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

COS-2473: Re-enable ostree-container-inject-openshift-cvo-labels knob #1374

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Sep 27, 2023

This was previously enabled (#1048) and then disabled again (#1084) because oc doesn't know how to handle multiple images with those labels in the release payload. We'll need to solve this eventually if we want to be able to ship multiple OS images in the payload (that's tracked in #1047), but we don't need to block on this if we can remove the legacy machine-os-content at the same time.

See also: openshift/driver-toolkit#101
See also: openshift/machine-config-operator#3364

@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
Contributor

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 27, 2023
@jlebon
Copy link
Member Author

jlebon commented Sep 27, 2023

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

@cgwalters
Copy link
Member

I'm ok with trying to just change everything at once, but I think we need to give a heads up to stakeholders.

@jlebon
Copy link
Member Author

jlebon commented Sep 27, 2023

I'm ok with trying to just change everything at once, but I think we need to give a heads up to stakeholders.

Sounds good. Anyone other than those listed in the previous comment? I guess there's the MCO team too.

image-c9s.yaml Outdated Show resolved Hide resolved
@cgwalters
Copy link
Member

I rebased openshift/machine-config-operator#3364 FWIW, so I think we can get down to just two things that need to change at the same time, not 3.

This was previously enabled (openshift#1048) and then disabled again (openshift#1084)
because `oc` doesn't know how to handle multiple images with those
labels in the release payload. We'll need to solve this eventually if
we want to be able to ship multiple OS images in the payload (that's
tracked in openshift#1047), but we don't need to block on this if we can remove
the legacy `machine-os-content` at the same time.

See also: openshift/driver-toolkit#101
See also: openshift/machine-config-operator#3364
@jlebon jlebon force-pushed the pr/rhel-coreos-labels branch from 32e890c to 12a9a69 Compare September 27, 2023 20:11
@jlebon
Copy link
Member Author

jlebon commented Nov 8, 2023

/hold

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Nov 8, 2023
@openshift-ci openshift-ci bot requested review from cgwalters and ravanelli November 8, 2023 14:23
@cgwalters
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2023
Copy link
Contributor

openshift-ci bot commented Nov 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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:
  • OWNERS [cgwalters,jlebon,travier]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Contributor

openshift-ci bot commented Nov 8, 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.

@jlebon
Copy link
Member Author

jlebon commented Nov 8, 2023

Just so we don't spend too much time playing the coordination game, let's go ahead and merge this. It seems OK if 4.15 is rocky for a bit until the other PRs merge.

/hold cancel

@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 8, 2023
@jlebon jlebon changed the title Re-enable ostree-container-inject-openshift-cvo-labels knob COS-2473: Re-enable ostree-container-inject-openshift-cvo-labels knob 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

openshift-ci-robot commented Nov 8, 2023

@jlebon: This pull request references COS-2473 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:

This was previously enabled (#1048) and then disabled again (#1084) because oc doesn't know how to handle multiple images with those labels in the release payload. We'll need to solve this eventually if we want to be able to ship multiple OS images in the payload (that's tracked in #1047), but we don't need to block on this if we can remove the legacy machine-os-content at the same time.

See also: openshift/driver-toolkit#101
See also: openshift/machine-config-operator#3364

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-merge-bot openshift-merge-bot bot merged commit 8204d33 into openshift:master Nov 8, 2023
7 checks passed
jlebon added a commit to jlebon/os that referenced this pull request Mar 7, 2024
This was previously enabled (openshift#1048, openshift#1374) and then disabled again (openshift#1084, openshift#1393).

The last time we tried it, the issue was that there were some references
remaining in openshift/kubernetes and openshift/release. Those have been
cleaned up now:

openshift/release#49156
openshift/kubernetes#1805

So... third time's the charm!
jlebon added a commit to jlebon/os that referenced this pull request Mar 7, 2024
This was previously enabled (openshift#1048, openshift#1374) and then disabled again (openshift#1084, openshift#1393).

The last time we tried it, the issue was that there were some references
remaining in openshift/kubernetes and openshift/release. Those have been
cleaned up now:

openshift/release#49156
openshift/kubernetes#1805

So... third time's the charm!
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.

5 participants