From d3201d490a0ef9a6fc9e4f1e73ddd7f95b8de5a4 Mon Sep 17 00:00:00 2001 From: aattuluri <44482891+aattuluri@users.noreply.github.com> Date: Fri, 4 Feb 2022 13:11:00 -0800 Subject: [PATCH] Ignore syncing istio resources in system namespaces and admiral-sync namespace (#192) --- admiral/pkg/clusters/handler.go | 45 +++++++++++++++------------- admiral/pkg/clusters/handler_test.go | 39 ++++++++++++++---------- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go index a3e21601..8aabc30a 100644 --- a/admiral/pkg/clusters/handler.go +++ b/admiral/pkg/clusters/handler.go @@ -104,46 +104,53 @@ func getDestinationRule(host string, locality string, gtpTrafficPolicy *model.Tr } func (se *ServiceEntryHandler) Added(obj *v1alpha3.ServiceEntry) { - if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations) { + if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { + log.Infof(LogFormat, "Add", "ServiceEntry", obj.Name, se.ClusterID, "Skipping resource from namespace=" + obj.Namespace) return } } func (se *ServiceEntryHandler) Updated(obj *v1alpha3.ServiceEntry) { - if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations) { + if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { + log.Infof(LogFormat, "Update", "ServiceEntry", obj.Name, se.ClusterID, "Skipping resource from namespace=" + obj.Namespace) return } } func (se *ServiceEntryHandler) Deleted(obj *v1alpha3.ServiceEntry) { - if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations) { + if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { + log.Infof(LogFormat, "Delete", "ServiceEntry", obj.Name, se.ClusterID, "Skipping resource from namespace=" + obj.Namespace) return } } func (dh *DestinationRuleHandler) Added(obj *v1alpha3.DestinationRule) { - if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations) { + if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { + log.Infof(LogFormat, "Add", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) return } handleDestinationRuleEvent(obj, dh, common.Add, common.DestinationRule) } func (dh *DestinationRuleHandler) Updated(obj *v1alpha3.DestinationRule) { - if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations) { + if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { + log.Infof(LogFormat, "Update", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) return } handleDestinationRuleEvent(obj, dh, common.Update, common.DestinationRule) } func (dh *DestinationRuleHandler) Deleted(obj *v1alpha3.DestinationRule) { - if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations) { + if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { + log.Infof(LogFormat, "Delete", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) return } handleDestinationRuleEvent(obj, dh, common.Delete, common.DestinationRule) } func (vh *VirtualServiceHandler) Added(obj *v1alpha3.VirtualService) { - if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations) { + if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { + log.Infof(LogFormat, "Add", "VirtualService", obj.Name, vh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) return } err := handleVirtualServiceEvent(obj, vh, common.Add, common.VirtualService) @@ -153,7 +160,8 @@ func (vh *VirtualServiceHandler) Added(obj *v1alpha3.VirtualService) { } func (vh *VirtualServiceHandler) Updated(obj *v1alpha3.VirtualService) { - if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations) { + if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { + log.Infof(LogFormat, "Update", "VirtualService", obj.Name, vh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) return } err := handleVirtualServiceEvent(obj, vh, common.Update, common.VirtualService) @@ -163,7 +171,8 @@ func (vh *VirtualServiceHandler) Updated(obj *v1alpha3.VirtualService) { } func (vh *VirtualServiceHandler) Deleted(obj *v1alpha3.VirtualService) { - if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations) { + if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) { + log.Infof(LogFormat, "Delete", "VirtualService", obj.Name, vh.ClusterID, "Skipping resource from namespace=" + obj.Namespace) return } err := handleVirtualServiceEvent(obj, vh, common.Delete, common.VirtualService) @@ -178,12 +187,16 @@ func (dh *SidecarHandler) Updated(obj *v1alpha3.Sidecar) {} func (dh *SidecarHandler) Deleted(obj *v1alpha3.Sidecar) {} -func IgnoreIstioResource(exportTo []string, annotations map[string]string) bool { +func IgnoreIstioResource(exportTo []string, annotations map[string]string, namespace string) bool { if len(annotations) > 0 && annotations[common.AdmiralIgnoreAnnotation] == "true" { return true } + if namespace == common.NamespaceIstioSystem || namespace == common.NamespaceKubeSystem || namespace == common.GetSyncNamespace() { + return true + } + if len(exportTo) == 0 { return false } else { @@ -209,11 +222,6 @@ func handleDestinationRuleEvent(obj *v1alpha3.DestinationRule, dh *DestinationRu r := dh.RemoteRegistry - if obj.Namespace == syncNamespace || obj.Namespace == common.NamespaceKubeSystem { - log.Infof(LogFormat, "Event", "DestinationRule", obj.Name, clusterId, "Skipping the namespace: "+obj.Namespace) - return - } - dependentClusters := r.AdmiralCache.CnameDependentClusterCache.Get(destinationRule.Host) if dependentClusters != nil && len(dependentClusters.Map()) > 0 { @@ -399,11 +407,6 @@ func handleVirtualServiceEvent(obj *v1alpha3.VirtualService, vh *VirtualServiceH syncNamespace := common.GetSyncNamespace() - if obj.Namespace == syncNamespace { - log.Infof(LogFormat, "Event", resourceType, obj.Name, clusterId, "Skipping the namespace: "+obj.Namespace) - return nil - } - if len(virtualService.Hosts) > 1 { log.Errorf(LogFormat, "Event", resourceType, obj.Name, clusterId, "Skipping as multiple hosts not supported for virtual service namespace="+obj.Namespace) return nil @@ -570,7 +573,7 @@ func addUpdateDestinationRule(obj *v1alpha3.DestinationRule, exist *v1alpha3.Des obj.Annotations = map[string]string{} } obj.Annotations["app.kubernetes.io/created-by"] = "admiral" - if exist == nil { + if exist == nil || exist.Name == "" || exist.Spec.Host == "" { obj.Namespace = namespace obj.ResourceVersion = "" _, err = rc.DestinationRuleController.IstioClient.NetworkingV1alpha3().DestinationRules(namespace).Create(obj) diff --git a/admiral/pkg/clusters/handler_test.go b/admiral/pkg/clusters/handler_test.go index 6e515e3c..1038f228 100644 --- a/admiral/pkg/clusters/handler_test.go +++ b/admiral/pkg/clusters/handler_test.go @@ -27,36 +27,56 @@ func TestIgnoreIstioResource(t *testing.T) { name string exportTo []string annotations map[string]string + namespace string expectedResult bool }{ { name: "Should return false when exportTo is not present", exportTo: nil, annotations: nil, + namespace: "random-ns", expectedResult: false, }, { name: "Should return false when its exported to *", exportTo: []string{"*"}, annotations: nil, + namespace: "random-ns", expectedResult: false, }, { name: "Should return true when its exported to .", exportTo: []string{"."}, annotations: nil, + namespace: "random-ns", expectedResult: true, }, { name: "Should return true when its exported to a handful of namespaces", exportTo: []string{"namespace1", "namespace2"}, annotations: nil, + namespace: "random-ns", expectedResult: true, }, { name: "Should return true when admiral ignore annotation is set", exportTo: []string{"*"}, annotations: map[string]string{common.AdmiralIgnoreAnnotation: "true"}, + namespace: "random-ns", + expectedResult: true, + }, + { + name: "Should return true when its from istio-system namespace", + exportTo: []string{"*"}, + annotations: nil, + namespace: "istio-system", + expectedResult: true, + }, + { + name: "Should return true when its from admiral sync namespace", + exportTo: []string{"*"}, + annotations: nil, + namespace: "ns", expectedResult: true, }, } @@ -64,7 +84,7 @@ func TestIgnoreIstioResource(t *testing.T) { //Run the test for every provided case for _, c := range testCases { t.Run(c.name, func(t *testing.T) { - result := IgnoreIstioResource(c.exportTo, c.annotations) + result := IgnoreIstioResource(c.exportTo, c.annotations, c.namespace) if result == c.expectedResult { //perfect } else { @@ -192,10 +212,6 @@ func TestGetDestinationRule(t *testing.T) { } func TestHandleVirtualServiceEvent(t *testing.T) { - //Do setup here - syncNs := v1alpha32.VirtualService{} - syncNs.Namespace = "ns" - tooManyHosts := v1alpha32.VirtualService{ Spec: v1alpha3.VirtualService{ Hosts: []string{"qa.blah.global", "e2e.blah.global"}, @@ -283,13 +299,6 @@ func TestHandleVirtualServiceEvent(t *testing.T) { expectedError error event common.Event }{ - { - name: "Virtual Service in sync namespace", - vs: &syncNs, - expectedError: nil, - handler: &noDependencClustersHandler, - event: 0, - }, { name: "Virtual Service with multiple hosts", vs: &tooManyHosts, @@ -305,21 +314,21 @@ func TestHandleVirtualServiceEvent(t *testing.T) { event: 0, }, { - name: "Add event for VS in SourceNamespace", + name: "Add event for VS not generated by Admiral", vs: &happyPath, expectedError: nil, handler: &handlerFullClient, event: 0, }, { - name: "Update event for VS in SourceNamespace", + name: "Update event for VS not generated by Admiral", vs: &vsNotGeneratedByAdmiral, expectedError: nil, handler: &handlerFullClient, event: 1, }, { - name: "Delete event for VS in SourceNamespace", + name: "Delete event for VS not generated by Admiral", vs: &vsNotGeneratedByAdmiral, expectedError: nil, handler: &handlerFullClient,