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

Improve Subs/CSV validation #452

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Improve Subs/CSV validation #452

merged 1 commit into from
Oct 2, 2024

Conversation

betoredhat
Copy link
Contributor

@betoredhat betoredhat commented Oct 2, 2024

SUMMARY
  • installPlanApproval for the initial subscription must be Automatic to complete the installation.
  • Use installedCSV to validate the subscription as it represents the current version installed. currentCSV represents the version available for upgrading.
ISSUE TYPE
  • Bug, Docs Fix or other nominal change
Tests

@betoredhat betoredhat requested a review from a team as a code owner October 2, 2024 18:42
Copy link

@dcibot
Copy link
Collaborator

dcibot commented Oct 2, 2024

Copy link
Contributor

@tonyskapunk tonyskapunk left a comment

Choose a reason for hiding this comment

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

LGTM!! 💯

@betoredhat betoredhat added this pull request to the merge queue Oct 2, 2024
Merged via the queue into main with commit a9e4db3 Oct 2, 2024
7 checks passed
@betoredhat betoredhat deleted the fix_csv branch October 2, 2024 22:38
@nsilla
Copy link
Contributor

nsilla commented Oct 3, 2024

I'd wish I had seen this PR before it got approved but I see it's causing some jobs to fail.

https://www.distributed-ci.io/jobs/1fd1de75-5ecf-4a0e-aa7f-73549c216797/jobStates?sort=date&task=dd157386-954d-41eb-b1a5-4be22f6b420c

The problem is hardcoding the approval method to "Automatic" is not compatible with using the starting_csv parameter.

With the automatic approval method, the OLM will install the starting_csv and then keep upgrading the CSVs until it reaches the currentCSV.

To start with, this renders the starting_csv parameter without effect, but, furthermore, when the olm_operator role waits for the subscription to be ready, the condition is that the installedCSV has the same value as the operator_csv variable, which is set to the starting_csv if set. As you can see in the linked job, with the automatic approval method these values are bound not to match and thus the job will fail.

@betoredhat
Copy link
Contributor Author

ouch, no worries... time travel is now possible. The following revert change will do the trick: #454. Will wait for you next time. :)

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.

4 participants