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.

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

Be pessimistic about legacy cm deletion #131

Merged
merged 1 commit into from
Jul 9, 2020

Conversation

timuthy
Copy link
Member

@timuthy timuthy commented Jul 9, 2020

How to categorize this PR?

/area control-plane
/kind bug
/priority normal
/platform azure

What this PR does / why we need it:
With the introduction of #99 we observed errors during the shoot deletion because the KAS deployment still referred to the legacy cloud-provider config map. This PR adds a check if the said config map is still referenced and skips the deletion. Please note that the deletion error only happens under special circumstances, e.g. if the shoot reconciliation is aborted after the control plane deployment and immediately deleted.

Special notes for your reviewer:
/cc @danielfoehrKn

Release note:

An issues has been fixed which caused unsuccessful shoot deletions due to the migration of the `cloud-provider-config` from a config map to a secret.

@timuthy timuthy requested review from a team as code owners July 9, 2020 07:40
@gardener-robot gardener-robot added area/control-plane Control plane related kind/bug Bug platform/azure Microsoft Azure platform/infrastructure priority/normal labels Jul 9, 2020
@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 9, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added 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 9, 2020
@timuthy timuthy force-pushed the fix.cloud-provider-config branch from 2fe67b4 to cd7f0ea Compare July 9, 2020 08:21
@gardener-robot-ci-3 gardener-robot-ci-3 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 9, 2020
@timuthy timuthy changed the title Be pessimitic about legacy cm deletion Be pessimistic about legacy cm deletion Jul 9, 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 9, 2020
Copy link
Member

@dkistner dkistner left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added the reviewed/lgtm Has approval for merging label Jul 9, 2020
@dkistner
Copy link
Member

dkistner commented Jul 9, 2020

@danielfoehrKn Can you have look?

Copy link
Contributor

@danielfoehrKn danielfoehrKn left a comment

Choose a reason for hiding this comment

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

Thanks!

@danielfoehrKn danielfoehrKn merged commit 6ba1432 into gardener:master Jul 9, 2020
@timuthy timuthy deleted the fix.cloud-provider-config branch July 9, 2020 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/control-plane Control plane related kind/bug Bug needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/azure Microsoft Azure platform/infrastructure reviewed/lgtm Has approval for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants