Fix race condition in setting initialized and finalizers

lifecycle/object.go

@@ -1,6 +1,8 @@
 package lifecycle
 
 import (
+	"reflect"
+
 	"github.com/rancher/norman/clientbase"
 	"github.com/rancher/norman/types/slice"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -53,17 +55,16 @@ func (o *objectLifecycleAdapter) sync(key string, obj runtime.Object) error {
 	}
 
 	obj = obj.DeepCopyObject()
-	if newObj, err := o.lifecycle.Updated(obj); err != nil {
+	newObj, err := o.lifecycle.Updated(obj)
-		if newObj != nil {
+	o.update(metadata.GetName(), obj, newObj)
-			o.objectClient.Update(metadata.GetName(), newObj)
-		}
 	return err
-	} else if newObj != nil {
+}
-		_, err = o.objectClient.Update(metadata.GetName(), newObj)
-		return err
-	}
 
-	return nil
+func (o *objectLifecycleAdapter) update(name string, orig, obj runtime.Object) (runtime.Object, error) {
+	if obj != nil && !reflect.DeepEqual(orig, obj) {
+		return o.objectClient.Update(name, obj)
+	}
+	return obj, nil
 }
 
 func (o *objectLifecycleAdapter) finalize(metadata metav1.Object, obj runtime.Object) (bool, error) {
@@ -78,9 +79,7 @@ func (o *objectLifecycleAdapter) finalize(metadata metav1.Object, obj runtime.Ob
 
 	obj = obj.DeepCopyObject()
 	if newObj, err := o.lifecycle.Finalize(obj); err != nil {
-		if newObj != nil {
+		o.update(metadata.GetName(), obj, newObj)
-			o.objectClient.Update(metadata.GetName(), newObj)
-		}
 		return false, err
 	} else if newObj != nil {
 		obj = newObj
@@ -121,36 +120,65 @@ func (o *objectLifecycleAdapter) constructFinalizerKey() string {
 }
 
 func (o *objectLifecycleAdapter) create(metadata metav1.Object, obj runtime.Object) (bool, error) {
-	initialized := o.createKey()
+	if o.isInitialized(metadata) {
-
-	if metadata.GetAnnotations()[initialized] == "true" {
 		return true, nil
 	}
 
-	obj = obj.DeepCopyObject()
+	// addFinalizer will always return a DeepCopy
-	if newObj, err := o.lifecycle.Create(obj); err != nil {
+	obj, err := o.addFinalizer(obj)
-		if newObj != nil {
+	if err != nil {
-			o.objectClient.Update(metadata.GetName(), newObj)
+		return false, err
 	}
+
+	orig := obj.DeepCopyObject()
+	if newObj, err := o.lifecycle.Create(obj); err != nil {
+		o.update(metadata.GetName(), orig, newObj)
 		return false, err
 	} else if newObj != nil {
 		obj = newObj
 	}
 
+	return false, o.setInitialized(obj)
+}
+
+func (o *objectLifecycleAdapter) isInitialized(metadata metav1.Object) bool {
+	initialized := o.createKey()
+	return metadata.GetAnnotations()[initialized] == "true"
+}
+
+func (o *objectLifecycleAdapter) setInitialized(obj runtime.Object) error {
 	metadata, err := meta.Accessor(obj)
 	if err != nil {
-		return false, err
+		return err
 	}
 
+	initialized := o.createKey()
+
 	if metadata.GetAnnotations() == nil {
 		metadata.SetAnnotations(map[string]string{})
 	}
-
-	if o.objectClient.GroupVersionKind().Kind != "Namespace" {
-		metadata.SetFinalizers(append(metadata.GetFinalizers(), o.constructFinalizerKey()))
-	}
 	metadata.GetAnnotations()[initialized] = "true"
 
 	_, err = o.objectClient.Update(metadata.GetName(), obj)
-	return false, err
+	return err
+}
+
+func (o *objectLifecycleAdapter) addFinalizer(obj runtime.Object) (runtime.Object, error) {
+	obj = obj.DeepCopyObject()
+
+	metadata, err := meta.Accessor(obj)
+	if err != nil {
+		return nil, err
+	}
+
+	if o.objectClient.GroupVersionKind().Kind == "Namespace" {
+		return obj, nil
+	}
+
+	if slice.ContainsString(metadata.GetFinalizers(), o.constructFinalizerKey()) {
+		return obj, nil
+	}
+
+	metadata.SetFinalizers(append(metadata.GetFinalizers(), o.constructFinalizerKey()))
+	return o.objectClient.Update(metadata.GetName(), obj)
 }