Updating generic registry to return UID while deleting the object

Kubernetes-commit: 44fc88cecd5ab175fe7907eb7b975f0a00cb2305

pkg/endpoints/handlers/rest.go

@@ -1059,7 +1059,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
 type resultFunc func() (runtime.Object, error)
 
 // finishRequest makes a given resultFunc asynchronous and handles errors returned by the response.
-// Any api.Status object returned is considered an "error", which interrupts the normal response flow.
+// An api.Status object with status != success is considered an "error", which interrupts the normal response flow.
 func finishRequest(timeout time.Duration, fn resultFunc) (result runtime.Object, err error) {
 	// these channels need to be buffered to prevent the goroutine below from hanging indefinitely
 	// when the select statement reads something other than the one the goroutine sends on.
@@ -1083,7 +1083,9 @@ func finishRequest(timeout time.Duration, fn resultFunc) (result runtime.Object,
 	select {
 	case result = <-ch:
 		if status, ok := result.(*metav1.Status); ok {
-			return nil, errors.FromObject(status)
+			if status.Status != metav1.StatusSuccess {
+				return nil, errors.FromObject(status)
+			}
 		}
 		return result, nil
 	case err = <-errCh:

pkg/endpoints/handlers/rest_test.go

@@ -597,3 +597,62 @@ func TestParseTimeout(t *testing.T) {
 		t.Errorf("10s timeout produced: %v", d)
 	}
 }
+
+func TestFinishRequest(t *testing.T) {
+	exampleObj := &example.Pod{}
+	exampleErr := fmt.Errorf("error")
+	successStatusObj := &metav1.Status{Status: metav1.StatusSuccess, Message: "success message"}
+	errorStatusObj := &metav1.Status{Status: metav1.StatusFailure, Message: "error message"}
+	testcases := []struct {
+		timeout     time.Duration
+		fn          resultFunc
+		expectedObj runtime.Object
+		expectedErr error
+	}{
+		{
+			// Expected obj is returned.
+			timeout: time.Second,
+			fn: func() (runtime.Object, error) {
+				return exampleObj, nil
+			},
+			expectedObj: exampleObj,
+			expectedErr: nil,
+		},
+		{
+			// Expected error is returned.
+			timeout: time.Second,
+			fn: func() (runtime.Object, error) {
+				return nil, exampleErr
+			},
+			expectedObj: nil,
+			expectedErr: exampleErr,
+		},
+		{
+			// Successful status object is returned as expected.
+			timeout: time.Second,
+			fn: func() (runtime.Object, error) {
+				return successStatusObj, nil
+			},
+			expectedObj: successStatusObj,
+			expectedErr: nil,
+		},
+		{
+			// Error status object is converted to StatusError.
+			timeout: time.Second,
+			fn: func() (runtime.Object, error) {
+				return errorStatusObj, nil
+			},
+			expectedObj: nil,
+			expectedErr: apierrors.FromObject(errorStatusObj),
+		},
+	}
+	for i, tc := range testcases {
+		obj, err := finishRequest(tc.timeout, tc.fn)
+		if (err == nil && tc.expectedErr != nil) || (err != nil && tc.expectedErr == nil) || (err != nil && tc.expectedErr != nil && err.Error() != tc.expectedErr.Error()) {
+			t.Errorf("%d: unexpected err. expected: %v, got: %v", i, tc.expectedErr, err)
+		}
+		if !apiequality.Semantic.DeepEqual(obj, tc.expectedObj) {
+			t.Errorf("%d: unexpected obj. expected %#v, got %#v", tc.expectedObj, obj)
+		}
+	}
+}

pkg/registry/generic/registry/store.go

@@ -992,7 +992,20 @@ func (e *Store) finalizeDelete(obj runtime.Object, runHooks bool) (runtime.Objec
 		}
 		return obj, nil
 	}
-	return &metav1.Status{Status: metav1.StatusSuccess}, nil
+	// Return information about the deleted object, which enables clients to
+	// verify that the object was actually deleted and not waiting for finalizers.
+	accessor, err := meta.Accessor(obj)
+	if err != nil {
+		return nil, err
+	}
+	details := &metav1.StatusDetails{
+		Name:  accessor.GetName(),
+		Group: e.QualifiedResource.Group,
+		Kind:  e.QualifiedResource.Resource, // Yes we set Kind field to resource.
+		UID:   accessor.GetUID(),
+	}
+	status := &metav1.Status{Status: metav1.StatusSuccess, Details: details}
+	return status, nil
 }
 
 // Watch makes a matcher for the given label and field, and calls

pkg/registry/generic/registry/store_test.go

@@ -1542,3 +1542,29 @@ func newTestGenericStoreRegistry(t *testing.T, scheme *runtime.Scheme, hasCacheE
 		Storage: s,
 	}
 }
+
+func TestFinalizeDelete(t *testing.T) {
+	// Verify that it returns the expected Status.
+	_, s := NewTestGenericStoreRegistry(t)
+	obj := &example.Pod{
+		ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "random-uid"},
+	}
+	result, err := s.finalizeDelete(obj, false)
+	if err != nil {
+		t.Fatalf("unexpected err: %s", err)
+	}
+	returnedObj := result.(*metav1.Status)
+
+	expectedObj := &metav1.Status{
+		Status: metav1.StatusSuccess,
+		Details: &metav1.StatusDetails{
+			Name:  "foo",
+			Group: s.QualifiedResource.Group,
+			Kind:  s.QualifiedResource.Resource,
+			UID:   "random-uid",
+		},
+	}
+	if !apiequality.Semantic.DeepEqual(expectedObj, returnedObj) {
+		t.Errorf("unexpected obj. expected %#v, got %#v", expectedObj, returnedObj)
+	}
+}