Bug 1866299: Adds support for GovCloud endpoints #441

danehans · 2020-08-20T21:40:11Z

A follow-on to #433. Based upon these docs, GovCloud does provide a specific Route 53 endpoint. This PR updates NewProvider() to set the GovCloud Route 53 and Tagging endpoints and validates custom endpoint url's.

openshift-ci-robot · 2020-08-20T21:41:36Z

@danehans: This pull request references Bugzilla bug 1866299, which is invalid:

expected the bug to be in one of the following states: NEW, ASSIGNED, ON_DEV, POST, POST, but it is MODIFIED instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1866299: Adds support for GovCloud endpoints

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

danehans · 2020-08-20T21:49:47Z

/bugzilla refresh

openshift-ci-robot · 2020-08-20T21:49:52Z

@danehans: This pull request references Bugzilla bug 1866299, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Details

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

danehans · 2020-08-21T01:08:54Z

error: some steps failed:
  * could not run steps: step e2e-aws-upgrade failed: template pod "e2e-aws-upgrade" failed: the pod ci-op-9g5dfn2s/e2e-aws-upgrade failed after 1h27m30s (failed containers: resourcewatch): ContainerFailed one or more containers exited

/test e2e-aws-upgrade

sgreene570 · 2020-08-21T14:45:06Z

pkg/dns/aws/dns.go

+				return true
+			}
+		}
+	default:


When exactly is this default case hit? When case ep.Name == ELBService:?
Could you add a unit test to cover it, if you don't mind?

ELBService is correct. Yes, I'll add a unit test.

sgreene570 · 2020-08-21T14:47:38Z

pkg/dns/aws/dns.go

+		// https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/using-govcloud-endpoints.html
+		// does not specify a tagging endpoint. I suspect tagging has the same behavior as the
+		// standard tagging endpoint (requires east-1 due to Route 53 hosted zone lookup).
+		tagConfig = tagConfig.WithRegion(endpoints.UsGovEast1RegionID)


Is there any way to verify that this suspicion is correct before we merge these changes? Or do you plan on verifying this after this PR merges?

@sgreene570 I'm going to push a new commit that removes the limitation and ask QE to confirm.

danehans · 2020-08-24T17:25:42Z

=== RUN   TestIngressControllerScale
I0822 00:22:16.268114    6144 request.go:645] Throttling request took 1.049586282s, request: GET:https://api.ci-op-7ml9t3d2-43abb.origin-ci-int-aws.dev.rhcloud.com:6443/apis/template.openshift.io/v1?timeout=32s
--- FAIL: TestIngressControllerScale (123.29s)
    operator_test.go:354: failed waiting deployment openshift-ingress-operator/default to scale to 3: failed to achieve expected replicas, last observed: 2

/test e2e-aws-operator

danehans · 2020-08-25T23:28:11Z

pkg/dns/aws/dns.go

+		// Route53 for GovCloud uses the "us-gov" region id:
+		// https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/using-govcloud-endpoints.html
+		r53Config = r53Config.WithRegion(govCloudRoute53Region)
+		tagConfig = tagConfig.WithRegion(region)


Note that I have not implemented the same tagging api region restrictions as standard aws since the govcloud service endpoints doc does not specify the tagging api. The PR needs to merge so @yunjiang29 can verify against aws govcloud.

sgreene570 · 2020-08-26T14:59:38Z

Looks good!

/lgtm

openshift-ci-robot · 2020-08-26T14:59:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danehans, sgreene570

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [danehans,sgreene570]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2020-08-26T15:05:21Z

@danehans: All pull requests linked via external trackers have merged:

Bugzilla bug 1866299 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1866299: Adds support for GovCloud endpoints

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Aug 20, 2020

openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Aug 20, 2020

openshift-ci-robot requested review from frobware and ironcladlou August 20, 2020 21:41

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 20, 2020

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Aug 20, 2020

sgreene570 reviewed Aug 21, 2020

View reviewed changes

danehans force-pushed the custom_endpoints_fix_part_2 branch from 7d04bf9 to 4223cf9 Compare August 21, 2020 23:34

Bug 1866299: Adds support for GovCloud endpoints

07c1694

danehans force-pushed the custom_endpoints_fix_part_2 branch from 4223cf9 to 07c1694 Compare August 25, 2020 21:02

danehans commented Aug 25, 2020

View reviewed changes

openshift-ci-robot assigned sgreene570 Aug 26, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2020

openshift-merge-robot merged commit 7a1a72e into openshift:master Aug 26, 2020

This was referenced Sep 8, 2020

Bug 1866299: Fixes Route53 Client Handling of GovCloud Partition #454

Merged

Bug 1866299: Fixes AWS Tagging Client for GovCloud East Region #455

Merged

Bug 1866299: Refactors AWS Provider Validation #457

Merged

openshift-ci-robot mentioned this pull request Sep 22, 2020

Bug 1866299: Forces Tagging Client to us-gov-west-1 Region #460

Merged

Bug 1866299: Adds support for GovCloud endpoints #441

Bug 1866299: Adds support for GovCloud endpoints #441

Uh oh!

Conversation

danehans commented Aug 20, 2020

Uh oh!

openshift-ci-robot commented Aug 20, 2020

Uh oh!

danehans commented Aug 20, 2020

Uh oh!

openshift-ci-robot commented Aug 20, 2020

Uh oh!

danehans commented Aug 21, 2020

Uh oh!

sgreene570 Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

danehans Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

sgreene570 Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

danehans Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

danehans commented Aug 24, 2020

Uh oh!

danehans Aug 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgreene570 commented Aug 26, 2020

Uh oh!

openshift-ci-robot commented Aug 26, 2020

Uh oh!

openshift-ci-robot commented Aug 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danehans Aug 25, 2020 •

edited

Loading