Bug 1945026: Changing the name to make OSBS auto repo/registry replacements to work #104
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: LalatenduMohanty 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 |
openshift-ci-robot
added
the
approved
LalatenduMohanty
force-pushed
the
change_operand_name
branch
2 times, most recently
from
16146d2 to
777fadb
Compare
LalatenduMohanty
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
Member
Author
|
The PR fixes https://bugzilla.redhat.com/show_bug.cgi?id=1945026 |
Member
Author
/test operator-e2e |
wking
changed the title
openshift-ci-robot
added
bugzilla/severity-high
|
@LalatenduMohanty: This pull request references Bugzilla bug 1945026, which is invalid:
Comment 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. |
Member
|
/bugzilla refresh |
openshift-ci-robot
added
bugzilla/valid-bug
|
@wking: This pull request references Bugzilla bug 1945026, which is valid. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Bugzilla ([email protected]), skipping review request. 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. |
Member
|
e2e: I think we have a ticket somewhere about bubbling up the Deployment status. Poking around now in assets to see if we include it there... |
Member
Author
|
Tested the PR and it works as expected now. It was able to pull in the operand image successfully. |
openshift-merge-robot
pushed a commit
to openshift/release
that referenced
this pull request
Apr 5, 2021
From [1]:
> Before OSBS can pin your pullspecs, it first needs to find
> them. Because it is practically impossible to tell if a string is a
> pullspec, atomic-reactor has a predefined set of locations where it
> will look for pullspecs.
>
> 1. metadata.annotations.containerImage anywhere in the file
> jq: .. | .metadata?.annotations.containerImage | select(. != null)
>
> 2. All containers in each deployment
> jq: .spec.install.spec.deployments[].spec.template.spec.containers[]
>
> 3. All initContainers in each deployment
> jq: .spec.install.spec.deployments[].spec.template.spec.initContainers[]
>
> 4. All RELATED_IMAGE_* variables for all containers and initContainers
> jq: .env[] | select(.name | test("RELATED_IMAGE_")) for each of [2], [3]
>
> 5. All pullspecs from all annotations. This is done heuristically
> (OSBS needs to guess what might be a pullspec). See heuristic
> annotations below...
This change allows us to pivot to the approach from (4) for 4.6 and
later [2,3].
[1]: https://osbs.readthedocs.io/en/latest/users.html#pullspec-locations
[2]: openshift/cincinnati-operator#104
[3]: https://bugzilla.redhat.com/show_bug.cgi?id=1945026
Member
Author
|
/retest |
Member
Author
|
/cherrypick release-4.6 |
|
@LalatenduMohanty: once the present PR merges, I will cherry-pick it on top of release-4.6 in a new PR and assign it to you. 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. |
Member
|
Do we need to get any/all of these too? $ git --no-pager grep 'OPERATOR_IMAGE\|OPERAND_IMAGE' origin/pr/104
origin/pr/104:config/olm-catalog/updateservice-operator/1.0.0/updateservice-operator.v1.0.0.clusterserviceversion.yaml: - name: OPERAND_IMAGE
origin/pr/104:hack/deploy.sh:# DEFAULT_OPERATOR_IMAGE is a placeholder for cincinnati-operator image placeholder
origin/pr/104:hack/deploy.sh:DEFAULT_OPERATOR_IMAGE="controller:latest"
origin/pr/104:hack/deploy.sh:DEFAULT_OPERAND_IMAGE="quay.io/app-sre/cincinnati:2873c6b"
origin/pr/104:hack/deploy.sh:OPERATOR_IMAGE="${RELATED_IMAGE_OPERATOR:-${DEFAULT_OPERATOR_IMAGE}}"
origin/pr/104:hack/deploy.sh:OPERAND_IMAGE="${RELATED_IMAGE_OPERAND:-${DEFAULT_OPERAND_IMAGE}}"
origin/pr/104:hack/deploy.sh: OPERATOR_IMAGE="registry.svc.ci.openshift.org/${OPENSHIFT_BUILD_NAMESPACE}/stable:updateservice-operator"
origin/pr/104:hack/deploy.sh: echo "Openshift CI detected, deploying using image $OPERATOR_IMAGE and ${GRAPH_DATA_IMAGE}"
origin/pr/104:hack/deploy.sh:sed -i "s|quay.io/cincinnati/cincinnati:latest|$OPERAND_IMAGE|" config/manager/manager.yaml
origin/pr/104:hack/deploy.sh:sed -i "s|$DEFAULT_OPERATOR_IMAGE|$OPERATOR_IMAGE|" config/manager/manager.yaml
origin/pr/104:tools/create-catalog-source.sh: echo "Usage: $0 OPERATOR_IMAGE_TAG" >&2 |
Member
Author
Nope, those the variable names which do not impact the CSV files. |
Member
Author
|
/cherrypick release-4.7 |
|
@LalatenduMohanty: once the present PR merges, I will cherry-pick it on top of release-4.7 in a new PR and assign it to you. 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. |
LalatenduMohanty
force-pushed
the
change_operand_name
branch
from
777fadb to
1b98382
Compare
Refer https://osbs.readthedocs.io/en/latest/users.html#pullspec-locations for details Also changed OPERATOR_IMAGE to RELATED_IMAGE_OPERATOR for consistency Signed-off-by: Lalatendu Mohanty <[email protected]>
As we do not use it for the operator Signed-off-by: Lalatendu Mohanty <[email protected]>
LalatenduMohanty
force-pushed
the
change_operand_name
branch
from
1b98382 to
afbf71a
Compare
Member
|
/lgtm |
|
@LalatenduMohanty: All pull requests linked via external trackers have merged: Bugzilla bug 1945026 has been moved to the MODIFIED state. 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. |
|
@LalatenduMohanty: new pull request created: #108 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. |
|
@LalatenduMohanty: new pull request created: #109 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. |
wking
added a commit
to wking/openshift-release
that referenced
this pull request
Nov 3, 2023
…p-published-graph-data, etc. Moving to a recent Go builder, based on [1] and: $ oc -n ocp get -o json imagestream builder | jq -r '.status.tags[] | select(.items | length > 0) | .items[0].created + " " + .tag' | sort | grep golang ... 2023-11-02T19:53:15Z rhel-8-golang-1.18-openshift-4.11 2023-11-02T19:53:23Z rhel-8-golang-1.17-openshift-4.11 2023-11-02T20:49:19Z rhel-8-golang-1.19-openshift-4.13 2023-11-02T20:49:25Z rhel-9-golang-1.19-openshift-4.13 2023-11-02T21:54:25Z rhel-9-golang-1.20-openshift-4.14 2023-11-02T21:54:46Z rhel-8-golang-1.20-openshift-4.14 2023-11-02T21:55:24Z rhel-8-golang-1.19-openshift-4.14 2023-11-02T21:55:29Z rhel-9-golang-1.19-openshift-4.14 I'd tried dropping the build_root stanza, because we didn't seem to need the functionality it delivers [2]. But that removal caused failures like [3]: Failed to load CI Operator configuration" error="invalid ci-operator config: invalid configuration: when 'images' are specified 'build_root' is required and must have image_stream_tag, project_image or from_repository set" source-file=ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml And [2] docs a need for Git, which apparently the UBI images don't have. So I'm using a Go image here still, even though we don't need Go, and although that means some tedious bumping to keep up with RHEL and Go versions instead of floating. The operators stanza doc'ed in [4] remains largely unchanged, although I did rename 'cincinnati_operand_latest' to 'cincinnati-operand', because these tests use a single operand image, and there is no need to distinguish between multiple operand images with "latest". The image used for operator-sdk (which I bump to an OpenShift 4.14 base) and its use are doc'ed in [5]. The 4.14 cluster-claim pool I'm transitioning to is listed as healthy in [6]. For the end-to-end tests, we install the operator via the test suite, so we do not need the SDK bits. I've dropped OPERATOR_IMAGE, because we are well past the transition initiated by eae9d38 (ci-operator/config/openshift/cincinnati-operator: Set RELATED_IMAGE_*, 2021-04-05, openshift#17435) and openshift/cincinnati-operator@799d18525b (Changing the name to make OSBS auto repo/registry replacements to work, 2021-04-06, openshift/cincinnati-operator#104). I'm consistently using the current Cincinnati operand instead of the pinned one, because we ship the OpenShift Update Service Operator as a bundle with the operator and operand, and while it might be useful to grow update-between-OSUS-releases test coverage, we do not expect long durations of new operators coexisting with old-image operand pods. And we never expect new operators to touch Deployments with old operand images, except to bump them to new operand images. We'd been using digest-pinned operand images here since efcafb6 (ci-operator/config/openshift/cincinnati-operator: Move e2e-operator to multi-step, 2020-10-06, openshift#12486), where I said: In a future pivot we'll pull the operand image out of CI too, instead of hard-coding. But with this change we at least move the hard-coding into the CI repository. 4f46d7e (cincinnati-operator: test operator against released OSUS version and latest master, 2022-01-11, openshift#25152) brought in that floating operand image, but neglected, for reasons that I am not clear on, did not drop the digest-pinned operand. I'm dropping it now. With "which operand image" removed as a differentiator, the remaining differentiators for the end-to-end tests are: * Which host OpenShift? * To protect from "new operators require new platform capabilities not present in older OpenShift releases", we have an old-ocp job. It's currently 4.11 for the oldest supported release [7]. * To protect from "new operators still use platform capabilities that have been removed from development branches of OpenShift", we have a new-ocp job. It's currently 4.14, as the most modern openshift-ci pool in [6], but if there was a 4.15 openshift-ci pool I'd us that to ensure we work on dev-branch engineering candidates like 4.15.0-ec.1. * To protect against "HyperShift does something the operator does not expect", we have a hypershift job. I'd prefer to defer "which version?" to the workflow, because we do not expect HyperShift-specific difference to evolve much between 4.y releases, while the APIs used by the operator (Deployments, Services, Routes, etc.) might. But perhaps I'm wrong, and we will see more API evolution during HyperShift minor versions. And in any case, today 4.14 fails with [8]: Unable to apply 4.14.1: some cluster operators are not available so in the short term I'm going with 4.13, but with a generic name so we only have to bump one place as HyperShift support improves. * I'm not worrying about enumerating all the current 4.y options like we had done before. That is more work to maintain, and renaming required jobs confuses Prow and requires an /override of the removed job. It seems unlikely that we work on 4.old, break on some 4.middle, and work again on 4.dev. Again, we can always revisit this if we change our minds about the exposure. * Which graph-data? * To protect against "I updated my OSUS without changing the graph-data image, and it broke", we have published-graph-data jobs. These consume images that were built by previous postsubmits in the cincinnati-graph-data repository. * We could theoretically also add coverage for older forms of graph-data images we suspect customers might be using. I'm punting this kind of thing to possible future work, if we decide the exposure is significant enough to warrant ongoing CI coverage. * To allow testing new features like serving signatures, we have a local-graph-data job. This consumes a graph-data image built from steps in the operator repository, allowing convenient testing of changes that simultaneously tweak the operator and how the graph-data image is built. For example, [9] injects an image signature into graph-data, and updates graph-data to serve it. I'm setting a GRAPH_DATA environment variable to 'local' to allow the test suite to easily distinguish this case. [1]: https://docs.ci.openshift.org/docs/architecture/images/#ci-images [2]: https://docs.ci.openshift.org/docs/architecture/ci-operator/#build-root-image [3]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/45245/pull-ci-openshift-release-master-generated-config/1720218786344210432 [4]: https://docs.ci.openshift.org/docs/how-tos/testing-operator-sdk-operators/#building-operator-bundles [5]: https://docs.ci.openshift.org/docs/how-tos/testing-operator-sdk-operators/#simple-operator-installation [6]: https://docs.ci.openshift.org/docs/how-tos/cluster-claim/#existing-cluster-pools [7]: https://access.redhat.com/support/policy/updates/openshift/#dates [8]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/45245/rehearse-45245-pull-ci-openshift-cincinnati-operator-master-operator-e2e-hypershift-local-graph-data/1720287506777247744 [9]: openshift/cincinnati-operator#176
openshift-merge-bot bot
pushed a commit
to openshift/release
that referenced
this pull request
Nov 7, 2023
…p-published-graph-data, etc. (#45245) Moving to a recent Go builder, based on [1] and: $ oc -n ocp get -o json imagestream builder | jq -r '.status.tags[] | select(.items | length > 0) | .items[0].created + " " + .tag' | sort | grep golang ... 2023-11-02T19:53:15Z rhel-8-golang-1.18-openshift-4.11 2023-11-02T19:53:23Z rhel-8-golang-1.17-openshift-4.11 2023-11-02T20:49:19Z rhel-8-golang-1.19-openshift-4.13 2023-11-02T20:49:25Z rhel-9-golang-1.19-openshift-4.13 2023-11-02T21:54:25Z rhel-9-golang-1.20-openshift-4.14 2023-11-02T21:54:46Z rhel-8-golang-1.20-openshift-4.14 2023-11-02T21:55:24Z rhel-8-golang-1.19-openshift-4.14 2023-11-02T21:55:29Z rhel-9-golang-1.19-openshift-4.14 I'd tried dropping the build_root stanza, because we didn't seem to need the functionality it delivers [2]. But that removal caused failures like [3]: Failed to load CI Operator configuration" error="invalid ci-operator config: invalid configuration: when 'images' are specified 'build_root' is required and must have image_stream_tag, project_image or from_repository set" source-file=ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml And [2] docs a need for Git, which apparently the UBI images don't have. So I'm using a Go image here still, even though we don't need Go, and although that means some tedious bumping to keep up with RHEL and Go versions instead of floating. The operators stanza doc'ed in [4] remains largely unchanged, although I did rename 'cincinnati_operand_latest' to 'cincinnati-operand', because these tests use a single operand image, and there is no need to distinguish between multiple operand images with "latest". The image used for operator-sdk (which I bump to an OpenShift 4.14 base) and its use are doc'ed in [5]. The 4.14 cluster-claim pool I'm transitioning to is listed as healthy in [6]. For the end-to-end tests, we install the operator via the test suite, so we do not need the SDK bits. I've dropped OPERATOR_IMAGE, because we are well past the transition initiated by eae9d38 (ci-operator/config/openshift/cincinnati-operator: Set RELATED_IMAGE_*, 2021-04-05, #17435) and openshift/cincinnati-operator@799d18525b (Changing the name to make OSBS auto repo/registry replacements to work, 2021-04-06, openshift/cincinnati-operator#104). I'm consistently using the current Cincinnati operand instead of the pinned one, because we ship the OpenShift Update Service Operator as a bundle with the operator and operand, and while it might be useful to grow update-between-OSUS-releases test coverage, we do not expect long durations of new operators coexisting with old-image operand pods. And we never expect new operators to touch Deployments with old operand images, except to bump them to new operand images. We'd been using digest-pinned operand images here since efcafb6 (ci-operator/config/openshift/cincinnati-operator: Move e2e-operator to multi-step, 2020-10-06, #12486), where I said: In a future pivot we'll pull the operand image out of CI too, instead of hard-coding. But with this change we at least move the hard-coding into the CI repository. 4f46d7e (cincinnati-operator: test operator against released OSUS version and latest master, 2022-01-11, #25152) brought in that floating operand image, but neglected, for reasons that I am not clear on, did not drop the digest-pinned operand. I'm dropping it now. With "which operand image" removed as a differentiator, the remaining differentiators for the end-to-end tests are: * Which host OpenShift? * To protect from "new operators require new platform capabilities not present in older OpenShift releases", we have an old-ocp job. It's currently 4.11 for the oldest supported release [7]. * To protect from "new operators still use platform capabilities that have been removed from development branches of OpenShift", we have a new-ocp job. It's currently 4.14, as the most modern openshift-ci pool in [6], but if there was a 4.15 openshift-ci pool I'd us that to ensure we work on dev-branch engineering candidates like 4.15.0-ec.1. * To protect against "HyperShift does something the operator does not expect", we have a hypershift job. I'd prefer to defer "which version?" to the workflow, because we do not expect HyperShift-specific difference to evolve much between 4.y releases, while the APIs used by the operator (Deployments, Services, Routes, etc.) might. But perhaps I'm wrong, and we will see more API evolution during HyperShift minor versions. And in any case, today 4.14 fails with [8]: Unable to apply 4.14.1: some cluster operators are not available so in the short term I'm going with 4.13, but with a generic name so we only have to bump one place as HyperShift support improves. * I'm not worrying about enumerating all the current 4.y options like we had done before. That is more work to maintain, and renaming required jobs confuses Prow and requires an /override of the removed job. It seems unlikely that we work on 4.old, break on some 4.middle, and work again on 4.dev. Again, we can always revisit this if we change our minds about the exposure. * Which graph-data? * To protect against "I updated my OSUS without changing the graph-data image, and it broke", we have published-graph-data jobs. These consume images that were built by previous postsubmits in the cincinnati-graph-data repository. * We could theoretically also add coverage for older forms of graph-data images we suspect customers might be using. I'm punting this kind of thing to possible future work, if we decide the exposure is significant enough to warrant ongoing CI coverage. * To allow testing new features like serving signatures, we have a local-graph-data job. This consumes a graph-data image built from steps in the operator repository, allowing convenient testing of changes that simultaneously tweak the operator and how the graph-data image is built. For example, [9] injects an image signature into graph-data, and updates graph-data to serve it. I'm setting a GRAPH_DATA environment variable to 'local' to allow the test suite to easily distinguish this case. [1]: https://docs.ci.openshift.org/docs/architecture/images/#ci-images [2]: https://docs.ci.openshift.org/docs/architecture/ci-operator/#build-root-image [3]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/45245/pull-ci-openshift-release-master-generated-config/1720218786344210432 [4]: https://docs.ci.openshift.org/docs/how-tos/testing-operator-sdk-operators/#building-operator-bundles [5]: https://docs.ci.openshift.org/docs/how-tos/testing-operator-sdk-operators/#simple-operator-installation [6]: https://docs.ci.openshift.org/docs/how-tos/cluster-claim/#existing-cluster-pools [7]: https://access.redhat.com/support/policy/updates/openshift/#dates [8]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/45245/rehearse-45245-pull-ci-openshift-cincinnati-operator-master-operator-e2e-hypershift-local-graph-data/1720287506777247744 [9]: openshift/cincinnati-operator#176
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changing the name to make OSBS auto repo/registry replacements to work.
Auto digest_pinning, repo_replacements, registry_replacements necessary for releasing the image to different repository (stage, prod) etc.
Refer
https://osbs.readthedocs.io/en/latest/users.html#pullspec-locations for
details
Also changed OPERATOR_IMAGE to RELATED_IMAGE_OPERATOR for consistency
Signed-off-by: Lalatendu Mohanty [email protected]