Skip to content

e2e-pool: Plumb in BASE_DOMAIN#2083

Merged
2uasimojo merged 1 commit into
openshift:masterfrom
2uasimojo:HIVE-2259/default-basedomain-pool
Aug 11, 2023
Merged

e2e-pool: Plumb in BASE_DOMAIN#2083
2uasimojo merged 1 commit into
openshift:masterfrom
2uasimojo:HIVE-2259/default-basedomain-pool

Conversation

@2uasimojo
Copy link
Copy Markdown
Member

@2uasimojo 2uasimojo commented Aug 9, 2023

hiveutil still defaults the base domain to new-installer.openshift.com, which is probably what we want since we (hive engineers) are the ones invoking it by hand, using the hive-team cluster as a hub, and will thus prefer less typing.

But now that we're switching to a new AWS account for CI, we want the AWS-based e2e suites to use the new base domain. We recognize and default a $BASE_DOMAIN variable for e2e itself, but weren't yet plumbing it through to e2e-pool. Fix.

HIVE-2259

hiveutil still defaults the base domain to new-installer.openshift.com,
which is probably what we want since we (hive engineers) are the ones
invoking it by hand, using the hive-team cluster as a hub, and will thus
prefer less typing.

But now that we're switching to a new AWS account for CI, we want the
AWS-based e2e suites to use the new base domain. We recognize and
default a `$BASE_DOMAIN` variable for e2e itself, but weren't yet
plumbing it through to e2e-pool. Fix.

HIVE-2259
@2uasimojo
Copy link
Copy Markdown
Member Author

/assign @dlom

Comment thread hack/e2e-pool-test.sh
${REGION_ARG} \
${CREDS_FILE_ARG} \
--pull-secret-file="${PULL_SECRET_FILE}" \
--base-domain="${CLUSTER_DOMAIN}" \
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ntr: CLUSTER_DOMAIN is set based on BASE_DOMAIN depending whether we're using managed DNS or not

@openshift-ci openshift-ci Bot requested review from dlom and suhanime August 9, 2023 13:49
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 9, 2023
@dlom
Copy link
Copy Markdown
Contributor

dlom commented Aug 9, 2023

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 9, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, dlom

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:

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
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD a0c10c8 and 2 for PR HEAD 5bbb192 in total

@2uasimojo
Copy link
Copy Markdown
Member Author

Well the plumbing is working.

But somehow we're enabling managed DNS, despite explicitly disabling it in the test config.

Actually, now that I look at it, we should be exporting that sucker. Fix on the way...

openshift/release#42172

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD ed65ea1 and 1 for PR HEAD 5bbb192 in total

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD 7461d16 and 0 for PR HEAD 5bbb192 in total

@2uasimojo
Copy link
Copy Markdown
Member Author

Of note: it doesn't look like we have a way to make clusterpool clusters use managed DNS. I'm not completely sure there's a use case for that in the first place, but I thought it was interesting.

@openshift-ci-robot
Copy link
Copy Markdown

/hold

Revision 5bbb192 was retested 3 times: holding

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 10, 2023
@2uasimojo
Copy link
Copy Markdown
Member Author

Actually, now that I look at it, we should be exporting that sucker. Fix on the way...

test e2e-pool

@2uasimojo
Copy link
Copy Markdown
Member Author

I mean

/test e2e-pool

@2uasimojo
Copy link
Copy Markdown
Member Author

also

/hold cancel

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 11, 2023
@2uasimojo
Copy link
Copy Markdown
Member Author

Okay, I'm sick of this -- infra flake on cleanup after the test succeeded.

/override ci/prow/e2e-pool

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 11, 2023

@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/e2e-pool

Details

In response to this:

Okay, I'm sick of this -- infra flake on cleanup after the test succeeded.

/override ci/prow/e2e-pool

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.

@2uasimojo 2uasimojo merged commit edebb8a into openshift:master Aug 11, 2023
@2uasimojo 2uasimojo deleted the HIVE-2259/default-basedomain-pool branch August 11, 2023 22:07
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 11, 2023

@2uasimojo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-pool 5bbb192 link unknown /test e2e-pool

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

2uasimojo added a commit to 2uasimojo/release that referenced this pull request Aug 15, 2023
Switch to the new AWS account which will be subject to the pruner.

Because this account is pruned automatically, remove commentary about
our bespoke one.

We changed the default BASE_DOMAIN in the test scripts themselves via
openshift/hive#2081 and
openshift/hive#2083, so it is removed from the
configuration.

HIVE-2259
openshift-merge-robot pushed a commit to openshift/release that referenced this pull request Aug 22, 2023
Switch to the new AWS account which will be subject to the pruner.

Because this account is pruned automatically, remove commentary about
our bespoke one.

We changed the default BASE_DOMAIN in the test scripts themselves via
openshift/hive#2081 and
openshift/hive#2083, so it is removed from the
configuration.

HIVE-2259
prb112 pushed a commit to prb112/openshift-release that referenced this pull request Sep 12, 2023
Switch to the new AWS account which will be subject to the pruner.

Because this account is pruned automatically, remove commentary about
our bespoke one.

We changed the default BASE_DOMAIN in the test scripts themselves via
openshift/hive#2081 and
openshift/hive#2083, so it is removed from the
configuration.

HIVE-2259
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants