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
[WIP] Set ownership of spec.replicas
to scale subresource
#83444
Conversation
... when RS is scaled via scale subresource
@mariantalla: GitHub didn't allow me to request PR reviews from the following users: julianvmodesto. Note that only kubernetes 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. |
Not sure if you've seen @jennybuckley's idea yet here: Seems nice since we could keep the logic to update fieldmanager in the internal package and we could re-use it for ReplicaSets, Deployments etc. as well as CRDs. |
Adding the entry in the ManagedFieldsEntry is not good enough to grab the ownership from other users. You also have to remove it from other users, which is definitely not trivial. The best way to do that is to call FieldManager.Update() with the old and new object (I think both are available in the ScaleREST). Jenny and Julian have discussed the wiring of the FieldManager in ScaleREST. |
Will look into FieldManager next. Signed-off-by: Maria Ntalla <mntalla@pivotal.io>
Signed-off-by: Maria Ntalla <mntalla@pivotal.io>
@mariantalla: PR needs rebase. 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mariantalla The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Alright, the commit state is a bit (or, a lot!) messy at the moment, so here's where work for this PR is at for now:
Next steps:
Yesterday at the working group meeting we said we'd speak with the rest of sig-api-machinery on whether a more reasonable approach would be to redirect subresource |
Signed-off-by: Maria Ntalla <mntalla@pivotal.io>
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
@mariantalla: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/close @nodo is working on an alternative implementation of this. |
@apelisse: Closed this PR. 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. |
(with @hoegaarden )
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR implements field ownership on
replicas.scale
for the scale subresource, so that changes to that field would behave the same way (including conflicts where applicable) as a server-side apply of the main resource.Which issue(s) this PR fixes:
xref: #82046
Special notes for your reviewer:
managedField
entry forspec.replicas
, on behalf of the scale subresource (i.e. the scale subresource appears as the manager of the field).Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
/cc @apelisse @jennybuckley @julianvmodesto @hoegaarden