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

Categorize certain aws-route53 errors as configuration problems #398

Merged
merged 2 commits into from
Aug 31, 2021
Merged

Categorize certain aws-route53 errors as configuration problems #398

merged 2 commits into from
Aug 31, 2021

Conversation

stoyanr
Copy link
Contributor

@stoyanr stoyanr commented Aug 27, 2021

How to categorize this PR?

/area ops-productivity
/kind enhancement
/platform aws

What this PR does / why we need it:
Categorizes certain aws-route53 errors as configuration problems as suggested in gardener/gardener#4294.

Which issue(s) this PR fixes:
Fixes gardener/gardener#4294

Special notes for your reviewer:

Release note:

Failures to reconcile `DNSRecords` due to a missing hosted zone or a DNS name not matching the zone name are now properly categorized as `ERR_CONFIGURATION_PROBLEM`.

@stoyanr stoyanr requested review from a team as code owners August 27, 2021 14:08
@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 Aug 27, 2021
ialidzhikov
ialidzhikov previously approved these changes Aug 30, 2021
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

dkistner
dkistner reviewed Aug 30, 2021
View reviewed changes
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.

Looks good to me. Just two missing doc strings.

pkg/aws/client/client_route53.go Show resolved Hide resolved
pkg/aws/client/client_route53.go Show resolved Hide resolved
rfranzke
rfranzke previously approved these changes Aug 30, 2021
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 apart from the nits mentioned by @dkistner

I guess we still
/hold
as there is the ongoing release validation? cc @ialidzhikov

@gardener-robot gardener-robot added reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies reviewed/lgtm Has approval for merging labels Aug 30, 2021
@ialidzhikov
Copy link
Member

I think it should be fine to include this one as it affects only the dnsrecord controller which is still not heavily used.

/unhold

@gardener-robot gardener-robot removed the reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies label Aug 30, 2021
@stoyanr stoyanr dismissed stale reviews from rfranzke and ialidzhikov via e8170f9 August 30, 2021 15:51
@gardener-robot gardener-robot added needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) and removed needs/review Needs review labels Aug 30, 2021
@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 Aug 30, 2021
@ialidzhikov
Copy link
Member

/milestone v1.28

@gardener-robot gardener-robot added this to the v1.28 milestone Aug 31, 2021
ialidzhikov
ialidzhikov approved these changes Aug 31, 2021
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 ialidzhikov merged commit ee1f49c into gardener:master Aug 31, 2021
@stoyanr stoyanr deleted the categorize-aws-route53-configuration-issues branch August 31, 2021 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkistner dkistner dkistner left review comments

@ialidzhikov ialidzhikov ialidzhikov approved these changes

@rfranzke rfranzke rfranzke 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) reviewed/lgtm Has approval for merging size/s Size of pull request is small (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
v1.28
Development

Successfully merging this pull request may close these issues.

Add error categorization for route53 errors
7 participants
@stoyanr @ialidzhikov @dkistner @rfranzke @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot