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

Add OLM upgrade test #2198

Merged
merged 1 commit into from
Apr 12, 2021
Merged

Add OLM upgrade test #2198

merged 1 commit into from
Apr 12, 2021

Conversation

bouskaJ
Copy link
Contributor

@bouskaJ bouskaJ commented Apr 9, 2021

Release Note

NONE

@bouskaJ
Copy link
Contributor Author

bouskaJ commented Apr 9, 2021

@astefanutti Finally, the PR is there. Please review.

@bouskaJ
Copy link
Contributor Author

bouskaJ commented Apr 9, 2021

@astefanutti the CI seems to be faulty. I see a lot of various errors that are not caused by my changes.

@astefanutti
Copy link
Member

@bouskaJ awesome!

It's possible that CI is currently broken as the Camel / Camel Quarkus upgrade is being rolling out and needs #2197 to complete.

@nicolaferraro
Copy link
Member

@bouskaJ awesome!

It's possible that CI is currently broken as the Camel / Camel Quarkus upgrade is being rolling out and needs #2197 to complete.

Yes, we may merge #2197 to have less errors, as the only issue seems in runtime: apache/camel-k-runtime#651. Wdyt?

@astefanutti
Copy link
Member

@bouskaJ awesome!
It's possible that CI is currently broken as the Camel / Camel Quarkus upgrade is being rolling out and needs #2197 to complete.

Yes, we may merge #2197 to have less errors, as the only issue seems in runtime: apache/camel-k-runtime#651. Wdyt?

Good idea, +1 for me, then we can rebase the pending PRs.

@bouskaJ
Copy link
Contributor Author

bouskaJ commented Apr 12, 2021

@astefanutti can you rerun the CI?

@astefanutti
Copy link
Member

@bouskaJ could you please rebase to have the changes from #2197 taken into account?

@nicolaferraro nicolaferraro merged commit 8c69a47 into apache:master Apr 12, 2021
@nicolaferraro nicolaferraro mentioned this pull request Apr 13, 2021
Copy link
Member

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

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

@bouskaJ thanks again for the awesome work.

It seems the test sometimes fails with the following error:

    olm_upgrade_test.go:94: 
        Timed out after 300.006s.
        Expected
            <v1alpha1.ClusterServiceVersionPhase>: Succeeded
        to equal
            <v1alpha1.ClusterServiceVersionPhase>: Replacing
    --- FAIL: TestOLMAutomaticUpgrade/OLM_upgrade (300.01s)

I wonder if that's just a race issue, or that the phase is not as expected.

Also, I understand that the operator upgrade is triggered by updating the index image in the CatalogSource. However, the CSV generated for the new version does not have a replaces field. So I wonder, are we sure that this sequence mimics exactly a typical upgrade performed by OLM?

I'd like to be sure that what the test covers is an actual upgrade, rather than just a replace.

@bouskaJ
Copy link
Contributor Author

bouskaJ commented Apr 16, 2021

Hi, @astefanutti yes, there is no replaces but the replace scenario is forced by the skipRange (see https://github.com/apache/camel-k/blob/master/.github/workflows/upgrade.yml#L138).
I was considering:

  • replaces because it was close to the real upstream releases
  • skipRange - used in the internal stream (at the day I was writing the test)

I decided on the skipRange because it is able to force an upgrade on the older releases as well so the test is not dependent on the very last release -> devel version. I think we can migrate to replaces since both streams use replaces at the moment in the real manifests.

@bouskaJ
Copy link
Contributor Author

bouskaJ commented Apr 16, 2021

I am not sure why, it's failing. Maybe, the OLM needs more time to start the upgrade and we should use TestTimeoutLong https://github.com/apache/camel-k/blob/master/e2e/upgrade/olm_upgrade_test.go#L94

@bouskaJ
Copy link
Contributor Author

bouskaJ commented Apr 16, 2021

Printing the patched CSV to the workflow log could help as well... (result of https://github.com/apache/camel-k/blob/master/.github/workflows/upgrade.yml#L143)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants