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
Lock ServerSideApply feature to true #112748
Conversation
f8c35e3
to
718fe22
Compare
718fe22
to
57c95fb
Compare
lgtm, will let @apelisse tag |
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.
So this keeps the feature-gate but basically ignores its value throughout? Shouldn't we at least warn people that the feature-gate is moot?
/lgtm
/approve
@@ -599,7 +599,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag | |||
if a.group.MetaGroupVersion != nil { | |||
reqScope.MetaGroupVersion = *a.group.MetaGroupVersion | |||
} | |||
if a.group.OpenAPIModels != nil && utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { | |||
if a.group.OpenAPIModels != nil { |
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.
Not necessarily in this pull-request, but I think we need to remove that condition. the scope.FieldManager
should always be set otherwise the handler is going to crash later when it's used, which is a worse experience.
If this causes flakiness due to extra memory-allocation, @alexzielenski fix for swagger unmarshaling will almost certainly solve the problem.
cc @Jefftree
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.
ack, I'll work on removing the OpenAPI conditions
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: apelisse, wojtek-t 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 |
Are we able to remove the feature gate without it being locked to default first? |
/triage accepted |
It's what we do with pretty much any other feature gate. |
I don't think it'd hurt to have something in the boot sequence that does:
or something? |
|
OK Thanks, I missed that, perfect :-) |
Ref #110173 (comment)
/kind cleanup
/priority important-soon
/sig api-machinery
/assign @apelisse @liggitt