Make a deep copy of cluster prior to calling actuator

kubernetes-sigs · Jul 12, 2018 · 2fce5e9 · 2fce5e9
1 parent b770dd7
commit 2fce5e9
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 8 deletions.
diff --git a/pkg/controller/cluster/controller.go b/pkg/controller/cluster/controller.go
@@ -59,33 +59,35 @@ func (c *ClusterControllerImpl) Init(arguments sharedinformers.ControllerInitArg
 
 // Reconcile handles enqueued messages. The delete will be handled by finalizer.
 func (c *ClusterControllerImpl) Reconcile(cluster *clusterv1.Cluster) error {
-	name := cluster.Name
+	// Deep-copy otherwise we are mutating our cache.
+	clusterCopy := cluster.DeepCopy()
+	name := clusterCopy.Name
 	glog.Infof("Running reconcile Cluster for %s\n", name)
 
-	if !cluster.ObjectMeta.DeletionTimestamp.IsZero() {
+	if !clusterCopy.ObjectMeta.DeletionTimestamp.IsZero() {
 		// no-op if finalizer has been removed.
-		if !util.Contains(cluster.ObjectMeta.Finalizers, clusterv1.ClusterFinalizer) {
+		if !util.Contains(clusterCopy.ObjectMeta.Finalizers, clusterv1.ClusterFinalizer) {
 			glog.Infof("reconciling cluster object %v causes a no-op as there is no finalizer.", name)
 			return nil
 		}
 
 		glog.Infof("reconciling cluster object %v triggers delete.", name)
-		if err := c.actuator.Delete(cluster); err != nil {
+		if err := c.actuator.Delete(clusterCopy); err != nil {
 			glog.Errorf("Error deleting cluster object %v; %v", name, err)
 			return err
 		}
 		// Remove finalizer on successful deletion.
 		glog.Infof("cluster object %v deletion successful, removing finalizer.", name)
-		cluster.ObjectMeta.Finalizers = util.Filter(cluster.ObjectMeta.Finalizers, clusterv1.ClusterFinalizer)
-		if _, err := c.clientSet.ClusterV1alpha1().Clusters(cluster.Namespace).Update(cluster); err != nil {
+		clusterCopy.ObjectMeta.Finalizers = util.Filter(clusterCopy.ObjectMeta.Finalizers, clusterv1.ClusterFinalizer)
+		if _, err := c.clientSet.ClusterV1alpha1().Clusters(clusterCopy.Namespace).Update(clusterCopy); err != nil {
 			glog.Errorf("Error removing finalizer from cluster object %v; %v", name, err)
 			return err
 		}
 		return nil
 	}
 
 	glog.Infof("reconciling cluster object %v triggers idempotent reconcile.", name)
-	err := c.actuator.Reconcile(cluster)
+	err := c.actuator.Reconcile(clusterCopy)
 	if err != nil {
 		glog.Errorf("Error reconciling cluster object %v; %v", name, err)
 		return err

diff --git a/pkg/controller/cluster/reconcile_test.go b/pkg/controller/cluster/reconcile_test.go
@@ -108,7 +108,14 @@ func TestClusterSetControllerReconcileHandler(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			finalizerRemoved := clusterUpdated && !util.Contains(clusterToTest.ObjectMeta.Finalizers, v1alpha1.ClusterFinalizer)
+			finalizerRemoved := false
+			if clusterUpdated {
+				updatedCluster, err := fakeClient.ClusterV1alpha1().Clusters(clusterToTest.Namespace).Get(clusterToTest.Name, metav1.GetOptions{})
+				if err != nil {
+					t.Fatalf("failed to get updated cluster.")
+				}
+				finalizerRemoved = !util.Contains(updatedCluster.ObjectMeta.Finalizers, v1alpha1.ClusterFinalizer)
+			}
 
 			if finalizerRemoved != test.expectFinalizerRemoved {
 				t.Errorf("Got finalizer removed %v, expected finalizer removed %v", finalizerRemoved, test.expectFinalizerRemoved)