Skip to content

Commit

Permalink
Ignore syncing istio resources in system namespaces and admiral-sync …
Browse files Browse the repository at this point in the history
…namespace (#192)
  • Loading branch information
aattuluri committed Feb 4, 2022
1 parent dea6a86 commit d3201d4
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 36 deletions.
45 changes: 24 additions & 21 deletions admiral/pkg/clusters/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
39 changes: 24 additions & 15 deletions admiral/pkg/clusters/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,44 +27,64 @@ 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,
},
}

//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 {
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit d3201d4

Please sign in to comment.