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.

Sign up for GitHub

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

Jump to bottom

Changes to ManagedFields is not mutation for GC #90534

Merged
merged 1 commit into from May 9, 2020
Merged

Changes to ManagedFields is not mutation for GC #90534

merged 1 commit into from May 9, 2020

Conversation

apelisse
Copy link
Member

@apelisse apelisse commented Apr 27, 2020 •
edited by liggitt

What type of PR is this?
/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Fixed a regression preventing garbage collection of RBAC role and binding objects

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 27, 2020
@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Apr 27, 2020
@apelisse
Copy link
Member Author

it's not super clear how we can know which new fields might want to be ignored? Should I just fail if any new field is added to ObjectMeta? @liggitt

cc @deads2k

@liggitt
Copy link
Member

liggitt commented Apr 27, 2020

Should I just fail if any new field is added to ObjectMeta?

That's a reasonable place to start, along with a comment about what should be used to decide if new fields should be cleared or added to the list to ignore in the test

@gabemontero gabemontero mentioned this pull request Apr 28, 2020
Closed
@apelisse apelisse force-pushed the mutatinggcfields-fieldmanager branch from 571c34f to fe8bd02 Compare April 28, 2020 20:40
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 28, 2020
@apelisse apelisse force-pushed the mutatinggcfields-fieldmanager branch from fe8bd02 to fcd0528 Compare April 28, 2020 20:52
@apelisse apelisse force-pushed the mutatinggcfields-fieldmanager branch from fcd0528 to 89b68ba Compare April 29, 2020 16:29
@apelisse
Copy link
Member Author

/retest

gabemontero added a commit to gabemontero/openshift-apiserver that referenced this pull request Apr 30, 2020
@gabemontero
copy/use kubernetes/kubernetes#90534 for IsMutationForGC check
6556e17
@openshift-ci-robot openshift-ci-robot mentioned this pull request Apr 30, 2020
Merged
@apelisse
Copy link
Member Author

@liggitt PTAL

gabemontero added a commit to gabemontero/openshift-apiserver that referenced this pull request May 1, 2020
@gabemontero
Bug 1731222: unwarranted SARs on foreground GC of objects with owner …
418f0ce 
…refs

copy/use kubernetes/kubernetes#90534 for IsMutationForGC check
gabemontero added a commit to gabemontero/openshift-apiserver that referenced this pull request May 8, 2020
@gabemontero
Bug 1731222: unwarranted SARs on foreground GC of objects with owner …
894f585 
…refs

copy/use kubernetes/kubernetes#90534 for IsMutationForGC check
@apelisse
Copy link
Member Author

apelisse commented May 8, 2020

/priority important-soon
/retest

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels May 8, 2020
@liggitt
Copy link
Member

liggitt commented May 8, 2020 •
edited

/kind regression

this will need picking to 1.16-1.18, right?

@k8s-ci-robot k8s-ci-robot added the kind/regression Categorizes issue or PR as related to a regression from a prior release. label May 8, 2020
@apelisse
Copy link
Member Author

apelisse commented May 8, 2020

Correct

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels May 8, 2020
@liggitt
Copy link
Member

liggitt commented May 8, 2020

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apelisse, liggitt

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 8, 2020
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

4 similar comments
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit 68ade56 into kubernetes:master May 9, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.19 milestone May 9, 2020
@liggitt
Copy link
Member

liggitt commented Jun 1, 2020

Thanks. this fix needs to be picked back to existing releases.

@liggitt liggitt mentioned this pull request Jun 1, 2020
Closed
This was referenced Jun 1, 2020
Merged
Merged
Merged
k8s-ci-robot added a commit that referenced this pull request Jun 9, 2020
@k8s-ci-robot
Merge pull request #91649 from apelisse/automated-cherry-pick-of-#905…
3f1712a 
…34-kubernetes-release-1.17

Automated cherry pick of #90534: Changes to ManagedFields is not mutation for GC
k8s-ci-robot added a commit that referenced this pull request Jun 9, 2020
@k8s-ci-robot
Merge pull request #91650 from apelisse/automated-cherry-pick-of-#905…
df2d315 
…34-kubernetes-release-1.16

Automated cherry pick of #90534: Changes to ManagedFields is not mutation for GC
k8s-ci-robot added a commit that referenced this pull request Jun 10, 2020
@k8s-ci-robot
Merge pull request #91648 from apelisse/automated-cherry-pick-of-#905…
22a86a7 
…34-kubernetes-release-1.18

Automated cherry pick of #90534: Changes to ManagedFields is not mutation for GC
@jannfis jannfis mentioned this pull request Jul 12, 2020
Closed
3 tasks
@jannfis jannfis mentioned this pull request Aug 25, 2020
Closed
3 tasks
@jannfis jannfis mentioned this pull request Mar 10, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@deads2k deads2k deads2k left review comments

@dims dims Awaiting requested review from dims

@jianhuiz jianhuiz Awaiting requested review from jianhuiz

Assignees

@liggitt liggitt

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.19
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@apelisse @liggitt @k8s-ci-robot @fejta-bot @deads2k