diff --git a/pkg/duck/v1alpha1/identifiable.go b/pkg/duck/v1alpha1/identifiable.go index 9a7761abc4..853b5b902d 100644 --- a/pkg/duck/v1alpha1/identifiable.go +++ b/pkg/duck/v1alpha1/identifiable.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + "k8s.io/apimachinery/pkg/runtime" "knative.dev/pkg/apis" "knative.dev/pkg/kmeta" @@ -24,6 +25,9 @@ import ( ) type Identifiable interface { + // runtime.Object can be removed once the old Topic and PullSubscription are removed. + runtime.Object + kmeta.OwnerRefable // IdentitySpec returns the IdentitySpec portion of the Spec. IdentitySpec() *duckv1alpha1.IdentitySpec diff --git a/pkg/reconciler/intevents/reconciler.go b/pkg/reconciler/intevents/reconciler.go index 1ded8ddc18..db243fdc96 100644 --- a/pkg/reconciler/intevents/reconciler.go +++ b/pkg/reconciler/intevents/reconciler.go @@ -22,6 +22,7 @@ import ( duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" inteventsv1alpha1 "github.com/google/knative-gcp/pkg/apis/intevents/v1alpha1" + pubsubv1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" clientset "github.com/google/knative-gcp/pkg/client/clientset/versioned" duck "github.com/google/knative-gcp/pkg/duck/v1alpha1" "github.com/google/knative-gcp/pkg/reconciler" @@ -62,39 +63,56 @@ type PubSubBase struct { // Also sets the following fields in the pubsubable.Status upon success // TopicID, ProjectID, and SinkURI func (psb *PubSubBase) ReconcilePubSub(ctx context.Context, pubsubable duck.PubSubable, topic, resourceGroup string) (*inteventsv1alpha1.Topic, *inteventsv1alpha1.PullSubscription, error) { + t, err := psb.reconcileTopic(ctx, pubsubable, topic) + if err != nil { + return t, nil, err + } + + ps, err := psb.ReconcilePullSubscription(ctx, pubsubable, topic, resourceGroup, false) + if err != nil { + return t, ps, err + } + return t, ps, nil +} + +func (psb *PubSubBase) reconcileTopic(ctx context.Context, pubsubable duck.PubSubable, topic string) (*inteventsv1alpha1.Topic, pkgreconciler.Event) { if pubsubable == nil { - return nil, nil, fmt.Errorf("nil pubsubable passed in") + return nil, fmt.Errorf("nil pubsubable passed in") } - namespace := pubsubable.GetObjectMeta().GetNamespace() - name := pubsubable.GetObjectMeta().GetName() - spec := pubsubable.PubSubSpec() - annotations := pubsubable.GetObjectMeta().GetAnnotations() - status := pubsubable.PubSubStatus() + name := pubsubable.GetObjectMeta().GetName() args := &resources.TopicArgs{ - Namespace: namespace, + Namespace: pubsubable.GetObjectMeta().GetNamespace(), Name: name, - Spec: spec, + Spec: pubsubable.PubSubSpec(), Owner: pubsubable, Topic: topic, Labels: resources.GetLabels(psb.receiveAdapterName, name), - Annotations: annotations, + Annotations: pubsubable.GetObjectMeta().GetAnnotations(), } newTopic := resources.MakeTopic(args) - topics := psb.pubsubClient.InternalV1alpha1().Topics(namespace) - t, err := topics.Get(name, v1.GetOptions{}) + // The old and new Topics use the same, deterministic names. So delete the old one before + // creating the new one. They cannot both be Ready=true at the same time, so by deleting the old + // Topic, we allow the new Topic to become ready. + err := psb.deleteOldPubSubTopic(ctx, pubsubable, newTopic) if err != nil { - if !apierrs.IsNotFound(err) { - logging.FromContext(ctx).Desugar().Error("Failed to get Topics", zap.Error(err)) - return nil, nil, fmt.Errorf("failed to get Topics: %w", err) - } + logging.FromContext(ctx).Desugar().Info("Unable to delete old Topic", zap.Error(err)) + return nil, err + } + + topics := psb.pubsubClient.InternalV1alpha1().Topics(newTopic.Namespace) + t, err := topics.Get(newTopic.Name, v1.GetOptions{}) + if apierrs.IsNotFound(err) { logging.FromContext(ctx).Desugar().Debug("Creating Topic", zap.Any("topic", newTopic)) t, err = topics.Create(newTopic) if err != nil { logging.FromContext(ctx).Desugar().Error("Failed to create Topic", zap.Any("topic", newTopic), zap.Error(err)) - return nil, nil, fmt.Errorf("failed to create Topic: %w", err) + return nil, fmt.Errorf("failed to create Topic: %w", err) } + } else if err != nil { + logging.FromContext(ctx).Desugar().Error("Failed to get Topic", zap.Error(err)) + return nil, fmt.Errorf("failed to get Topic: %w", err) // Check whether the specs differ and update the Topic if so. } else if !equality.Semantic.DeepDerivative(newTopic.Spec, t.Spec) { // Don't modify the informers copy. @@ -104,21 +122,17 @@ func (psb *PubSubBase) ReconcilePubSub(ctx context.Context, pubsubable duck.PubS t, err = topics.Update(desired) if err != nil { logging.FromContext(ctx).Desugar().Error("Failed to update Topic", zap.Any("topic", t), zap.Error(err)) - return nil, nil, fmt.Errorf("failed to update Topic: %w", err) + return nil, fmt.Errorf("failed to update Topic: %w", err) } } + status := pubsubable.PubSubStatus() cs := pubsubable.ConditionSet() - if err := propagateTopicStatus(t, status, cs, topic); err != nil { - return t, nil, err + return t, err } - ps, err := psb.ReconcilePullSubscription(ctx, pubsubable, topic, resourceGroup, false) - if err != nil { - return t, ps, err - } - return t, ps, nil + return t, nil } func (psb *PubSubBase) ReconcilePullSubscription(ctx context.Context, pubsubable duck.PubSubable, topic, resourceGroup string, isPushCompatible bool) (*inteventsv1alpha1.PullSubscription, pkgreconciler.Event) { @@ -181,9 +195,97 @@ func (psb *PubSubBase) ReconcilePullSubscription(ctx context.Context, pubsubable } status.SinkURI = ps.Status.SinkURI + + // The old and new PullSubscriptions can co-exist without any problems. So to bias in favor of + // double event delivery over dropped events, don't delete the old one until the new one is + // ready. + if ps.Status.IsReady() { + err = psb.deleteOldPubSubPullSubscription(ctx, pubsubable, ps) + if err != nil { + return ps, err + } + } + return ps, nil } +func (psb *PubSubBase) deleteOldPubSubTopic(_ context.Context, pubsubable duck.PubSubable, t *inteventsv1alpha1.Topic) pkgreconciler.Event { + // TODO This will be deleted at the same time as the old pubsub.cloud.google.com CRDs. That is + // expected to happen after 0.15, before 0.16. + oldT, err := psb.pubsubClient.PubsubV1alpha1().Topics(t.Namespace).Get(t.Name, v1.GetOptions{}) + if apierrs.IsNotFound(err) { + // It doesn't exist, so there is nothing to delete. + return nil + } else if err != nil { + return pkgreconciler.NewEvent(corev1.EventTypeWarning, "OldTopicGetFailed", "unable to get old Topic in the `pubsub.events.cloud.google.com` API group: %w", err) + } + if !v1.IsControlledBy(oldT, pubsubable.GetObjectMeta()) { + // If this pubsubable doesn't own it, then just ignore it. Generate an event in case users + // are interested, but do not stop reconciliation of pubsubable, nor give it a Ready=false + // status. + psb.Recorder.Eventf(pubsubable, + corev1.EventTypeWarning, + "OldTopicNotControlled", + "old Topic '%s/%s' in the `pubsub.events.cloud.google.com` API group is not controlled by this pubsubable, so won't be deleted. Actual owners: %v", + oldT.Namespace, oldT.Name, oldT.OwnerReferences) + return nil + } + + // First, to make sure the Topic is not deleted in GCP, update the Topic with a new deletion + // policy. + switch pp := oldT.Spec.PropagationPolicy; pp { + case pubsubv1alpha1.TopicPolicyCreateDelete: + c := oldT.DeepCopy() + c.Spec.PropagationPolicy = pubsubv1alpha1.TopicPolicyCreateNoDelete + oldT, err = psb.pubsubClient.PubsubV1alpha1().Topics(oldT.Namespace).Update(c) + if err != nil { + return pkgreconciler.NewEvent(corev1.EventTypeWarning, "OldTopicUpdateFailed", "unable to update propagation policy on old Topic: %w", err) + } + case pubsubv1alpha1.TopicPolicyCreateNoDelete: + // Already marked for non-deletion. + break + case pubsubv1alpha1.TopicPolicyNoCreateNoDelete: + // Already marked for non-deletion. + break + default: + return pkgreconciler.NewEvent(corev1.EventTypeWarning, "OldTopicUnknownPropagationPolicy", "unknown propagation policy on old Topic: %v", pp) + } + + err = psb.pubsubClient.PubsubV1alpha1().Topics(oldT.Namespace).Delete(oldT.Name, nil) + if err != nil { + return pkgreconciler.NewEvent(corev1.EventTypeWarning, "OldTopicDeletionFailed", "unable to delete old Topic in the `pubsub.events.cloud.google.com` API group: %w", err) + } + return nil +} + +func (psb *PubSubBase) deleteOldPubSubPullSubscription(_ context.Context, pubsubable duck.PubSubable, ps *inteventsv1alpha1.PullSubscription) pkgreconciler.Event { + // TODO This will be deleted at the same time as the old pubsub.cloud.google.com CRDs. That is + // expected to happen after 0.15, before 0.16. + oldPS, err := psb.pubsubClient.PubsubV1alpha1().PullSubscriptions(ps.Namespace).Get(ps.Name, v1.GetOptions{}) + if apierrs.IsNotFound(err) { + // It doesn't exist, so there is nothing to delete. + return nil + } else if err != nil { + return pkgreconciler.NewEvent(corev1.EventTypeWarning, "OldPullSubscriptionGetFailed", "unable to get old PullSubscription in the `pubsub.events.cloud.google.com` API group: %w", err) + } + if !v1.IsControlledBy(oldPS, pubsubable.GetObjectMeta()) { + // If this pubsubable doesn't own it, then just ignore it. Generate an event in case users + // are interested, but do not stop reconciliation of pubsubable, nor give it a Ready=false + // status. + psb.Recorder.Eventf(pubsubable, + corev1.EventTypeWarning, + "oldPullSubscriptionNotControlled", + "old PullSubscription '%s/%s' in the `pubsub.events.cloud.google.com` API group is not controlled by this pubsubable, so won't be deleted. Actual owners: %v", + oldPS.Namespace, oldPS.Name, oldPS.OwnerReferences) + return nil + } + err = psb.pubsubClient.PubsubV1alpha1().PullSubscriptions(oldPS.Namespace).Delete(oldPS.Name, nil) + if err != nil { + return pkgreconciler.NewEvent(corev1.EventTypeWarning, "OldPullSubscriptionDeletionFailed", "unable to delete old PullSubscription in the `pubsub.events.cloud.google.com` API group: %w", err) + } + return nil +} + func propagatePullSubscriptionStatus(ps *inteventsv1alpha1.PullSubscription, status *duckv1alpha1.PubSubStatus, cs *apis.ConditionSet) error { pc := ps.Status.GetTopLevelCondition() if pc == nil { diff --git a/pkg/reconciler/intevents/reconciler_test.go b/pkg/reconciler/intevents/reconciler_test.go index fb17941524..8ac63be746 100644 --- a/pkg/reconciler/intevents/reconciler_test.go +++ b/pkg/reconciler/intevents/reconciler_test.go @@ -25,12 +25,14 @@ import ( "testing" "github.com/google/go-cmp/cmp/cmpopts" + pubsubv1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" clientgotesting "k8s.io/client-go/testing" + "k8s.io/client-go/tools/record" "knative.dev/pkg/apis" "knative.dev/pkg/kmeta" @@ -55,7 +57,6 @@ const ( testProjectID = "project" receiveAdapterName = "test-receive-adapter" resourceGroup = "test-resource-group" - sinkName = "sink" failedToPropagatePullSubscriptionStatusMsg = `Failed to propagate PullSubscription status` ) @@ -832,90 +833,319 @@ func TestCreates(t *testing.T) { defer logtesting.ClearAll() for _, tc := range testCases { - cs := fakePubsubClient.NewSimpleClientset(tc.objects...) + t.Run(tc.name, func(t *testing.T) { + cs := fakePubsubClient.NewSimpleClientset(tc.objects...) - psBase := &PubSubBase{ - Base: &reconciler.Base{}, - pubsubClient: cs, - receiveAdapterName: receiveAdapterName, - } - psBase.Logger = logtesting.TestLogger(t) + psBase := &PubSubBase{ + Base: &reconciler.Base{}, + pubsubClient: cs, + receiveAdapterName: receiveAdapterName, + } + psBase.Logger = logtesting.TestLogger(t) - arl := pkgtesting.ActionRecorderList{cs} - topic, ps, err := psBase.ReconcilePubSub(context.Background(), pubsubable, testTopicID, resourceGroup) + arl := pkgtesting.ActionRecorderList{cs} + topic, ps, err := psBase.ReconcilePubSub(context.Background(), pubsubable, testTopicID, resourceGroup) - if (tc.expectedErr != "" && err == nil) || - (tc.expectedErr == "" && err != nil) || - (tc.expectedErr != "" && err != nil && tc.expectedErr != err.Error()) { - t.Errorf("Test case %q, Error mismatch, want: %q got: %q", tc.name, tc.expectedErr, err) + if (tc.expectedErr != "" && err == nil) || + (tc.expectedErr == "" && err != nil) || + (tc.expectedErr != "" && err != nil && tc.expectedErr != err.Error()) { + t.Errorf("Test case %q, Error mismatch, want: %q got: %q", tc.name, tc.expectedErr, err) + } + if diff := cmp.Diff(tc.expectedTopic, topic, ignoreLastTransitionTime); diff != "" { + t.Errorf("Test case %q, unexpected topic (-want, +got) = %v", tc.name, diff) + } + if diff := cmp.Diff(tc.expectedPS, ps, ignoreLastTransitionTime); diff != "" { + t.Errorf("Test case %q, unexpected pullsubscription (-want, +got) = %v", tc.name, diff) + } + + // Validate creates. + actions, err := arl.ActionsByVerb() + if err != nil { + t.Errorf("Error capturing actions by verb: %q", err) + } + + verifyCreateActions(t, actions.Creates, tc.wantCreates) + }) + } +} + +func TestDeletesOldPubSub(t *testing.T) { + testCases := []struct { + name string + objects []runtime.Object + expectedErr string + wantUpdates []runtime.Object + wantDeletes []clientgotesting.DeleteActionImpl + }{ + { + name: "old topic exists - CreateDelete", + objects: []runtime.Object{ + rectesting.NewPubSubTopic(name, testNS, + rectesting.WithPubSubTopicSpec(pubsubv1alpha1.TopicSpec{ + Topic: testTopicID, + PropagationPolicy: "CreateDelete", + }), + rectesting.WithPubSubTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), + }, + wantUpdates: []runtime.Object{ + rectesting.NewPubSubTopic(name, testNS, + rectesting.WithPubSubTopicSpec(pubsubv1alpha1.TopicSpec{ + Topic: testTopicID, + // Policy is changed to NoDelete version. + PropagationPolicy: "CreateNoDelete", + }), + rectesting.WithPubSubTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), + }, + wantDeletes: []clientgotesting.DeleteActionImpl{ + { + ActionImpl: clientgotesting.ActionImpl{ + Namespace: testNS, + Resource: schema.GroupVersionResource{ + Group: "pubsub.cloud.google.com", + Version: "v1alpha1", + Resource: "topics", + }, + }, + Name: name, + }, + }, + }, + { + name: "old topic exists - CreateNoDelete", + objects: []runtime.Object{ + rectesting.NewPubSubTopic(name, testNS, + rectesting.WithPubSubTopicSpec(pubsubv1alpha1.TopicSpec{ + Topic: testTopicID, + PropagationPolicy: "CreateNoDelete", + }), + rectesting.WithPubSubTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), + }, + wantDeletes: []clientgotesting.DeleteActionImpl{ + { + ActionImpl: clientgotesting.ActionImpl{ + Namespace: testNS, + Resource: schema.GroupVersionResource{ + Group: "pubsub.cloud.google.com", + Version: "v1alpha1", + Resource: "topics", + }, + }, + Name: name, + }, + }, + }, + { + name: "old topic exists - NoCreateNoDelete", + objects: []runtime.Object{ + rectesting.NewPubSubTopic(name, testNS, + rectesting.WithPubSubTopicSpec(pubsubv1alpha1.TopicSpec{ + Topic: testTopicID, + PropagationPolicy: "NoCreateNoDelete", + }), + rectesting.WithPubSubTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), + }, + wantDeletes: []clientgotesting.DeleteActionImpl{ + { + ActionImpl: clientgotesting.ActionImpl{ + Namespace: testNS, + Resource: schema.GroupVersionResource{ + Group: "pubsub.cloud.google.com", + Version: "v1alpha1", + Resource: "topics", + }, + }, + Name: name, + }, + }, + }, + { + name: "old topic exists - Bad Propagation Policy", + objects: []runtime.Object{ + rectesting.NewPubSubTopic(name, testNS, + rectesting.WithPubSubTopicSpec(pubsubv1alpha1.TopicSpec{ + Topic: testTopicID, + PropagationPolicy: "bad-policy", + }), + rectesting.WithPubSubTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), + }, + expectedErr: "unknown propagation policy on old Topic: bad-policy", + }, + { + name: "old topic exists - Not owned by the Pubsubable", + objects: []runtime.Object{ + rectesting.NewPubSubTopic(name, testNS, + rectesting.WithPubSubTopicSpec(pubsubv1alpha1.TopicSpec{ + Topic: testTopicID, + PropagationPolicy: "bad-policy", + }), + ), + }, + }, + { + name: "old PullSubscription exists - Not Owned by Pubsubable", + objects: []runtime.Object{ + rectesting.NewPubSubPullSubscription(name, testNS), + }, + }, + { + name: "old PullSubscription exists - Deleted", + objects: []runtime.Object{ + rectesting.NewPubSubPullSubscription(name, testNS, + rectesting.WithPubSubPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), + }, + wantDeletes: []clientgotesting.DeleteActionImpl{ + { + ActionImpl: clientgotesting.ActionImpl{ + Namespace: testNS, + Resource: schema.GroupVersionResource{ + Group: "pubsub.cloud.google.com", + Version: "v1alpha1", + Resource: "pullsubscriptions", + }, + }, + Name: name, + }, + }, + }, + } + + defer logtesting.ClearAll() + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + objects := append(tc.objects, []runtime.Object{ + // Ready Topic. + rectesting.NewTopic(name, testNS, + rectesting.WithTopicProjectID(testProjectID), + rectesting.WithTopicReady(testTopicID), + rectesting.WithTopicAddress(testTopicURI), + ), + // Ready PullSubscription + rectesting.NewPullSubscriptionWithNoDefaults(name, testNS, + rectesting.WithPullSubscriptionReady(apis.HTTP("foo")), + ), + }...) + cs := fakePubsubClient.NewSimpleClientset(objects...) + + psBase := &PubSubBase{ + Base: &reconciler.Base{ + Recorder: &record.FakeRecorder{}, + }, + pubsubClient: cs, + receiveAdapterName: receiveAdapterName, + } + psBase.Logger = logtesting.TestLogger(t) + + arl := pkgtesting.ActionRecorderList{cs} + _, _, err := psBase.ReconcilePubSub(context.Background(), pubsubable, testTopicID, resourceGroup) + + if (tc.expectedErr != "" && err == nil) || + (tc.expectedErr == "" && err != nil) || + (tc.expectedErr != "" && err != nil && tc.expectedErr != err.Error()) { + t.Errorf("Test case %q, Error mismatch, want: %q got: %q", tc.name, tc.expectedErr, err) + } + + actions, err := arl.ActionsByVerb() + if err != nil { + t.Errorf("Error capturing actions by verb: %q", err) + } + + verifyUpdateActions(t, actions.Updates, tc.wantUpdates, tc.objects) + verifyDeleteActions(t, actions.Deletes, tc.wantDeletes) + }) + } +} + +func verifyCreateActions(t *testing.T, actual []clientgotesting.CreateAction, expected []runtime.Object) { + for i, want := range expected { + if i >= len(actual) { + t.Errorf("Missing create: %#v", want) + continue } - if diff := cmp.Diff(tc.expectedTopic, topic, ignoreLastTransitionTime); diff != "" { - t.Errorf("Test case %q, unexpected topic (-want, +got) = %v", tc.name, diff) + got := actual[i] + obj := got.GetObject() + if diff := cmp.Diff(want, obj); diff != "" { + t.Errorf("Unexpected create (-want, +got): %s", diff) } - if diff := cmp.Diff(tc.expectedPS, ps, ignoreLastTransitionTime); diff != "" { - t.Errorf("Test case %q, unexpected pullsubscription (-want, +got) = %v", tc.name, diff) + } + if got, want := len(actual), len(expected); got > want { + for _, extra := range actual[want:] { + t.Errorf("Extra create: %#v", extra.GetObject()) } + } +} - // Previous state is used to diff resource expected state for update requests that were missed. - objPrevState := make(map[string]runtime.Object, len(tc.objects)) - for _, o := range tc.objects { - objPrevState[objKey(o)] = o - } +func verifyUpdateActions(t *testing.T, actual []clientgotesting.UpdateAction, expected []runtime.Object, originalObjects []runtime.Object) { + // Previous state is used to diff resource expected state for update requests that were missed. + objPrevState := make(map[string]runtime.Object, len(originalObjects)) + for _, o := range originalObjects { + objPrevState[objKey(o)] = o + } - // Validate creates. - actions, err := arl.ActionsByVerb() - if err != nil { - t.Errorf("Error capturing actions by verb: %q", err) - } - for i, want := range tc.wantCreates { - if i >= len(actions.Creates) { - t.Errorf("Missing create: %#v", want) + updates := filterUpdatesWithSubresource("", actual) + for i, want := range actual { + if i >= len(updates) { + wo := want.GetObject() + key := objKey(wo) + oldObj, ok := objPrevState[key] + if !ok { + t.Errorf("Object %s was never created: want: %#v", key, wo) continue } - got := actions.Creates[i] - obj := got.GetObject() - if diff := cmp.Diff(want, obj); diff != "" { - t.Errorf("Unexpected create (-want, +got): %s", diff) - } + t.Errorf("Missing update for %s (-want, +prevState): %s", key, + cmp.Diff(wo, oldObj, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty())) + continue } - if got, want := len(actions.Creates), len(tc.wantCreates); got > want { - for _, extra := range actions.Creates[want:] { - t.Errorf("Extra create: %#v", extra.GetObject()) - } + + if want.GetSubresource() != "" { + t.Errorf("Expectation was invalid - it should not include a subresource: %#v", want) } - updates := filterUpdatesWithSubresource("", actions.Updates) - for i, want := range tc.wantUpdates { - if i >= len(updates) { - wo := want.GetObject() - key := objKey(wo) - oldObj, ok := objPrevState[key] - if !ok { - t.Errorf("Object %s was never created: want: %#v", key, wo) - continue - } - t.Errorf("Missing update for %s (-want, +prevState): %s", key, - cmp.Diff(wo, oldObj, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty())) - continue - } + got := updates[i].GetObject() - if want.GetSubresource() != "" { - t.Errorf("Expectation was invalid - it should not include a subresource: %#v", want) - } + // Update the object state. + objPrevState[objKey(got)] = got - got := updates[i].GetObject() + if diff := cmp.Diff(want.GetObject(), got, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("Unexpected update (-want, +got): %s", diff) + } + } + if got, want := len(updates), len(actual); got > want { + for _, extra := range updates[want:] { + t.Errorf("Extra update: %#v", extra.GetObject()) + } + } +} - // Update the object state. - objPrevState[objKey(got)] = got +func verifyDeleteActions(t *testing.T, actual []clientgotesting.DeleteAction, expected []clientgotesting.DeleteActionImpl) { + for i, want := range expected { + if i >= len(actual) { + t.Errorf("Missing delete: %#v", want) + continue + } + got := actual[i] - if diff := cmp.Diff(want.GetObject(), got, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty()); diff != "" { - t.Errorf("Unexpected update (-want, +got): %s", diff) - } + wantGVR := want.GetResource() + gotGVR := got.GetResource() + if diff := cmp.Diff(wantGVR, gotGVR); diff != "" { + t.Errorf("Unexpected delete GVR (-want +got): %s", diff) } - if got, want := len(updates), len(tc.wantUpdates); got > want { - for _, extra := range updates[want:] { - t.Errorf("Extra update: %#v", extra.GetObject()) - } + if w, g := want.Namespace, got.GetNamespace(); w != g { + t.Errorf("Unexpected delete namespace. Expected %q, actually %q", w, g) + } + if w, g := want.Name, got.GetName(); w != g { + t.Errorf("Unexpected delete name. Expected %q, actually %q", w, g) + } + } + if got, want := len(actual), len(expected); got > want { + for _, extra := range actual[want:] { + t.Errorf("Extra delete: %#v", extra) } } }