Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GAP-23] Inbound network proxy - concept #53

Merged
merged 5 commits into from
Jun 1, 2023

Conversation

mfranciszkiewicz
Copy link
Contributor

No description provided.

@mfranciszkiewicz mfranciszkiewicz linked an issue Jun 29, 2022 that may be closed by this pull request
@mfranciszkiewicz mfranciszkiewicz marked this pull request as ready for review July 13, 2022 08:27
@stranger80 stranger80 changed the title GAP - Inbound network Proxy [GAP-23] Inbound network proxy Jul 25, 2022
Copy link
Contributor

@stranger80 stranger80 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, let me say that I admire this GAP, as it is descriptive, meaningful, and includes specification-level details aimed at explaining the concept to an avid reader. The idea is well-justified, and proposal seems thoroughly considered (eg. it includes the on-negotiation connectivity verification using "probe" endpoints, which significantly improves reliability of the proxy providers).

There are a few elements which IMHO would improve the readability of the GAP and make the article easier to follow:

  • Proposed extensions to the standards - properties, usage counters, etc.:

    • How will the proxy service be offered on Golem market? Presumably the Offers will indicate a specific golem.runtime.name value to flag the proxy service on the market? What other properties are required to describe the features of the proxy?
    • How will the Requestor Agent obtain the address of the "probe" endpoint during negotiations? Will this be via a property in the Offer proposal?
    • I would assume the Requestor indicates the requested proxy port number via a property in the Demand?
    • What will be the usage vector of the proxy runtime?
  • ExeScript commands semantics table - this feels like a default element of new runtime specification: a table explaining semantincs/implementation summary of respective exescript commands (also explicitly listing those commands, which will not be implemented), with meaningful examples, eg:

| Command | Examples | Implementation summary |
|-|-|
| DEPLOY | N/A | Not implemented |
| START | START | Start the 'proxy' runtime |
| RUN | RUN "GET /probe-api/v1/", RUN "PUT /TCP/121.32.25.54:1224 {requestorId, token}" | Send command to proxy runtime's management/admin APIs |
| ... | ... | ... |

@mfranciszkiewicz
Copy link
Contributor Author

mfranciszkiewicz commented Jul 27, 2022

@stranger80 The idea implemented here was to approach #35 in 3 separate GAPs: the proxy (new core functionality), runtime and market negotiations. It's tempting to include the whole solution in a single document, but IMO the amount of information would be overwhelming, at least in comparison to the existing documents. Notably, market negotiations (golemfactory/yagna#2009) would be rich in sequence charts and at least as long as this GAP, which IMO deserves a separate document.

On the other hand, the least impressive GAP would describe the runtime itself, as it's going to be a sibling of ya-runtime-http-auth. At least, I owe that explanation to the readers.

Would you agree that the points above can be addressed in the remaining GAPs?

@stranger80
Copy link
Contributor

@mfranciszkiewicz yep, I'm on board with the intent to compose 3 GAPs to cover all the aspects of the proposal.

@stranger80 stranger80 changed the title [GAP-23] Inbound network proxy [GAP-23] Inbound network proxy - concept Jul 31, 2022
@rad9k
Copy link

rad9k commented Aug 8, 2022

why "The initial implementation assumes that there exists a single forwardee and the traffic is sent over Golem Virtual Private Network."? Do we consider other (non single) scenarios? If yes, what determinates non-single implementation?

@mfranciszkiewicz
Copy link
Contributor Author

why "The initial implementation assumes that there exists a single forwardee and the traffic is sent over Golem Virtual Private Network."? Do we consider other (non single) scenarios? If yes, what determinates non-single implementation?

We might want to consider implementing load balancing over multiple nodes or a single destination (proxy) that does the load balancing

@stranger80
Copy link
Contributor

why "The initial implementation assumes that there exists a single forwardee and the traffic is sent over Golem Virtual Private Network."? Do we consider other (non single) scenarios? If yes, what determinates non-single implementation?

As @mfranciszkiewicz responded - this "inbound proxy" is purely about forwarding traffic. We don't want to add more features to it (as per the "single responsibility" design principle). For load balancing we should rather propose a separate "Application Load Balancer" or "Network Load Balancer" component/feature.

@golmek
Copy link

golmek commented Jun 1, 2023

@stranger80 - Can You merge this PR or is it something we are waiting for / missing ? Thx!

cc: @jalas167

@stranger80 stranger80 merged commit b2790f1 into master Jun 1, 2023
@stranger80 stranger80 deleted the mf/GAP/inbound_net_proxy branch June 1, 2023 14:15
@stranger80
Copy link
Contributor

@stranger80 - Can You merge this PR or is it something we are waiting for / missing ? Thx!

cc: @jalas167

Hi, yes, this has been merged now.
What is the status of the implementation? (I'm asking as this GAP is quite old, and from times where GAPs would be created after the implementation was almost complete...)

@golmek
Copy link

golmek commented Jun 2, 2023

@stranger80 Big Thx!

Answering Your question. Inbound was not yet touched when it comes to Implementation. Currently, it is also hard for me to state when it might be taken into our plan :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GAP for inbound network traffic proxy
4 participants