Remove Delete from handler

Signed-off-by: Xabier Larrakoetxea <[email protected]>

CHANGELOG.md

@@ -18,6 +18,7 @@ NOTE: Breaking release in controllers.
 - Add multiretriever to retriever different resource types on the same controller.
 - Refactor metrics recorder implementation including the prometheus backend.
 - Refactor internal controller queue into a decorator implementation approach.
+- Remove `Delete` method from `controller.Handler` and simplify to only `Handle` method
 
 ## [0.8.0] - 2019-12-11
 

controller/generic_test.go

@@ -104,7 +104,7 @@ func createNamespaceList(prefix string, q int) (*corev1.NamespaceList, []*corev1
 	return nsl, nss
 }
 
-func TestGenericControllerHandleAdds(t *testing.T) {
+func TestGenericControllerHandle(t *testing.T) {
 	nsList, expNSAdds := createNamespaceList("testing", 10)
 
 	tests := []struct {
@@ -113,7 +113,7 @@ func TestGenericControllerHandleAdds(t *testing.T) {
 		expNSAdds []*corev1.Namespace
 	}{
 		{
-			name:      "Listing multiple namespaces should call as add handlers for every namespace on list.",
+			name:      "Listing multiple namespaces should execute the handling for every namespace on list.",
 			nsList:    nsList,
 			expNSAdds: expNSAdds,
 		},
@@ -134,7 +134,7 @@ func TestGenericControllerHandleAdds(t *testing.T) {
 			callHandling := 0 // used to track the number of calls.
 			mh := &mcontroller.Handler{}
 			for _, ns := range test.expNSAdds {
-				mh.On("Add", mock.Anything, ns).Once().Return(nil).Run(func(args mock.Arguments) {
+				mh.On("Handle", mock.Anything, ns).Once().Return(nil).Run(func(args mock.Arguments) {
 					callHandling++
 					// Check last call, if is the last call expected then stop the controller so
 					// we can assert the expectations of the calls and finish the test.
@@ -172,80 +172,6 @@ func TestGenericControllerHandleAdds(t *testing.T) {
 	}
 }
 
-func TestGenericControllerHandleDeletes(t *testing.T) {
-
-	startNSList, expNSAdds := createNamespaceList("testing", 10)
-	nsDels := []*corev1.Namespace{expNSAdds[0], expNSAdds[4], expNSAdds[1]}
-
-	tests := []struct {
-		name        string
-		startNSList *corev1.NamespaceList
-		deleteNs    []*corev1.Namespace
-		expDeleteNs []*corev1.Namespace
-	}{
-		{
-			name:        "Deleting multiple namespaces should call as delete handlers for every namespace on deleted.",
-			startNSList: startNSList,
-			deleteNs:    nsDels,
-			expDeleteNs: nsDels,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			assert := assert.New(t)
-			require := require.New(t)
-			controllerStopperC := make(chan struct{})
-			resultC := make(chan error)
-
-			// Mocks kubernetes  client.
-			mc := &fake.Clientset{}
-			// Populate cache so we ensure deletes are correctly delivered.
-			onKubeClientListNamespaceReturn(mc, test.startNSList)
-			onKubeClientWatchNamespaceReturn(mc, nil, nil, test.deleteNs)
-
-			// Mock our handler and set expects.
-			callHandling := 0 // used to track the number of calls.
-			mh := &mcontroller.Handler{}
-			mh.On("Add", mock.Anything, mock.Anything).Return(nil)
-			for _, ns := range test.expDeleteNs {
-				mh.On("Delete", mock.Anything, ns.ObjectMeta.Name).Once().Return(nil).Run(func(args mock.Arguments) {
-					// Check last call, if is the last call expected then stop the controller so
-					// we can assert the expectations of the calls and finish the test.
-					callHandling++
-					if callHandling == len(test.expDeleteNs) {
-						close(controllerStopperC)
-					}
-				})
-			}
-
-			c, err := controller.New(&controller.Config{
-				Name:      "test",
-				Handler:   mh,
-				Retriever: newNamespaceRetriever(mc),
-				Logger:    log.Dummy,
-			})
-			require.NoError(err)
-
-			// Run Controller in background.
-			go func() {
-				resultC <- c.Run(controllerStopperC)
-			}()
-
-			// Wait for different results. If no result means error failure.
-			select {
-			case err := <-resultC:
-				if assert.NoError(err) {
-					// Check handles from the controller.
-					mh.AssertExpectations(t)
-				}
-			case <-time.After(1 * time.Second):
-				assert.Fail("timeout waiting for controller handling, this could mean the controller is not receiving resources")
-			}
-		})
-	}
-}
-
 func TestGenericControllerErrorRetries(t *testing.T) {
 	nsList, _ := createNamespaceList("testing", 11)
 
@@ -281,7 +207,7 @@ func TestGenericControllerErrorRetries(t *testing.T) {
 			// Expect all the retries
 			for range test.nsList.Items {
 				callsPerNS := test.retryNumber + 1 // initial call + retries.
-				mh.On("Add", mock.Anything, mock.Anything).Return(err).Times(callsPerNS).Run(func(args mock.Arguments) {
+				mh.On("Handle", mock.Anything, mock.Anything).Return(err).Times(callsPerNS).Run(func(args mock.Arguments) {
 					totalCalls--
 					// Check last call, if is the last call expected then stop the controller so
 					// we can assert the expectations of the calls and finish the test.
@@ -350,7 +276,7 @@ func TestGenericControllerWithLeaderElection(t *testing.T) {
 
 			// Expect the calls on the lead (mh1) and no calls on the other ones.
 			totalCalls := len(test.nsList.Items)
-			mh1.On("Add", mock.Anything, mock.Anything).Return(nil).Times(totalCalls).Run(func(args mock.Arguments) {
+			mh1.On("Handle", mock.Anything, mock.Anything).Return(nil).Times(totalCalls).Run(func(args mock.Arguments) {
 				totalCalls--
 				// Check last call, if is the last call expected then stop the controller so
 				// we can assert the expectations of the calls and finish the test.

controller/handler.go

@@ -9,35 +9,16 @@ import (
 
 // Handler knows how to handle the received resources from a kubernetes cluster.
 type Handler interface {
-	Add(context.Context, runtime.Object) error
-	Delete(context.Context, string) error
+	Handle(context.Context, runtime.Object) error
 }
 
-// AddFunc knows how to handle resource adds.
-type AddFunc func(context.Context, runtime.Object) error
+// HandlerFunc knows how to handle resource adds.
+type HandlerFunc func(context.Context, runtime.Object) error
 
-// DeleteFunc knows how to handle resource deletes.
-type DeleteFunc func(context.Context, string) error
-
-// HandlerFunc is a handler that is created from functions that the
-// Handler interface requires.
-type HandlerFunc struct {
-	AddFunc    AddFunc
-	DeleteFunc DeleteFunc
-}
-
-// Add satisfies Handler interface.
-func (h *HandlerFunc) Add(ctx context.Context, obj runtime.Object) error {
-	if h.AddFunc == nil {
-		return fmt.Errorf("function can't be nil")
-	}
-	return h.AddFunc(ctx, obj)
-}
-
-// Delete satisfies Handler interface.
-func (h *HandlerFunc) Delete(ctx context.Context, s string) error {
-	if h.DeleteFunc == nil {
-		return fmt.Errorf("function can't be nil")
+// Handle satisfies controller.Handler interface.
+func (h HandlerFunc) Handle(ctx context.Context, obj runtime.Object) error {
+	if h == nil {
+		return fmt.Errorf("handle func is required")
 	}
-	return h.DeleteFunc(ctx, s)
+	return h(ctx, obj)
 }

controller/processor.go

@@ -32,12 +32,11 @@ func newIndexerProcessor(indexer cache.Indexer, handler Handler) processor {
 			return err
 		}
 
-		// Handle the object.
 		if !exists {
-			return handler.Delete(ctx, key)
+			return nil
 		}
 
-		return handler.Add(ctx, obj.(runtime.Object))
+		return handler.Handle(ctx, obj.(runtime.Object))
 	})
 }
 

examples/config-custom-controller/main.go

@@ -55,17 +55,11 @@ func run() error {
 	})
 
 	// Our domain logic that will print every add/sync/update and delete event we .
-	hand := &controller.HandlerFunc{
-		AddFunc: func(_ context.Context, obj runtime.Object) error {
-			pod := obj.(*corev1.Pod)
-			logger.Infof("Pod added: %s/%s", pod.Namespace, pod.Name)
-			return nil
-		},
-		DeleteFunc: func(_ context.Context, s string) error {
-			logger.Infof("Pod deleted: %s", s)
-			return nil
-		},
-	}
+	hand := controller.HandlerFunc(func(_ context.Context, obj runtime.Object) error {
+		pod := obj.(*corev1.Pod)
+		logger.Infof("Pod added: %s/%s", pod.Namespace, pod.Name)
+		return nil
+	})
 
 	// Create the controller with custom configuration.
 	cfg := &controller.Config{

examples/controller-concurrency-handling/main.go

@@ -102,19 +102,12 @@ func run() error {
 	})
 
 	// Our domain logic that will print every add/sync/update and delete event we.
-	hand := &controller.HandlerFunc{
-		AddFunc: func(_ context.Context, obj runtime.Object) error {
-			pod := obj.(*corev1.Pod)
-			sleep()
-			logger.Infof("Pod added: %s/%s", pod.Namespace, pod.Name)
-			return nil
-		},
-		DeleteFunc: func(_ context.Context, s string) error {
-			sleep()
-			logger.Infof("Pod deleted: %s", s)
-			return nil
-		},
-	}
+	hand := controller.HandlerFunc(func(_ context.Context, obj runtime.Object) error {
+		pod := obj.(*corev1.Pod)
+		sleep()
+		logger.Infof("Pod added: %s/%s", pod.Namespace, pod.Name)
+		return nil
+	})
 
 	// Create the controller.
 	cfg := &controller.Config{

examples/leader-election-controller/main.go

@@ -87,17 +87,11 @@ func run() error {
 	})
 
 	// Our domain logic that will print every add/sync/update and delete event we .
-	hand := &controller.HandlerFunc{
-		AddFunc: func(_ context.Context, obj runtime.Object) error {
-			pod := obj.(*corev1.Pod)
-			logger.Infof("Pod added: %s/%s", pod.Namespace, pod.Name)
-			return nil
-		},
-		DeleteFunc: func(_ context.Context, s string) error {
-			logger.Infof("Pod deleted: %s", s)
-			return nil
-		},
-	}
+	hand := controller.HandlerFunc(func(_ context.Context, obj runtime.Object) error {
+		pod := obj.(*corev1.Pod)
+		logger.Infof("Pod added: %s/%s", pod.Namespace, pod.Name)
+		return nil
+	})
 
 	// Leader election service.
 	lesvc, err := leaderelection.NewDefault(leaderElectionKey, fl.Namespace, k8scli, logger)

examples/metrics-controller/main.go

@@ -114,16 +114,10 @@ func run() error {
 	})
 
 	// Our domain logic that will print every add/sync/update and delete event we .
-	hand := &controller.HandlerFunc{
-		AddFunc: func(_ context.Context, obj runtime.Object) error {
-			sleepRandomly()
-			return errRandomly()
-		},
-		DeleteFunc: func(_ context.Context, s string) error {
-			sleepRandomly()
-			return errRandomly()
-		},
-	}
+	hand := controller.HandlerFunc(func(_ context.Context, obj runtime.Object) error {
+		sleepRandomly()
+		return errRandomly()
+	})
 
 	// Create the controller that will refresh every 30 seconds.
 	cfg := &controller.Config{

examples/multi-resource-controller/main.go

@@ -73,26 +73,21 @@ func run() error {
 	}
 
 	// Our domain logic that will print every add/sync/update and delete event we .
-	hand := &controller.HandlerFunc{
-		AddFunc: func(_ context.Context, obj runtime.Object) error {
-			dep, ok := obj.(*appsv1.Deployment)
-			if ok {
-				logger.Infof("Deployment added: %s/%s", dep.Namespace, dep.Name)
-				return nil
-			}
-
-			st, ok := obj.(*appsv1.StatefulSet)
-			if ok {
-				logger.Infof("Statefulset added: %s/%s", st.Namespace, st.Name)
-				return nil
-			}
-
+	hand := controller.HandlerFunc(func(_ context.Context, obj runtime.Object) error {
+		dep, ok := obj.(*appsv1.Deployment)
+		if ok {
+			logger.Infof("Deployment added: %s/%s", dep.Namespace, dep.Name)
 			return nil
-		},
-		DeleteFunc: func(_ context.Context, s string) error {
+		}
+
+		st, ok := obj.(*appsv1.StatefulSet)
+		if ok {
+			logger.Infof("Statefulset added: %s/%s", st.Namespace, st.Name)
 			return nil
-		},
-	}
+		}
+
+		return nil
+	})
 
 	// Create the controller with custom configuration.
 	cfg := &controller.Config{

examples/pod-terminator-operator/apis/chaos/v1alpha1/types.go

@@ -4,11 +4,12 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// PodTerminator represents a pod terminator.
+//
 // +genclient
 // +genclient:nonNamespaced
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
-
-// PodTerminator represents a pod terminator.
+// +kubebuilder:resource:singular=podterminator,path=podterminators,shortName=pt,scope=Cluster,categories=terminators;killers;gc
 type PodTerminator struct {
 	metav1.TypeMeta `json:",inline"`
 	// Standard object's metadata.

examples/pod-terminator-operator/manifest-examples/pause.yaml

@@ -1,7 +1,8 @@
-apiVersion: apps/v1beta2
+apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: pause-pods
+  namespace: kooper-test
   labels:
     application: pause
 spec:

examples/pod-terminator-operator/manifests/chaos.spotahome.com_podterminators.yaml

@@ -10,11 +10,17 @@ metadata:
 spec:
   group: chaos.spotahome.com
   names:
+    categories:
+    - terminators
+    - killers
+    - gc
     kind: PodTerminator
     listKind: PodTerminatorList
     plural: podterminators
+    shortNames:
+    - pt
     singular: podterminator
-  scope: Namespaced
+  scope: Cluster
   validation:
     openAPIV3Schema:
       description: PodTerminator represents a pod terminator.