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

MCO-392: Start using rhel-coreos image rather than machine-os-content #135

Merged
merged 1 commit into from Nov 9, 2023

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Sep 27, 2023

The machine-os-content is deprecated and we'd like to stop shipping it entirely in the release payload. Point the DTK imagestream to the rhel-coreos image instead.

Closes: #101

The `machine-os-content` is deprecated and we'd like to stop shipping it
entirely in the release payload. Point the DTK imagestream to the `rhel-
coreos` image instead.

Closes: openshift#101
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 27, 2023
@openshift-ci
Copy link

openshift-ci bot commented Sep 27, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jlebon
Copy link
Member Author

jlebon commented Sep 27, 2023

This would go at the same time as openshift/os#1374.
We need to make sure DTK, ART, and RHCOS teams are aligned before doing this change.

@ybettan
Copy link
Contributor

ybettan commented Sep 28, 2023

Thanks for this PR @jlebon.

Is openshift/os#1374 adding the "io.openshift.build.versions": "machine-os=.." label to the rhel-coreos container?

Do we need anything else to make it propagate into the payload or it is happening automatically.

If that's all we need I am good with merging.

@ybettan
Copy link
Contributor

ybettan commented Sep 28, 2023

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 28, 2023
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 28, 2023
@openshift-ci
Copy link

openshift-ci bot commented Sep 28, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlebon, travier, ybettan

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

@LorbusChris
Copy link
Member

This will require stopping driver-toolkit image mirroring first, as the release controller will otherwise barf when there's an unknown image (rhel-coreos does not exist in OKD imagestreams): openshift/release#43816

@LorbusChris
Copy link
Member

@jlebon
Copy link
Member Author

jlebon commented Oct 26, 2023

Also note that the machine-os-content ref is also still used by the installer: openshift/installer@c723ac8/data/data/bootstrap/files/usr/local/bin/bootkube.sh.template#L46

Opened openshift/installer#7636!

@ybettan
Copy link
Contributor

ybettan commented Nov 7, 2023

@LorbusChris Do we need anything else before undrafting this PR and merging it?

@jlebon
Copy link
Member Author

jlebon commented Nov 7, 2023

@LorbusChris Do we need anything else before undrafting this PR and merging it?

We were waiting on ART bits to catch up, but looks like we might be there. Discussions in openshift-eng/ocp-build-data#3851.

@ybettan
Copy link
Contributor

ybettan commented Nov 8, 2023

Great.
Please undraft the PR so we make sure CI passes.

/hold
In order to sync the merge with other PRs required.

@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 Nov 8, 2023
@jlebon jlebon marked this pull request as ready for review November 8, 2023 14:21
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 8, 2023
@jlebon
Copy link
Member Author

jlebon commented Nov 8, 2023

OK as expected, CI is failing with:

 error: failed to push image registry.build05.ci.openshift.org/ci-op-1p5hwli8/release:latest: unable to upload new layer (0): Patch "https://registry.build05.ci.openshift.org/v2/ci-op-1p5hwli8/release/blobs/uploads/c58b18af-367c-4f7e-9749-17e89f6f1635?_state=Hj3H_lI5ysPOLpl-oZTVM9_QqDZM8oTjQv0x9aPIvLl7Ik5hbWUiOiJjaS1vcC0xcDVod2xpOC9yZWxlYXNlIiwiVVVJRCI6ImM1OGIxOGFmLTM2N2MtNGY3ZS05NzQ5LTE3ZTg5ZjZmMTYzNSIsIk9mZnNldCI6MCwiU3RhcnRlZEF0IjoiMjAyMy0xMS0wOFQxNDoyOToxMy42NDcxMDI0ODZaIn0%3D": unknown version reference "machine-os" 

So I think we're ready here to merge them all together!

openshift/os#1374 is ready to merge, so if you agree and override the CI failures to get this in, I'll get that one in too.

@jlebon jlebon changed the title Start using rhel-coreos image rather than machine-os-content MCO-392: Start using rhel-coreos image rather than machine-os-content Nov 8, 2023
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 8, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 8, 2023

@jlebon: This pull request references MCO-392 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.15.0" version, but no target version was set.

In response to this:

The machine-os-content is deprecated and we'd like to stop shipping it entirely in the release payload. Point the DTK imagestream to the rhel-coreos image instead.

Closes: #101

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.

@jlebon
Copy link
Member Author

jlebon commented Nov 8, 2023

Actually, we went ahead and merged openshift/os#1374. This should be good to go too now!

@sosiouxme
Copy link
Member

ART bits are merged too

@ybettan
Copy link
Contributor

ybettan commented Nov 9, 2023

/retest

@ybettan
Copy link
Contributor

ybettan commented Nov 9, 2023

/unhold

@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 Nov 9, 2023
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD c410e9f and 2 for PR HEAD 06237b7 in total

@jlebon
Copy link
Member Author

jlebon commented Nov 9, 2023

Will we actually be able to achieve green CI here or do we have to force merge it?

I do see we have an RHCOS build now with openshift/os#1374 (415.92.202311082221-0). It was pushed as quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:12ff620fd1eebe68c006cbe0b971ed570a0d304c5992048b898c6be176dad6eb. But I'm not familiar enough with OpenShift CI internals to know if that'll end up in the right place on its own for CI to pick it up here or if e.g. it'll fail to get there anyway until this PR is forced in.

@jlebon
Copy link
Member Author

jlebon commented Nov 9, 2023

I do see we have an RHCOS build now with openshift/os#1374 (415.92.202311082221-0). It was pushed as quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:12ff620fd1eebe68c006cbe0b971ed570a0d304c5992048b898c6be176dad6eb.

BTW, I can confirm that image has the expected labels:

$ skopeo inspect -n docker://quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:12ff620fd1eebe68c006cbe0b971ed570a0d304c5992048b898c6be176dad6eb  | grep io.openshift.build
        "io.openshift.build.version-display-names": "machine-os=Red Hat Enterprise Linux CoreOS",
        "io.openshift.build.versions": "machine-os=415.92.202311082221-0",

@sosiouxme
Copy link
Member

/retest

rhel-coreos image updated for CI

@sosiouxme
Copy link
Member

/retest
after removing machine-os-content

@ybettan
Copy link
Contributor

ybettan commented Nov 9, 2023

/override ci/prow/e2e-aws
/override ci/prow/e2e-aws-driver-toolkit-presubmi
/override ci/prow/e2e-upgrade
/override ci/prow/images

https://redhat-internal.slack.com/archives/CB95J6R4N/p1699558337383699?thread_ts=1699386359.814909&cid=CB95J6R4N

@ybettan
Copy link
Contributor

ybettan commented Nov 9, 2023

/override ci/prow/images

Copy link

openshift-ci bot commented Nov 9, 2023

@ybettan: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

  • ci/prow/e2e-aws-driver-toolkit-presubmi

Only the following failed contexts/checkruns were expected:

  • ci/prow/check-commits-count
  • ci/prow/e2e-aws
  • ci/prow/e2e-aws-driver-toolkit-presubmit
  • ci/prow/e2e-upgrade
  • ci/prow/image-contents
  • ci/prow/images
  • ci/prow/verify
  • ci/prow/verify-imagestream
  • pull-ci-openshift-driver-toolkit-master-check-commits-count
  • pull-ci-openshift-driver-toolkit-master-e2e-aws
  • pull-ci-openshift-driver-toolkit-master-e2e-aws-driver-toolkit-presubmit
  • pull-ci-openshift-driver-toolkit-master-e2e-upgrade
  • pull-ci-openshift-driver-toolkit-master-image-contents
  • pull-ci-openshift-driver-toolkit-master-images
  • pull-ci-openshift-driver-toolkit-master-verify
  • pull-ci-openshift-driver-toolkit-master-verify-imagestream
  • tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override ci/prow/e2e-aws
/override ci/prow/e2e-aws-driver-toolkit-presubmi
/override ci/prow/e2e-upgrade
/override ci/prow/images

https://redhat-internal.slack.com/archives/CB95J6R4N/p1699558337383699?thread_ts=1699386359.814909&cid=CB95J6R4N

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.

Copy link

openshift-ci bot commented Nov 9, 2023

@ybettan: Overrode contexts on behalf of ybettan: ci/prow/images

In response to this:

/override ci/prow/images

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.

@ybettan
Copy link
Contributor

ybettan commented Nov 9, 2023

/override ci/prow/e2e-aws
/override ci/prow/e2e-aws-driver-toolkit-presubmit
/override ci/prow/e2e-upgrade

Copy link

openshift-ci bot commented Nov 9, 2023

@ybettan: Overrode contexts on behalf of ybettan: ci/prow/e2e-aws, ci/prow/e2e-aws-driver-toolkit-presubmit, ci/prow/e2e-upgrade

In response to this:

/override ci/prow/e2e-aws
/override ci/prow/e2e-aws-driver-toolkit-presubmit
/override ci/prow/e2e-upgrade

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.

Copy link

openshift-ci bot commented Nov 9, 2023

@jlebon: all tests passed!

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 86bbea7 into openshift:master Nov 9, 2023
9 checks passed
jlebon added a commit to jlebon/driver-toolkit that referenced this pull request Nov 14, 2023
…nt` (openshift#135)"

This reverts commit 86bbea7.

We need to revert the openshift/os side of this because of remaining
references to machine-os-content:

openshift/os#1393

So we have to revert this too.
@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

This PR has been included in build driver-toolkit-container-v4.15.0-202311150309.p0.g86bbea7.assembly.stream for distgit driver-toolkit.
All builds following this will include this PR.

openshift-merge-bot bot pushed a commit that referenced this pull request Nov 15, 2023
…nt` (#135)" (#136)

This reverts commit 86bbea7.

We need to revert the openshift/os side of this because of remaining
references to machine-os-content:

openshift/os#1393

So we have to revert this too.
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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please drop machine-os-content reference
7 participants