diff --git a/cmd/app/controller.go b/cmd/app/controller.go index a21e8b51..49d8a072 100644 --- a/cmd/app/controller.go +++ b/cmd/app/controller.go @@ -413,8 +413,8 @@ func (c *Controller) syncHandler(wqKey images.WorkQueueKey) error { // Convert the namespace/name string into a distinct namespace and name namespace, name, err := cache.SplitMetaNamespaceKey(wqKey.ObjKey) if err != nil { - runtime.HandleError(fmt.Errorf("invalid resource key: %s", wqKey.ObjKey)) - return nil + glog.Errorf("Error from cache.SplitMetaNamespaceKey(): %v", err) + return err } glog.Infof("Starting to sync image cache %s(%s)", name, wqKey.WorkType) @@ -427,10 +427,7 @@ func (c *Controller) syncHandler(wqKey images.WorkQueueKey) error { if err != nil { // The ImageCache resource may no longer exist, in which case we stop // processing. - if errors.IsNotFound(err) { - runtime.HandleError(fmt.Errorf("ImageCache '%s' in work queue no longer exists", wqKey.ObjKey)) - return nil - } + glog.Errorf("Error getting imagecache(%s): %v", name, err) return err } @@ -440,7 +437,7 @@ func (c *Controller) syncHandler(wqKey images.WorkQueueKey) error { status.Reason = fledgedv1alpha1.ImageCacheReasonCacheSpecValidationFailed status.Message = err.Error() - if err = c.updateImageCacheStatus(imageCache, status); err != nil { + if err := c.updateImageCacheStatus(imageCache, status); err != nil { glog.Errorf("Error updating imagecache status to %s: %v", status.Status, err) return err } @@ -453,7 +450,7 @@ func (c *Controller) syncHandler(wqKey images.WorkQueueKey) error { status.Reason = fledgedv1alpha1.ImageCacheReasonOldImageCacheNotFound status.Message = fledgedv1alpha1.ImageCacheMessageOldImageCacheNotFound - if err = c.updateImageCacheStatus(imageCache, status); err != nil { + if err := c.updateImageCacheStatus(imageCache, status); err != nil { glog.Errorf("Error updating imagecache status to %s: %v", status.Status, err) return err } diff --git a/cmd/app/controller_test.go b/cmd/app/controller_test.go index 7f0af0a1..cc6fd934 100644 --- a/cmd/app/controller_test.go +++ b/cmd/app/controller_test.go @@ -27,7 +27,9 @@ import ( fledgedclientsetfake "github.com/senthilrch/kube-fledged/pkg/client/clientset/versioned/fake" informers "github.com/senthilrch/kube-fledged/pkg/client/informers/externalversions" fledgedinformers "github.com/senthilrch/kube-fledged/pkg/client/informers/externalversions/fledged/v1alpha1" + "github.com/senthilrch/kube-fledged/pkg/images" batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -540,3 +542,380 @@ func TestAddRemoveFinalizers(t *testing.T) { } t.Logf("%d tests passed", len(tests)) } + +func TestSyncHandler(t *testing.T) { + type ActionReaction struct { + action string + reaction string + } + now := metav1.Now() + defaultImageCache := fledgedv1alpha1.ImageCache{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "kube-fledged", + }, + Spec: fledgedv1alpha1.ImageCacheSpec{ + CacheSpec: []fledgedv1alpha1.CacheSpecImages{ + { + Images: []string{"foo"}, + }, + }, + }, + } + defaultNodeList := &corev1.NodeList{ + Items: []corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "fakenode", + Labels: map[string]string{"foo": "bar"}, + }, + }, + }, + } + + tests := []struct { + name string + imageCache fledgedv1alpha1.ImageCache + wqKey images.WorkQueueKey + nodeList *corev1.NodeList + expectedActions []ActionReaction + expectErr bool + expectedErrString string + }{ + { + name: "#1: Invalid imagecache resource key", + wqKey: images.WorkQueueKey{ + ObjKey: "foo/bar/car", + }, + expectErr: true, + expectedErrString: "unexpected key format", + }, + { + name: "#2: Create - Invalid imagecache spec (no images specified)", + imageCache: fledgedv1alpha1.ImageCache{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "kube-fledged", + }, + Spec: fledgedv1alpha1.ImageCacheSpec{ + CacheSpec: []fledgedv1alpha1.CacheSpecImages{ + { + Images: []string{}, + }, + }, + }, + }, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheCreate, + }, + expectedActions: []ActionReaction{{action: "update", reaction: ""}}, + expectErr: true, + expectedErrString: "No images specified within image list", + }, + { + name: "#3: Update - Old imagecache pointer is nil", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheUpdate, + OldImageCache: nil, + }, + nodeList: defaultNodeList, + expectedActions: []ActionReaction{{action: "update", reaction: ""}}, + expectErr: true, + expectedErrString: "OldImageCacheNotFound", + }, + { + name: "#4: Update - No. of imagelists not equal", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheUpdate, + OldImageCache: &fledgedv1alpha1.ImageCache{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "kube-fledged", + }, + Spec: fledgedv1alpha1.ImageCacheSpec{ + CacheSpec: []fledgedv1alpha1.CacheSpecImages{ + { + Images: []string{"foo"}, + }, + { + Images: []string{"bar"}, + }, + }, + }, + }, + }, + nodeList: defaultNodeList, + expectedActions: []ActionReaction{{action: "update", reaction: ""}}, + expectErr: true, + expectedErrString: "NotSupportedUpdates", + }, + { + name: "#5: Update - Change in NodeSelectors", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheUpdate, + OldImageCache: &fledgedv1alpha1.ImageCache{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "kube-fledged", + }, + Spec: fledgedv1alpha1.ImageCacheSpec{ + CacheSpec: []fledgedv1alpha1.CacheSpecImages{ + { + Images: []string{"foo"}, + NodeSelector: map[string]string{"foo": "bar"}, + }, + }, + }, + }, + }, + nodeList: defaultNodeList, + expectedActions: []ActionReaction{{action: "update", reaction: ""}}, + expectErr: true, + expectedErrString: "NotSupportedUpdates", + }, + { + name: "#6: Create - Error in adding finalizer", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheCreate, + }, + nodeList: defaultNodeList, + expectedActions: []ActionReaction{{action: "update", reaction: "fake error"}}, + expectErr: true, + expectedErrString: "Internal error occurred: fake error", + }, + { + name: "#7: Refresh - Update status to processing", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheRefresh, + }, + nodeList: defaultNodeList, + expectedActions: []ActionReaction{ + {action: "get", reaction: ""}, + {action: "update", reaction: "fake error"}, + }, + expectErr: true, + expectedErrString: "Internal error occurred: fake error", + }, + { + name: "#8: Purge - Update status to processing", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCachePurge, + }, + nodeList: defaultNodeList, + expectedActions: []ActionReaction{ + {action: "get", reaction: ""}, + {action: "update", reaction: "fake error"}, + }, + expectErr: true, + expectedErrString: "Internal error occurred: fake error", + }, + { + name: "#9: Create - Successfully firing imagepull requests", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheCreate, + }, + nodeList: defaultNodeList, + expectedActions: []ActionReaction{ + {action: "get", reaction: ""}, + {action: "update", reaction: ""}, + }, + expectErr: false, + expectedErrString: "", + }, + { + name: "#10: Update - Successfully firing imagepull & imagedelete requests", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheUpdate, + OldImageCache: &fledgedv1alpha1.ImageCache{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "kube-fledged", + }, + Spec: fledgedv1alpha1.ImageCacheSpec{ + CacheSpec: []fledgedv1alpha1.CacheSpecImages{ + { + Images: []string{"foo", "bar"}, + }, + }, + }, + }, + }, + nodeList: defaultNodeList, + expectedActions: []ActionReaction{ + {action: "get", reaction: ""}, + {action: "update", reaction: ""}, + }, + expectErr: false, + expectedErrString: "", + }, + { + name: "#11: StatusUpdate - ImagesPulledSuccessfully", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheStatusUpdate, + Status: &map[string]images.ImageWorkResult{ + "job1": { + Status: images.ImageWorkResultStatusSucceeded, + ImageWorkRequest: images.ImageWorkRequest{ + WorkType: images.ImageCacheCreate, + }, + }, + }, + }, + expectedActions: []ActionReaction{ + {action: "get", reaction: ""}, + {action: "update", reaction: ""}, + }, + expectErr: false, + expectedErrString: "", + }, + { + name: "#12: StatusUpdate - ImagesDeletedSuccessfully", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheStatusUpdate, + Status: &map[string]images.ImageWorkResult{ + "job1": { + Status: images.ImageWorkResultStatusSucceeded, + ImageWorkRequest: images.ImageWorkRequest{ + WorkType: images.ImageCachePurge, + }, + }, + }, + }, + expectedActions: []ActionReaction{ + {action: "get", reaction: ""}, + {action: "update", reaction: ""}, + }, + expectErr: false, + expectedErrString: "", + }, + { + name: "#13: StatusUpdate - ImagePullFailedForSomeImages", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheStatusUpdate, + Status: &map[string]images.ImageWorkResult{ + "job1": { + Status: images.ImageWorkResultStatusFailed, + ImageWorkRequest: images.ImageWorkRequest{ + WorkType: images.ImageCacheCreate, + }, + }, + }, + }, + expectedActions: []ActionReaction{ + {action: "get", reaction: ""}, + {action: "update", reaction: ""}, + }, + expectErr: false, + expectedErrString: "", + }, + { + name: "#14: StatusUpdate - ImageDeleteFailedForSomeImages", + imageCache: defaultImageCache, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheStatusUpdate, + Status: &map[string]images.ImageWorkResult{ + "job1": { + Status: images.ImageWorkResultStatusFailed, + ImageWorkRequest: images.ImageWorkRequest{ + WorkType: images.ImageCachePurge, + }, + }, + }, + }, + expectedActions: []ActionReaction{ + {action: "get", reaction: ""}, + {action: "update", reaction: ""}, + }, + expectErr: false, + expectedErrString: "", + }, + { + name: "#15: Delete - Remove Finalizer", + imageCache: fledgedv1alpha1.ImageCache{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "kube-fledged", + DeletionTimestamp: &now, + Finalizers: []string{"fledged"}, + }, + Spec: fledgedv1alpha1.ImageCacheSpec{ + CacheSpec: []fledgedv1alpha1.CacheSpecImages{ + { + Images: []string{"foo"}, + }, + }, + }, + }, + wqKey: images.WorkQueueKey{ + ObjKey: "kube-fledged/foo", + WorkType: images.ImageCacheDelete, + }, + expectedActions: []ActionReaction{ + {action: "get", reaction: ""}, + {action: "update", reaction: ""}, + }, + expectErr: false, + expectedErrString: "", + }, + } + + for _, test := range tests { + fakekubeclientset := &fakeclientset.Clientset{} + fakefledgedclientset := &fledgedclientsetfake.Clientset{} + for _, ar := range test.expectedActions { + if ar.reaction != "" { + apiError := apierrors.NewInternalError(fmt.Errorf(ar.reaction)) + fakefledgedclientset.AddReactor(ar.action, "imagecaches", func(action core.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, apiError + }) + } + fakefledgedclientset.AddReactor(ar.action, "imagecaches", func(action core.Action) (handled bool, ret runtime.Object, err error) { + return false, &test.imageCache, nil + }) + } + + controller, nodeInformer, imagecacheInformer := newTestController(fakekubeclientset, fakefledgedclientset) + if test.nodeList != nil && len(test.nodeList.Items) > 0 { + for _, node := range test.nodeList.Items { + nodeInformer.Informer().GetIndexer().Add(&node) + } + } + imagecacheInformer.Informer().GetIndexer().Add(&test.imageCache) + err := controller.syncHandler(test.wqKey) + if test.expectErr { + if err == nil { + t.Errorf("Test: %s failed: expectedError=%s, actualError=nil", test.name, test.expectedErrString) + } + if err != nil && !strings.HasPrefix(err.Error(), test.expectedErrString) { + t.Errorf("Test: %s failed: expectedError=%s, actualError=%s", test.name, test.expectedErrString, err.Error()) + } + } else if err != nil { + t.Errorf("Test: %s failed. expectedError=nil, actualError=%s", test.name, err.Error()) + } + } + t.Logf("%d tests passed", len(tests)) +}