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

[v1.0.x] *: fix run packagemanifests #3895

Conversation

joelanford
Copy link
Member

Description of the change:
Backport #3856 and #3867

Motivation for the change:
Supported branches should get bug fixes. This unblocks CI, which is currently failing due to issues with OLM v0.16.1's caching of catalog data.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

…ifests (operator-framework#3856)

* internal/olm/operator: fix operator-registry image for run packagemanifests

* internal/olm/client/client.go: return error for failed CSV

* use switch for DoCSVWait phase logic

* pin to olm 0.15.1 for integration tests
@joelanford joelanford force-pushed the v1.0.x-fix-run-packagemanifests branch from 223d94b to ce9cf4f Compare September 17, 2020 02:26
if isRunningOnKind() {
By("loading the bundle image into Kind cluster")
err = tc.LoadImageToKindClusterWithName(bundleImage)
Expect(err).Should(Succeed())
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I know this is a backport and this line is basically unchanged in this PR but for better readability this probably should be:

Expect(err).NotTo(HaveOccurred())

Succeed() is used more for the function itself:

Ω(DoSomethingSimple()).Should(Succeed())

https://onsi.github.io/gomega/#handling-errors

Copy link
Member Author

Choose a reason for hiding this comment

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

+1 I'll make this comment into an issue. There are probably lots of instances of this that we can fix on master (probably not worth backporting those changes though).

Copy link
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

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

LGTM

@jmrodri
Copy link
Member

jmrodri commented Sep 17, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 17, 2020
@joelanford joelanford merged commit 53ab05e into operator-framework:v1.0.x Sep 17, 2020
@joelanford joelanford deleted the v1.0.x-fix-run-packagemanifests branch September 17, 2020 03:52
@joelanford joelanford added this to the v1.0.1 milestone Sep 17, 2020
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.

None yet

4 participants