fix: Admiral crash when ArgoRolloutsEnabled set to false (istio-ecosy…

…stem#195) * fix panic: argoRolloutsEnabled=false Signed-off-by: nbn01 <nandan_bn@intuit.com> * Update test case with `argoRolloutsEnabled=true` Signed-off-by: nbn01 <nandan_bn@intuit.com> Co-authored-by: nbn01 <nandan_bn@intuit.com> Signed-off-by: psikka1 <pankaj_sikka@intuit.com>
psikka1 · Jun 15, 2022 · 739623c · 739623c
1 parent 62c1a6d
commit 739623c
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 7 deletions.
diff --git a/admiral/pkg/clusters/types.go b/admiral/pkg/clusters/types.go
@@ -276,7 +276,9 @@ func (gtp *GlobalTrafficHandler) Added(obj *v1.GlobalTrafficPolicy) {
 	//IMPORTANT: The deployment/Rollout matched with a GTP will not necessarily be from the same cluster. This is because the same service could be deployed in multiple clusters and we need to guarantee consistent behavior
 	for _, remoteCluster := range gtp.RemoteRegistry.RemoteControllers {
 		matchedDeployments = append(matchedDeployments, remoteCluster.DeploymentController.GetDeploymentByLabel(obj.Labels[common.GetGlobalTrafficDeploymentLabel()], obj.Namespace)...)
-		matchedRollouts = append(matchedRollouts, remoteCluster.RolloutController.GetRolloutByLabel(obj.Labels[common.GetGlobalTrafficDeploymentLabel()], obj.Namespace)...)
+		if gtp.RemoteRegistry.AdmiralCache.argoRolloutsEnabled {
+			matchedRollouts = append(matchedRollouts, remoteCluster.RolloutController.GetRolloutByLabel(obj.Labels[common.GetGlobalTrafficDeploymentLabel()], obj.Namespace)...)
+		}
 	}
 
 	deployments := common.MatchDeploymentsToGTP(obj, matchedDeployments)
@@ -317,7 +319,9 @@ func (gtp *GlobalTrafficHandler) Updated(obj *v1.GlobalTrafficPolicy) {
 	//IMPORTANT: The deployment/Rollout matched with a GTP will not necessarily be from the same cluster. This is because the same service could be deployed in multiple clusters and we need to guarantee consistent behavior
 	for _, remoteCluster := range gtp.RemoteRegistry.RemoteControllers {
 		matchedDeployments = append(matchedDeployments, remoteCluster.DeploymentController.GetDeploymentByLabel(obj.Labels[common.GetGlobalTrafficDeploymentLabel()], obj.Namespace)...)
-		matchedRollouts = append(matchedRollouts, remoteCluster.RolloutController.GetRolloutByLabel(obj.Labels[common.GetGlobalTrafficDeploymentLabel()], obj.Namespace)...)
+		if gtp.RemoteRegistry.AdmiralCache.argoRolloutsEnabled {
+			matchedRollouts = append(matchedRollouts, remoteCluster.RolloutController.GetRolloutByLabel(obj.Labels[common.GetGlobalTrafficDeploymentLabel()], obj.Namespace)...)
+		}
 	}
 
 	deployments := common.MatchDeploymentsToGTP(obj, matchedDeployments)

diff --git a/admiral/pkg/clusters/types_test.go b/admiral/pkg/clusters/types_test.go
@@ -2,22 +2,23 @@ package clusters
 
 import (
 	"context"
+	"log"
+	"sync"
+	"testing"
+	"time"
+
 	argo "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
 	argofake "github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
-	"github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
+	v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
 	admiralFake "github.com/istio-ecosystem/admiral/admiral/pkg/client/clientset/versioned/fake"
 	"github.com/istio-ecosystem/admiral/admiral/pkg/controller/admiral"
 	"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
 	v12 "k8s.io/api/apps/v1"
 	v13 "k8s.io/api/core/v1"
 	time2 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/fake"
-	"log"
-	"sync"
-	"testing"
-	"time"
 )
 
 var ignoreUnexported = cmpopts.IgnoreUnexported(v1.GlobalTrafficPolicy{}.Status)
@@ -860,6 +861,7 @@ func TestGlobalTrafficHandler_Updated_ForRollouts(t *testing.T) {
 
 	admiralCacle.GlobalTrafficCache = gtpCache
 	registry.AdmiralCache = admiralCacle
+	admiralCacle.argoRolloutsEnabled = true
 	handler.RemoteRegistry = registry
 
 	e2eGtp := v1.GlobalTrafficPolicy{}