Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Offer auto-delete on invalid update operation for managed resources #69

Merged

Conversation

rfranzke
Copy link
Contributor

@rfranzke rfranzke commented Jul 2, 2020

How to categorize this PR?

/area usability
/kind enhancement
/priority normal

What this PR does / why we need it:
The ManagedResource controller is now capable of auto-deleting a resource in case an update fails with the "invalid" error and the resource is properly annotated to allow this behaviour.
This is useful if a managed resource, e.g. a StorageClass or Pod, shall be updated and those updates are not allowed by the API server.

Special notes for your reviewer:
The Kubernetes API server sends a 422 Unprocessable Entity error in case it static validation fails. Unfortunately, its status codes returned on errors of admission plugins as well as the status codes returned by (validating) admission webhooks are not necessarily streamlined. Consequently, I decided to exclude those cases for now and limit the solution on errors happening during static validation. Probably, this is fine for the vast majority of our use-cases within the Gardener project.

Release note:

Resources annotated with `resources.gardener.cloud/delete-on-invalid-update=true` will now be deleted in case the Gardener-Resource-Manager fails to update them and receives an `422 Unprocessable Entity` error. This error is usually sent by the Kubernetes API server in case its static validation fails.

@rfranzke rfranzke requested a review from a team as a code owner July 2, 2020 11:13
@gardener-robot gardener-robot added area/usability Usability related kind/enhancement Enhancement, improvement, extension priority/normal labels Jul 2, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jul 2, 2020
@rfranzke rfranzke force-pushed the feature/delete-on-forbidden-changes branch from 5379456 to 5496754 Compare July 2, 2020 11:22
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 2, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 2, 2020
@timebertt
Copy link
Contributor

/needs/rebase
/status author-action

@gardener-robot gardener-robot added needs/rebase Needs git rebase status/author-action Issue requires issue author action labels Jul 3, 2020
@gardener-robot
Copy link
Contributor

@rfranzke The pull request was assigned to you under author-action. Please unassign yourself when you are done. Thank you.

@rfranzke rfranzke force-pushed the feature/delete-on-forbidden-changes branch from 5496754 to 47c49a8 Compare July 3, 2020 09:37
@rfranzke
Copy link
Contributor Author

rfranzke commented Jul 3, 2020

/unassign
/needs review
/remove needs/rebase status/author-action

@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 3, 2020
@gardener-robot gardener-robot added needs/review Needs review and removed needs/rebase Needs git rebase labels Jul 3, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 3, 2020
@gardener-robot gardener-robot removed the status/author-action Issue requires issue author action label Jul 3, 2020
@timuthy
Copy link
Contributor

timuthy commented Jul 8, 2020

/assign

Copy link
Contributor

@timuthy timuthy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I also tested it locally and it worked as expected.
/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Jul 15, 2020
@timuthy timuthy merged commit b511ef0 into gardener-attic:master Jul 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/usability Usability related kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants