OSD-6918 Rename managed CR to managed-upgrade-config #224

mrbarge · 2021-04-20T11:07:48Z

What type of PR is this?

Refactor

What this PR does / why we need it?

OSD-6918 largely turned into a small refactoring exercise, as fortunately all of the building blocks to support multi-types are already built in the controller and left me with very little to actually do to support separate upgrade types.

So what this PR does is rename the managed Custom Resource from osd-upgrade-config to managed-upgrade-config. The UpgradeConfig controller will now only reconcile over CRs that are named that.

This has minimal impact except if the operator happens to be mid-upgrade when this change lands (it will never report upgrade completion to OCM), or if a cluster has an automatic upgrade scheduled (it will leave an orphaned osd-upgrade-config CR).

I am proposing a temporary managed-cluster-config job which will rename the osd-upgrade-config CR on any clusters:

https://github.com/mrbarge/managed-cluster-config/tree/osd-6918/deploy/osd-6918-muo-cr-rename

This cronjob has been successfully tested on a MUO upgrade to rename the CR in-flight during an upgrade.

Which Jira/Github issue(s) this PR fixes?

OSD-6918

Special notes for your reviewer:

Pre-checks (if applicable):

Tested latest changes against a cluster (OSD upgrade type)
Tested latest changes against a cluster (ARO upgrade type)
Tested latest changes against a cluster (updated operator to this release mid-upgrade and applied cron CR fix)

mrbarge · 2021-04-20T11:07:55Z

/hold

ravitri · 2021-04-20T13:05:46Z

@mrbarge - Thank you Matt! Had a quick glance for osd-upgrade-config in your branch and came across following matches:

$ grep -ri osd-upgrade-config
README.md:  name: osd-upgrade-config
pkg/controller/upgradeconfig/upgradeconfig_controller_test.go:			Name:      "osd-upgrade-config",
pkg/eventmanager/eventmanager_test.go:	TEST_UPGRADECONFIG_CR   = "osd-upgrade-config"
test/deploy/upgrade.managed.openshift.io_v1alpha1_upgradeconfig_cr.yaml:  name: osd-upgrade-config
$

I think the major functional change is done already by UPGRADECONFIG_CR_NAME = "managed-upgrade-config" but was curious about test/deploy/upgrade.managed.openshift.io_v1alpha1_upgradeconfig_cr.yaml which was brought in via Boilerplate change in #218. Should it be changed too? The README change could be done too alongside. For the test files, make test runs fine as such so don't think any change is necessary. Let me know if I missed anything.

mrbarge · 2021-04-20T20:37:29Z

@mrbarge - Thank you Matt! Had a quick glance for osd-upgrade-config in your branch and came across following matches:

Thanks for spotting - lingering references to the old naming convention have been updated.

The files in test/deploy/ aren't used by boilerplate or any other testing processes, they're just supporting materials to assist with performing local development.

dofinn · 2021-04-23T04:41:59Z

/lgtm

dofinn · 2021-04-23T04:42:06Z

please unhold when ready

openshift-ci-robot · 2021-04-23T04:42:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dofinn, mrbarge

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [dofinn,mrbarge]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dofinn · 2021-04-23T04:47:25Z

pkg/upgradeconfigmanager/upgradeconfigmanager.go

 const (
 	// Name of the Custom Resource that the provider will manage
-	UPGRADECONFIG_CR_NAME = "osd-upgrade-config"
+	UPGRADECONFIG_CR_NAME = "managed-upgrade-config"


I feel like this should be defined at higher level:https://github.com/openshift/managed-upgrade-operator/blob/master/config/config.go ?

T0MASD · 2021-04-29T22:21:58Z

pkg/controller/upgradeconfig/upgradeconfig_controller.go


 			if remoteChanged {
-				reqLogger.Info("The remote upgrade policy does not match the local upgrade config, applying the new upgrade policy")
+				reqLogger.Info("The cluster's upgrade policy has changed, so the operator will re-reconcile.")


should this be 'reconcile'?

T0MASD

looks good to me

T0MASD · 2021-04-29T22:50:20Z

pkg/upgradeconfigmanager/upgradeconfigmanager.go

 const (
 	// Name of the Custom Resource that the provider will manage
-	UPGRADECONFIG_CR_NAME = "osd-upgrade-config"
+	UPGRADECONFIG_CR_NAME = "managed-upgrade-config"


mrbarge · 2021-05-31T22:42:33Z

/hold cancel

Conventions: - openshift/golang-osd-operator: Update --- openshift/boilerplate@2a6a579...1e947d2 commit: 1e947d2b7daee575dbc0283c647b9945a8081c8f author: Haoran Wang 1. Set a home dir that are writable when do go test to workaround the (openshift#224) issue we have in openshift ci 2. Set the test binary bin dir to a tmp dir commit: 8f3dfee52954fecbb9b834676f3bfc435619d9d7 author: Supreeth Basabattini Remove explicit configuration of envtest bin directory commit: 95509602ef9a3fe37a23f3b15d0f013918d2cebb author: Supreeth Basabattini Fix openapi-gen versioning for new o-sdk commit: 39cc9e896380f04987f92d5a5aeec331966e2d1e author: Supreeth Basabattini Include openapi-gen and setup-envtest in the backing image commit: 1d6d39398ec892deec3add7f9b0c3d616af1a60a author: Supreeth Basabattini Fix incorrect bash syntax commit: bac2488ea130d26848df3cba7ccc1ef98496b436 author: Haoran Wang Update boilerplate to support latest osdk use controller-gen v0.3.0 when it's using old osdk Address some comments use controller-gen in the baking image commit: 210292d58116f6c5981e1ab43b5d6543fb8a070d author: Haoran Wang install v0.8.0 controller-gen in the backing image commit: 9fa3022a5f8c00b7f5e1e10f6f935162c1741a21 author: John Roche make target for fips on osd operators commit: e35d0f5c1aa9ef8b22ede5e2bd7f674d2f07b3ba author: Michael Shen Initial prow-config addition Signed-off-by: Michael Shen <[email protected]> commit: a078e1ce4ffd8607b13f398e7bcab5cc759fa864 author: Michael Shen Unexport GOFLAGS to fix bug when using container-make Signed-off-by: Michael Shen <[email protected]> commit: b99a046991521f41600c897354009b93dee15e33 author: Benjamin Dematteo Fixing errors from golang-lint (in standard.mk) commit: 3560610f126217211ad9663f8e5729ffee2735c6 author: Benson Ngoy USER 1001 doesn't have pip install permissions + updating pip commit: bbf4703f01903064bf1d2f79a0e893f8535ce048 author: Wesley Hearn [OSD-10491] Bump urllib3 version (openshift#209) * [OSD-10491] Bump urllib3 version * Update catalog-build.sh Remove the --upgrade from the pip3 install commit: c06911de4e086ff74e40cc31f018a71f72b9b408 author: Michael Shen Initial commit for osd-container-image convention Signed-off-by: Michael Shen <[email protected]> commit: b924e51f50330cd4e7279acc102b7e1adc29f338 author: Benjamin Dematteo update README for new conventions commit: 7788244648e463fac115937799297d6eab204179 author: Benjamin Dematteo updating with PR Review comments commit: 7d81a9d4ba6ed1b17ed0f0ceee85cb9def9884d4 author: Benjamin Dematteo Initial commit for golang-codecov and golang-lint conventions

Conventions: - openshift/golang-osd-operator: Update --- openshift/boilerplate@2a6a579...dcfe34f commit: ec6e7a3e70bb23401df5f7e333569fdef2535c4e author: Supreeth Basabattini Remove support for CRDv1beta1 in boilerplate Removed op-generate-crd-fixup test case commit: 16924d60224e73f7d5b82c652fc9e28fb18794a5 author: Ravi Trivedi Adding initializer for debugging purpose commit: ffc15d3682ce1f84f37499fd65c472dc60623ecf author: Haoran Wang generate CRD v1 by default commit: 1e947d2b7daee575dbc0283c647b9945a8081c8f author: Haoran Wang 1. Set a home dir that are writable when do go test to workaround the (openshift#224) issue we have in openshift ci 2. Set the test binary bin dir to a tmp dir commit: 8f3dfee52954fecbb9b834676f3bfc435619d9d7 author: Supreeth Basabattini Remove explicit configuration of envtest bin directory commit: 95509602ef9a3fe37a23f3b15d0f013918d2cebb author: Supreeth Basabattini Fix openapi-gen versioning for new o-sdk commit: 39cc9e896380f04987f92d5a5aeec331966e2d1e author: Supreeth Basabattini Include openapi-gen and setup-envtest in the backing image commit: 1d6d39398ec892deec3add7f9b0c3d616af1a60a author: Supreeth Basabattini Fix incorrect bash syntax commit: 1f0fa3dc4630b115897dcadf1cf2f25edfa3a731 author: Ravi Trivedi Rebuild registry image from 4.10.0 into a ubi-micro image commit: bac2488ea130d26848df3cba7ccc1ef98496b436 author: Haoran Wang Update boilerplate to support latest osdk use controller-gen v0.3.0 when it's using old osdk Address some comments use controller-gen in the baking image commit: 210292d58116f6c5981e1ab43b5d6543fb8a070d author: Haoran Wang install v0.8.0 controller-gen in the backing image commit: 9fa3022a5f8c00b7f5e1e10f6f935162c1741a21 author: John Roche make target for fips on osd operators commit: e35d0f5c1aa9ef8b22ede5e2bd7f674d2f07b3ba author: Michael Shen Initial prow-config addition Signed-off-by: Michael Shen <[email protected]> commit: a078e1ce4ffd8607b13f398e7bcab5cc759fa864 author: Michael Shen Unexport GOFLAGS to fix bug when using container-make Signed-off-by: Michael Shen <[email protected]> commit: b99a046991521f41600c897354009b93dee15e33 author: Benjamin Dematteo Fixing errors from golang-lint (in standard.mk) commit: 3560610f126217211ad9663f8e5729ffee2735c6 author: Benson Ngoy USER 1001 doesn't have pip install permissions + updating pip commit: bbf4703f01903064bf1d2f79a0e893f8535ce048 author: Wesley Hearn [OSD-10491] Bump urllib3 version (openshift#209) * [OSD-10491] Bump urllib3 version * Update catalog-build.sh Remove the --upgrade from the pip3 install commit: c06911de4e086ff74e40cc31f018a71f72b9b408 author: Michael Shen Initial commit for osd-container-image convention Signed-off-by: Michael Shen <[email protected]> commit: b924e51f50330cd4e7279acc102b7e1adc29f338 author: Benjamin Dematteo update README for new conventions commit: 7788244648e463fac115937799297d6eab204179 author: Benjamin Dematteo updating with PR Review comments commit: 7d81a9d4ba6ed1b17ed0f0ceee85cb9def9884d4 author: Benjamin Dematteo Initial commit for golang-codecov and golang-lint conventions

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 20, 2021

openshift-ci-robot requested review from T0MASD and ravitri April 20, 2021 11:08

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 20, 2021

OSD-6918 Rename managed CR to managed-upgrade-config

635b575

mrbarge force-pushed the osd-6918 branch from cc5631c to 635b575 Compare April 20, 2021 20:34

mrbarge mentioned this pull request Apr 22, 2021

Add MUO UpgradeConfig CR renaming job openshift/managed-cluster-config#794

Merged

openshift-ci-robot assigned dofinn Apr 23, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 23, 2021

dofinn reviewed Apr 23, 2021

View reviewed changes

T0MASD reviewed Apr 29, 2021

View reviewed changes

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 31, 2021

openshift-merge-robot merged commit ecba07f into openshift:master May 31, 2021

maorfr mentioned this pull request Jul 11, 2021

[openshift-upgrade-watcher] fix missing upgrades due to resource name app-sre/qontract-reconcile#1688

Merged

sedroche mentioned this pull request May 17, 2022

Boilerplate: Update to 1e947d2b7daee575dbc0283c647b9945a8081c8f #316

Closed

sedroche mentioned this pull request May 18, 2022

Boilerplate: Update to dcfe34f6135d3d22735eb5374a8fc31b4d4cca81 #318

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OSD-6918 Rename managed CR to managed-upgrade-config #224

OSD-6918 Rename managed CR to managed-upgrade-config #224

Uh oh!

mrbarge commented Apr 20, 2021 •

edited by openshift-ci-robot

Loading

Uh oh!

mrbarge commented Apr 20, 2021

Uh oh!

ravitri commented Apr 20, 2021

Uh oh!

mrbarge commented Apr 20, 2021

Uh oh!

dofinn commented Apr 23, 2021

Uh oh!

dofinn commented Apr 23, 2021

Uh oh!

openshift-ci-robot commented Apr 23, 2021

Uh oh!

dofinn Apr 23, 2021

Uh oh!

T0MASD Apr 29, 2021

Uh oh!

T0MASD Apr 29, 2021

Uh oh!

T0MASD left a comment

Uh oh!

T0MASD Apr 29, 2021

Uh oh!

mrbarge commented May 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

OSD-6918 Rename managed CR to managed-upgrade-config #224

OSD-6918 Rename managed CR to managed-upgrade-config #224

Uh oh!

Conversation

mrbarge commented Apr 20, 2021 • edited by openshift-ci-robot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it?

Which Jira/Github issue(s) this PR fixes?

Special notes for your reviewer:

Pre-checks (if applicable):

Uh oh!

mrbarge commented Apr 20, 2021

Uh oh!

ravitri commented Apr 20, 2021

Uh oh!

mrbarge commented Apr 20, 2021

Uh oh!

dofinn commented Apr 23, 2021

Uh oh!

dofinn commented Apr 23, 2021

Uh oh!

openshift-ci-robot commented Apr 23, 2021

Uh oh!

dofinn Apr 23, 2021

Choose a reason for hiding this comment

Uh oh!

T0MASD Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

T0MASD Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

T0MASD left a comment

Choose a reason for hiding this comment

Uh oh!

T0MASD Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

mrbarge commented May 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mrbarge commented Apr 20, 2021 •

edited by openshift-ci-robot

Loading