update EnqueueControllerOf to properly handle Delete (#146)

* update EnqueueControllerOf to properly handle Delete

* update existing test

* add enqueue label tests

* address comments

controller/controller.go

@@ -123,12 +123,9 @@ func (c *Impl) Enqueue(obj interface{}) {
 // EnqueueControllerOf takes a resource, identifies its controller resource,
 // converts it into a namespace/name string, and passes that to EnqueueKey.
 func (c *Impl) EnqueueControllerOf(obj interface{}) {
-	// TODO(mattmoor): This will not properly handle Delete, which we do
+	object, err := getObject(obj)
-	// not currently use.  Consider using "cache.DeletedFinalStateUnknown"
-	// to enqueue the last known owner.
-	object, err := meta.Accessor(obj)
 	if err != nil {
-		c.logger.Error(zap.Error(err))
+		c.logger.Error(err)
 		return
 	}
 
@@ -139,6 +136,61 @@ func (c *Impl) EnqueueControllerOf(obj interface{}) {
 	}
 }
 
+// EnqueueLabelOf returns with an Enqueue func that takes a resource,
+// identifies its controller resource through given namespace and name labels,
+// converts it into a namespace/name string, and passes that to EnqueueKey.
+// Callers should pass in an empty string as namespace label key for obj
+// whose controller is of cluster-scoped resource.
+func (c *Impl) EnqueueLabelOf(namespaceLabel, nameLabel string) func(obj interface{}) {
+	return func(obj interface{}) {
+		object, err := getObject(obj)
+		if err != nil {
+			c.logger.Error(err)
+			return
+		}
+
+		labels := object.GetLabels()
+		controllerKey, ok := labels[nameLabel]
+		if !ok {
+			c.logger.Infof("Object %s/%s does not have a referring name label %s",
+				object.GetNamespace(), object.GetName(), nameLabel)
+			return
+		}
+
+		if namespaceLabel != "" {
+			controllerNamespace, ok := labels[namespaceLabel]
+			if !ok {
+				c.logger.Infof("Object %s/%s does not have a referring namespace label %s",
+					object.GetNamespace(), object.GetName(), namespaceLabel)
+				return
+			}
+
+			controllerKey = fmt.Sprintf("%s/%s", controllerNamespace, controllerKey)
+		}
+
+		c.EnqueueKey(controllerKey)
+	}
+}
+
+// getObject tries to get runtime Object from given interface in the way of Accessor first;
+// and to handle deletion, it try to fetch info from DeletedFinalStateUnknown on failure.
+func getObject(obj interface{}) (metav1.Object, error) {
+	object, err := meta.Accessor(obj)
+	if err != nil {
+		// To handle obj deletion, try to fetch info from DeletedFinalStateUnknown.
+		tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
+		if !ok {
+			return nil, fmt.Errorf("Couldn't get object from tombstone %#v", obj)
+		}
+		object, ok = tombstone.Obj.(metav1.Object)
+		if !ok {
+			return nil, fmt.Errorf("The object that Tombstone contained is not of metav1.Object %#v", obj)
+		}
+	}
+
+	return object, nil
+}
+
 // EnqueueKey takes a namespace/name string and puts it onto the work queue.
 func (c *Impl) EnqueueKey(key string) {
 	c.WorkQueue.AddRateLimited(key)

controller/controller_test.go

@@ -230,6 +230,120 @@ func TestEnqueues(t *testing.T) {
 			})
 		},
 		wantQueue: []string{"bar/baz"},
+	}, {
+		name: "enqueue controller of deleted resource with owner",
+		work: func(impl *Impl) {
+			impl.EnqueueControllerOf(cache.DeletedFinalStateUnknown{
+				Key: "foo/bar",
+				Obj: &Resource{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "foo",
+						Namespace: "bar",
+						OwnerReferences: []metav1.OwnerReference{{
+							APIVersion: gvk.GroupVersion().String(),
+							Kind:       gvk.Kind,
+							Name:       "baz",
+							Controller: &boolTrue,
+						}},
+					},
+				},
+			})
+		},
+		wantQueue: []string{"bar/baz"},
+	}, {
+		name: "enqueue controller of deleted bad resource",
+		work: func(impl *Impl) {
+			impl.EnqueueControllerOf(cache.DeletedFinalStateUnknown{
+				Key: "foo/bar",
+				Obj: "bad-resource",
+			})
+		},
+	}, {
+		name: "enqueue label of bad resource",
+		work: func(impl *Impl) {
+			impl.EnqueueLabelOf("test-ns", "test-name")("baz/blah")
+		},
+	}, {
+		name: "enqueue label of resource without label",
+		work: func(impl *Impl) {
+			impl.EnqueueLabelOf("ns-key", "name-key")(&Resource{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "bar",
+					Labels: map[string]string{
+						"ns-key": "bar",
+					},
+				},
+			})
+		},
+	}, {
+		name: "enqueue label of resource without namespace label",
+		work: func(impl *Impl) {
+			impl.EnqueueLabelOf("ns-key", "name-key")(&Resource{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "bar",
+					Labels: map[string]string{
+						"name-key": "baz",
+					},
+				},
+			})
+		},
+	}, {
+		name: "enqueue label of resource with labels",
+		work: func(impl *Impl) {
+			impl.EnqueueLabelOf("ns-key", "name-key")(&Resource{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "bar",
+					Labels: map[string]string{
+						"ns-key": "bar",
+						"name-key": "baz",
+					},
+				},
+			})
+		},
+		wantQueue: []string{"bar/baz"},
+	}, {
+		name: "enqueue label of resource with empty namespace label (cluster-scoped resource)",
+		work: func(impl *Impl) {
+			impl.EnqueueLabelOf("", "name-key")(&Resource{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "bar",
+					Labels: map[string]string{
+						"name-key": "baz",
+					},
+				},
+			})
+		},
+		wantQueue: []string{"baz"},
+	}, {
+		name: "enqueue label of deleted resource with label",
+		work: func(impl *Impl) {
+			impl.EnqueueLabelOf("ns-key", "name-key")(cache.DeletedFinalStateUnknown{
+				Key: "foo/bar",
+				Obj: &Resource{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "foo",
+						Namespace: "bar",
+						Labels: map[string]string{
+							"ns-key": "bar",
+							"name-key": "baz",
+						},
+					},
+				},
+			})
+		},
+		wantQueue: []string{"bar/baz"},
+	}, {
+		name: "enqueue controller of deleted bad resource",
+		work: func(impl *Impl) {
+			impl.EnqueueLabelOf("ns-key", "name-key")(cache.DeletedFinalStateUnknown{
+				Key: "foo/bar",
+				Obj: "bad-resource",
+			})
+		},
 	}}
 
 	for _, test := range tests {