Adding graduation criteria to GEP 1619 #2432
Conversation
k8s-ci-robot
added
the
release-note-none
|
@robscott: GitHub didn't allow me to request PR reviews from the following users: gcs278. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: robscott The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
| scope of this policy? | ||
| 2. Should we leave room for configuring different forms of Session Persistence? | ||
| If so, what would that look like? | ||
|
|
There was a problem hiding this comment.
Do you think this question is worth a call out? This one is from the Open Questions section, but maybe it's worth calling out specifically for graduation.
| 3. Should there be an API configuration field that specifies how already established sessions are handled? |
We have more open questions, but I suppose I'm trying to decide what's worthy of blocking Implementable status.
There was a problem hiding this comment.
Added a comment below that may cover this, but although it's probably worth some due diligence to see what underlying implementations like NGINX and Envoy can support here, I'm guessing we may not need a field for this.
| from 2 maintainers had been received. Before this graduates to implementable, | ||
| we need to get at least one of @robscott or @youngnick to also approve this GEP. | ||
|
|
||
| There are some open questions that will need to be answered before this can |
There was a problem hiding this comment.
This might be an overly specific question: To me, this implies we don't necessarily need to answer all of the questions we have in Open Questions in order to graduate to Implementable, is that fair?
Sounds like you are addressing specific, but questions you found deserving of special attention here.
There was a problem hiding this comment.
Yeah good question. The things I'm trying to highlight are potentially big API changes that we need to figure out before moving to implementable. IE if we're increasing the potential scope of the resource, that probably means a new name, maybe an enum field for a type of persistence, etc. Really just an overall plan for how this policy resource could grow to support related use cases in the future.
On the other hand, I think that many of the "open questions" in the section below this could likely be addressed at a future point. Here's a quick run through of my thoughts on each of them:
- What happens when session persistence is broken because the backend is not up or healthy? If that's an error case, how should that be handled? Should the API dictate the http error code? Or should the API dictate fall back behavior?
Would really need to look at what implementations are capable of here. Probably worth answering prior to release, but the answer may be that we can't dictate anything if behavior ends up being fairly impl-specific here.
- What happens when session persistence causes traffic splitting scenarios to overload a backend?
This seems like something we likely can't prevent, but we may need to document the risks of persistence. Similar to above, this may end up being a documentation-only thing.
- Should we add status somewhere when a user gets in to a "risky" configuration with session persistence?
I'm not sure how we'd define these risky states, but this feels like it could be handled with a follow up change so should not block.
- Should there be an API configuration field that specifies how already established sessions are handled?
Not sure I get this one? Is this trying to ask what would happen to existing sessions as session persistence is being enabled or disabled? I think this is referring to the potential addition of a new field, so I think this is also probably good criteria for a non-blocking follow up, but may be misunderstanding.
There was a problem hiding this comment.
Thanks for your thought process, it makes sense now that you used the ones with the highest level of impact to the GEP.
Not sure I get this one? Is this trying to ask what would happen to existing sessions as session persistence is being enabled or disabled?
Correct, more so on the enabled side-of-things. An API field to control how to handle an existing session with the same name, whether it should forcibly "override" or "rewrite", or it should be passive and not enable persistence if there is a collision.
I think this is referring to the potential addition of a new field, so I think this is also probably good criteria for a non-blocking follow up, but may be misunderstanding.
Correct, that's fair.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
Updates #1619 |
What type of PR is this?
/kind gep
What this PR does / why we need it:
There was a bit of a mixup where this GEP was merged a bit earlier than intended. Although I think there's lots of good content here, I don't think @youngnick or I have had sufficient time to look through this in detail yet, and likely won't until v1.0 is ready 😞.
Although I haven't had time to take a detailed look at this GEP yet, I did add some initial higher level questions as part of the graduation criteria here that I think we should address before this GEP graduates. Particularly interested in how we can leave room for future expansion here.
Does this PR introduce a user-facing change?:
/cc @gcs278 @shaneutt @youngnick