Use lister instead of to load service instead of k8s api call

argoproj · Jun 5, 2019 · 079e527 · 079e527
1 parent 1efc05a
commit 079e527
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 53 deletions.
diff --git a/controller/bluegreen_test.go b/controller/bluegreen_test.go
@@ -54,12 +54,12 @@ func TestBlueGreenHandleResetPreviewAfterActiveSet(t *testing.T) {
 
 	previewSvc := newService("preview", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash})
 	f.kubeobjects = append(f.kubeobjects, previewSvc)
+	f.serviceLister = append(f.serviceLister, previewSvc)
 
 	activeSvc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash})
 	f.kubeobjects = append(f.kubeobjects, activeSvc)
+	f.serviceLister = append(f.serviceLister, activeSvc)
 
-	f.expectGetServiceAction(previewSvc)
-	f.expectGetServiceAction(activeSvc)
 	f.expectPatchServiceAction(previewSvc, "")
 	f.expectPatchRolloutAction(r2)
 	f.run(getKey(r2, t))
@@ -75,12 +75,11 @@ func TestBlueGreenCreatesReplicaSet(t *testing.T) {
 	previewSvc := newService("preview", 80, nil)
 	activeSvc := newService("active", 80, nil)
 	f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc)
+	f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
 	rs := newReplicaSet(r, "foo-895c6c4f9", 1)
 	generatedConditions := generateConditionsPatch(false, conditions.NewReplicaSetReason, rs, false)
 
-	f.expectGetServiceAction(activeSvc)
-	f.expectGetServiceAction(previewSvc)
 	f.expectCreateReplicaSetAction(rs)
 	updatedRolloutIndex := f.expectUpdateRolloutAction(r)
 	expectedPatchWithoutSubs := `{
@@ -119,9 +118,8 @@ func TestBlueGreenSetPreviewService(t *testing.T) {
 	selector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "test"}
 	activeSvc := newService("active", 80, selector)
 	f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc)
+	f.serviceLister = append(f.serviceLister, previewSvc, activeSvc)
 
-	f.expectGetServiceAction(activeSvc)
-	f.expectGetServiceAction(previewSvc)
 	f.expectPatchServiceAction(previewSvc, "")
 	f.expectPatchRolloutAction(r)
 	f.run(getKey(r, t))
@@ -148,9 +146,8 @@ func TestBlueGreenHandlePause(t *testing.T) {
 		f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc, rs1, rs2)
 		f.rolloutLister = append(f.rolloutLister, r2)
 		f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
+		f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-		f.expectGetServiceAction(activeSvc)
-		f.expectGetServiceAction(previewSvc)
 		addPauseConditionPatchIndex := f.expectPatchRolloutAction(r2)
 		f.run(getKey(r2, t))
 
@@ -191,9 +188,8 @@ func TestBlueGreenHandlePause(t *testing.T) {
 		f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc, rs1, rs2)
 		f.rolloutLister = append(f.rolloutLister, r2)
 		f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
+		f.serviceLister = append(f.serviceLister, previewSvc, activeSvc)
 
-		f.expectGetServiceAction(activeSvc)
-		f.expectGetServiceAction(previewSvc)
 		addPauseConditionPatchIndex := f.expectPatchRolloutAction(r2)
 		f.expectPatchRolloutAction(r2)
 		f.run(getKey(r2, t))
@@ -232,9 +228,8 @@ func TestBlueGreenHandlePause(t *testing.T) {
 		f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc, rs1, rs2)
 		f.rolloutLister = append(f.rolloutLister, r2)
 		f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
+		f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-		f.expectGetServiceAction(activeSvc)
-		f.expectGetServiceAction(previewSvc)
 		patchIndex := f.expectPatchRolloutActionWithPatch(r2, OnlyObservedGenerationPatch)
 		f.run(getKey(r2, t))
 		patch := f.getPatchedRollout(patchIndex)
@@ -266,9 +261,8 @@ func TestBlueGreenHandlePause(t *testing.T) {
 		f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc, rs1, rs2)
 		f.rolloutLister = append(f.rolloutLister, r2)
 		f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
+		f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-		f.expectGetServiceAction(activeSvc)
-		f.expectGetServiceAction(previewSvc)
 		patchIndex := f.expectPatchRolloutActionWithPatch(r2, OnlyObservedGenerationPatch)
 		f.run(getKey(r2, t))
 		patch := f.getPatchedRollout(patchIndex)
@@ -303,9 +297,8 @@ func TestBlueGreenHandlePause(t *testing.T) {
 		f.kubeobjects = append(f.kubeobjects, activeSvc, previewSvc, rs1, rs2)
 		f.rolloutLister = append(f.rolloutLister, r2)
 		f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
+		f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-		f.expectGetServiceAction(activeSvc)
-		f.expectGetServiceAction(previewSvc)
 		expectedPatchWithoutSubs := `{
 			"spec": {
 				"paused": null
@@ -344,8 +337,8 @@ func TestBlueGreenHandlePause(t *testing.T) {
 		f.kubeobjects = append(f.kubeobjects, activeSvc, rs1, rs2)
 		f.rolloutLister = append(f.rolloutLister, r2)
 		f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
+		f.serviceLister = append(f.serviceLister, activeSvc)
 
-		f.expectGetServiceAction(activeSvc)
 		servicePatchIndex := f.expectPatchServiceAction(activeSvc, rs2PodHash)
 
 		generatedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, true)
@@ -389,9 +382,8 @@ func TestBlueGreenHandlePause(t *testing.T) {
 		f.kubeobjects = append(f.kubeobjects, activeSvc, previewSvc, rs1)
 		f.rolloutLister = append(f.rolloutLister, r1)
 		f.replicaSetLister = append(f.replicaSetLister, rs1)
+		f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-		f.expectGetServiceAction(activeSvc)
-		f.expectGetServiceAction(previewSvc)
 		servicePatchIndex := f.expectPatchServiceAction(activeSvc, rs1PodHash)
 		expectedPatchWithoutSubs := `{
 			"status": {
@@ -441,9 +433,8 @@ func TestBlueGreenHandlePause(t *testing.T) {
 		f.kubeobjects = append(f.kubeobjects, activeSvc, previewSvc, rs1, rs2)
 		f.rolloutLister = append(f.rolloutLister, r2)
 		f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
+		f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-		f.expectGetServiceAction(activeSvc)
-		f.expectGetServiceAction(previewSvc)
 		servicePatchIndex := f.expectPatchServiceAction(activeSvc, rs2PodHash)
 		unpausePatchIndex := f.expectPatchRolloutAction(r2)
 		patchRolloutIndex := f.expectPatchRolloutAction(r2)
@@ -495,9 +486,8 @@ func TestBlueGreenSkipPreviewUpdateActive(t *testing.T) {
 	previewSvc := newService("preview", 80, nil)
 	activeSvc := newService("active", 80, nil)
 	f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc)
+	f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-	f.expectGetServiceAction(activeSvc)
-	f.expectGetServiceAction(previewSvc)
 	f.expectPatchServiceAction(activeSvc, rs.Labels[v1alpha1.DefaultRolloutUniqueLabelKey])
 	f.expectPatchRolloutAction(r)
 	f.run(getKey(r, t))
@@ -522,8 +512,8 @@ func TestBlueGreenAddScaleDownDelayStartTime(t *testing.T) {
 	r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, 2, 1, 1, false, true)
 	f.rolloutLister = append(f.rolloutLister, r2)
 	f.objects = append(f.objects, r2)
+	f.serviceLister = append(f.serviceLister, s)
 
-	f.expectGetServiceAction(s)
 	f.expectPatchServiceAction(s, rs2PodHash)
 	patchIndex := f.expectPatchRolloutAction(r2)
 	f.run(getKey(r2, t))
@@ -573,10 +563,10 @@ func TestBlueGreenWaitForScaleDownDelay(t *testing.T) {
 	serviceSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash}
 	s := newService("bar", 80, serviceSelector)
 	f.kubeobjects = append(f.kubeobjects, s)
+	f.serviceLister = append(f.serviceLister, s)
 
 	expRS := rs2.DeepCopy()
 	expRS.Annotations[annotations.DesiredReplicasAnnotation] = "0"
-	f.expectGetServiceAction(s)
 	patchIndex := f.expectPatchRolloutAction(r1)
 
 	f.run(getKey(r2, t))
@@ -607,10 +597,10 @@ func TestBlueGreenScaleDownOldRS(t *testing.T) {
 	serviceSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash}
 	s := newService("bar", 80, serviceSelector)
 	f.kubeobjects = append(f.kubeobjects, s)
+	f.serviceLister = append(f.serviceLister, s)
 
 	expRS := rs2.DeepCopy()
 	expRS.Annotations[annotations.DesiredReplicasAnnotation] = "0"
-	f.expectGetServiceAction(s)
 	f.expectUpdateReplicaSetAction(expRS)
 	f.expectPatchRolloutAction(r1)
 
@@ -641,6 +631,7 @@ func TestBlueGreenRolloutStatusHPAStatusFieldsActiveSelectorSet(t *testing.T) {
 
 	f.kubeobjects = append(f.kubeobjects, rs1, rs2, previewSvc, activeSvc)
 	f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
+	f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
 	expectedPatchWithoutSubs := `{
 		"status":{
@@ -654,8 +645,6 @@ func TestBlueGreenRolloutStatusHPAStatusFieldsActiveSelectorSet(t *testing.T) {
 	//_, availableStr := newAvailableCondition(true)
 	expectedPatch := calculatePatch(r2, fmt.Sprintf(expectedPatchWithoutSubs, rs1PodHash))
 
-	f.expectGetServiceAction(activeSvc)
-	f.expectGetServiceAction(previewSvc)
 	patchIndex := f.expectPatchRolloutActionWithPatch(r2, expectedPatch)
 	f.run(getKey(r2, t))
 
@@ -715,9 +704,8 @@ func TestBlueGreenRolloutScaleUpdateActiveRS(t *testing.T) {
 	previewSvc := newService("preview", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash})
 	activeSvc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash})
 	f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc)
+	f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-	f.expectGetServiceAction(previewSvc)
-	f.expectGetServiceAction(activeSvc)
 	f.expectUpdateReplicaSetAction(rs1)
 	f.expectPatchRolloutAction(r1)
 
@@ -748,9 +736,8 @@ func TestBlueGreenRolloutScaleUpdatePreviewRS(t *testing.T) {
 	f.rolloutLister = append(f.rolloutLister, r2)
 	f.objects = append(f.objects, r2)
 	f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc)
+	f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-	f.expectGetServiceAction(previewSvc)
-	f.expectGetServiceAction(activeSvc)
 	rs2idx := f.expectUpdateReplicaSetAction(rs2)
 	f.expectPatchRolloutAction(r1)
 
@@ -780,9 +767,8 @@ func TestBlueGreenRolloutScalePreviewActiveRS(t *testing.T) {
 	previewSvc := newService("preview", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash})
 	activeSvc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash})
 	f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc)
+	f.serviceLister = append(f.serviceLister, activeSvc, previewSvc)
 
-	f.expectGetServiceAction(previewSvc)
-	f.expectGetServiceAction(activeSvc)
 	f.expectUpdateReplicaSetAction(rs2)
 	f.expectPatchRolloutAction(r1)
 
@@ -809,8 +795,8 @@ func TestBlueGreenRolloutCompleted(t *testing.T) {
 
 	f.rolloutLister = append(f.rolloutLister, r2)
 	f.objects = append(f.objects, r2)
+	f.serviceLister = append(f.serviceLister, s)
 
-	f.expectGetServiceAction(s)
 	patchIndex := f.expectPatchRolloutAction(r1)
 
 	f.run(getKey(r2, t))

diff --git a/controller/controller_test.go b/controller/controller_test.go
@@ -447,14 +447,6 @@ func filterInformerActions(actions []core.Action) []core.Action {
 	return ret
 }
 
-func (f *fixture) expectGetServiceAction(s *corev1.Service) {
-	serviceSchema := schema.GroupVersionResource{
-		Resource: "services",
-		Version:  "v1",
-	}
-	f.kubeactions = append(f.kubeactions, core.NewGetAction(serviceSchema, s.Namespace, s.Name))
-}
-
 func (f *fixture) expectPatchServiceAction(s *corev1.Service, newLabel string) int {
 	patch := fmt.Sprintf(switchSelectorPatch, v1alpha1.DefaultRolloutUniqueLabelKey, newLabel)
 	serviceSchema := schema.GroupVersionResource{
@@ -597,9 +589,8 @@ func TestAdoptReplicaSet(t *testing.T) {
 	rs := newReplicaSet(r, "foo-895c6c4f9", 1)
 	f.kubeobjects = append(f.kubeobjects, previewSvc, activeSvc, rs)
 	f.replicaSetLister = append(f.replicaSetLister, rs)
+	f.serviceLister = append(f.serviceLister, previewSvc, activeSvc)
 
-	f.expectGetServiceAction(activeSvc)
-	f.expectGetServiceAction(previewSvc)
 	updatedRolloutIndex := f.expectUpdateRolloutAction(r)
 	f.expectPatchRolloutAction(r)
 	f.run(getKey(r, t))
@@ -756,12 +747,12 @@ requests:
 		activeSvc := newService("active", 80, nil)
 		f.kubeobjects = append(f.kubeobjects, activeSvc)
 		f.rolloutLister = append(f.rolloutLister, r)
+		f.serviceLister = append(f.serviceLister, activeSvc)
 		f.objects = append(f.objects, r)
 
 		_ = f.expectUpdateRolloutAction(r)
 		f.expectPatchRolloutAction(r)
 		rs := newReplicaSet(r, expectedReplicaSetName, 1)
-		f.expectGetServiceAction(activeSvc)
 		rsIdx := f.expectCreateReplicaSetAction(rs)
 		f.run(getKey(r, t))
 		rs = f.getCreatedReplicaSet(rsIdx)

diff --git a/controller/service.go b/controller/service.go
@@ -5,8 +5,7 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
-	errors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	patchtypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/client-go/tools/cache"
@@ -125,7 +124,7 @@ func (c *Controller) getPreviewAndActiveServices(r *v1alpha1.Rollout) (*corev1.S
 	var activeSvc *corev1.Service
 	var err error
 	if r.Spec.Strategy.BlueGreenStrategy.PreviewService != "" {
-		previewSvc, err = c.kubeclientset.CoreV1().Services(r.Namespace).Get(r.Spec.Strategy.BlueGreenStrategy.PreviewService, metav1.GetOptions{})
+		previewSvc, err = c.servicesLister.Services(r.Namespace).Get(r.Spec.Strategy.BlueGreenStrategy.PreviewService)
 		if err != nil {
 			if errors.IsNotFound(err) {
 				msg := fmt.Sprintf(conditions.ServiceNotFoundMessage, r.Spec.Strategy.BlueGreenStrategy.PreviewService)
@@ -141,7 +140,7 @@ func (c *Controller) getPreviewAndActiveServices(r *v1alpha1.Rollout) (*corev1.S
 	if r.Spec.Strategy.BlueGreenStrategy.ActiveService == "" {
 		return nil, nil, fmt.Errorf("Invalid Spec: Rollout missing field ActiveService")
 	}
-	activeSvc, err = c.kubeclientset.CoreV1().Services(r.Namespace).Get(r.Spec.Strategy.BlueGreenStrategy.ActiveService, metav1.GetOptions{})
+	activeSvc, err = c.servicesLister.Services(r.Namespace).Get(r.Spec.Strategy.BlueGreenStrategy.ActiveService)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			msg := fmt.Sprintf(conditions.ServiceNotFoundMessage, r.Spec.Strategy.BlueGreenStrategy.ActiveService)

diff --git a/controller/service_test.go b/controller/service_test.go
@@ -153,7 +153,11 @@ func TestGetPreviewAndActiveServices(t *testing.T) {
 	expPreview := newService("preview", 80, nil)
 	f.kubeobjects = append(f.kubeobjects, expActive)
 	f.kubeobjects = append(f.kubeobjects, expPreview)
+	f.serviceLister = append(f.serviceLister, expActive, expPreview)
 	rollout := &v1alpha1.Rollout{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: metav1.NamespaceDefault,
+		},
 		Spec: v1alpha1.RolloutSpec{
 			Strategy: v1alpha1.RolloutStrategy{
 				BlueGreenStrategy: &v1alpha1.BlueGreenStrategy{
@@ -205,10 +209,9 @@ func TestActiveServiceNotFound(t *testing.T) {
 	previewSvc := newService("preview-svc", 80, nil)
 	notUsedActiveSvc := newService("active-svc", 80, nil)
 	f.kubeobjects = append(f.kubeobjects, previewSvc)
+	f.serviceLister = append(f.serviceLister, previewSvc)
 
 	patchIndex := f.expectPatchRolloutAction(r)
-	f.expectGetServiceAction(previewSvc)
-	f.expectGetServiceAction(notUsedActiveSvc)
 	f.runExpectError(getKey(r, t), true)
 
 	patch := f.getPatchedRollout(patchIndex)
@@ -231,8 +234,8 @@ func TestPreviewServiceNotFound(t *testing.T) {
 	activeSvc := newService("active-svc", 80, nil)
 	notUsedPreviewSvc := newService("preview-svc", 80, nil)
 	f.kubeobjects = append(f.kubeobjects, activeSvc)
+	f.serviceLister = append(f.serviceLister)
 
-	f.expectGetServiceAction(notUsedPreviewSvc)
 	patchIndex := f.expectPatchRolloutAction(r)
 	f.runExpectError(getKey(r, t), true)