MCO-1972: Removes OSImageURLConfig from the build controller#5424
MCO-1972: Removes OSImageURLConfig from the build controller#5424cheesesashimi wants to merge 6 commits intoopenshift:mainfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
/test unit verify e2e-gcp-op-ocl |
| BaseOSContainerImage: m.MachineConfig.Spec.OSImageURL, | ||
| BaseOSExtensionsContainerImage: m.MachineConfig.Spec.BaseOSExtensionsContainerImage, | ||
| // This value is purposely left empty because the ConfigMap does not actually | ||
| // populate this value. However, we want the hashing to be stable. |
There was a problem hiding this comment.
Note to reviewer: This might be a moot point if someone is upgrading from one OCP release to another, the hashes will change. However, that means that old images may get rebuilt in the process, which is undesirable.
cheesesashimi
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@cheesesashimi: This pull request references MCO-1972 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 story to target the "4.21.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
approved
|
/test e2e-gcp-op-ocl |
cheesesashimi
force-pushed
the
zzlotnik/osimageurl-redux
branch
from
49a39f6 to
3ffaec0
Compare
|
/test unit verify e2e-gcp-op-ocl |
|
/test e2e-gcp-op-ocl |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/test unit |
cheesesashimi
force-pushed
the
zzlotnik/osimageurl-redux
branch
from
adb5a66 to
b66544e
Compare
|
/retest-required |
|
Pre-merge verified: Environment Setup: Pre-requisites
Steps
|
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
/test verify |
Ensures that a cluster admin may only override the OSImageURL field or set the desired OSImageStream name; but not both. This ensures that either the cluster admin or the MCO will manage the OS image and prevents the MCO from overriding this setting.
cheesesashimi
force-pushed
the
zzlotnik/osimageurl-redux
branch
from
ce83ad7 to
6303082
Compare
|
/test e2e-gcp-op-techpreview |
|
/lgtm |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
Case1: Verify when osstream and osimageurl MC is applied the MCP is degraded
Case2: Verify the off-cluster and on-cluster layering case
MC templateoc create -f - << EOF
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
labels:
machineconfiguration.openshift.io/role: custom3
name: os-layer-custom3
spec:
osImageURL: "quay.io/mcoqe/layering@sha256:3760b74a58b882494c5e99d7191647822a2dfea43983cb5882887325b99327a7"
EOF
machineconfig.machineconfiguration.openshift.io/os-layer-custom3 created
OCL template$ sh ../ocl/create-long-lived-token.sh pull-copy
$ oc create -f - << EOF
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineOSConfig
metadata:
name: custom3
spec:
machineConfigPool:
name: custom3
imageBuilder:
imageBuilderType: Job
baseImagePullSecret:
name: pull-copy
renderedImagePushSecret:
name: $(oc get -n openshift-machine-config-operator sa builder -ojsonpath='{.secrets[0].name}')
renderedImagePushSpec: "image-registry.openshift-image-registry.svc:5000/openshift-machine-config-operator/ocb-image:latest"
containerFile:
- content: |-
RUN echo "This is from ON-CLUSTER layering" > /etc/test-onlayering.test
EOF
|
|
Case3: patch the osstream for OCL enabled bool and check the build is triggred.
/label qe-approved |
openshift-ci-robot
added
the
verified
|
@ptalgulk01: This PR has been marked as verified by 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
qe-approved
|
@cheesesashimi: This pull request references MCO-1972 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 story to target the "4.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/test e2e-aws-ovn |
|
/retest-required |
1 similar comment
|
/retest-required |
|
/test e2e-gcp-op-part2 |
1 similar comment
|
/test e2e-gcp-op-part2 |
|
/override ci/prow/e2e-gcp-op-part2 |
|
@pablintino: Overrode contexts on behalf of pablintino: ci/prow/e2e-gcp-op-part2 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-sigs/prow repository. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheesesashimi, pablintino 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 |
|
@pablintino: Overrode contexts on behalf of pablintino: ci/prow/e2e-gcp-op-part2, ci/prow/e2e-hypershift 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-sigs/prow repository. |
|
@cheesesashimi: 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-sigs/prow repository. I understand the commands that are listed here. |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. 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-sigs/prow repository. |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
5 participants
- What I did
This decouples the Build Controller from
OSImageURLConfigand makes the OSImageURL and BaseOSExtensionsImage fields on the rendered MachineConfig the source of truth for the base OS and extensions images to use for Image-Mode OpenShift. The idea is that if a different OS image is selected on a per-pool basis (e.g., one is RHEL9 and one is RHEL10 for dual-streams), then the Build Controller should use the appropriate source of truth for the appropriate pool.However, if one also sets the OSImageStream name on the MachineConfigPool and also sets OSImageURL on a MachineConfig, the MCO should degrade in this state because it would override value provided by the cluster admin. This PR also includes an E2E test which verifies that this is the case. This new E2E test will not be automatically ran until openshift/release#75329 is merged.
- How to verify it
The best way to verify this is to create a cluster and then create a MachineConfig which overrides the OSImageURL value. The Build Controller should build a new OS image based upon the new OSImageURL value.
- Description for the changelog
MachineConfigs should be the source of truth for the Build Controller