Skip to content

Conversation

@mhsreddy
Copy link
Collaborator

@mhsreddy mhsreddy commented Nov 25, 2022

Which issue this PR addresses:

"Fixes a bug in the certificate PUCM process that does not take into account clusters that have not been successfully PUCM'ed since a large infra change PR was merged in Feb 2021".

Previous ADO Card for Certificate PUCM : https://msazure.visualstudio.com/AzureRedHatOpenShift/_workitems/edit/15581636/

What this PR does / why we need it:

"Fixes a bug in the certificate PUCM process that does not take into account clusters that have not been successfully PUCM'ed since a large infra change PR was merged in Feb 2021". At present fixMCSCert step failing as it could not find the Properties.APIServerProfile.IntIP entry in the Cluster Document.

Test plan for issue:

Create Test Cluster without the Properties.APIServerProfile.IntIP entry in the Cluster Document and run the Certificate PUCM and it will create a new entry for Properties.APIServerProfile.IntIP and update the Cluster Document in Cosmos DB.

CURL Command to perform Admin Update just for certificates renewal:
curl -X PATCH -k "https://localhost:8443/subscriptions/$AZURE_SUBSCRIPTION_ID/resourceGroups/$RESOURCEGROUP/providers/Microsoft.RedHatOpenShift/openShiftClusters/$CLUSTER?api-version=admin" --header "Content-Type: application/json" -d '{"properties":{"maintenanceTask": "CertificatesRenewal"}}'

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

@darthhexx darthhexx added the bug Something isn't working label Nov 25, 2022
@mhsreddy mhsreddy marked this pull request as ready for review November 25, 2022 08:44
@mhsreddy mhsreddy force-pushed the fixing-mcscert-adminupdate branch from 3fe4f94 to 68121cc Compare November 25, 2022 11:17
Copy link
Contributor

@SrinivasAtmakuri SrinivasAtmakuri left a comment

Choose a reason for hiding this comment

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

LGTM!

@darthhexx darthhexx added next-release To be included in the next RP release rollout ready-for-review labels Nov 28, 2022

if isEverything || isRenewCerts {
toRun = append(toRun,
steps.Action(m.populateDatabaseIntIP),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can this additional step be moved down to within the condition on line 85 where we check the same isEverything || isRenewCerts?

@cblecker cblecker merged commit 0b196ae into Azure:master Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working next-release To be included in the next RP release rollout ready-for-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants