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

WIP: Verify that RHCOS major version matches expected #2532

Closed
wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Member

This is just a draft, proposal untested WIP.

I've seen people in the past trying to use the wrong RHCOS
bootimages. I'd like to make that a clean, clear error.
Various weird things can go wrong if e.g. one tries to use
a RHCOS 4.2 bootimage for 4.1 installer.

I'd also like to change this code at least warn if
the bootimage doesn't exactly match what's pinned in the
installer.

Cleaning this up will need to plumb through the pinned
install data into this script.

Also we'll need to add an environment variable like
OPENSHIFT_INSTALL_ALLOW_OS_IMAGE_OVERRIDE which
makes this script not error, so that one can easily
test new RHCOS versions without rebuilding/hacking the
installer.

This is just a draft, proposal untested WIP.

I've seen people in the past trying to use the wrong RHCOS
bootimages.  I'd like to make that a clean, clear error.
Various weird things can go wrong if e.g. one tries to use
a RHCOS 4.2 bootimage for 4.1 installer.

I'd also like to change this code at least *warn* if
the bootimage doesn't *exactly* match what's pinned in the
installer.

Cleaning this up will need to plumb through the pinned
install data into this script.

Also we'll need to add an environment variable like
`OPENSHIFT_INSTALL_ALLOW_OS_IMAGE_OVERRIDE` which
makes this script not error, so that one can easily
test new RHCOS versions without rebuilding/hacking the
installer.
@cgwalters
Copy link
Member Author

/hold

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 18, 2019
@cgwalters cgwalters changed the title Verify that RHCOS major version matches expected WIP: Verify that RHCOS major version matches expected Oct 18, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cgwalters
To complete the pull request process, please assign wking
You can assign the PR to them by writing /assign @wking in a comment when ready.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cgwalters
Copy link
Member Author

And this should probably be a separate service that doesn't go in a retry loop, so we fail fast.

@abhinavdahiya
Copy link
Contributor

I'd also like to change this code at least warn if
the bootimage doesn't exactly match what's pinned in the
installer.

that's not advisable or feasible for UPI customers.

I've seen people in the past trying to use the wrong RHCOS
bootimages. I'd like to make that a clean, clear error.
Various weird things can go wrong if e.g. one tries to use
a RHCOS 4.2 bootimage for 4.1 installer.

personally until we support in cluster component that managed the bootimages, this is not the correct stance to make.

@cgwalters
Copy link
Member Author

personally until we support in cluster component that managed the bootimages, this is not the correct stance to make.

This is about the bootstrap image - you're talking about openshift/os#381 which is distinct.

Now, I could imagine that e.g. we could also pivot the bootstrap image. That would make the bootstrap case symmetrical.

But it absolutely doesn't work today to use e.g. the 4.1 bootimage with 4.2 installer (due to crio.service changes at least). So I think it's better to make the code reflect reality.

@cgwalters
Copy link
Member Author

See openshift/enhancements#78 (comment) which would obviate this.

@abhinavdahiya
Copy link
Contributor

Closing due to this being open for a long time, Please feel free to reopen

/close

@openshift-ci-robot
Copy link
Contributor

@abhinavdahiya: Closed this PR.

In response to this:

Closing due to this being open for a long time, Please feel free to reopen

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants