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

data/aws: Consolidate pivate and public load balancer target groups #609

Conversation

wking
Copy link
Member

@wking wking commented Nov 4, 2018

The listeners may be private or public, but as far as the bootstrap and master modules are concerned, these are just lists of ARNs that need attaching. This partially unwinds changes from 16dfbb3 (#594), before which the bootstrap module used a unified list of load balancers while the master module made private/public distinctions.

CC @crawford

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 4, 2018
@wking wking force-pushed the collapse-private-and-public-load-balancer-target-groups branch 2 times, most recently from d59dd15 to 24b53c6 Compare November 4, 2018 18:14
@wking
Copy link
Member Author

wking commented Nov 4, 2018

Rebased around #608 with d59dd15 -> 24b53c6.

@crawford
Copy link
Contributor

crawford commented Nov 4, 2018

2018 and we're still putting up with out-of-bounds errors...

2018/11/04 18:18:32 Copying artifacts from release-latest into /logs/artifacts/release-latest
2018/11/04 18:18:32 Copied 0.87Mi of artifacts from release-latest to /logs/artifacts/release-latest
panic: runtime error: index out of range

goroutine 113 [running]:
github.com/openshift/ci-operator/pkg/steps.waitForPodCompletion(0x1285700, 0xc4200f67c0, 0xc420265620, 0xe, 0x1275cc0, 0xc4202800f0, 0x7ffd47f1eee7, 0xf, 0x0, 0xc420721520)
	/go/src/github.com/openshift/ci-operator/pkg/steps/template.go:572 +0x3f7
github.com/openshift/ci-operator/pkg/steps.(*podStep).Run(0xc42044a000, 0x1275a40, 0xc4202ab6c0, 0x0, 0x5, 0xc42057e400)
	/go/src/github.com/openshift/ci-operator/pkg/steps/test.go:120 +0xc4f
github.com/openshift/ci-operator/pkg/steps/release.(*assembleReleaseStep).Run(0xc42028ba40, 0x1275a40, 0xc4202ab6c0, 0xbeefebce406abe00, 0x1d67a188d8, 0x1974400)
	/go/src/github.com/openshift/ci-operator/pkg/steps/release/create_release.go:83 +0x773
github.com/openshift/ci-operator/pkg/steps.runStep(0x1275a40, 0xc4202ab6c0, 0xc420307980, 0xc4206b61e0, 0x0)
	/go/src/github.com/openshift/ci-operator/pkg/steps/run.go:105 +0x92
created by github.com/openshift/ci-operator/pkg/steps.Run
	/go/src/github.com/openshift/ci-operator/pkg/steps/run.go:71 +0x3a1

@crawford
Copy link
Contributor

crawford commented Nov 4, 2018

/retest

@crawford
Copy link
Contributor

crawford commented Nov 4, 2018

Error: module "masters": "private_endpoints" is not a valid argument

@wking wking force-pushed the collapse-private-and-public-load-balancer-target-groups branch from 24b53c6 to 920cca4 Compare November 4, 2018 18:49
@wking
Copy link
Member Author

wking commented Nov 4, 2018

Error: module "masters": "private_endpoints" is not a valid argument

Fixed with 24b53c6 -> 920cca4.

data/data/aws/vpc/outputs.tf Outdated Show resolved Hide resolved
The listeners may be private or public, but as far as the bootstrap
and master modules are concerned, these are just lists of ARNs that
need attaching.  This partially unwinds changes from 16dfbb3
(data/aws: use nlbs instead of elbs, 2018-01-01, openshift#594), before which
the bootstrap module used a unified list of load balancers while the
master module made private/public distinctions.

While I'm touching variables.tf, I've alphabetized the dns_server_ip
and kubeconfig_content entries as well.
@wking wking force-pushed the collapse-private-and-public-load-balancer-target-groups branch from 920cca4 to 7931855 Compare November 5, 2018 03:13
@wking
Copy link
Member Author

wking commented Nov 5, 2018

All green :)

@crawford
Copy link
Contributor

crawford commented Nov 5, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 5, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, wking

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-merge-robot openshift-merge-robot merged commit 39523ae into openshift:master Nov 5, 2018
@wking wking deleted the collapse-private-and-public-load-balancer-target-groups branch November 5, 2018 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants