Skip to content
This repository has been archived by the owner on Apr 7, 2020. It is now read-only.

Delete remnant route entries #49

Merged
merged 1 commit into from
Apr 15, 2019
Merged

Delete remnant route entries #49

merged 1 commit into from
Apr 15, 2019

Conversation

DockToFuture
Copy link
Contributor

@DockToFuture DockToFuture commented Apr 11, 2019 •
edited
Loading

What this PR does / why we need it:
Assures that k8s created route entries for gcp shoots are deleted if they are not properly removed by the cloud provider.
Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:
-->

Remnant route entries are now explicitly deleted by the gcp-provider extension controller during the infrastructure deletion.

@DockToFuture DockToFuture requested a review from a team as a code owner April 11, 2019 14:26
@DockToFuture DockToFuture force-pushed the feature/delete-routes-on-gcp-provider-extension branch from 75cde7e to 6b658d9 Compare April 11, 2019 14:52
timuthy
timuthy suggested changes Apr 12, 2019
View reviewed changes
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.

I don't expect any changes in the contorller-registration.yaml files, as well as ^ is not a valid Base64 character.

Please revert those changes.

@DockToFuture
Copy link
Contributor Author

DockToFuture commented Apr 12, 2019 •
edited
Loading

I don't expect any changes in the contorller-registration.yaml files, as well as ^ is not a valid Base64 character.

Please revert those changes.

The generate script somehow adds linebreaks which are ^M in base64. Would be nice to found out what causes them when calling make generate and of course I will remove them.

@rfranzke rfranzke added area/robustness Robustness, reliability, resilience related exp/intermediate Issue that requires some project experience kind/enhancement Enhancement, improvement, extension needs/review Needs review platform/gcp Google Cloud Platform (GCP) platform/infrastructure priority/normal Standard backlog priority, that can be worked on now or later reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/s A few hours of work or small change status/in-progress Issue is in progress/work status/under-investigation Issue is under investigation topology/seed Affects Seed clusters labels Apr 12, 2019
@DockToFuture DockToFuture force-pushed the feature/delete-routes-on-gcp-provider-extension branch from 6b658d9 to 6dde153 Compare April 12, 2019 07:11
@gardener-robot-ci-1 gardener-robot-ci-1 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 Apr 12, 2019
@rfranzke
Copy link
Contributor

The generate script somehow adds linebreaks which are ^M in base64. Would be nice to found out what causes them when calling make generate and of course I will remove them.

@timuthy suspects that you don't have gnutar. Is that true?

@DockToFuture
Copy link
Contributor Author

DockToFuture commented Apr 12, 2019 •
edited
Loading

The generate script somehow adds linebreaks which are ^M in base64. Would be nice to found out what causes them when calling make generate and of course I will remove them.

@timuthy suspects that you don't have gnutar. Is that true?

For make generate generate I've used gnutar and locally the files are correctly shown, the newline seperator ˆM is added when pushing to git. At least I've checked out the old files and pushed them again with the same result.

@DockToFuture DockToFuture force-pushed the feature/delete-routes-on-gcp-provider-extension branch 2 times, most recently from 05d5c60 to e959602 Compare April 12, 2019 13:28
@DockToFuture DockToFuture force-pushed the feature/delete-routes-on-gcp-provider-extension branch from e959602 to d43b33a Compare April 15, 2019 06:37
@rfranzke rfranzke 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 Apr 15, 2019
@gardener-robot-ci-1 gardener-robot-ci-1 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 Apr 15, 2019
rfranzke
rfranzke approved these changes Apr 15, 2019
View reviewed changes
Copy link
Contributor

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

@rfranzke rfranzke removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review labels Apr 15, 2019
@rfranzke rfranzke added reviewed/lgtm Has approval for merging status/accepted Issue was accepted as something we need to work on and removed status/in-progress Issue is in progress/work status/under-investigation Issue is under investigation labels Apr 15, 2019
adracus
adracus approved these changes Apr 15, 2019
View reviewed changes
Copy link
Contributor

@adracus adracus left a comment

Choose a reason for hiding this comment

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

/lgtm

@rfranzke rfranzke merged commit 6371a97 into gardener-attic:master Apr 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@timuthy timuthy timuthy requested changes

@rfranzke rfranzke rfranzke approved these changes

@adracus adracus adracus approved these changes

Assignees
No one assigned
Labels
area/robustness Robustness, reliability, resilience related exp/intermediate Issue that requires some project experience kind/enhancement Enhancement, improvement, extension platform/gcp Google Cloud Platform (GCP) platform/infrastructure priority/normal Standard backlog priority, that can be worked on now or later reviewed/lgtm Has approval for merging size/s A few hours of work or small change status/accepted Issue was accepted as something we need to work on topology/seed Affects Seed clusters
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@DockToFuture @rfranzke @adracus @timuthy @gardener-robot-ci-1