check deployment name before deleting deployment from cache

Signed-off-by: Anubhav Aeron <anubhav_aeron@intuit.com>
istio-ecosystem · Nov 10, 2022 · 6ebfa50 · 6ebfa50
1 parent 13daaf0
commit 6ebfa50
Show file tree

Hide file tree

Showing 2 changed files with 128 additions and 6 deletions.
diff --git a/admiral/pkg/controller/admiral/deployment.go b/admiral/pkg/controller/admiral/deployment.go
@@ -80,13 +80,23 @@ func (p *deploymentCache) UpdateDeploymentToClusterCache(key string, deployment
 func (p *deploymentCache) DeleteFromDeploymentClusterCache(key string, deployment *k8sAppsV1.Deployment) {
 	defer p.mutex.Unlock()
 	p.mutex.Lock()
-
-	env := common.GetEnv(deployment)
-
-	dce := p.cache[key]
+	var (
+		env = common.GetEnv(deployment)
+		dce = p.cache[key]
+	)
 
 	if dce != nil {
-		delete(dce.Deployments, env)
+		if dce.Deployments[env] != nil && deployment.Name == dce.Deployments[env].Name {
+			log.Infof("op=%s type=%v name=%v namespace=%s cluster=%s message=%s", "Delete", "Deployment",
+				deployment.Name, deployment.Namespace, "", "ignoring deployment and deleting from cache")
+			delete(dce.Deployments, env)
+		} else {
+			log.Warnf("op=%s type=%v name=%v namespace=%s cluster=%s message=%s", "Get", "Deployment",
+				deployment.Name, deployment.Namespace, "", "ignoring deployment delete as it doesn't match the one in cache")
+		}
+	} else {
+		log.Infof("op=%s type=%v name=%v namespace=%s cluster=%s message=%s", "Delete", "Deployment",
+			deployment.Name, deployment.Namespace, "", "nothing to delete, deployment not found in cache")
 	}
 }
 
@@ -138,7 +148,6 @@ func HandleAddUpdateDeployment(ctx context.Context, ojb interface{}, d *Deployme
 			d.DeploymentHandler.Added(ctx, deployment)
 		} else {
 			d.Cache.DeleteFromDeploymentClusterCache(key, deployment)
-			log.Debugf("ignoring deployment %v based on labels", deployment.Name)
 		}
 	}
 }

diff --git a/admiral/pkg/controller/admiral/deployment_test.go b/admiral/pkg/controller/admiral/deployment_test.go
@@ -327,3 +327,116 @@ func TestDeploymentController_GetDeploymentBySelectorInNamespace(t *testing.T) {
 		})
 	}
 }
+
+func TestDeleteFromDeploymentClusterCache(t *testing.T) {
+	var (
+		identity = "app1"
+		env      = "prd"
+
+		deploymentWithoutEnvAnnotation1 = k8sAppsV1.Deployment{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      "abc-service",
+				Namespace: "namespace-" + env,
+			},
+			Spec: k8sAppsV1.DeploymentSpec{
+				Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": identity}},
+				Template: coreV1.PodTemplateSpec{
+					ObjectMeta: v1.ObjectMeta{
+						Labels: map[string]string{"identity": identity},
+					},
+				},
+			},
+		}
+		deploymentWithoutEnvAnnotation2 = k8sAppsV1.Deployment{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      "debug",
+				Namespace: "namespace-" + env,
+			},
+			Spec: k8sAppsV1.DeploymentSpec{
+				Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": identity}},
+				Template: coreV1.PodTemplateSpec{
+					ObjectMeta: v1.ObjectMeta{
+						Labels: map[string]string{"identity": identity},
+					},
+				},
+			},
+		}
+		deploymentWitEnvAnnotation1 = k8sAppsV1.Deployment{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      "abc-service",
+				Namespace: "namespace-" + env,
+			},
+			Spec: k8sAppsV1.DeploymentSpec{
+				Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": identity}},
+				Template: coreV1.PodTemplateSpec{
+					ObjectMeta: v1.ObjectMeta{
+						Labels: map[string]string{"identity": identity, "env": "prd"},
+					},
+				},
+			},
+		}
+	)
+
+	testCases := []struct {
+		name                        string
+		deploymentInCache           *k8sAppsV1.Deployment
+		deploymentToDelete          *k8sAppsV1.Deployment
+		expectedDeploymentCacheSize int
+	}{
+		{
+			name: "Given deployment cache has a valid deployment in its cache, " +
+				"And the deployment does not have an env annotation" +
+				"And a debug deployment exists, without the env annotation" +
+				"And the env key is derived from namespace " +
+				"When DeleteFromDeploymentClusterCache is called for a debug deployment" +
+				"Then, the valid deployment should not be deleted from the cache",
+			deploymentInCache:           &deploymentWithoutEnvAnnotation1,
+			deploymentToDelete:          &deploymentWithoutEnvAnnotation2,
+			expectedDeploymentCacheSize: 1,
+		},
+		{
+			name: "Given deployment cache has a valid deployment in its cache, " +
+				"And the deployment does not have an env annotation" +
+				"And the env key is derived from namespace " +
+				"When DeleteFromDeploymentClusterCache is called for the same deployment" +
+				"Then, the deployment should be deleted from the cache",
+			deploymentInCache:           &deploymentWithoutEnvAnnotation1,
+			deploymentToDelete:          &deploymentWithoutEnvAnnotation1,
+			expectedDeploymentCacheSize: 0,
+		},
+		{
+			name: "Given deployment cache has a valid deployment in its cache, " +
+				"And the deployment as a valid env annotation" +
+				"And a debug deployment exists, without an env annotation" +
+				"When DeleteFromDeploymentClusterCache is called for a debug deployment" +
+				"Then, the valid deployment should not be deleted from the cache",
+			deploymentInCache:           &deploymentWitEnvAnnotation1,
+			deploymentToDelete:          &deploymentWithoutEnvAnnotation2,
+			expectedDeploymentCacheSize: 1,
+		},
+		{
+			name: "Given deployment cache has a valid deployment in its cache, " +
+				"And the deployment as a valid env annotation" +
+				"And a debug deployment exists, without an env annotation" +
+				"When DeleteFromDeploymentClusterCache is called for the same deployment" +
+				"Then, the deployment should be deleted from the cache",
+			deploymentInCache:           &deploymentWitEnvAnnotation1,
+			deploymentToDelete:          &deploymentWitEnvAnnotation1,
+			expectedDeploymentCacheSize: 0,
+		},
+	}
+	for _, c := range testCases {
+		t.Run(c.name, func(t *testing.T) {
+			deploymentCache := deploymentCache{
+				cache: make(map[string]*DeploymentClusterEntry),
+				mutex: &sync.Mutex{},
+			}
+			deploymentCache.UpdateDeploymentToClusterCache(identity, c.deploymentInCache)
+			deploymentCache.DeleteFromDeploymentClusterCache(identity, c.deploymentToDelete)
+			if len(deploymentCache.cache[identity].Deployments) != c.expectedDeploymentCacheSize {
+				t.Fatalf("expected deployment cache size to have size: %v, but got: %v",
+					c.expectedDeploymentCacheSize, len(deploymentCache.cache))
+			}
+		})
+	}
+}