Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Apr 5, 2021

From the OSBS docs:

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, rhbz#1945026 and openshift/cincinnati-operator#104.

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
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 5, 2021
Copy link
Member

@LalatenduMohanty LalatenduMohanty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 5, 2021
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LalatenduMohanty, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 5, 2021

@wking: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/rehearse/openshift/cincinnati-operator/release-4.6/operator-e2e eae9d38 link /test pj-rehearse
ci/rehearse/openshift/cincinnati-operator/release-4.7/operator-e2e eae9d38 link /test pj-rehearse

Full PR test history. Your PR dashboard.

Details

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-robot openshift-merge-robot merged commit bd42b53 into openshift:master Apr 5, 2021
@openshift-ci-robot
Copy link
Contributor

@wking: Updated the following 5 configmaps:

  • ci-operator-4.8-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cincinnati-operator-release-4.8.yaml using file ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-release-4.8.yaml
  • ci-operator-4.9-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cincinnati-operator-release-4.9.yaml using file ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-release-4.9.yaml
  • ci-operator-master-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cincinnati-operator-master.yaml using file ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml
  • ci-operator-4.6-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cincinnati-operator-release-4.6.yaml using file ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-release-4.6.yaml
  • ci-operator-4.7-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cincinnati-operator-release-4.7.yaml using file ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-release-4.7.yaml
Details

In response to this:

From the OSBS docs:

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, rhbz#1945026 and openshift/cincinnati-operator#104.

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 wking deleted the related-image-variables-for-update-service branch April 5, 2021 21:56
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 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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants