From 83968d92e91565632cd302448457bce72bdedaf6 Mon Sep 17 00:00:00 2001 From: Nandan B N Date: Fri, 4 Mar 2022 18:25:14 +0530 Subject: [PATCH 1/2] fix panic: argoRolloutsEnabled=false Signed-off-by: nbn01 --- admiral/pkg/clusters/types.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/admiral/pkg/clusters/types.go b/admiral/pkg/clusters/types.go index 72fabbc3..1fead796 100644 --- 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) From f7e6dd97c7a55b4d95c5ff737029172a03fff95b Mon Sep 17 00:00:00 2001 From: nbn01 Date: Wed, 9 Mar 2022 14:52:58 +0530 Subject: [PATCH 2/2] Update test case with `argoRolloutsEnabled=true` Signed-off-by: nbn01 --- admiral/pkg/clusters/types_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/admiral/pkg/clusters/types_test.go b/admiral/pkg/clusters/types_test.go index 1c06e2c9..f2fb24dc 100644 --- a/admiral/pkg/clusters/types_test.go +++ b/admiral/pkg/clusters/types_test.go @@ -2,11 +2,16 @@ 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" @@ -14,10 +19,6 @@ import ( 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{}