-
Notifications
You must be signed in to change notification settings - Fork 471
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
USHIFT-2245: Add router namespace ownership enhancement #1552
Conversation
@pacevedom: This pull request references USHIFT-2245 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.16.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
b5ce26f
to
c5010a9
Compare
775ff0f
to
fc0b75a
Compare
@pacevedom: This pull request references USHIFT-2245 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.16.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
fc0b75a
to
02ed929
Compare
#1555 is changing the enhancement template in a way that will cause the header check in the linter job to fail for existing PRs. If this PR is merged within the development period for 4.16 you may override the linter if the only failures are caused by issues with the headers (please make sure the markdown formatting is correct). If this PR is not merged before 4.16 development closes, please update the enhancement to conform to the new template. |
02ed929
to
55017e4
Compare
/assign @alebedev87 |
|
||
## Graduation Criteria | ||
### Dev Preview -> Tech Preview | ||
- Ability to utilize the enhancement end to end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the status of this feature for MicroShift 4.16? Tech Preview or GA?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be GA.
|
||
MicroShift is also focused on single-node deployments, and runs without an | ||
external load balancer and in environments without access to deep DNS | ||
integration. The primary means of accessing the apps on the MicroShift host |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a secondary means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could have a few of them, but depends on the application. Additional routers, different ingress controllers, direct service exposure, etc.
All of those are out of control for MicroShift, the default router is provided as a helper and a means of easily accessing applications.
### Workflow Description | ||
|
||
### API Extensions | ||
To allow configuring the router a new option is exposed through the MicroShift |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the user going to add this, or is part of the default config and set to InterNamespaceAllowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default is InterNamespaceAllowed and will always be present. Users may modify this field to whichever value they need (Strict or InterNamespaceAllowed).
## Upgrade / Downgrade Strategy | ||
N/A | ||
|
||
## Version Skew Strategy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OCP enhancement notes, "The API defines empty
equivalent to Strict
, therefore all the clients will have to keep that check to remain backwards compatible."
How will this work in MicroShift with the Y+2 skew? Is it a concern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MicroShift will always default this field to InterNamespaceAllowed
if not present. In the event of a skew where this field is not present, MicroShift ignores the unknown config parameters and does nothing with them.
@pacevedom: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
The implementation for this has already started to land. If we end up needing substantial changes we can come back and update the doc.
@pacevedom , please get someone to apply a lgtm when you're ready.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhellmann 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 |
/lgtm |
Targeting 4.16, include the namespace ownership configuration parameter for the router in MicroShift.