Skip to content

Commit

Permalink
ignore argo tracking tags during VS copy
Browse files Browse the repository at this point in the history
Signed-off-by: Ryan Tay <[email protected]>
  • Loading branch information
Ryan Tay committed Jan 7, 2025
1 parent 957ef3b commit bc55f95
Show file tree
Hide file tree
Showing 7 changed files with 10,770 additions and 8 deletions.
2 changes: 1 addition & 1 deletion admiral/cmd/admiral/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,11 @@ func GetRootCmd(args []string) *cobra.Command {
rootCmd.PersistentFlags().BoolVar(&params.EnableSWAwareNSCaches, "enable_sw_aware_ns_caches", false, "Enable/Disable SW Aware NS Caches")
rootCmd.PersistentFlags().BoolVar(&params.AdmiralStateSyncerMode, "admiral_state_syncer_mode", false, "Enable/Disable admiral to run as state syncer only")
rootCmd.PersistentFlags().Int64Var(&params.DefaultWarmupDurationSecs, "default_warmup_duration_in_seconds", 45, "The default value for the warmupDurationSecs to be used on Destination Rules created by admiral")

rootCmd.PersistentFlags().BoolVar(&params.EnableGenerationCheck, "enable_generation_check", true, "Enable/Disable Generation Check")
rootCmd.PersistentFlags().BoolVar(&params.EnableIsOnlyReplicaCountChangedCheck, "enable_replica_count_check", false, "Enable/Disable Replica Count Check")
rootCmd.PersistentFlags().BoolVar(&params.ClientInitiatedProcessingEnabled, "client_initiated_processing_enabled", true, "Enable/Disable Client Initiated Processing")
rootCmd.PersistentFlags().BoolVar(&params.PreventSplitBrain, "prevent_split_brain", true, "Enable/Disable Explicit Split Brain prevention logic")
rootCmd.PersistentFlags().StringSliceVar(&params.IgnoreLabelsAnnotationsList, "ignore_labels_annotations_list", []string{"applications.argoproj.io/app-name", "app.kubernetes.io/instance", "argocd.argoproj.io/tracking-id"}, "Labels and annotations that should not be preserved during VS copy")

//Admiral 2.0 flags
rootCmd.PersistentFlags().BoolVar(&params.AdmiralOperatorMode, "admiral_operator_mode", false, "Enable/Disable admiral operator functionality")
Expand Down
12 changes: 9 additions & 3 deletions admiral/pkg/clusters/virtualservice_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,12 @@ func addUpdateVirtualService(
skipAddingExportTo = true
}

// remove ignored labels and annotations from NewCopy (deleting on nil map or nonexistent keys is a no-op)
for _, ignored := range common.GetIgnoreLabelsAnnotations() {
delete(newCopy.Labels, ignored)
delete(newCopy.Annotations, ignored)
}

if common.EnableExportTo(newCopy.Spec.Hosts[0]) && !skipAddingExportTo {
sortedDependentNamespaces := getSortedDependentNamespaces(rr.AdmiralCache, newCopy.Spec.Hosts[0], rc.ClusterID, ctxLogger)
newCopy.Spec.ExportTo = sortedDependentNamespaces
Expand All @@ -542,7 +548,7 @@ func addUpdateVirtualService(
rc.ClusterID, newCopy.Name))
newCopy.Namespace = namespace
newCopy.ResourceVersion = ""
_, err = rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(namespace).Create(ctx, newCopy, metav1.CreateOptions{})
_, err = rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(namespace).Create(ctx, newCopy, metav1.CreateOptions{}) //HERE RTAY
if k8sErrors.IsAlreadyExists(err) {
ctxLogger.Infof(LogFormat, op, common.VirtualServiceResourceType, newCopy.Name, rc.ClusterID,
fmt.Sprintf("skipping create virtualservice and it already exists for cluster: %s VirtualService name=%s",
Expand Down Expand Up @@ -573,7 +579,7 @@ func addUpdateVirtualService(
exist.Annotations = newCopy.Annotations
//nolint
exist.Spec = newCopy.Spec
_, err = rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(namespace).Update(ctx, exist, metav1.UpdateOptions{})
_, err = rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(namespace).Update(ctx, exist, metav1.UpdateOptions{}) //HERE RTAY
if err != nil {
err = retryUpdatingVS(ctxLogger, ctx, newCopy, exist, namespace, rc, err, op)
}
Expand Down Expand Up @@ -613,7 +619,7 @@ func retryUpdatingVS(ctxLogger *log.Entry, ctx context.Context, obj *v1alpha3.Vi
updatedVS.Spec = obj.Spec
updatedVS.Labels = obj.Labels
updatedVS.Annotations = obj.Annotations
_, err = rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(namespace).Update(ctx, updatedVS, metav1.UpdateOptions{})
_, err = rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(namespace).Update(ctx, updatedVS, metav1.UpdateOptions{}) //HERE RTAY
if err == nil {
return nil
}
Expand Down
25 changes: 21 additions & 4 deletions admiral/pkg/clusters/virtualservice_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1773,7 +1773,9 @@ func TestAddUpdateVirtualService(t *testing.T) {
namespace = "testns"
fooVS = &apiNetworkingV1Alpha3.VirtualService{
ObjectMeta: metaV1.ObjectMeta{
Name: "stage.test00.foo-vs",
Name: "stage.test00.foo-vs",
Labels: map[string]string{"applications.argoproj.io/app-name": "test", "app.kubernetes.io/instance": "test"},
Annotations: map[string]string{"argocd.argoproj.io/tracking-id": "test"},
},
Spec: networkingV1Alpha3.VirtualService{
Hosts: []string{"stage.test00.foo", "stage.test00.bar"},
Expand All @@ -1789,10 +1791,13 @@ func TestAddUpdateVirtualService(t *testing.T) {
},
}
admiralParams := common.AdmiralParams{
LabelSet: &common.LabelSet{},
SyncNamespace: "test-sync-ns",
EnableSWAwareNSCaches: true,
LabelSet: &common.LabelSet{},
SyncNamespace: "test-sync-ns",
EnableSWAwareNSCaches: true,
IgnoreLabelsAnnotationsList: []string{"applications.argoproj.io/app-name", "app.kubernetes.io/instance", "argocd.argoproj.io/tracking-id"},
}
common.ResetSync()
common.InitializeConfig(admiralParams)
rr := NewRemoteRegistry(ctx, admiralParams)

cases := []struct {
Expand All @@ -1818,6 +1823,18 @@ func TestAddUpdateVirtualService(t *testing.T) {
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
addUpdateVirtualService(ctxLogger, ctx, c.newVS, c.existingVS, namespace, rc, rr)
vs, err := istioClientWithExistingVS.NetworkingV1alpha3().VirtualServices(namespace).Get(ctx, fooVS.Name, metaV1.GetOptions{})
if err != nil {
t.Errorf("failed to get VS with error: %v", err)
}
for _, ignored := range common.GetIgnoreLabelsAnnotations() {
if vs.Labels[ignored] != "" {
t.Errorf("expected VS to not ignored labels, but got labels: %v", ignored)
}
if vs.Annotations[ignored] != "" {
t.Errorf("expected VS to not ignored annotations, but got annotations: %v", ignored)
}
}
})
}
}
6 changes: 6 additions & 0 deletions admiral/pkg/controller/common/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,3 +576,9 @@ func GetOperatorSecretFilterTags() string {
defer wrapper.RUnlock()
return wrapper.params.OperatorSecretFilterTags
}

func GetIgnoreLabelsAnnotations() []string {
wrapper.RLock()
defer wrapper.RUnlock()
return wrapper.params.IgnoreLabelsAnnotationsList
}
5 changes: 5 additions & 0 deletions admiral/pkg/controller/common/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func setupForConfigTests() {
DiscoveryClustersForNumaflow: make([]string, 0),
ClientDiscoveryClustersForJobs: make([]string, 0),
EnableClientDiscovery: true,
IgnoreLabelsAnnotationsList: []string{"applications.argoproj.io/app-name", "app.kubernetes.io/instance", "argocd.argoproj.io/tracking-id"},
}
ResetSync()
initHappened = true
Expand Down Expand Up @@ -327,6 +328,10 @@ func TestConfigManagement(t *testing.T) {
if len(GetClientDiscoveryClustersForNumaflow()) != 0 {
t.Errorf("clusters for numaflow client discovery mismatch, expected 0, got %v", GetClientDiscoveryClustersForNumaflow())
}

if len(GetIgnoreLabelsAnnotations()) != 3 {
t.Errorf("ignored labels and annotations for VS copy mismatch, expected 3, got %v", GetIgnoreLabelsAnnotations())
}
}

func TestGetCRDIdentityLabelWithCRDIdentity(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions admiral/pkg/controller/common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ type AdmiralParams struct {
EnableGenerationCheck bool
EnableIsOnlyReplicaCountChangedCheck bool
PreventSplitBrain bool
IgnoreLabelsAnnotationsList []string

// Cartographer specific params
TrafficConfigPersona bool
Expand Down
Loading

0 comments on commit bc55f95

Please sign in to comment.