-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Create ImageContentSourcePolicy for disconnected upgrade #25130
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
Create ImageContentSourcePolicy for disconnected upgrade #25130
Conversation
031aa17 to
4cfc9cc
Compare
|
/test ci/rehearse/openshift/metallb/release-4.10/e2e-metal-ipi-upgrade-ovn-ipv6 |
|
@ardaguclu: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
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/test-infra repository. |
|
/uncc @smg247 @petr-muller You'll need approvals from ci-operator/config/openshift/cluster-baremetal-operator/OWNERS |
e0820ca to
75ffa0b
Compare
1702c65 to
a0ae011
Compare
|
Changes look good to me, anyhow as discussed in another thread with @wking I think that moving also the upgrade release mirroring code in dev-scripts would be beneficial (to be done into another PR): all the mirroring code would stay in one place, and the task could be addressed during the setup step (and not during the test step, as it happens now). |
Thanks @andfasano, it is a very good point and I agree with that. After that PR, we can move all mirroring in dev-scripts. |
|
periodic-ci-openshift-release-master-nightly-4.10-e2e-metal-ipi-upgrade-ovn-ipv6 failed in setup stage which is not related to the code changes of this PR. |
|
/assign @andfasano could you please take a look?, if you'd like to see upgrade-ovn-ipv6 result, we can retest one more time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we keep this echo? I guess it would be useful when troubleshooting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not overriding image anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using actual image as upgrade and it is automatically mirrored to our registry via ImageContentSourcePolicy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the canonical env var name used for injecting the release:latest pullspec and still used:
oc adm release mirror --registry-config ${DS_WORKING_DIR}/pull_secret.json \
--from=${OPENSHIFT_UPGRADE_RELEASE_IMAGE_OVERRIDE} \
The echo will be just a tip in the log to specify to which version we're currently upgrading to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But since we are not overriding, it is in;
openshift-tests run-upgrade all --to-image registry.build03.ci.openshift.org/ci-op-3g123y5y/release@sha256:5d19aec75540e8056cca0ab3ab1ee9c4098c0a703b1264ded57a477cae0c496e --provider '{"type":"baremetal","disconnected":true}' --from-repository virthost.ostest.test.metalkube.org:5000/localimages/local-test-image -o /logs/artifacts/e2e.log --junit-dir /logs/artifacts/junit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the var name it's a bit unfortunate, anyhow I noticed that the info I was really looking for is also reported in a better way later, ie:
"desired": {
"version": "4.10.0-0.ci.test-2022-01-12-125752-ci-op-3g123y5y-latest",
"image": "registry.build03.ci.openshift.org/ci-op-3g123y5y/release@sha256:5d19aec75540e8056cca0ab3ab1ee9c4098c0a703b1264ded57a477cae0c496e"
}
ci-operator/step-registry/baremetalds/e2e/test/baremetalds-e2e-test-commands.sh
Outdated
Show resolved
Hide resolved
a0ae011 to
73b7dce
Compare
|
Both upgrade-ovn-ipv6 jobs passed; |
In disconnected upgrades, namely e2e-metal-ipi-upgrade-ovn-ipv6, release image is mirrored to be used, however since ImageContentSourcePolicy is not created, pods get image pull errors and upgrade fails in presubmit jobs. Upgrade passes in periodics because from and to releases use same registry. This PR adds ImageContentSourcePolicy.
ce2d1b6 to
71b29a1
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andfasano, ardaguclu 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 |
|
@ardaguclu: 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/test-infra repository. I understand the commands that are listed here. |
|
@ardaguclu: Updated the
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/test-infra repository. |
In disconnected upgrades, namely e2e-metal-ipi-upgrade-ovn-ipv6,
release image is mirrored to be used, however since ImageContentSourcePolicy
is not created, pods get image pull errors and upgrade fails in presubmit jobs.
Upgrade passes in periodics because from and to releases use same registry.
This PR adds ImageContentSourcePolicy.