Add orphaning finalizer logic to gc

kubernetes · May 13, 2016 · d90165a · d90165a
1 parent fcf7fd2
commit d90165a
Show file tree

Hide file tree

Showing 11 changed files with 350 additions and 5 deletions.
diff --git a/pkg/api/meta.go b/pkg/api/meta.go
@@ -90,6 +90,8 @@ func (meta *ObjectMeta) GetLabels() map[string]string                 { return m
 func (meta *ObjectMeta) SetLabels(labels map[string]string)           { meta.Labels = labels }
 func (meta *ObjectMeta) GetAnnotations() map[string]string            { return meta.Annotations }
 func (meta *ObjectMeta) SetAnnotations(annotations map[string]string) { meta.Annotations = annotations }
+func (meta *ObjectMeta) GetFinalizers() []string                      { return meta.Finalizers }
+func (meta *ObjectMeta) SetFinalizers(finalizers []string)            { meta.Finalizers = finalizers }
 
 func (meta *ObjectMeta) GetOwnerReferences() []metatypes.OwnerReference {
 	ret := make([]metatypes.OwnerReference, len(meta.OwnerReferences))

diff --git a/pkg/api/meta/interfaces.go b/pkg/api/meta/interfaces.go
@@ -58,6 +58,8 @@ type Object interface {
 	SetLabels(labels map[string]string)
 	GetAnnotations() map[string]string
 	SetAnnotations(annotations map[string]string)
+	GetFinalizers() []string
+	SetFinalizers(finalizers []string)
 	GetOwnerReferences() []metatypes.OwnerReference
 	SetOwnerReferences([]metatypes.OwnerReference)
 }

diff --git a/pkg/api/meta/meta.go b/pkg/api/meta/meta.go
@@ -407,6 +407,7 @@ type genericAccessor struct {
 	labels            *map[string]string
 	annotations       *map[string]string
 	ownerReferences   reflect.Value
+	finalizers        *[]string
 }
 
 func (a genericAccessor) GetNamespace() string {
@@ -539,6 +540,17 @@ func (a genericAccessor) SetAnnotations(annotations map[string]string) {
 	*a.annotations = annotations
 }
 
+func (a genericAccessor) GetFinalizers() []string {
+	if a.finalizers == nil {
+		return nil
+	}
+	return *a.finalizers
+}
+
+func (a genericAccessor) SetFinalizers(finalizers []string) {
+	*a.finalizers = finalizers
+}
+
 func (a genericAccessor) GetOwnerReferences() []metatypes.OwnerReference {
 	var ret []metatypes.OwnerReference
 	s := a.ownerReferences
@@ -611,6 +623,9 @@ func extractFromObjectMeta(v reflect.Value, a *genericAccessor) error {
 	if err := runtime.FieldPtr(v, "Annotations", &a.annotations); err != nil {
 		return err
 	}
+	if err := runtime.FieldPtr(v, "Finalizers", &a.finalizers); err != nil {
+		return err
+	}
 	ownerReferences := v.FieldByName("OwnerReferences")
 	if !ownerReferences.IsValid() {
 		return fmt.Errorf("struct %#v lacks OwnerReferences type", v)

diff --git a/pkg/api/meta/meta_test.go b/pkg/api/meta/meta_test.go
@@ -43,6 +43,10 @@ func TestAPIObjectMeta(t *testing.T) {
 			SelfLink:        "some/place/only/we/know",
 			Labels:          map[string]string{"foo": "bar"},
 			Annotations:     map[string]string{"x": "y"},
+			Finalizers: []string{
+				"finalizer.1",
+				"finalizer.2",
+			},
 		},
 	}
 	var _ meta.Object = &j.ObjectMeta
@@ -72,6 +76,9 @@ func TestAPIObjectMeta(t *testing.T) {
 	if e, a := "some/place/only/we/know", accessor.GetSelfLink(); e != a {
 		t.Errorf("expected %v, got %v", e, a)
 	}
+	if e, a := []string{"finalizer.1", "finalizer.2"}, accessor.GetFinalizers(); !reflect.DeepEqual(e, a) {
+		t.Errorf("expected %v, got %v", e, a)
+	}
 
 	typeAccessor, err := meta.TypeAccessor(j)
 	if err != nil {
@@ -92,6 +99,7 @@ func TestAPIObjectMeta(t *testing.T) {
 	typeAccessor.SetKind("d")
 	accessor.SetResourceVersion("2")
 	accessor.SetSelfLink("google.com")
+	accessor.SetFinalizers([]string{"finalizer.3"})
 
 	// Prove that accessor changes the original object.
 	if e, a := "baz", j.Namespace; e != a {
@@ -118,6 +126,9 @@ func TestAPIObjectMeta(t *testing.T) {
 	if e, a := "google.com", j.SelfLink; e != a {
 		t.Errorf("expected %v, got %v", e, a)
 	}
+	if e, a := []string{"finalizer.3"}, j.Finalizers; !reflect.DeepEqual(e, a) {
+		t.Errorf("expected %v, got %v", e, a)
+	}
 
 	typeAccessor.SetAPIVersion("d")
 	typeAccessor.SetKind("e")
@@ -143,6 +154,7 @@ func TestGenericTypeMeta(t *testing.T) {
 		Labels            map[string]string    `json:"labels,omitempty"`
 		Annotations       map[string]string    `json:"annotations,omitempty"`
 		OwnerReferences   []api.OwnerReference `json:"ownerReferences,omitempty"`
+		Finalizers        []string             `json:"finalizers,omitempty"`
 	}
 	type Object struct {
 		TypeMeta `json:",inline"`
@@ -159,6 +171,7 @@ func TestGenericTypeMeta(t *testing.T) {
 			SelfLink:        "some/place/only/we/know",
 			Labels:          map[string]string{"foo": "bar"},
 			Annotations:     map[string]string{"x": "y"},
+			Finalizers:      []string{"finalizer.1", "finalizer.2"},
 		},
 	}
 	accessor, err := meta.Accessor(&j)
@@ -183,6 +196,9 @@ func TestGenericTypeMeta(t *testing.T) {
 	if e, a := "some/place/only/we/know", accessor.GetSelfLink(); e != a {
 		t.Errorf("expected %v, got %v", e, a)
 	}
+	if e, a := []string{"finalizer.1", "finalizer.2"}, accessor.GetFinalizers(); !reflect.DeepEqual(e, a) {
+		t.Errorf("expected %v, got %v", e, a)
+	}
 
 	typeAccessor, err := meta.TypeAccessor(&j)
 	if err != nil {
@@ -203,6 +219,7 @@ func TestGenericTypeMeta(t *testing.T) {
 	typeAccessor.SetKind("d")
 	accessor.SetResourceVersion("2")
 	accessor.SetSelfLink("google.com")
+	accessor.SetFinalizers([]string{"finalizer.3"})
 
 	// Prove that accessor changes the original object.
 	if e, a := "baz", j.Namespace; e != a {
@@ -229,6 +246,9 @@ func TestGenericTypeMeta(t *testing.T) {
 	if e, a := "google.com", j.SelfLink; e != a {
 		t.Errorf("expected %v, got %v", e, a)
 	}
+	if e, a := []string{"finalizer.3"}, j.Finalizers; !reflect.DeepEqual(e, a) {
+		t.Errorf("expected %v, got %v", e, a)
+	}
 
 	typeAccessor.SetAPIVersion("d")
 	typeAccessor.SetKind("e")
@@ -252,6 +272,7 @@ type InternalTypeMeta struct {
 	APIVersion        string               `json:"apiVersion,omitempty"`
 	Labels            map[string]string    `json:"labels,omitempty"`
 	Annotations       map[string]string    `json:"annotations,omitempty"`
+	Finalizers        []string             `json:"finalizers,omitempty"`
 	OwnerReferences   []api.OwnerReference `json:"ownerReferences,omitempty"`
 }
 
@@ -435,6 +456,7 @@ func TestGenericObjectMeta(t *testing.T) {
 		ResourceVersion   string               `json:"resourceVersion,omitempty"`
 		Labels            map[string]string    `json:"labels,omitempty"`
 		Annotations       map[string]string    `json:"annotations,omitempty"`
+		Finalizers        []string             `json:"finalizers,omitempty"`
 		OwnerReferences   []api.OwnerReference `json:"ownerReferences,omitempty"`
 	}
 	type Object struct {
@@ -455,6 +477,10 @@ func TestGenericObjectMeta(t *testing.T) {
 			SelfLink:        "some/place/only/we/know",
 			Labels:          map[string]string{"foo": "bar"},
 			Annotations:     map[string]string{"a": "b"},
+			Finalizers: []string{
+				"finalizer.1",
+				"finalizer.2",
+			},
 		},
 	}
 	accessor, err := meta.Accessor(&j)
@@ -485,6 +511,9 @@ func TestGenericObjectMeta(t *testing.T) {
 	if e, a := 1, len(accessor.GetAnnotations()); e != a {
 		t.Errorf("expected %v, got %v", e, a)
 	}
+	if e, a := []string{"finalizer.1", "finalizer.2"}, accessor.GetFinalizers(); !reflect.DeepEqual(e, a) {
+		t.Errorf("expected %v, got %v", e, a)
+	}
 
 	typeAccessor, err := meta.TypeAccessor(&j)
 	if err != nil {
@@ -507,6 +536,7 @@ func TestGenericObjectMeta(t *testing.T) {
 	accessor.SetSelfLink("google.com")
 	accessor.SetLabels(map[string]string{"other": "label"})
 	accessor.SetAnnotations(map[string]string{"c": "d"})
+	accessor.SetFinalizers([]string{"finalizer.3"})
 
 	// Prove that accessor changes the original object.
 	if e, a := "baz", j.Namespace; e != a {
@@ -539,6 +569,9 @@ func TestGenericObjectMeta(t *testing.T) {
 	if e, a := map[string]string{"c": "d"}, j.Annotations; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %#v, got %#v", e, a)
 	}
+	if e, a := []string{"finalizer.3"}, j.Finalizers; !reflect.DeepEqual(e, a) {
+		t.Errorf("expected %v, got %v", e, a)
+	}
 }
 
 func TestGenericListMeta(t *testing.T) {

diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go
@@ -395,7 +395,7 @@ func ValidateObjectMetaUpdate(newMeta, oldMeta *api.ObjectMeta, fldPath *field.P
 	allErrs = append(allErrs, ValidateImmutableField(newMeta.Namespace, oldMeta.Namespace, fldPath.Child("namespace"))...)
 	allErrs = append(allErrs, ValidateImmutableField(newMeta.UID, oldMeta.UID, fldPath.Child("uid"))...)
 	allErrs = append(allErrs, ValidateImmutableField(newMeta.CreationTimestamp, oldMeta.CreationTimestamp, fldPath.Child("creationTimestamp"))...)
-	allErrs = append(allErrs, ValidateImmutableField(newMeta.Finalizers, oldMeta.Finalizers, fldPath.Child("finalizers"))...)
+	// allErrs = append(allErrs, ValidateImmutableField(newMeta.Finalizers, oldMeta.Finalizers, fldPath.Child("finalizers"))...)
 
 	allErrs = append(allErrs, unversionedvalidation.ValidateLabels(newMeta.Labels, fldPath.Child("labels"))...)
 	allErrs = append(allErrs, ValidateAnnotations(newMeta.Annotations, fldPath.Child("annotations"))...)

diff --git a/pkg/client/cache/delta_fifo.go b/pkg/client/cache/delta_fifo.go
@@ -84,7 +84,7 @@ func NewDeltaFIFO(keyFunc KeyFunc, compressor DeltaCompressor, knownObjects KeyL
 // A note on the KeyLister used by the DeltaFIFO: It's main purpose is
 // to list keys that are "known", for the purpose of figuring out which
 // items have been deleted when Replace() or Delete() are called. The deleted
-// objet will be included in the DeleteFinalStateUnknown markers. These objects
+// object will be included in the DeleteFinalStateUnknown markers. These objects
 // could be stale.
 //
 // You may provide a function to compress deltas (e.g., represent a

diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go
@@ -43,6 +43,8 @@ import (
 
 const ResourceResyncTime = 30 * time.Second
 
+const OrphanFinalizerID = "orphanFinalizer"
+
 type monitor struct {
 	store      cache.Store
 	controller *framework.Controller
@@ -62,6 +64,7 @@ func (s objectReference) String() string {
 // Propagator.processEvent() is the sole writer of the nodes. The multi-threaded
 // GarbageCollector.processItem() reads the nodes, but it only reads the fields
 // that never get changed by Propagator.processEvent().
+// TODO: now "dependents" will be read by the orphanFinalizer routine, we need to protect it with a lock.
 type node struct {
 	identity   objectReference
 	dependents map[*node]struct{}
@@ -171,6 +174,86 @@ func referencesDiffs(old []metatypes.OwnerReference, new []metatypes.OwnerRefere
 	return added, removed
 }
 
+func shouldFinalize(e event, accessor meta.Object) bool {
+	// The delta_fifo may combine the creation and update of the object into one
+	// event, so we need to check AddEvent as well.
+	if e.oldObj == nil {
+		if accessor.GetDeletionTimestamp() == nil {
+			return false
+		}
+	} else {
+		oldAccessor, err := meta.Accessor(e.oldObj)
+		if err != nil {
+			utilruntime.HandleError(fmt.Errorf("cannot access oldObj: %v", err))
+			return false
+		}
+		// ignore the event if it's not updating DeletionTimestamp from non-nil to nil.
+		if accessor.GetDeletionTimestamp() == nil || oldAccessor.GetDeletionTimestamp() != nil {
+			return false
+		}
+	}
+	return sets.NewString(accessor.GetFinalizers()...).Has(OrphanFinalizerID)
+}
+
+func orphan(gc *GarbageCollector, owner *node) {
+	// TODO: This requires lock
+	dependents := owner.dependents
+	for dependent := range dependents {
+		deleteOwnerRefPatch := fmt.Sprintf(`{"metadata":{"ownerReferences":[{"$patch":"delete","uid":"%s"}]}}`, owner.identity.UID)
+		_, err := gc.patchObject(dependent.identity, []byte(deleteOwnerRefPatch))
+		if err != nil {
+			glog.V(6).Infof("orphaning %s failed with %v", dependent.identity, err)
+		}
+	}
+	// update the owner, remove "orphaningFinalizer" from its finalizers list
+	// TODO: maybe we should retry infinitely?
+	// TODO: Using Patch when strategicmerge supports deleting an entry from a
+	// slice of a base type.
+	maxRetries := 10
+	count := 0
+	for {
+		ownerObject, err := gc.getObject(owner.identity)
+		if err != nil {
+			utilruntime.HandleError(fmt.Errorf("cannot finalize owner %s, because cannot get it", owner.identity))
+		}
+		accessor, err := meta.Accessor(ownerObject)
+		if err != nil {
+			utilruntime.HandleError(fmt.Errorf("cannot access the owner object: %v", err))
+			break
+		}
+		finalizers := accessor.GetFinalizers()
+		var newFinalizers []string
+		found := false
+		for _, f := range finalizers {
+			if f == OrphanFinalizerID {
+				found = true
+			} else {
+				newFinalizers = append(newFinalizers, f)
+			}
+		}
+		if !found {
+			glog.V(6).Infof("the orphan finalizer is already removed from object %s", owner.identity)
+			break
+		}
+		// remove the owner from dependent's OwnerReferences
+		ownerObject.SetFinalizers(newFinalizers)
+		_, err = gc.updateObject(owner.identity, ownerObject)
+		if err == nil {
+			break
+		}
+		if err != nil && !errors.IsConflict(err) {
+			utilruntime.HandleError(fmt.Errorf("cannot update the finalizers of owner %s, with error: %v", owner.identity, err))
+			return
+		}
+		// retry if it's a conflict
+		if count < maxRetries {
+			count++
+			glog.V(6).Infof("got conflict updating the owner object %s, retry the %d time", owner.identity, count)
+		}
+		utilruntime.HandleError(fmt.Errorf("maxRetries reached, giving up on finalizing the owner object %s", owner.identity))
+	}
+}
+
 // Dequeueing an event from eventQueue, updating graph, populating dirty_queue.
 func (p *Propagator) processEvent() {
 	key, quit := p.eventQueue.Get()
@@ -214,7 +297,11 @@ func (p *Propagator) processEvent() {
 		}
 		p.insertNode(newNode)
 	case (event.eventType == addEvent || event.eventType == updateEvent) && found:
-		// TODO: finalizer: Check if ObjectMeta.DeletionTimestamp is updated from nil to non-nil
+		// caveat: if GC observes the creation of the dependents later than the
+		// deletion of the owner, then the orphaning finalizer won't be effective.
+		if shouldFinalize(event, accessor) {
+			go orphan(p.gc, existingNode)
+		}
 		// We only need to add/remove owner refs for now
 		added, removed := referencesDiffs(existingNode.owners, accessor.GetOwnerReferences())
 		if len(added) == 0 && len(removed) == 0 {
@@ -396,6 +483,26 @@ func (gc *GarbageCollector) getObject(item objectReference) (*runtime.Unstructur
 	return client.Resource(resource, item.Namespace).Get(item.Name)
 }
 
+func (gc *GarbageCollector) updateObject(item objectReference, obj *runtime.Unstructured) (*runtime.Unstructured, error) {
+	fqKind := unversioned.FromAPIVersionAndKind(item.APIVersion, item.Kind)
+	client, err := gc.clientPool.ClientForGroupVersion(fqKind.GroupVersion())
+	resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0)
+	if err != nil {
+		return nil, err
+	}
+	return client.Resource(resource, item.Namespace).Update(obj)
+}
+
+func (gc *GarbageCollector) patchObject(item objectReference, patch []byte) (*runtime.Unstructured, error) {
+	fqKind := unversioned.FromAPIVersionAndKind(item.APIVersion, item.Kind)
+	client, err := gc.clientPool.ClientForGroupVersion(fqKind.GroupVersion())
+	resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0)
+	if err != nil {
+		return nil, err
+	}
+	return client.Resource(resource, item.Namespace).Patch(item.Name, api.StrategicMergePatchType, patch)
+}
+
 func objectReferenceToUnstructured(ref objectReference) *runtime.Unstructured {
 	ret := &runtime.Unstructured{}
 	ret.SetKind(ref.Kind)