Get updated cluster before removing finalizers (#1173)

If the cluster actuator's Delete() call updates the cluster in the
apiserver, there is a possibility that in-memory cluster object in the
cluster controller is now out of date. We either need to get-then-update
the cluster to remove the finalizer, or we need to use patch. The
version of controller-runtime we're using in release-0.1 does not
support patch in its generic client, so this change adds the
get-then-update logic, and wraps it inside a retry on conflict loop.

Signed-off-by: Andy Goldstein <goldsteina@vmware.com>

pkg/controller/cluster/BUILD.bazel

@@ -17,6 +17,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/k8s.io/client-go/util/retry:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
         "//vendor/sigs.k8s.io/controller-runtime/pkg/client:go_default_library",
         "//vendor/sigs.k8s.io/controller-runtime/pkg/controller:go_default_library",

pkg/controller/cluster/cluster_controller.go

@@ -23,6 +23,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/util/retry"
 	"k8s.io/klog"
 	clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
 	clusterv1alpha1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
@@ -127,13 +128,32 @@ func (r *ReconcileCluster) Reconcile(request reconcile.Request) (reconcile.Resul
 			klog.Errorf("Error deleting cluster object %v; %v", name, err)
 			return reconcile.Result{}, err
 		}
+
 		// Remove finalizer on successful deletion.
 		klog.Infof("cluster object %v deletion successful, removing finalizer.", name)
-		cluster.ObjectMeta.Finalizers = util.Filter(cluster.ObjectMeta.Finalizers, clusterv1.ClusterFinalizer)
-		if err := r.Client.Update(context.Background(), cluster); err != nil {
+		err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
+			// It's possible the actuator's Delete call modified the cluster. We can't guarantee that every provider
+			// updated the in memory cluster object with the latest copy of the cluster, so try to get a fresh copy.
+			//
+			// Note, because the get from the client is a cached read from the shared informer's cache, there's still a
+			// chance this could be a stale read.
+			//
+			// Note 2, this is not a Patch call because the version of controller-runtime in the release-0.1 branch
+			// does not support patching.
+			err := r.Get(context.Background(), request.NamespacedName, cluster)
+			if err != nil {
+				return err
+			}
+
+			cluster.ObjectMeta.Finalizers = util.Filter(cluster.ObjectMeta.Finalizers, clusterv1.ClusterFinalizer)
+
+			return r.Client.Update(context.Background(), cluster)
+		})
+		if err != nil {
 			klog.Errorf("Error removing finalizer from cluster object %v; %v", name, err)
 			return reconcile.Result{}, err
 		}
+
 		return reconcile.Result{}, nil
 	}