CORS-3462: use static openshift-installer when possible #1651
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @r4f4. Thanks for your PR. I'm waiting for a openshift-metal3 member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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
bot
added
the
needs-ok-to-test
|
/ok-to-test |
openshift-ci
bot
added
ok-to-test
|
/retitle CORS-3024,CORS-2797: use static openshift-installer when possible |
|
@r4f4: Re-titling can only be requested by trusted users, like repository collaborators. In 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. |
r4f4
changed the title
|
/retitle CORS-3462: use static openshift-installer when possible |
openshift-ci
bot
changed the title
common.sh
Outdated
| export OPENSHIFT_INSTALLER=${OPENSHIFT_INSTALLER:-${OCP_DIR}/openshift-baremetal-install} | ||
| # On 4.16+ the baremetall installer was merged into the regular installer | ||
| if is_lower_version $OPENSHIFT_VERSION 4.16; then | ||
| export OPENSHIFT_INSTALLER=${OPENSHIFT_INSTALLER:-${OCP_DIR}/openshift-baremetal-install} |
There was a problem hiding this comment.
We also have OPENSHIFT_INSTALLER_CMD used in a couple of places, but not here (probably because of dependencies).
I think we need to define a DEFAULT_OPENSHIFT_INSTALLER_CMD and set it on the basis of the release version, then use that as the binary name here and as the default for OPENSHIFT_INSTALLER_CMD in the 2 places where that is defined.
r4f4
force-pushed
the
use-static-installer
branch
2 times, most recently
from
14f440d
to
2576c9a
Compare
|
I wonder if this actually works. The CI fails with This file uses variable called OPENSHIFT_INSTALLER. I'm not sure why @zaneb suggested _CMD suffix, but we now need to update a lot more places. |
|
/retest-required @r4f4 in general the changes look fine to me, leaving the final approval to the metal folks since it will impact mostly metal jobs |
common.sh
Outdated
| @@ -174,6 +174,12 @@ fi | |||
|
|
|||
| export OPENSHIFT_RELEASE_TAG=$(echo $OPENSHIFT_RELEASE_IMAGE | sed -E 's/[[:alnum:]\/.-]*(release|okd).*://') | |||
|
|
|||
| # On 4.16+ the baremetall installer was merged into the regular installer | |||
| export DEFAULT_OPENSHIFT_INSTALLER_CMD="openshift-install" | |||
| if is_lower_version ${OPENSHIFT_VERSION:-""} 4.16; then | |||
There was a problem hiding this comment.
OPENSHIFT_VERSION is empty in the CI run
++(common.sh:179): source(): is_lower_version '' 4.16
+++(common.sh:34): is_lower_version(): echo ' 4.16'
+++(common.sh:34): is_lower_version(): tr ' ' '\n'
+++(common.sh:34): is_lower_version(): sort -V
+++(common.sh:34): is_lower_version(): head -n1
++(common.sh:34): is_lower_version(): [[ '' != 4.16 ]]
++(common.sh:35): is_lower_version(): return 0
++(common.sh:180): source(): export DEFAULT_OPENSHIFT_INSTALLER_CMD=openshift-baremetal-install
++(common.sh:180): source(): DEFAULT_OPENSHIFT_INSTALLER_CMD=openshift-baremetal-install
There was a problem hiding this comment.
Any suggestions on how to get the release version so it works also in CI?
There was a problem hiding this comment.
Ops! It's the lowercase function openshift_version that returns the current payload version
There was a problem hiding this comment.
Then there's existing code that's wrong:
Line 206 in 6f67488
r4f4
force-pushed
the
use-static-installer
branch
4 times, most recently
from
9809b0c
to
8a645a6
Compare
With openshift/installer#8161, the baremetal installer is now part of the regular installer which is now statically linked. Using the static binary has the advantage of working on both CS8 and CS9 VMs and is a necessary step for moving the Installer images to a RHEL9 base.
r4f4
force-pushed
the
use-static-installer
branch
from
8a645a6
to
454c29c
Compare
|
@r4f4: 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/test-infra repository. I understand the commands that are listed here. |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
/close |
|
@zaneb: Closed this PR. In 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. |
With openshift/installer#8161, the baremetal installer is now part of the regular installer which is now statically linked.
Using the static binary has the advantage of working on both CS8 and CS9 VMs and is a necessary step for moving the Installer images to a RHEL9 base.