Remove baseDomain for azure #69970
Remove baseDomain for azure #69970
Conversation
openshift-ci
bot
added
the
approved
|
fixing the config-metadata check |
deepsm007
force-pushed
the
azure-basedomain-fix
branch
from
ae9ef22 to
c9d359f
Compare
|
/pj-rehearse |
|
@deepsm007: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
|
||
| if [[ ! -r "${CLUSTER_PROFILE_DIR}/baseDomain" ]]; then | ||
| echo "Using default value: ${BASE_DOMAIN}" | ||
| AZURE_BASE_DOMAIN="${BASE_DOMAIN}" |
There was a problem hiding this comment.
This feels backwards to me. I'd expect most jobs that requested a custom base domain to have that ask respected, regardless of a cluster-profile-level default. And if the job didn't have a custom preference, it would fall back to the cluster-profile-level default. And that most (all?) jobs wouldn't have a custom preference. If that sounds good, something like:
if [[ -n "${BASE_DOMAIN}" ]]; then
echo "Using default value: ${BASE_DOMAIN}"
AZURE_BASE_DOMAIN="${BASE_DOMAIN}"
elif [[ -f "${CLUSTER_PROFILE_DIR}/baseDomain" ]; then
AZURE_BASE_DOMAIN=$(< ${CLUSTER_PROFILE_DIR}/baseDomain)
else
echo "Neither BASE_DOMAIN nor cluster profile baseDomain set, but this step requires a configured base domain." >&2
exit 1
fi
There was a problem hiding this comment.
i would actually suggest to use -n than -r since not readable is another error (it should not happen)
| documentation: |- | ||
| The architecture of the control plane nodes (e.g., amd64, arm64). | ||
| - name: BASE_DOMAIN | ||
| default: ci.azure.devcluster.openshift.com |
There was a problem hiding this comment.
If you buy this argument, you'll want to drop the default here, or set it equal to the empty string like SIZE_VARIANT and some of the other variables in this file.
There was a problem hiding this comment.
I set similar expectations for AWS, we can discuss more on this next week. If this isnt blocking we can merge hoping azure issue will be relieved
|
@deepsm007: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
/pj-rehearse ack |
|
@deepsm007: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-ci-robot
added
the
rehearsals-ack
|
|
||
| if [[ ! -r "${CLUSTER_PROFILE_DIR}/baseDomain" ]]; then | ||
| echo "Using default value: ${BASE_DOMAIN}" | ||
| AZURE_BASE_DOMAIN="${BASE_DOMAIN}" |
There was a problem hiding this comment.
i would actually suggest to use -n than -r since not readable is another error (it should not happen)
deepsm007
force-pushed
the
azure-basedomain-fix
branch
from
c9d359f to
aa170cf
Compare
deepsm007
force-pushed
the
azure-basedomain-fix
branch
from
aa170cf to
7ef097f
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deepsm007, pruan-rht 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 |
openshift-ci-robot
removed
the
rehearsals-ack
|
[REHEARSALNOTIFIER]
A total of 3925 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
[REHEARSALNOTIFIER]
A total of 3925 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
openshift-merge-bot
bot
merged commit 80bf8e8
into
openshift:master
5 participants
https://redhat.enterprise.slack.com/archives/CBN38N3MW/p1759526570267999?thread_ts=1759502756.084789&channel=CBN38N3MW&message_ts=1759526570.267999
https://issues.redhat.com/browse/DPTP-4566
/cc @openshift/test-platform @wking