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

Allow customers to annotate CloudCredential resource on update #3575

Merged
merged 7 commits into from
Jun 4, 2024

Conversation

cadenmarchese
Copy link
Collaborator

Which issue this PR addresses:

Fixes ARO-7348

What this PR does / why we need it:

Clusters with manual credentials mode need to be annotated in order to be upgraded: https://docs.openshift.com/container-platform/4.14/updating/preparing_for_updates/preparing-manual-creds-update.html#cco-manual-upgrade-annotation_preparing-manual-creds-update

This PR adds a new API field that the user can patch on update, which will then annotate the cluster itself with the version.

Requires #3563 as this will also require some validation to ensure that the user is passing in a version and not some other string.

Test plan for issue:

Is there any documentation that needs to be updated for this PR?

How do you know this will function as expected in production?

@cadenmarchese cadenmarchese added work-in-progress chainsaw Pull requests or issues owned by Team Chainsaw labels May 10, 2024
cadenmarchese and others added 3 commits May 22, 2024 16:36
- Initialize the operatorcli in both the real code and the unit tests
- Compare the actual annotations on the CloudCredentials to the
  wantAnnotations
allow existing cc annotations, more test cases
@cadenmarchese
Copy link
Collaborator Author

/azp run ci,e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Collaborator

@rajdeepc2792 rajdeepc2792 left a comment

Choose a reason for hiding this comment

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

Left 2 comments:-

  • I believe a Mutable tag would be needed for the ExternalAPI.
    Everything else seems good.

pkg/api/v20240812preview/openshiftcluster.go Outdated Show resolved Hide resolved
pkg/operator/deploy/deploy.go Show resolved Hide resolved
@cadenmarchese
Copy link
Collaborator Author

/azp run e2e

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@kimorris27 kimorris27 left a comment

Choose a reason for hiding this comment

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

The logic LGTM, but I suggested some small changes.

pkg/operator/deploy/deploy.go Show resolved Hide resolved
pkg/api/admin/openshiftcluster.go Outdated Show resolved Hide resolved
pkg/api/openshiftcluster.go Outdated Show resolved Hide resolved
pkg/api/v20240812preview/openshiftcluster.go Outdated Show resolved Hide resolved
pkg/cluster/arooperator.go Outdated Show resolved Hide resolved
pkg/operator/deploy/deploy.go Outdated Show resolved Hide resolved
@cadenmarchese
Copy link
Collaborator Author

/azp run ci,e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

.sha256sum Show resolved Hide resolved
@cadenmarchese
Copy link
Collaborator Author

/azp run ci,e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@hlipsig hlipsig merged commit 0f8830b into master Jun 4, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chainsaw Pull requests or issues owned by Team Chainsaw ready-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants