ClusterRelocate to move ClusterDeployments between Hive instances #1011

staebler · 2020-05-28T21:08:30Z

When a ClusterRelocate has a label selector that matches with a ClusterDeployment, the clusterrelocate controller will relocate the matching ClusterDeployment.

Set the hive.openshift.io/relocate annotation to outgoing on the ClusterDeployment and the DNSZone.
Copy secrets, configmaps, machinesets, syncsets, and syncidentityproviders.
Copy the DNSZone for the ClusterDeployment, with the relocate annotation set to incoming.
Copy the ClusterDeployment, with the relocate annotation set to incoming.
Set the relocate annotation on the DNSZone and ClusterDeployemnt to complete.

When a ClusterDeployment with a relocate annotation set to incoming is reconciled by the clusterRelocate controller, the relocate annotation is removed from the DNSZone and the ClusterDeployment. This indicates that the relocate has completed on the destination side.

When a ClusterDeployment has the relocated annotation, no controllers will do any mutation of the remote cluster (such as syncing syncsets). When the ClusterDeployment has the relocated annotation, then controllers will not run finalizer code when the ClusterDeployment is deleted.

Add a hive_cluster_relocations counter vec that tracks the total number of successful cluster relocations. The metric has a single
cluster_relocate label that is the name of the ClusterRelocate directing the relocation.

Add a hive_aborted_cluster_relocations counter vec that tracks the total number of aborted cluster relocations. The metrics has two labels: cluster_relocate and reason. The cluster_relocate label is the name of the ClusterRelocate directing the aborted relocation. The reason label is the reason why the relocate was aborted. Possible values for the reason label are "no_match", "multiple_matches", and "new_match".

The number of failing cluster relocates can be seen by looking at the hive_cluster_deployments_conditions metric and filtering on
a value of RelocationFailed for the condition label.

https://issues.redhat.com/browse/CO-653

pkg/controller/add_clusterrelocate.go

pkg/controller/clusterrelocate/clusterrelocate_controller.go

joelddiaz · 2020-06-02T16:24:24Z

pkg/controller/clusterrelocate/clusterrelocate_controller.go

+	}
+
+	for _, cd := range clusterDeployments.Items {
+		if cd.Annotations[constants.RelocatingAnnotation] != clusterRelocate.Name &&


This wasn't clear to me. Should it be if annotation != name OR !labelMatch ? I suppose the question is, what exactly is this condition catching?

This check is meant to filter out the ClusterDeployments that are not related to the ClusterRelocate. If the ClusterDeployment does not have a relocating annotation matching the ClusterRelocate AND does not have a label matching the ClusterRelocate, then the ClusterDeployment is not queued.

pkg/controller/clusterrelocate/clusterrelocate_controller.go

staebler · 2020-06-18T21:41:27Z

This has been tested now and is ready for further review.

dgoodwin · 2020-06-22T12:59:24Z

pkg/apis/hive/v1/clusterdeployment_types.go

@@ -431,6 +431,18 @@ type CertificateBundleStatus struct {
 	Generated bool `json:"generated"`
 }

+// RelocateStatus is the status of a cluster relocate
+type RelocateStatus string


Noting the annotation it's used in would be useful context.

I'm still not seeing the annotation it's used in mentioned here, something missed?

Oh, I added it to ClusterRelocateStatus but not here. That brings to light that I have a ClusterRelocateStatus and a RelocateStatus. That is too confusing. I need to rename one or both of those.

pkg/constants/constants.go

pkg/controller/clusterdeployment/clusterdeployment_controller.go

pkg/controller/clusterrelocate/clusterrelocate_controller.go

pkg/installmanager/installmanager.go

joelddiaz · 2020-06-22T15:53:29Z

pkg/controller/clusterrelocate/clusterrelocate_controller.go

 		if err := r.copyResource(dnsZone, destClient, false, logger); err != nil {
 			return errors.Wrap(err, "failed to copy dnszone")
 		}
 	}

 	// copy clusterdeployment
-	if err := r.copyResource(cd, destClient, true, logger); err != nil {
-		return errors.Wrap(err, "failed to copy clusterdeployment")
+	{


why the naked curly block?

To keep the logger variable isolate to inside the scope, in case there is other code added after the code block later that may use logger and not want the type and resource field. But, now I see that I replaced the function-scoped logger variable instead of creating a new logger variable.

joelddiaz · 2020-06-24T13:40:21Z

pkg/controller/utils/dnszone.go

+
+	"github.com/pkg/errors"
+	log "github.com/sirupsen/logrus"
+	"sigs.k8s.io/controller-runtime/pkg/handler"


the grouping of the imports seems inconsistent. only one controller-runtime import is by itself.

pkg/controller/clusterrelocate/clusterrelocate_controller.go

dgoodwin · 2020-06-26T11:12:06Z

Despite the minor annotation thing I'm going to lgtm and see if we can get this to merge today.

dgoodwin · 2020-06-26T11:12:10Z

/lgtm

openshift-bot · 2020-06-26T11:56:52Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-26T12:22:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

staebler · 2020-06-26T13:46:17Z

Let me fix the annotation comment and address Joel's comment and squash everything down.

dgoodwin · 2020-06-26T13:51:35Z

/hold

staebler · 2020-06-26T13:56:08Z

/hold cancel

When a ClusterRelocate has a label selector that matches with a ClusterDeployment, the clusterrelocate controller will relocate the matching ClusterDeployment. 1. Set the hive.openshift.io/relocate annotation to outgoing on the ClusterDeployment and the DNSZone. 2. Copy secrets, configmaps, machinesets, syncsets, and syncidentityproviders. 3. Copy the DNSZone for the ClusterDeployment, with the relocate annotation set to incoming. 4. Copy the ClusterDeployment, with the relocate annotation set to incoming. 5. Set the relocate annotation on the DNSZone and ClusterDeployemnt to complete. When a ClusterDeployment with a relocate annotation set to incoming is reconciled by the clusterRelocate controller, the relocate annotation is removed from the DNSZone and the ClusterDeployment. This indicates that the relocate has completed on the destination side. When a ClusterDeployment has the relocated annotation, no controllers will do any mutation of the remote cluster (such as syncing syncsets). When the ClusterDeployment has the relocated annotation, then controllers will not run finalizer code when the ClusterDeployment is deleted. Add a hive_cluster_relocations counter vec that tracks the total number of successful cluster relocations. The metric has a single cluster_relocate label that is the name of the ClusterRelocate directing the relocation. Add a hive_aborted_cluster_relocations counter vec that tracks the total number of aborted cluster relocations. The metrics has two labels: cluster_relocate and reason. The cluster_relocate label is the name of the ClusterRelocate directing the aborted relocation. The reason label is the reason why the relocate was aborted. Possible values for the reason label are "no_match", "multiple_matches", and "new_match". The number of failing cluster relocates can be seen by looking at the hive_cluster_deployments_conditions metric and filtering on a value of RelocationFailed for the condition label. https://issues.redhat.com/browse/CO-653

staebler · 2020-06-26T16:38:15Z

/test unit

dgoodwin · 2020-06-26T16:55:30Z

/lgtm

openshift-ci-robot · 2020-06-26T16:55:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgoodwin, staebler

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [dgoodwin,staebler]

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

openshift-bot · 2020-06-26T20:36:58Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-26T21:16:26Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 28, 2020

openshift-ci-robot requested review from joelddiaz and twiest May 28, 2020 21:08

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 28, 2020

staebler force-pushed the rework_cluster_relocate branch from 98f2b15 to a473798 Compare May 28, 2020 21:09

joelddiaz reviewed Jun 2, 2020

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 4, 2020

staebler force-pushed the rework_cluster_relocate branch from a473798 to be97dcd Compare June 17, 2020 21:29

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 17, 2020

staebler changed the title ~~WIP: controller: re-work clusterrelocate to add annotation to dnszone~~ controller: re-work clusterrelocate to add annotation to dnszone Jun 18, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 18, 2020

dgoodwin reviewed Jun 22, 2020

View reviewed changes

pkg/constants/constants.go Outdated Show resolved Hide resolved

dgoodwin reviewed Jun 22, 2020

View reviewed changes

pkg/controller/clusterdeployment/clusterdeployment_controller.go Show resolved Hide resolved

dgoodwin reviewed Jun 22, 2020

View reviewed changes

pkg/controller/clusterrelocate/clusterrelocate_controller.go Show resolved Hide resolved

dgoodwin reviewed Jun 22, 2020

View reviewed changes

pkg/controller/clusterrelocate/clusterrelocate_controller.go Outdated Show resolved Hide resolved

pkg/controller/clusterrelocate/clusterrelocate_controller.go Outdated Show resolved Hide resolved

pkg/installmanager/installmanager.go Outdated Show resolved Hide resolved

joelddiaz reviewed Jun 22, 2020

View reviewed changes

staebler force-pushed the rework_cluster_relocate branch 3 times, most recently from 17031c1 to 91144bf Compare June 24, 2020 13:30

joelddiaz reviewed Jun 24, 2020

View reviewed changes

dgoodwin reviewed Jun 25, 2020

View reviewed changes

pkg/controller/clusterrelocate/clusterrelocate_controller.go Show resolved Hide resolved

openshift-ci-robot assigned dgoodwin Jun 26, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2020

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

staebler force-pushed the rework_cluster_relocate branch from 91144bf to c753c45 Compare June 26, 2020 13:54

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 26, 2020

staebler changed the title ~~controller: re-work clusterrelocate to add annotation to dnszone~~ ClusterRelocate to move ClusterDeployments between Hive instances Jun 26, 2020

staebler force-pushed the rework_cluster_relocate branch 2 times, most recently from 26fec3c to cb2aa73 Compare June 26, 2020 14:07

staebler force-pushed the rework_cluster_relocate branch from cb2aa73 to dc6f5db Compare June 26, 2020 16:24

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2020

openshift-merge-robot merged commit 64c4461 into openshift:master Jun 27, 2020

dgoodwin mentioned this pull request Feb 12, 2021

Add some documentation around ClusterRelocate. #1286

Merged

2uasimojo mentioned this pull request Dec 22, 2022

Hive Cluster Relocation is stuck on kube-root-ca.crt and openshift-service-ca.crt already exists #1932

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ClusterRelocate to move ClusterDeployments between Hive instances #1011

ClusterRelocate to move ClusterDeployments between Hive instances #1011

staebler commented May 28, 2020 •

edited

Loading

joelddiaz Jun 2, 2020

staebler Jun 17, 2020

staebler commented Jun 18, 2020

dgoodwin Jun 22, 2020

dgoodwin Jun 25, 2020

staebler Jun 25, 2020

joelddiaz Jun 22, 2020

staebler Jun 22, 2020

joelddiaz Jun 24, 2020

dgoodwin commented Jun 26, 2020

dgoodwin commented Jun 26, 2020

openshift-bot commented Jun 26, 2020

openshift-bot commented Jun 26, 2020

staebler commented Jun 26, 2020

dgoodwin commented Jun 26, 2020

staebler commented Jun 26, 2020

staebler commented Jun 26, 2020

dgoodwin commented Jun 26, 2020

openshift-ci-robot commented Jun 26, 2020

openshift-bot commented Jun 26, 2020

openshift-bot commented Jun 26, 2020

ClusterRelocate to move ClusterDeployments between Hive instances #1011

ClusterRelocate to move ClusterDeployments between Hive instances #1011

Conversation

staebler commented May 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staebler commented Jun 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgoodwin commented Jun 26, 2020

dgoodwin commented Jun 26, 2020

openshift-bot commented Jun 26, 2020

openshift-bot commented Jun 26, 2020

staebler commented Jun 26, 2020

dgoodwin commented Jun 26, 2020

staebler commented Jun 26, 2020

staebler commented Jun 26, 2020

dgoodwin commented Jun 26, 2020

openshift-ci-robot commented Jun 26, 2020

openshift-bot commented Jun 26, 2020

openshift-bot commented Jun 26, 2020

staebler commented May 28, 2020 •

edited

Loading