stashed · tamalsaha · Oct 8, 2017 · Oct 8, 2017
diff --git a/pkg/cmds/schedule.go b/pkg/cmds/schedule.go
@@ -65,36 +65,36 @@ func NewCmdSchedule() *cobra.Command {
 				if err != nil {
 					_, err := kubeClient.ExtensionsV1beta1().Deployments(opt.Namespace).Get(opt.AppName, metav1.GetOptions{})
 					if err != nil {
-						log.Fatalf(`Unknown Deployment %s@%s`, opt.AppName, opt.Namespace)
+						log.Fatalf(`Unknown Deployment %s/%s`, opt.Namespace, opt.AppName)
 					}
 				}
 			case "ReplicaSets", "ReplicaSet", "replicasets", "replicaset", "rs":
 				opt.AppKind = "ReplicaSet"
 				opt.SmartPrefix = ""
 				_, err := kubeClient.ExtensionsV1beta1().ReplicaSets(opt.Namespace).Get(opt.AppName, metav1.GetOptions{})
 				if err != nil {
-					log.Fatalf(`Unknown ReplicaSet %s@%s`, opt.AppName, opt.Namespace)
+					log.Fatalf(`Unknown ReplicaSet %s/%s`, opt.Namespace, opt.AppName)
 				}
 			case "ReplicationControllers", "ReplicationController", "replicationcontrollers", "replicationcontroller", "rc":
 				opt.AppKind = "ReplicationController"
 				opt.SmartPrefix = ""
 				_, err := kubeClient.CoreV1().ReplicationControllers(opt.Namespace).Get(opt.AppName, metav1.GetOptions{})
 				if err != nil {
-					log.Fatalf(`Unknown ReplicationController %s@%s`, opt.AppName, opt.Namespace)
+					log.Fatalf(`Unknown ReplicationController %s/%s`, opt.Namespace, opt.AppName)
 				}
 			case "StatefulSets", "StatefulSet":
 				opt.AppKind = "StatefulSet"
 				opt.SmartPrefix = opt.PodName
 				_, err := kubeClient.AppsV1beta1().StatefulSets(opt.Namespace).Get(opt.AppName, metav1.GetOptions{})
 				if err != nil {
-					log.Fatalf(`Unknown StatefulSet %s@%s`, opt.AppName, opt.Namespace)
+					log.Fatalf(`Unknown StatefulSet %s/%s`, opt.Namespace, opt.AppName)
 				}
 			case "DaemonSets", "DaemonSet", "daemonsets", "daemonset":
 				opt.AppKind = "DaemonSet"
 				opt.SmartPrefix = opt.NodeName
 				_, err := kubeClient.ExtensionsV1beta1().DaemonSets(opt.Namespace).Get(opt.AppName, metav1.GetOptions{})
 				if err != nil {
-					log.Fatalf(`Unknown DaemonSet %s@%s`, opt.AppName, opt.Namespace)
+					log.Fatalf(`Unknown DaemonSet %s/%s`, opt.Namespace, opt.AppName)
 				}
 			default:
 				log.Fatalf(`Unrecognized workload "Kind" %v`, opt.AppKind)

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -88,13 +88,13 @@ func (c *StashController) Setup() error {
 	if err := c.ensureCustomResourceDefinitions(); err != nil {
 		return err
 	}
-	c.initNamespaceWatcher()
-	c.initResticWatcher()
 	c.initDaemonSetWatcher()
 	c.initDeploymentWatcher()
 	c.initRCWatcher()
 	c.initReplicaSetWatcher()
 	c.initStatefulSetWatcher()
+	c.initNamespaceWatcher()
+	c.initResticWatcher()
 	return nil
 }
 
@@ -140,7 +140,7 @@ func (c *StashController) Run(threadiness int, stopCh chan struct{}) {
 	defer c.rcQueue.ShutDown()
 	defer c.rsQueue.ShutDown()
 	defer c.ssQueue.ShutDown()
-	glog.Info("Starting Vault controller")
+	glog.Info("Starting Stash controller")
 
 	go c.nsInformer.Run(stopCh)
 	go c.rInformer.Run(stopCh)
@@ -190,5 +190,5 @@ func (c *StashController) Run(threadiness int, stopCh chan struct{}) {
 	}
 
 	<-stopCh
-	glog.Info("Stopping Vault controller")
+	glog.Info("Stopping Stash controller")
 }
diff --git a/pkg/controller/daemonsets.go b/pkg/controller/daemonsets.go
@@ -180,11 +180,6 @@ func (c *StashController) EnsureDaemonSetSidecar(resource *extensions.DaemonSet,
 }
 
 func (c *StashController) EnsureDaemonSetSidecarDeleted(resource *extensions.DaemonSet, restic *api.Restic) (err error) {
-	if name := util.GetString(resource.Annotations, api.ConfigName); name == "" {
-		log.Infof("Restic sidecar already removed for DaemonSet %s@%s.", resource.Name, resource.Namespace)
-		return nil
-	}
-
 	_, err = ext_util.PatchDaemonSet(c.k8sClient, resource, func(obj *extensions.DaemonSet) *extensions.DaemonSet {
 		obj.Spec.Template.Spec.Containers = core_util.EnsureContainerDeleted(obj.Spec.Template.Spec.Containers, util.StashContainer)
 		obj.Spec.Template.Spec.Volumes = util.EnsureVolumeDeleted(obj.Spec.Template.Spec.Volumes, util.ScratchDirVolumeName)

diff --git a/pkg/controller/deployment.go b/pkg/controller/deployment.go
@@ -182,11 +182,6 @@ func (c *StashController) EnsureDeploymentSidecar(resource *apps.Deployment, old
 }
 
 func (c *StashController) EnsureDeploymentSidecarDeleted(resource *apps.Deployment, restic *api.Restic) (err error) {
-	if name := util.GetString(resource.Annotations, api.ConfigName); name == "" {
-		log.Infof("Restic sidecar already removed for Deployment %s@%s.", resource.Name, resource.Namespace)
-		return nil
-	}
-
 	_, err = apps_util.PatchDeployment(c.k8sClient, resource, func(obj *apps.Deployment) *apps.Deployment {
 		obj.Spec.Template.Spec.Containers = core_util.EnsureContainerDeleted(obj.Spec.Template.Spec.Containers, util.StashContainer)
 		obj.Spec.Template.Spec.Volumes = util.EnsureVolumeDeleted(obj.Spec.Template.Spec.Volumes, util.ScratchDirVolumeName)

diff --git a/pkg/controller/namespaces.go b/pkg/controller/namespaces.go
@@ -2,6 +2,7 @@ package controller
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	rt "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/watch"
 	apiv1 "k8s.io/client-go/pkg/api/v1"
@@ -18,16 +19,12 @@ func (c *StashController) initNamespaceWatcher() {
 		},
 	}
 
-	// Bind the workqueue to a cache with the help of an informer. This way we make sure that
-	// whenever the cache is updated, the pod key is added to the workqueue.
-	// Note that when we finally process the item from the workqueue, we might see a newer version
-	// of the Namespace than the version which was responsible for triggering the update.
 	c.nsIndexer, c.nsInformer = cache.NewIndexerInformer(lw, &apiv1.Namespace{}, c.options.ResyncPeriod, cache.ResourceEventHandlerFuncs{
 		DeleteFunc: func(obj interface{}) {
 			if ns, ok := obj.(*apiv1.Namespace); ok {
-				restics, err := c.stashClient.Restics(ns.Name).List(metav1.ListOptions{})
+				restics, err := c.rLister.Restics(ns.Name).List(labels.Everything())
 				if err == nil {
-					for _, restic := range restics.Items {
+					for _, restic := range restics {
 						c.stashClient.Restics(restic.Namespace).Delete(restic.Name, &metav1.DeleteOptions{})
 					}
 				}

diff --git a/pkg/controller/rcs.go b/pkg/controller/rcs.go
@@ -180,11 +180,6 @@ func (c *StashController) EnsureReplicationControllerSidecar(resource *apiv1.Rep
 }
 
 func (c *StashController) EnsureReplicationControllerSidecarDeleted(resource *apiv1.ReplicationController, restic *api.Restic) (err error) {
-	if name := util.GetString(resource.Annotations, api.ConfigName); name == "" {
-		log.Infof("Restic sidecar already removed for ReplicationController %s@%s.", resource.Name, resource.Namespace)
-		return nil
-	}
-
 	_, err = core_util.PatchRC(c.k8sClient, resource, func(obj *apiv1.ReplicationController) *apiv1.ReplicationController {
 		obj.Spec.Template.Spec.Containers = core_util.EnsureContainerDeleted(obj.Spec.Template.Spec.Containers, util.StashContainer)
 		obj.Spec.Template.Spec.Volumes = util.EnsureVolumeDeleted(obj.Spec.Template.Spec.Volumes, util.ScratchDirVolumeName)

diff --git a/pkg/controller/replicasets.go b/pkg/controller/replicasets.go
@@ -182,11 +182,6 @@ func (c *StashController) EnsureReplicaSetSidecar(resource *extensions.ReplicaSe
 }
 
 func (c *StashController) EnsureReplicaSetSidecarDeleted(resource *extensions.ReplicaSet, restic *api.Restic) (err error) {
-	if name := util.GetString(resource.Annotations, api.ConfigName); name == "" {
-		log.Infof("Restic sidecar already removed for ReplicaSet %s@%s.", resource.Name, resource.Namespace)
-		return nil
-	}
-
 	_, err = ext_util.PatchReplicaSet(c.k8sClient, resource, func(obj *extensions.ReplicaSet) *extensions.ReplicaSet {
 		obj.Spec.Template.Spec.Containers = core_util.EnsureContainerDeleted(obj.Spec.Template.Spec.Containers, util.StashContainer)
 		obj.Spec.Template.Spec.Volumes = util.EnsureVolumeDeleted(obj.Spec.Template.Spec.Volumes, util.ScratchDirVolumeName)

diff --git a/pkg/controller/statefulsets.go b/pkg/controller/statefulsets.go
@@ -182,11 +182,6 @@ func (c *StashController) EnsureStatefulSetSidecar(resource *apps.StatefulSet, o
 }
 
 func (c *StashController) EnsureStatefulSetSidecarDeleted(resource *apps.StatefulSet, restic *api.Restic) (err error) {
-	if name := util.GetString(resource.Annotations, api.ConfigName); name == "" {
-		log.Infof("Restic sidecar already removed for StatefulSet %s@%s.", resource.Name, resource.Namespace)
-		return nil
-	}
-
 	_, err = apps_util.PatchStatefulSet(c.k8sClient, resource, func(obj *apps.StatefulSet) *apps.StatefulSet {
 		obj.Spec.Template.Spec.Containers = core_util.EnsureContainerDeleted(obj.Spec.Template.Spec.Containers, util.StashContainer)
 		obj.Spec.Template.Spec.Volumes = util.EnsureVolumeDeleted(obj.Spec.Template.Spec.Volumes, util.ScratchDirVolumeName)

diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go
@@ -216,12 +216,12 @@ func (c *Scheduler) configureScheduler() error {
 func (c *Scheduler) runOnce() (err error) {
 	select {
 	case <-c.locked:
-		log.Infof("Acquired lock for Restic %s@%s", c.opt.ResticName, c.opt.Namespace)
+		log.Infof("Acquired lock for Restic %s/%s", c.opt.Namespace, c.opt.ResticName)
 		defer func() {
 			c.locked <- struct{}{}
 		}()
 	default:
-		log.Warningf("Skipping backup schedule for Restic %s@%s", c.opt.ResticName, c.opt.Namespace)
+		log.Warningf("Skipping backup schedule for Restic %s/%s", c.opt.Namespace, c.opt.ResticName)
 		return
 	}
 
@@ -235,7 +235,7 @@ func (c *Scheduler) runOnce() (err error) {
 	}
 
 	if resource.Spec.Backend.StorageSecretName == "" {
-		err = errors.New("Missing repository secret name")
+		err = errors.New("missing repository secret name")
 		return
 	}
 	var secret *apiv1.Secret
@@ -315,7 +315,7 @@ func (c *Scheduler) runOnce() (err error) {
 			if err == nil {
 				break
 			}
-			log.Errorf("Attempt %d failed to update status for Restic %s@%s due to %s.", attempt, resource.Name, resource.Namespace, err)
+			log.Errorf("Attempt %d failed to update status for Restic %s/%s due to %s.", attempt, resource.Namespace, resource.Name, err)
 			time.Sleep(msec10)
 			if kerr.IsConflict(err) {
 				resource, err = c.stashClient.Restics(resource.Namespace).Get(resource.Name, metav1.GetOptions{})
@@ -325,7 +325,7 @@ func (c *Scheduler) runOnce() (err error) {
 			}
 		}
 		if attempt >= maxAttempts {
-			err = fmt.Errorf("Failed to add sidecar for ReplicaSet %s@%s after %d attempts.", resource.Name, resource.Namespace, attempt)
+			err = fmt.Errorf("failed to add sidecar for ReplicaSet %s/%s after %d attempts", resource.Namespace, resource.Name, attempt)
 			return
 		}
 	}()
@@ -334,7 +334,7 @@ func (c *Scheduler) runOnce() (err error) {
 		backupOpMetric := restic_session_duration_seconds.WithLabelValues(sanitizeLabelValue(fg.Path), "backup")
 		err = c.measure(c.resticCLI.Backup, resource, fg, backupOpMetric)
 		if err != nil {
-			log.Errorln("Backup operation failed for Reestic %s@%s due to %s", resource.Name, resource.Namespace, err)
+			log.Errorln("Backup operation failed for Reestic %s/%s due to %s", resource.Namespace, resource.Name, err)
 			c.recorder.Event(resource, apiv1.EventTypeNormal, eventer.EventReasonFailedToBackup, " Error taking backup: "+err.Error())
 			return
 		} else {
@@ -345,7 +345,7 @@ func (c *Scheduler) runOnce() (err error) {
 		forgetOpMetric := restic_session_duration_seconds.WithLabelValues(sanitizeLabelValue(fg.Path), "forget")
 		err = c.measure(c.resticCLI.Forget, resource, fg, forgetOpMetric)
 		if err != nil {
-			log.Errorln("Failed to forget old snapshots for Restic %s@%s due to %s", resource.Name, resource.Namespace, err)
+			log.Errorln("Failed to forget old snapshots for Restic %s/%s due to %s", resource.Namespace, resource.Name, err)
 			c.recorder.Event(resource, apiv1.EventTypeNormal, eventer.EventReasonFailedToRetention, " Error forgetting snapshots: "+err.Error())
 			return
 		}

diff --git a/test/e2e/daemonset_test.go b/test/e2e/daemonset_test.go
@@ -166,7 +166,7 @@ var _ = Describe("DaemonSet", func() {
 			}, BeNumerically(">=", 1)))
 
 			By("Change selector of Restic " + restic.Name)
-			err = f.UpdateRestic(restic.ObjectMeta, func(in api.Restic) api.Restic {
+			err = f.UpdateRestic(restic.ObjectMeta, func(in *api.Restic) *api.Restic {
 				in.Spec.Selector = metav1.LabelSelector{
 					MatchLabels: map[string]string{
 						"app": "unmatched",

diff --git a/test/e2e/deployment_test.go b/test/e2e/deployment_test.go
@@ -166,7 +166,7 @@ var _ = Describe("Deployment", func() {
 			}, BeNumerically(">=", 1)))
 
 			By("Change selector of Restic " + restic.Name)
-			err = f.UpdateRestic(restic.ObjectMeta, func(in api.Restic) api.Restic {
+			err = f.UpdateRestic(restic.ObjectMeta, func(in *api.Restic) *api.Restic {
 				in.Spec.Selector = metav1.LabelSelector{
 					MatchLabels: map[string]string{
 						"app": "unmatched",

diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go
@@ -63,12 +63,9 @@ var _ = BeforeSuite(func() {
 	root.EventuallyCRD("restic." + sapi.GroupName).Should(Succeed())
 
 	// Now let's start the controller
-	stop := make(chan struct{})
-	defer close(stop)
-	go ctrl.Run(1, stop)
-
-	// Wait forever
-	select {}
+	// stop := make(chan struct{})
+	// defer close(stop)
+	go ctrl.Run(1, nil)
 })
 
 var _ = AfterSuite(func() {

diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go
@@ -8,11 +8,11 @@ import (
 
 type Framework struct {
 	KubeClient  kubernetes.Interface
-	StashClient cs.ResticsGetter
+	StashClient cs.StashV1alpha1Interface
 	namespace   string
 }
 
-func New(kubeClient kubernetes.Interface, extClient cs.ResticsGetter) *Framework {
+func New(kubeClient kubernetes.Interface, extClient cs.StashV1alpha1Interface) *Framework {
 	return &Framework{
 		KubeClient:  kubeClient,
 		StashClient: extClient,

diff --git a/test/e2e/framework/restic.go b/test/e2e/framework/restic.go
@@ -1,14 +1,10 @@
 package framework
 
 import (
-	"fmt"
-	"time"
-
 	"github.com/appscode/go/crypto/rand"
-	"github.com/appscode/go/log"
 	api "github.com/appscode/stash/apis/stash/v1alpha1"
+	stash_util "github.com/appscode/stash/client/typed/stash/v1alpha1/util"
 	. "github.com/onsi/gomega"
-	kerr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apiv1 "k8s.io/client-go/pkg/api/v1"
 )
@@ -120,23 +116,9 @@ func (f *Framework) DeleteRestic(meta metav1.ObjectMeta) error {
 	return f.StashClient.Restics(meta.Namespace).Delete(meta.Name, deleteInForeground())
 }
 
-func (f *Framework) UpdateRestic(meta metav1.ObjectMeta, transformer func(api.Restic) api.Restic) error {
-	attempt := 0
-	for ; attempt < maxAttempts; attempt = attempt + 1 {
-		cur, err := f.StashClient.Restics(meta.Namespace).Get(meta.Name, metav1.GetOptions{})
-		if kerr.IsNotFound(err) {
-			return nil
-		} else if err == nil {
-			modified := transformer(*cur)
-			_, err = f.StashClient.Restics(cur.Namespace).Update(&modified)
-			if err == nil {
-				return nil
-			}
-		}
-		log.Errorf("Attempt %d failed to update Restic %s@%s due to %s.", attempt, cur.Name, cur.Namespace, err)
-		time.Sleep(updateRetryInterval)
-	}
-	return fmt.Errorf("Failed to update Restic %s@%s after %d attempts.", meta.Name, meta.Namespace, attempt)
+func (f *Framework) UpdateRestic(meta metav1.ObjectMeta, transformer func(*api.Restic) *api.Restic) error {
+	_, err := stash_util.TryUpdateRestic(f.StashClient, meta, transformer)
+	return err
 }
 
 func (f *Framework) EventuallyRestic(meta metav1.ObjectMeta) GomegaAsyncAssertion {

diff --git a/test/e2e/rc_test.go b/test/e2e/rc_test.go
@@ -165,7 +165,7 @@ var _ = Describe("ReplicationController", func() {
 			}, BeNumerically(">=", 1)))
 
 			By("Change selector of Restic " + restic.Name)
-			err = f.UpdateRestic(restic.ObjectMeta, func(in api.Restic) api.Restic {
+			err = f.UpdateRestic(restic.ObjectMeta, func(in *api.Restic) *api.Restic {
 				in.Spec.Selector = metav1.LabelSelector{
 					MatchLabels: map[string]string{
 						"app": "unmatched",

diff --git a/test/e2e/replicaset_test.go b/test/e2e/replicaset_test.go
@@ -166,7 +166,7 @@ var _ = Describe("ReplicaSet", func() {
 			}, BeNumerically(">=", 1)))
 
 			By("Change selector of Restic " + restic.Name)
-			err = f.UpdateRestic(restic.ObjectMeta, func(in api.Restic) api.Restic {
+			err = f.UpdateRestic(restic.ObjectMeta, func(in *api.Restic) *api.Restic {
 				in.Spec.Selector = metav1.LabelSelector{
 					MatchLabels: map[string]string{
 						"app": "unmatched",