Improve conflict errors in namespace controller

pkg/controller/namespace/namespace_controller.go

@@ -59,7 +59,7 @@ func NewNamespaceController(kubeClient client.Interface, experimentalMode bool,
 		framework.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
 				namespace := obj.(*api.Namespace)
-				if err := syncNamespace(kubeClient, experimentalMode, *namespace); err != nil {
+				if err := syncNamespace(kubeClient, experimentalMode, namespace); err != nil {
 					if estimate, ok := err.(*contentRemainingError); ok {
 						go func() {
 							// Estimate is the aggregate total of TerminationGracePeriodSeconds, which defaults to 30s
@@ -81,7 +81,7 @@ func NewNamespaceController(kubeClient client.Interface, experimentalMode bool,
 			},
 			UpdateFunc: func(oldObj, newObj interface{}) {
 				namespace := newObj.(*api.Namespace)
-				if err := syncNamespace(kubeClient, experimentalMode, *namespace); err != nil {
+				if err := syncNamespace(kubeClient, experimentalMode, namespace); err != nil {
 					if estimate, ok := err.(*contentRemainingError); ok {
 						go func() {
 							t := estimate.Estimate/2 + 1
@@ -121,12 +121,12 @@ func (nm *NamespaceController) Stop() {
 }
 
 // finalized returns true if the spec.finalizers is empty list
-func finalized(namespace api.Namespace) bool {
+func finalized(namespace *api.Namespace) bool {
 	return len(namespace.Spec.Finalizers) == 0
 }
 
 // finalize will finalize the namespace for kubernetes
-func finalize(kubeClient client.Interface, namespace api.Namespace) (*api.Namespace, error) {
+func finalizeNamespaceFunc(kubeClient client.Interface, namespace *api.Namespace) (*api.Namespace, error) {
 	namespaceFinalize := api.Namespace{}
 	namespaceFinalize.ObjectMeta = namespace.ObjectMeta
 	namespaceFinalize.Spec = namespace.Spec
@@ -209,25 +209,56 @@ func deleteAllContent(kubeClient client.Interface, experimentalMode bool, namesp
 	return estimate, nil
 }
 
-// syncNamespace makes namespace life-cycle decisions
-func syncNamespace(kubeClient client.Interface, experimentalMode bool, namespace api.Namespace) (err error) {
+// updateNamespaceFunc is a function that makes an update to a namespace
+type updateNamespaceFunc func(kubeClient client.Interface, namespace *api.Namespace) (*api.Namespace, error)
+
+// retryOnConflictError retries the specified fn if there was a conflict error
+// TODO RetryOnConflict should be a generic concept in client code
+func retryOnConflictError(kubeClient client.Interface, namespace *api.Namespace, fn updateNamespaceFunc) (result *api.Namespace, err error) {
+	latestNamespace := namespace
+	for {
+		result, err = fn(kubeClient, latestNamespace)
+		if err == nil {
+			return result, nil
+		}
+		if !errors.IsConflict(err) {
+			return nil, err
+		}
+		latestNamespace, err = kubeClient.Namespaces().Get(latestNamespace.Name)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return
+}
+
+// updateNamespaceStatusFunc will verify that the status of the namespace is correct
+func updateNamespaceStatusFunc(kubeClient client.Interface, namespace *api.Namespace) (*api.Namespace, error) {
+	if namespace.DeletionTimestamp.IsZero() || namespace.Status.Phase == api.NamespaceTerminating {
+		return namespace, nil
+	}
+	newNamespace := api.Namespace{}
+	newNamespace.ObjectMeta = namespace.ObjectMeta
+	newNamespace.Status = namespace.Status
+	newNamespace.Status.Phase = api.NamespaceTerminating
+	return kubeClient.Namespaces().Status(&newNamespace)
+}
+
+// syncNamespace orchestrates deletion of a Namespace and its associated content.
+func syncNamespace(kubeClient client.Interface, experimentalMode bool, namespace *api.Namespace) (err error) {
 	if namespace.DeletionTimestamp == nil {
 		return nil
 	}
 	glog.V(4).Infof("Syncing namespace %s", namespace.Name)
 
-	// if there is a deletion timestamp, and the status is not terminating, then update status
-	if !namespace.DeletionTimestamp.IsZero() && namespace.Status.Phase != api.NamespaceTerminating {
-		newNamespace := api.Namespace{}
-		newNamespace.ObjectMeta = namespace.ObjectMeta
-		newNamespace.Status = namespace.Status
-		newNamespace.Status.Phase = api.NamespaceTerminating
-		result, err := kubeClient.Namespaces().Status(&newNamespace)
-		if err != nil {
-			return err
+	// ensure that the status is up to date on the namespace
+	// if we get a not found error, we assume the namespace is truly gone
+	namespace, err = retryOnConflictError(kubeClient, namespace, updateNamespaceStatusFunc)
+	if err != nil {
+		if errors.IsNotFound(err) {
+			return nil
 		}
-		// work with the latest copy so we can proceed to clean up right away without another interval
-		namespace = *result
+		return err
 	}
 
 	// if the namespace is already finalized, delete it
@@ -249,13 +280,13 @@ func syncNamespace(kubeClient client.Interface, experimentalMode bool, namespace
 	}
 
 	// we have removed content, so mark it finalized by us
-	result, err := finalize(kubeClient, namespace)
+	result, err := retryOnConflictError(kubeClient, namespace, finalizeNamespaceFunc)
 	if err != nil {
 		return err
 	}
 
 	// now check if all finalizers have reported that we delete now
-	if finalized(*result) {
+	if finalized(result) {
 		err = kubeClient.Namespaces().Delete(namespace.Name)
 		if err != nil && !errors.IsNotFound(err) {
 			return err