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

Add infrastructure ConfigValidator for checking NAT IPs #312

Merged
merged 3 commits into from
Aug 31, 2021

Conversation

stoyanr
Copy link
Contributor

@stoyanr stoyanr commented Aug 25, 2021

How to categorize this PR?

/area ops-productivity
/area quality
/kind bug
/platform gcp

What this PR does / why we need it:

  • Adds an implementation of the infrastructure.ConfigValidator for checking whether NAT IP names refer to existing and available external IP addresses. See Add infrastructure config validator gardener#4547.
  • Adds unit tests for the infrastructure.ConfigValidator implementation.
  • Vendors g/g v1.31.0-dev (in a separate commit).

Which issue(s) this PR fixes:
Fixes #309

Special notes for your reviewer:

Release note:

NAT IP names in infrastructure config are now checked if they exist and are available, and if not the issue is reported as `ERR_CONFIGURATION_PROBLEM` with a clear error message.

@stoyanr stoyanr requested review from a team as code owners August 25, 2021 08:09
@gardener-robot gardener-robot added the platform/gcp Google cloud platform/infrastructure label Aug 25, 2021
@gardener-robot
Copy link

@stoyanr Labels area/todo, kind/todo do not exist.

@gardener-robot gardener-robot added needs/review Needs review size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Aug 25, 2021
@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 Aug 25, 2021
@gardener-robot gardener-robot added area/ops-productivity Operator productivity related (how to improve operations) area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/bug Bug labels Aug 25, 2021
@stoyanr
Copy link
Contributor Author

stoyanr commented Aug 25, 2021

/invite @ialidzhikov

@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 Aug 25, 2021
@stoyanr stoyanr marked this pull request as draft August 25, 2021 08:10
@gardener-robot gardener-robot added size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) and removed size/l Size of pull request is large (see gardener-robot robot/bots/size.py) labels Aug 25, 2021
@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 Aug 25, 2021
@stoyanr
Copy link
Contributor Author

stoyanr commented Aug 25, 2021

@kon-angelo Thanks for your comments, addressed them and also added some unit tests.

@stoyanr
Copy link
Contributor Author

stoyanr commented Aug 26, 2021

/squash

@stoyanr
Copy link
Contributor Author

stoyanr commented Aug 27, 2021

/test

@testmachinery
Copy link

testmachinery bot commented Aug 27, 2021

Testrun: e2e-7mwxl
Workflow: e2e-7mwxl-wf
Phase: Succeeded

+---------------------+---------------------+-----------+----------+
|        NAME         |        STEP         |   PHASE   | DURATION |
+---------------------+---------------------+-----------+----------+
| infrastructure-test | infrastructure-test | Succeeded | 9m45s    |
+---------------------+---------------------+-----------+----------+

@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 Aug 27, 2021
@stoyanr stoyanr marked this pull request as ready for review August 27, 2021 12:03
@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 Aug 27, 2021
@stoyanr
Copy link
Contributor Author

stoyanr commented Aug 27, 2021

/test

@testmachinery
Copy link

testmachinery bot commented Aug 27, 2021

Testrun: e2e-vvlzm
Workflow: e2e-vvlzm-wf
Phase: Succeeded

+---------------------+---------------------+-----------+----------+
|        NAME         |        STEP         |   PHASE   | DURATION |
+---------------------+---------------------+-----------+----------+
| infrastructure-test | infrastructure-test | Succeeded | 10m30s   |
+---------------------+---------------------+-----------+----------+

rfranzke
rfranzke previously approved these changes Aug 30, 2021
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

pkg/gcp/client/compute.go Show resolved Hide resolved
@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review needs/second-opinion Needs second review by someone else labels Aug 30, 2021
dkistner
dkistner previously approved these changes Aug 30, 2021
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

@stoyanr stoyanr dismissed stale reviews from dkistner and rfranzke via 066b27a August 30, 2021 15:42
@gardener-robot gardener-robot added needs/review Needs review size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else and removed needs/review Needs review labels Aug 30, 2021
@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 Aug 30, 2021
@gardener-robot gardener-robot removed size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) reviewed/lgtm Has approval for merging labels Aug 30, 2021
@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 Aug 30, 2021
@rfranzke rfranzke merged commit 255a6a6 into gardener:master Aug 31, 2021
@stoyanr stoyanr deleted the add-infra-config-validator branch August 31, 2021 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ops-productivity Operator productivity related (how to improve operations) area/quality Output qualification (tests, checks, scans, automation in general, etc.) 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) needs/second-opinion Needs second review by someone else platform/gcp Google cloud platform/infrastructure size/l Size of pull request is large (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Classify missing CloudNAT IPs as configuration error
8 participants