-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Enable arm64 Azure UPI tests and initial test config for QE e2e tests #35520
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
Enable arm64 Azure UPI tests and initial test config for QE e2e tests #35520
Conversation
|
@aleskandro: the
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/hold |
af51d52 to
0adc64d
Compare
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.13-arm64-nightly-azure-upi-p3-f28 |
5b66db8 to
4fddd29
Compare
|
/pj-rehearse refresh |
|
@aleskandro: the
A total of 37 jobs have been affected by this change. The above listing is non-exhaustive and limited to 35 jobs. A full list of affected jobs can be found here |
|
/pj-rehearse |
|
/unhold |
4fddd29 to
9e9bb70
Compare
|
/pj-rehearse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: maybe COMPUTE_INSTANCE_TYPE for consistence with the others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that COMPUTE_NODE_TYPE is already defined in other steps of the involved workflows, while the others aren't . I was unsure to do the contrary but also I'd like that we use the same naming across cloud providers. So, (a) COMPUTE_NODE_TYPE is used in some other steps related to Azure, (b) the others are the ones used by AWS UPI.
I'm ok with any solution here: ideally having more consistency would be great. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel strongly about this, it was just a nitpick. I didn't know about the usage in other files. I think it would be good to keep some consistency at least in the Azure steps/files, so maybe {BOOTSTRAP/CONTROL_PLANE}_NODE_TYPE is an Ok-ish compromise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
ci-operator/step-registry/upi/install/azure/upi-install-azure-ref.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option:
az_deployment_bootstrap_instance=""
if [ -n "${BOOTSTRAP_INSTANCE_TYPE}" ]; then
az_deployment_bootstrap_instance="--parameter bootstrapVMSize=${BOOTSTRAP_INSTANCE_TYPE}"
fi
[...]
--parameters baseName="$INFRA_ID" ${az_deployment_bootstrap_instance} ${az_deployment_optional_parameters}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have avoided the command substitution as well as the definition of a new var. Let me see and propose something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As disscussed offline, changed the len > 0 condition to -n "$...
- Introduce node size variables - Change the OCP_ARCH logic to be consistent with other uses in the repo's steps - Handle the OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE variable
9e9bb70 to
fdd02ce
Compare
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.10-amd64-nightly-azure-upi-p3-f28 |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.13-arm64-nightly-azure-upi-p3-f28 |
|
@aleskandro, |
fdd02ce to
a769c4d
Compare
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.10-amd64-nightly-azure-upi-p3-f28 |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.13-arm64-nightly-azure-upi-p3-f28 |
|
/pj-rehearse pull-ci-openshift-installer-master-e2e-azure-ovn-upi |
|
Install phase succeeded for |
|
/assign @patrickdillon |
|
The QE jobs' installation steps look good as well |
|
/approve |
1 similar comment
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aleskandro, patrickdillon, r4f4, smg247 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse ack |
|
@aleskandro: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
@aleskandro: Updated the following 2 configmaps:
DetailsIn response to this:
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#35520) * Enable support for arm64 in the upi-install-azure step - Introduce node size variables - Change the OCP_ARCH logic to be consistent with other uses in the repo's steps - Handle the OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE variable * Initial test config for the arm64 Azure UPI QE tests
This PR enables the support for arm64 in the upi-install-azure step as a follow up to openshift/installer#6684 and adds an initial config for the QE tests.
The installation succeeded here.