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

Validate zones order #30

Merged
merged 2 commits into from
Feb 11, 2020
Merged

Validate zones order #30

merged 2 commits into from
Feb 11, 2020

Conversation

timuthy
Copy link
Member

@timuthy timuthy commented Feb 11, 2020 •
edited
Loading

What this PR does / why we need it:
This PR forces a zone order immutability for updates on the infrastructure and worker portions of a shoot. Since the AWS provider uses the zone index to create networks via Terraform as well as MachineDeployments, the index should be kept stable.

Special notes for your reviewer:
/cc @rfranzke - thanks for noticing!

Release note:

The AWS validator now checks that zone orders in a shoot resources is retained (only additions are allowed).

@timuthy timuthy requested a review from a team as a code owner February 11, 2020 06:56
@gardener-robot-ci-2 gardener-robot-ci-2 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 Feb 11, 2020
@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 Feb 11, 2020
rfranzke
rfranzke previously approved these changes Feb 11, 2020
View reviewed changes
Copy link
Member

@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, thanks for fixing!

@timuthy timuthy force-pushed the enhancement.validation branch from 561ead6 to e471d9d Compare February 11, 2020 07:04
@gardener-robot-ci-2 gardener-robot-ci-2 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 Feb 11, 2020
@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 Feb 11, 2020
ialidzhikov
ialidzhikov previously approved these changes Feb 11, 2020
View reviewed changes
Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

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

/lgtm

@ialidzhikov
Copy link
Member

$index is used also in the alicloud infra chart. We can keep in mind to add the same validation check also there once we introduce the validator for provider-alicloud.

@rfranzke
Copy link
Member

@ismail can you add this info to gardener/gardener-extension-provider-alicloud#15?

@ialidzhikov ialidzhikov mentioned this pull request Feb 11, 2020
Closed
@timuthy timuthy force-pushed the enhancement.validation branch from e471d9d to 272be7e Compare February 11, 2020 09:19
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has 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 Feb 11, 2020
@timuthy
Copy link
Member Author

timuthy commented Feb 11, 2020

As discussed with @rfranzke, I added the validation for workers as well.

@timuthy timuthy force-pushed the enhancement.validation branch from 272be7e to e9441ee Compare February 11, 2020 11:19
@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) 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 Feb 11, 2020
timebertt
timebertt previously approved these changes Feb 11, 2020
View reviewed changes
Copy link
Member

@timebertt timebertt left a comment

Choose a reason for hiding this comment

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

/lgtm

ialidzhikov
ialidzhikov previously approved these changes Feb 11, 2020
View reviewed changes
Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

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

/lgtm

@timuthy timuthy dismissed stale reviews from ialidzhikov and timebertt via 2031a69 February 11, 2020 12:02
@timuthy timuthy force-pushed the enhancement.validation branch from e9441ee to 2031a69 Compare February 11, 2020 12:02
@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 Feb 11, 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 Feb 11, 2020
@timuthy timuthy mentioned this pull request Feb 11, 2020
Merged
@timuthy timuthy changed the title Validate zones order for infrastructure Validate zones order Feb 11, 2020
rfranzke
rfranzke approved these changes Feb 11, 2020
View reviewed changes
Copy link
Member

@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 merged commit 4152a53 into gardener:master Feb 11, 2020
@timuthy timuthy deleted the enhancement.validation branch February 11, 2020 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfranzke rfranzke rfranzke approved these changes

@ialidzhikov ialidzhikov ialidzhikov left review comments

@timebertt timebertt timebertt left review comments

Assignees
No one assigned
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@timuthy @ialidzhikov @rfranzke @timebertt @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3