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

🌱 update skopeo openshift test #3298

Merged
merged 2 commits into from
May 30, 2024

Conversation

perdasilva
Copy link
Collaborator

Description of the change:
This update does not affect OLM. This updates an e2e test that is used downstream by Red Hat. This PR updates the version of skopeo used in the test as well as the way the skopeo pod is built for it. Now, we mount the secret as a volume and convert its contents to an auth.json file that skopeo can use and also mount an emptydir as a local cache for skopeo (something the logs showed skopeo was complaining about). I've reproduced this issue and tested this fix using an Openshift cluster.

Motivation for the change:
The test expected Openshift to add credential annotations in a secret that contains the auth configuration for the internal registry.

Architectural changes:

Testing remarks:

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Bug fixes are accompanied by regression test(s)
  • e2e tests and flake fixes are accompanied evidence of flake testing, e.g. executing the test 100(0) times
  • tech debt/todo is accompanied by issue link(s) in comments in the surrounding code
  • Tests are comprehensible, e.g. Ginkgo DSL is being used appropriately
  • Docs updated or added to /doc
  • Commit messages sensible and descriptive
  • Tests marked as [FLAKE] are truly flaky and have an issue
  • Code is properly formatted

@perdasilva perdasilva changed the title update skopeo openshift test 🌱 update skopeo openshift test May 30, 2024
@openshift-ci openshift-ci bot requested review from dtfranz and stevekuznetsov May 30, 2024 12:40
Signed-off-by: Per Goncalves da Silva <[email protected]>
@@ -2601,13 +2601,13 @@ var _ = Describe("Subscription", func() {
sub, err = fetchSubscription(crc, generatedNamespace.GetName(), subName, subscriptionHasCurrentCSV("example-operator.v0.3.0"))
Expect(err).Should(BeNil())

By("waiting for the subscription to have v0.3.0 installed with a Package deprecated condition")
By("waiting for the subscription to have v0.3.0 installed without a bundle deprecated condition")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What this test really wants to test is that bundle deprecation condition is gone. So, let's just test for that.

@kevinrizza
Copy link
Member

If this is the fastest way to handle this, I'm not opposed to the change, but is there a reason we can't shift openshift specific stuff out of this repository entirely?

@perdasilva
Copy link
Collaborator Author

If this is the fastest way to handle this, I'm not opposed to the change, but is there a reason we can't shift openshift specific stuff out of this repository entirely?

I have that on my list of things. Let's deal with getting downstream green first though and yanks this out as it's own task.

Signed-off-by: Per Goncalves da Silva <[email protected]>
Copy link
Contributor

@tmshort tmshort 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 openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 30, 2024
@joelanford joelanford added this pull request to the merge queue May 30, 2024
Merged via the queue into operator-framework:master with commit 37dcff4 May 30, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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