From 3722ad1f5a6d6fce5aff1ce24c48fe32d5337602 Mon Sep 17 00:00:00 2001 From: Joe Peacock Date: Fri, 27 Mar 2020 13:48:53 -0700 Subject: [PATCH 1/5] Added tests for destination rule creation Signed-off-by: Joe Peacock --- admiral/pkg/clusters/handler_test.go | 157 +++++++++++++++++++++++++-- 1 file changed, 146 insertions(+), 11 deletions(-) diff --git a/admiral/pkg/clusters/handler_test.go b/admiral/pkg/clusters/handler_test.go index 12fece7d..a219320d 100644 --- a/admiral/pkg/clusters/handler_test.go +++ b/admiral/pkg/clusters/handler_test.go @@ -1,6 +1,11 @@ package clusters import ( + "github.com/gogo/protobuf/types" + "github.com/google/go-cmp/cmp" + "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/model" + "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" + "istio.io/api/networking/v1alpha3" "testing" ) @@ -8,28 +13,28 @@ func TestIgnoreIstioResource(t *testing.T) { //Struct of test case info. Name is required. testCases := []struct { - name string - exportTo []string + name string + exportTo []string expectedResult bool }{ { - name: "Should return false when exportTo is not present", - exportTo: nil, + name: "Should return false when exportTo is not present", + exportTo: nil, expectedResult: false, }, { - name: "Should return false when its exported to *", - exportTo: []string {"*"}, + name: "Should return false when its exported to *", + exportTo: []string{"*"}, expectedResult: false, }, { - name: "Should return true when its exported to .", - exportTo: []string {"."}, + name: "Should return true when its exported to .", + exportTo: []string{"."}, expectedResult: true, }, { - name: "Should return true when its exported to a handful of namespaces", - exportTo: []string {"namespace1", "namespace2"}, + name: "Should return true when its exported to a handful of namespaces", + exportTo: []string{"namespace1", "namespace2"}, expectedResult: true, }, } @@ -41,7 +46,137 @@ func TestIgnoreIstioResource(t *testing.T) { if result == c.expectedResult { //perfect } else { - t.Errorf("Failed. Got %v, expected %v",result, c.expectedResult) + t.Errorf("Failed. Got %v, expected %v", result, c.expectedResult) + } + }) + } +} + +func TestGetDestinationRule(t *testing.T) { + //Do setup here + mTLS := &v1alpha3.TrafficPolicy{Tls: &v1alpha3.TLSSettings{Mode: v1alpha3.TLSSettings_ISTIO_MUTUAL}} + + noGtpDr := v1alpha3.DestinationRule{ + Host: "qa.myservice.global", + TrafficPolicy: mTLS, + } + + basicGtpDr := v1alpha3.DestinationRule{ + Host: "qa.myservice.global", + TrafficPolicy: &v1alpha3.TrafficPolicy{ + Tls: &v1alpha3.TLSSettings{Mode: v1alpha3.TLSSettings_ISTIO_MUTUAL}, + LoadBalancer: &v1alpha3.LoadBalancerSettings{ + LbPolicy: &v1alpha3.LoadBalancerSettings_Simple{Simple: v1alpha3.LoadBalancerSettings_ROUND_ROBIN}, + LocalityLbSetting: &v1alpha3.LocalityLoadBalancerSetting{}, + }, + OutlierDetection: &v1alpha3.OutlierDetection{ + BaseEjectionTime: &types.Duration{Seconds: 120}, + ConsecutiveErrors: 10, + Interval: &types.Duration{Seconds: 60}, + }, + }, + } + + failoverGtpDr := v1alpha3.DestinationRule{ + Host: "qa.myservice.global", + TrafficPolicy: &v1alpha3.TrafficPolicy{ + Tls: &v1alpha3.TLSSettings{Mode: v1alpha3.TLSSettings_ISTIO_MUTUAL}, + LoadBalancer: &v1alpha3.LoadBalancerSettings{ + LbPolicy: &v1alpha3.LoadBalancerSettings_Simple{Simple: v1alpha3.LoadBalancerSettings_ROUND_ROBIN}, + LocalityLbSetting: &v1alpha3.LocalityLoadBalancerSetting{ + Distribute: []*v1alpha3.LocalityLoadBalancerSetting_Distribute{ + { + From: "uswest2/*", + To: map[string]uint32{"us-west-2": 100}, + }, + }, + }, + }, + OutlierDetection: &v1alpha3.OutlierDetection{ + BaseEjectionTime: &types.Duration{Seconds: 120}, + ConsecutiveErrors: 10, + Interval: &types.Duration{Seconds: 60}, + }, + }, + } + + topologyGTPBody := model.GlobalTrafficPolicy{ + Policy: []*model.TrafficPolicy{ + { + LbType: model.TrafficPolicy_TOPOLOGY, + Target: []*model.TrafficGroup{ + { + Region: "us-west-2", + Weight: 100, + }, + }, + }, + }, + } + + topologyGTP := v1.GlobalTrafficPolicy{ + Spec: topologyGTPBody, + } + topologyGTP.Name = "myGTP" + topologyGTP.Namespace = "myNS" + + failoverGTPBody := model.GlobalTrafficPolicy{ + Policy: []*model.TrafficPolicy{ + { + LbType: model.TrafficPolicy_FAILOVER, + Target: []*model.TrafficGroup{ + { + Region: "us-west-2", + Weight: 100, + }, + }, + }, + }, + } + + failoverGTP := v1.GlobalTrafficPolicy{ + Spec: failoverGTPBody, + } + failoverGTP.Name = "myGTP" + failoverGTP.Namespace = "myNS" + + //Struct of test case info. Name is required. + testCases := []struct { + name string + host string + locality string + gtp *v1.GlobalTrafficPolicy + destinationRule *v1alpha3.DestinationRule + }{ + { + name: "Should handle a nil GTP", + host: "qa.myservice.global", + locality: "uswest2", + gtp: nil, + destinationRule: &noGtpDr, + }, + { + name: "Should handle a topology GTP", + host: "qa.myservice.global", + locality: "uswest2", + gtp: &topologyGTP, + destinationRule: &basicGtpDr, + }, + { + name: "Should handle a failover GTP", + host: "qa.myservice.global", + locality: "uswest2", + gtp: &failoverGTP, + destinationRule: &failoverGtpDr, + }, + } + + //Run the test for every provided case + for _, c := range testCases { + t.Run(c.name, func(t *testing.T) { + result := getDestinationRule(c.host, c.locality, c.gtp) + if !cmp.Equal(result, c.destinationRule) { + t.Fatalf("DestinationRule Mismatch. Diff: %v", cmp.Diff(result, c.destinationRule)) } }) } From d0f7f6201fa1e0a5b1bfecefc9939218cd23a9c1 Mon Sep 17 00:00:00 2001 From: Joe Peacock Date: Tue, 31 Mar 2020 09:49:59 -0700 Subject: [PATCH 2/5] WIP commit Signed-off-by: Joe Peacock --- admiral/pkg/clusters/handler.go | 19 +++-- admiral/pkg/clusters/handler_test.go | 75 +++++++++++++++++++ .../pkg/controller/istio/virtualservice.go | 2 +- go.sum | 11 +++ 4 files changed, 100 insertions(+), 7 deletions(-) diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go index 7248becf..bd0fb9a8 100644 --- a/admiral/pkg/clusters/handler.go +++ b/admiral/pkg/clusters/handler.go @@ -409,7 +409,7 @@ func createDestinationRuleForLocal(remoteController *RemoteController, localDrNa } } -func handleVirtualServiceEvent(obj *v1alpha3.VirtualService, vh *VirtualServiceHandler, event common.Event, resourceType common.ResourceType) { +func handleVirtualServiceEvent(obj *v1alpha3.VirtualService, vh *VirtualServiceHandler, event common.Event, resourceType common.ResourceType) error { log.Infof(LogFormat, "Event", resourceType, obj.Name, vh.ClusterID, "Received event") @@ -423,19 +423,19 @@ func handleVirtualServiceEvent(obj *v1alpha3.VirtualService, vh *VirtualServiceH if obj.Namespace == syncNamespace { log.Infof(LogFormat, "Event", resourceType, obj.Name, clusterId, "Skipping the namespace: "+obj.Namespace) - return + 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 + return nil } dependentClusters := r.AdmiralCache.CnameDependentClusterCache.Get(virtualService.Hosts[0]) if dependentClusters == nil { log.Infof(LogFormat, "Event", resourceType, obj.Name, clusterId, "No dependent clusters found") - return + return nil } log.Infof(LogFormat, "Event", "VirtualService", obj.Name, clusterId, "Processing") @@ -448,11 +448,17 @@ func handleVirtualServiceEvent(obj *v1alpha3.VirtualService, vh *VirtualServiceH if event == common.Delete { log.Infof(LogFormat, "Delete", "VirtualService", obj.Name, clusterId, "Success") - rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(syncNamespace).Delete(obj.Name, &v12.DeleteOptions{}) + err := rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(syncNamespace).Delete(obj.Name, &v12.DeleteOptions{}) + if err != nil { + return err + } } else { - exist, _ := rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(syncNamespace).Get(obj.Name, v12.GetOptions{}) + exist, err := rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(syncNamespace).Get(obj.Name, v12.GetOptions{}) + if err != nil { + return err + } //change destination host for all http routes .. to same as host on the virtual service for _, httpRoute := range virtualService.Http { @@ -474,6 +480,7 @@ func handleVirtualServiceEvent(obj *v1alpha3.VirtualService, vh *VirtualServiceH } } + return nil } func addUpdateVirtualService(obj *v1alpha3.VirtualService, exist *v1alpha3.VirtualService, namespace string, rc *RemoteController) { diff --git a/admiral/pkg/clusters/handler_test.go b/admiral/pkg/clusters/handler_test.go index a219320d..a58acc03 100644 --- a/admiral/pkg/clusters/handler_test.go +++ b/admiral/pkg/clusters/handler_test.go @@ -5,7 +5,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/model" "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" + "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" "istio.io/api/networking/v1alpha3" + v1alpha32 "istio.io/client-go/pkg/apis/networking/v1alpha3" "testing" ) @@ -181,3 +183,76 @@ 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"}, + }, + } + tooManyHosts.Namespace = "other-ns" + + happyPath := v1alpha32.VirtualService{ + Spec: v1alpha3.VirtualService{ + Hosts: []string{"e2e.blah.global"}, + }, + } + happyPath.Namespace = "other-ns" + + cnameCache := common.NewMapOfMaps() + noDependencClustersHandler := VirtualServiceHandler{ + RemoteRegistry: &RemoteRegistry{ + remoteControllers: map[string]*RemoteController{}, + AdmiralCache: &AdmiralCache{ + CnameDependentClusterCache: cnameCache, + }, + }, + } + + + //Struct of test case info. Name is required. + testCases := []struct { + name string + vs *v1alpha32.VirtualService + handler *VirtualServiceHandler + 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, + expectedError: nil, + handler: &noDependencClustersHandler, + event: 0, + }, + { + name: "No dependent clusters", + vs: &happyPath, + expectedError: nil, + handler: &noDependencClustersHandler, + event: 0, + }, + } + + //Run the test for every provided case + for _, c := range testCases { + t.Run(c.name, func(t *testing.T) { + err := handleVirtualServiceEvent(c.vs, c.handler, c.event, common.VirtualService) + if err != c.expectedError { + t.Fatalf("Error mismatch, expected %v but got %v", c.expectedError, err) + } + }) + } +} diff --git a/admiral/pkg/controller/istio/virtualservice.go b/admiral/pkg/controller/istio/virtualservice.go index 21c7f1b3..081b6fae 100644 --- a/admiral/pkg/controller/istio/virtualservice.go +++ b/admiral/pkg/controller/istio/virtualservice.go @@ -21,7 +21,7 @@ type VirtualServiceHandler interface { } type VirtualServiceController struct { - IstioClient *versioned.Clientset + IstioClient versioned.Interface VirtualServiceHandler VirtualServiceHandler informer cache.SharedIndexInformer ctl *admiral.Controller diff --git a/go.sum b/go.sum index cab1d165..bea750c3 100644 --- a/go.sum +++ b/go.sum @@ -397,6 +397,8 @@ golang.org/x/crypto v0.0.0-20190923035154-9ee001bba392/go.mod h1:/lpIB1dKB+9EgE3 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191108234033-bd318be0434a h1:R/qVym5WAxsZWQqZCwDY/8sdVKV1m1WgU4/S5IRQAzc= golang.org/x/crypto v0.0.0-20191108234033-bd318be0434a/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200220183623-bac4c82f6975 h1:/Tl7pH94bvbAAHBdZJT947M/+gp0+CqQXDtMRC0fseo= +golang.org/x/crypto v0.0.0-20200220183623-bac4c82f6975/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -606,10 +608,14 @@ istio.io/api v0.0.0-20191107224652-6818c03d25b2 h1:rIP85xkbLc9Df20/Tnz2LxNck7uhO istio.io/api v0.0.0-20191107224652-6818c03d25b2/go.mod h1:+cyHH83OwC0rFpwk8eXctzPNpiCAbB+r6kmMiAxxBHw= istio.io/api v0.0.0-20200226024546-cca495b82b03 h1:c0Lsnavz2gRslJxqtqiP7VOxr8EJHJJWgfSJabfnZ2s= istio.io/api v0.0.0-20200226024546-cca495b82b03/go.mod h1:bcY3prusO/6vA6zGHz4PNG2v79clPyTw06Xx3fprJSQ= +istio.io/api v0.0.0-20200325005357-8217d7225b6d h1:qOJdOmqJdBZJSrQWgYImY0JTqqRIV3iturAW6gpN4Zs= +istio.io/api v0.0.0-20200325005357-8217d7225b6d/go.mod h1:bcY3prusO/6vA6zGHz4PNG2v79clPyTw06Xx3fprJSQ= istio.io/client-go v0.0.0-20191104174404-7b65e62d85b0 h1:6DWOkG2XZQRAwthO71a0v+AFcONHF6Wo9mtfMNUly24= istio.io/client-go v0.0.0-20191104174404-7b65e62d85b0/go.mod h1:jdpC7NC/WdBGIxcIHPzlurKaCZb+8LnsJW4U4mbX6n0= istio.io/client-go v0.0.0-20200226182959-cde3e69bd9dd h1:U5eWo5yvlRsk4pr5s6ky63G1dJkAgugXzoiKzkMQxGc= istio.io/client-go v0.0.0-20200226182959-cde3e69bd9dd/go.mod h1:wLFtAm266NbVvt1Y8ZwbZXCdp1kgnBuxZTAIaMwUkto= +istio.io/client-go v0.0.0-20200325170329-dc00bbff4229 h1:io9dTYbZm5iOo18iplGH0SCH4ovGfYctUlVzVolWx4I= +istio.io/client-go v0.0.0-20200325170329-dc00bbff4229/go.mod h1:SurC7RfK3Js8hEAPa0xZXxw8xk2d9tZx7YRsvbLlrBo= istio.io/gogo-genproto v0.0.0-20190930162913-45029607206a h1:w7zILua2dnYo9CxImhpNW4NE/8ZxEoc/wfBfHrhUhrE= istio.io/gogo-genproto v0.0.0-20190930162913-45029607206a/go.mod h1:OzpAts7jljZceG4Vqi5/zXy/pOg1b209T3jb7Nv5wIs= istio.io/gogo-genproto v0.0.0-20191024203824-d079cc8b1d55 h1:nvpx66mnuGvXYP4IfCWfUqB9YhiXBF3MvUDsclNnDzI= @@ -627,6 +633,8 @@ k8s.io/api v0.0.0-20191108065827-59e77acf588f/go.mod h1:uQDmBYHoPSuhbg8FGTRzrOda k8s.io/api v0.17.0/go.mod h1:npsyOePkeP0CPwyGfXDHxvypiYMJxBWAMpQxCaJ4ZxI= k8s.io/api v0.17.2 h1:NF1UFXcKN7/OOv1uxdRz3qfra8AHsPav5M93hlV9+Dc= k8s.io/api v0.17.2/go.mod h1:BS9fjjLc4CMuqfSO8vgbHPKMt5+SF0ET6u/RVDihTo4= +k8s.io/api v0.17.4 h1:HbwOhDapkguO8lTAE8OX3hdF2qp8GtpC9CW/MQATXXo= +k8s.io/api v0.17.4/go.mod h1:5qxx6vjmwUVG2nHQTKGlLts8Tbok8PzHl4vHtVFuZCA= k8s.io/apiextensions-apiserver v0.0.0-20181204003618-e419c5771cdc h1:IOukeE9HtTwpLslbujLDfRpfFU6tsjq28yO0fjnl/hk= k8s.io/apiextensions-apiserver v0.0.0-20181204003618-e419c5771cdc/go.mod h1:IxkesAMoaCRoLrPJdZNZUQp9NfZnzqaVzLhb2VEQzXE= k8s.io/apiextensions-apiserver v0.0.0-20191014073952-6b6acca910e3 h1:11T1avcLWsxcVNglsvkStUw3GJFjOYxuCb17+88+GI8= @@ -644,12 +652,15 @@ k8s.io/apimachinery v0.17.2 h1:hwDQQFbdRlpnnsR64Asdi55GyCaIP/3WQpMmbNBeWr4= k8s.io/apimachinery v0.17.2/go.mod h1:b9qmWdKlLuU9EBh+06BtLcSf/Mu89rWL33naRxs1uZg= k8s.io/apimachinery v0.17.3 h1:f+uZV6rm4/tHE7xXgLyToprg6xWairaClGVkm2t8omg= k8s.io/apimachinery v0.17.3/go.mod h1:gxLnyZcGNdZTCLnq3fgzyg2A5BVCHTNDFrw8AmuJ+0g= +k8s.io/apimachinery v0.17.4 h1:UzM+38cPUJnzqSQ+E1PY4YxMHIzQyCg29LOoGfo79Zw= +k8s.io/apimachinery v0.17.4/go.mod h1:gxLnyZcGNdZTCLnq3fgzyg2A5BVCHTNDFrw8AmuJ+0g= k8s.io/client-go v0.0.0-20190117233410-4022682532b3 h1:7VVBo3+/iX6dzB8dshNuo6Duds/6AoNP5R59IUnwoxg= k8s.io/client-go v0.0.0-20190117233410-4022682532b3/go.mod h1:7vJpHMYJwNQCWgzmNV+VYUl1zCObLyodBc8nIyt8L5s= k8s.io/client-go v0.0.0-20191016111102-bec269661e48 h1:C2XVy2z0dV94q9hSSoCuTPp1KOG7IegvbdXuz9VGxoU= k8s.io/client-go v0.0.0-20191016111102-bec269661e48/go.mod h1:hrwktSwYGI4JK+TJA3dMaFyyvHVi/aLarVHpbs8bgCU= k8s.io/client-go v0.17.2 h1:ndIfkfXEGrNhLIgkr0+qhRguSD3u6DCmonepn1O6NYc= k8s.io/client-go v0.17.2/go.mod h1:QAzRgsa0C2xl4/eVpeVAZMvikCn8Nm81yqVx3Kk9XYI= +k8s.io/client-go v0.17.4/go.mod h1:ouF6o5pz3is8qU0/qYL2RnoxOPqgfuidYLowytyLJmc= k8s.io/client-go v11.0.0+incompatible h1:LBbX2+lOwY9flffWlJM7f1Ct8V2SRNiMRDFeiwnJo9o= k8s.io/client-go v11.0.0+incompatible/go.mod h1:7vJpHMYJwNQCWgzmNV+VYUl1zCObLyodBc8nIyt8L5s= k8s.io/code-generator v0.0.0-20191108065441-3c1097069dc3 h1:qcl3ardofg2+EAvhYmwX+XMLXyDQ6dOp8yL2ir1JfsM= From 982d0fd434c25e6ecd6bd0fed729594d3834d6ec Mon Sep 17 00:00:00 2001 From: Joe Peacock Date: Tue, 31 Mar 2020 15:38:48 -0700 Subject: [PATCH 3/5] WIP commit Signed-off-by: Joe Peacock --- admiral/pkg/clusters/handler.go | 5 +-- admiral/pkg/clusters/handler_test.go | 67 ++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go index bd0fb9a8..8d550005 100644 --- a/admiral/pkg/clusters/handler.go +++ b/admiral/pkg/clusters/handler.go @@ -455,10 +455,7 @@ func handleVirtualServiceEvent(obj *v1alpha3.VirtualService, vh *VirtualServiceH } else { - exist, err := rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(syncNamespace).Get(obj.Name, v12.GetOptions{}) - if err != nil { - return err - } + exist, _ := rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(syncNamespace).Get(obj.Name, v12.GetOptions{}) //change destination host for all http routes .. to same as host on the virtual service for _, httpRoute := range virtualService.Http { diff --git a/admiral/pkg/clusters/handler_test.go b/admiral/pkg/clusters/handler_test.go index a58acc03..bbeba7c4 100644 --- a/admiral/pkg/clusters/handler_test.go +++ b/admiral/pkg/clusters/handler_test.go @@ -6,9 +6,13 @@ import ( "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/model" "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" + "github.com/istio-ecosystem/admiral/admiral/pkg/controller/istio" "istio.io/api/networking/v1alpha3" v1alpha32 "istio.io/client-go/pkg/apis/networking/v1alpha3" + v12 "k8s.io/apimachinery/pkg/apis/meta/v1" "testing" + + istiofake "istio.io/client-go/pkg/clientset/versioned/fake" ) func TestIgnoreIstioResource(t *testing.T) { @@ -203,6 +207,7 @@ func TestHandleVirtualServiceEvent(t *testing.T) { }, } happyPath.Namespace = "other-ns" + happyPath.Name = "vs-name" cnameCache := common.NewMapOfMaps() noDependencClustersHandler := VirtualServiceHandler{ @@ -214,6 +219,47 @@ func TestHandleVirtualServiceEvent(t *testing.T) { }, } + fakeIstioClient := istiofake.NewSimpleClientset() + goodCnameCache := common.NewMapOfMaps() + goodCnameCache.Put("e2e.blah.global", "cluster.k8s.global", "cluster.k8s.global") + handlerEmptyClient := VirtualServiceHandler{ + RemoteRegistry: &RemoteRegistry{ + remoteControllers: map[string]*RemoteController{ + "cluster.k8s.global": &RemoteController{ + VirtualServiceController: &istio.VirtualServiceController{ + IstioClient: fakeIstioClient, + }, + }, + }, + AdmiralCache: &AdmiralCache{ + CnameDependentClusterCache: goodCnameCache, + }, + }, + } + + fullFakeIstioClient := istiofake.NewSimpleClientset() + fullFakeIstioClient.NetworkingV1alpha3().VirtualServices("ns").Create(&v1alpha32.VirtualService{ + ObjectMeta: v12.ObjectMeta{ + Name: "vs-name", + }, + Spec: v1alpha3.VirtualService{ + Hosts: []string{"e2e.blah.global"}, + }, + }) + handlerFullClient := VirtualServiceHandler{ + RemoteRegistry: &RemoteRegistry{ + remoteControllers: map[string]*RemoteController{ + "cluster.k8s.global": &RemoteController{ + VirtualServiceController: &istio.VirtualServiceController{ + IstioClient: fullFakeIstioClient, + }, + }, + }, + AdmiralCache: &AdmiralCache{ + CnameDependentClusterCache: goodCnameCache, + }, + }, + } //Struct of test case info. Name is required. testCases := []struct { @@ -244,6 +290,27 @@ func TestHandleVirtualServiceEvent(t *testing.T) { handler: &noDependencClustersHandler, event: 0, }, + { + name: "New Virtual Service", + vs: &happyPath, + expectedError: nil, + handler: &handlerEmptyClient, + event: 0, + }, + { + name: "Existing Virtual Service", + vs: &happyPath, + expectedError: nil, + handler: &handlerFullClient, + event: 1, + }, + { + name: "Deleted Virtual Service", + vs: &happyPath, + expectedError: nil, + handler: &handlerFullClient, + event: 2, + }, } //Run the test for every provided case From 701672d9c72f5384f600ba13affff7ad9e09e69c Mon Sep 17 00:00:00 2001 From: Joe Peacock Date: Tue, 31 Mar 2020 16:08:37 -0700 Subject: [PATCH 4/5] Uncommented/fixed a bunch of SE tests Signed-off-by: Joe Peacock --- admiral/pkg/clusters/serviceentry_test.go | 564 ++++++++++-------- .../pkg/controller/istio/destinationrule.go | 2 +- admiral/pkg/controller/istio/serviceentry.go | 2 +- 3 files changed, 303 insertions(+), 265 deletions(-) diff --git a/admiral/pkg/clusters/serviceentry_test.go b/admiral/pkg/clusters/serviceentry_test.go index bd8cc32e..77f5dbec 100644 --- a/admiral/pkg/clusters/serviceentry_test.go +++ b/admiral/pkg/clusters/serviceentry_test.go @@ -1,281 +1,319 @@ package clusters import ( + "context" + "errors" "github.com/google/go-cmp/cmp" + v13 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1" + "github.com/istio-ecosystem/admiral/admiral/pkg/controller/admiral" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/istio" + "github.com/istio-ecosystem/admiral/admiral/pkg/test" "gopkg.in/yaml.v2" istionetworkingv1alpha3 "istio.io/api/networking/v1alpha3" "istio.io/client-go/pkg/apis/networking/v1alpha3" istiofake "istio.io/client-go/pkg/clientset/versioned/fake" + v14 "k8s.io/api/apps/v1" "k8s.io/api/core/v1" v12 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + "reflect" + "strconv" + "sync" "testing" + "time" ) -//func TestCreateSeWithDrLabels(t *testing.T) { -// -// se := networking.ServiceEntry{ -// Hosts: []string{"test.com"}, -// Endpoints: []*networking.ServiceEntry_Endpoint{ -// {Address: "127.0.0.1", Ports: map[string]uint32{"https": 80}, Labels: map[string]string{}, Network: "mesh1", Locality: "us-west", Weight: 100}, -// }, -// } -// -// des := networking.DestinationRule{ -// Host: "test.com", -// Subsets: []*networking.Subset{ -// {Name: "subset1", Labels: map[string]string{"foo": "bar"}, TrafficPolicy: nil}, -// }, -// } -// -// cacheWithNoEntry := ServiceEntryAddressStore{ -// EntryAddresses: map[string]string{"test-se": "1.2.3.4"}, -// Addresses: []string{"1.2.3.4"}, -// } -// -// emptyCacheController := test.FakeConfigMapController{ -// GetError: nil, -// PutError: nil, -// ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithNoEntry, "123"), -// } -// -// -// res := createSeWithDrLabels(nil, false, "", "test-se", &se, &des, &cacheWithNoEntry, &emptyCacheController) -// -// if res == nil { -// t.Fail() -// } -// -// newSe := res["test-se"] -// -// value := newSe.Endpoints[0].Labels["foo"] -// -// if value != "bar" { -// t.Fail() -// } -// -// if newSe.Addresses[0] != "1.2.3.4" { -// t.Errorf("Address set incorrectly from cache, expected 1.2.3.4, got %v", newSe.Addresses[0]) -// } -//} -// -//func TestAddServiceEntriesWithDr(t *testing.T) { -// admiralCache := AdmiralCache{} -// -// cnameIdentityCache := sync.Map{} -// cnameIdentityCache.Store("dev.bar.global", "bar") -// admiralCache.CnameIdentityCache = &cnameIdentityCache -// -// se := networking.ServiceEntry{ -// Hosts: []string{"dev.bar.global"}, -// Endpoints: []*networking.ServiceEntry_Endpoint{ -// {Address: "127.0.0.1", Ports: map[string]uint32{"https": 80}, Labels: map[string]string{}, Network: "mesh1", Locality: "us-west", Weight: 100}, -// }, -// } -// -// rc, _ := createMockRemoteController(func(i interface{}) { -// //res := i.(istio.Config) -// //se, ok := res.Spec.(*networking.ServiceEntry) -// //if ok { -// // if se.Hosts[0] != "dev.bar.global" { -// // t.Errorf("Host mismatch. Expected dev.bar.global, got %v", se.Hosts[0]) -// // } -// //} -// }) -// -// seConfig, _ := createIstioConfig(istio.ServiceEntryProto, &se, "se1", "admiral-sync") -// _, err := rc.IstioConfigStore.Create(*seConfig) -// if err != nil { -// t.Errorf("%v", err) -// -// } -// -// AddServiceEntriesWithDr(&admiralCache, map[string]string{"cl1":"cl1"}, map[string]*RemoteController{"cl1":rc}, map[string]*networking.ServiceEntry{"se1": &se}, "admiral-sync") -// } -// -//func TestCreateServiceEntryForNewServiceOrPod(t *testing.T) { -// -// p := AdmiralParams{ -// KubeconfigPath: "testdata/fake.config", -// } -// rr, _ := InitAdmiral(context.Background(), p) -// -// rc, _ := createMockRemoteController(func(i interface{}) { -// res := i.(istio.Config) -// se, ok := res.Spec.(*networking.ServiceEntry) -// if ok { -// if se.Hosts[0] != "dev.bar.global" { -// t.Fail() -// } -// } -// }) -// -// rr.remoteControllers["test.cluster"] = rc -// createServiceEntryForNewServiceOrPod("test", "bar", rr) -// -//} -// -//func TestGetLocalAddressForSe(t *testing.T) { -// t.Parallel() -// cacheWithEntry := ServiceEntryAddressStore{ -// EntryAddresses: map[string]string{"e2e.a.mesh": common.LocalAddressPrefix + ".10.1"}, -// Addresses: []string{common.LocalAddressPrefix + ".10.1"}, -// } -// cacheWithNoEntry := ServiceEntryAddressStore{ -// EntryAddresses: map[string]string{}, -// Addresses: []string{}, -// } -// cacheWith255Entries := ServiceEntryAddressStore{ -// EntryAddresses: map[string]string{}, -// Addresses: []string{}, -// } -// -// for i := 1; i <= 255; i++ { -// address := common.LocalAddressPrefix + ".10." + strconv.Itoa(i) -// cacheWith255Entries.EntryAddresses[strconv.Itoa(i) + ".mesh"] = address -// cacheWith255Entries.Addresses = append(cacheWith255Entries.Addresses, address) -// } -// -// emptyCacheController := test.FakeConfigMapController{ -// GetError: nil, -// PutError: nil, -// ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithNoEntry, "123"), -// } -// -// cacheController := test.FakeConfigMapController{ -// GetError: nil, -// PutError: nil, -// ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithEntry, "123"), -// } -// -// cacheControllerWith255Entries := test.FakeConfigMapController{ -// GetError: nil, -// PutError: nil, -// ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWith255Entries, "123"), -// } -// -// cacheControllerGetError := test.FakeConfigMapController{ -// GetError: errors.New("BAD THINGS HAPPENED"), -// PutError: nil, -// ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithEntry, "123"), -// } -// -// cacheControllerPutError := test.FakeConfigMapController{ -// PutError: errors.New("BAD THINGS HAPPENED"), -// GetError: nil, -// ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithEntry, "123"), -// } -// -// -// testCases := []struct { -// name string -// seName string -// seAddressCache ServiceEntryAddressStore -// wantAddess string -// cacheController admiral.ConfigMapControllerInterface -// expectedCacheUpdate bool -// wantedError error -// }{ -// { -// name: "should return new available address", -// seName: "e2e.a.mesh", -// seAddressCache: cacheWithNoEntry, -// wantAddess: common.LocalAddressPrefix + ".10.1", -// cacheController: &emptyCacheController, -// expectedCacheUpdate: true, -// wantedError: nil, -// }, -// { -// name: "should return address from map", -// seName: "e2e.a.mesh", -// seAddressCache: cacheWithEntry, -// wantAddess: common.LocalAddressPrefix + ".10.1", -// cacheController: &cacheController, -// expectedCacheUpdate: false, -// wantedError: nil, -// }, -// { -// name: "should return new available address", -// seName: "e2e.b.mesh", -// seAddressCache: cacheWithEntry, -// wantAddess: common.LocalAddressPrefix + ".10.2", -// cacheController: &cacheController, -// expectedCacheUpdate: true, -// wantedError: nil, -// }, -// { -// name: "should return new available address in higher subnet", -// seName: "e2e.a.mesh", -// seAddressCache: cacheWith255Entries, -// wantAddess: common.LocalAddressPrefix + ".11.1", -// cacheController: &cacheControllerWith255Entries, -// expectedCacheUpdate: true, -// wantedError: nil, -// }, -// { -// name: "should gracefully propagate get error", -// seName: "e2e.a.mesh", -// seAddressCache: cacheWith255Entries, -// wantAddess: "", -// cacheController: &cacheControllerGetError, -// expectedCacheUpdate: true, -// wantedError: errors.New("BAD THINGS HAPPENED"), -// }, -// { -// name: "Should not return address on put error", -// seName: "e2e.abcdefghijklmnop.mesh", -// seAddressCache: cacheWith255Entries, -// wantAddess: "", -// cacheController: &cacheControllerPutError, -// expectedCacheUpdate: true, -// wantedError: errors.New("BAD THINGS HAPPENED"), -// }, -// } -// -// for _, c := range testCases { -// t.Run(c.name, func(t *testing.T) { -// seAddress, needsCacheUpdate, err := GetLocalAddressForSe(c.seName, &c.seAddressCache, c.cacheController) -// if c.wantAddess != "" { -// if !reflect.DeepEqual(seAddress, c.wantAddess) { -// t.Errorf("Wanted se address: %s, got: %s", c.wantAddess, seAddress) -// } -// if err==nil && c.wantedError==nil { -// //we're fine -// } else if err.Error() != c.wantedError.Error() { -// t.Errorf("Error mismatch. Expected %v but got %v", c.wantedError, err) -// } -// if needsCacheUpdate != c.expectedCacheUpdate { -// t.Errorf("Expected %v, got %v for needs cache update", c.expectedCacheUpdate, needsCacheUpdate) -// } -// } else { -// if seAddress != "" { -// t.Errorf("Unexpectedly found address: %s", seAddress) -// } -// } -// }) -// } -// -//} -// -//func TestMakeRemoteEndpointForServiceEntry(t *testing.T) { -// address := "1.2.3.4" -// locality := "us-west-2" -// portName := "port" -// -// endpoint := makeRemoteEndpointForServiceEntry(address, locality, portName) -// -// if endpoint.Address != address { -// t.Errorf("Address mismatch. Got: %v, expected: %v", endpoint.Address, address) -// } -// if endpoint.Locality != locality { -// t.Errorf("Locality mismatch. Got: %v, expected: %v", endpoint.Locality, locality) -// } -// if endpoint.Ports[portName] != 15443 { -// t.Errorf("Incorrect port found") -// } -//} -// +func TestCreateSeWithDrLabels(t *testing.T) { + + se := istionetworkingv1alpha3.ServiceEntry{ + Hosts: []string{"test.com"}, + Endpoints: []*istionetworkingv1alpha3.ServiceEntry_Endpoint{ + {Address: "127.0.0.1", Ports: map[string]uint32{"https": 80}, Labels: map[string]string{}, Network: "mesh1", Locality: "us-west", Weight: 100}, + }, + } + + des := istionetworkingv1alpha3.DestinationRule{ + Host: "test.com", + Subsets: []*istionetworkingv1alpha3.Subset{ + {Name: "subset1", Labels: map[string]string{"foo": "bar"}, TrafficPolicy: nil}, + }, + } + + cacheWithNoEntry := ServiceEntryAddressStore{ + EntryAddresses: map[string]string{"test-se": "1.2.3.4"}, + Addresses: []string{"1.2.3.4"}, + } + + emptyCacheController := test.FakeConfigMapController{ + GetError: nil, + PutError: nil, + ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithNoEntry, "123"), + } + + + res := createSeWithDrLabels(nil, false, "", "test-se", &se, &des, &cacheWithNoEntry, &emptyCacheController) + + if res == nil { + t.Fail() + } + + newSe := res["test-se"] + + value := newSe.Endpoints[0].Labels["foo"] + + if value != "bar" { + t.Fail() + } + + if newSe.Addresses[0] != "1.2.3.4" { + t.Errorf("Address set incorrectly from cache, expected 1.2.3.4, got %v", newSe.Addresses[0]) + } +} + +func TestAddServiceEntriesWithDr(t *testing.T) { + admiralCache := AdmiralCache{} + + cnameIdentityCache := sync.Map{} + cnameIdentityCache.Store("dev.bar.global", "bar") + admiralCache.CnameIdentityCache = &cnameIdentityCache + + gtpCache := &globalTrafficCache{} + gtpCache.identityCache = make(map[string]*v13.GlobalTrafficPolicy) + gtpCache.dependencyCache = make(map[string]*v14.Deployment) + gtpCache.mutex = &sync.Mutex{} + admiralCache.GlobalTrafficCache = gtpCache + + se := istionetworkingv1alpha3.ServiceEntry{ + Hosts: []string{"dev.bar.global"}, + Endpoints: []*istionetworkingv1alpha3.ServiceEntry_Endpoint{ + {Address: "127.0.0.1", Ports: map[string]uint32{"https": 80}, Labels: map[string]string{}, Network: "mesh1", Locality: "us-west", Weight: 100}, + }, + } + + seConfig := v1alpha3.ServiceEntry{ + Spec: se, + } + seConfig.Name = "se1" + seConfig.Namespace = "admiral-sync" + + fakeIstioClient := istiofake.NewSimpleClientset() + fakeIstioClient.NetworkingV1alpha3().ServiceEntries("admiral-sync").Create(&seConfig) + rc := &RemoteController{ + ServiceEntryController: &istio.ServiceEntryController{ + IstioClient: fakeIstioClient, + }, + DestinationRuleController: &istio.DestinationRuleController{ + IstioClient: fakeIstioClient, + }, + NodeController: &admiral.NodeController{ + Locality: &admiral.Locality{ + Region: "us-west-2", + }, + }, + } + + AddServiceEntriesWithDr(&admiralCache, map[string]string{"cl1":"cl1"}, map[string]*RemoteController{"cl1":rc}, map[string]*istionetworkingv1alpha3.ServiceEntry{"se1": &se}) + } + +func TestCreateServiceEntryForNewServiceOrPod(t *testing.T) { + + p := common.AdmiralParams{ + KubeconfigPath: "testdata/fake.config", + } + rr, _ := InitAdmiral(context.Background(), p) + + config := rest.Config{ + Host: "localhost", + } + + d, e := admiral.NewDeploymentController(make(chan struct{}), &test.MockDeploymentHandler{}, &config, time.Second*time.Duration(300)) + + if e != nil { + t.Fail() + } + + fakeIstioClient := istiofake.NewSimpleClientset() + rc := &RemoteController{ + ServiceEntryController: &istio.ServiceEntryController{ + IstioClient: fakeIstioClient, + }, + DestinationRuleController: &istio.DestinationRuleController{ + IstioClient: fakeIstioClient, + }, + NodeController: &admiral.NodeController{ + Locality: &admiral.Locality{ + Region: "us-west-2", + }, + }, + DeploymentController: d, + } + + rr.remoteControllers["test.cluster"] = rc + createServiceEntryForNewServiceOrPod("test", "bar", rr) + +} + +func TestGetLocalAddressForSe(t *testing.T) { + t.Parallel() + cacheWithEntry := ServiceEntryAddressStore{ + EntryAddresses: map[string]string{"e2e.a.mesh": common.LocalAddressPrefix + ".10.1"}, + Addresses: []string{common.LocalAddressPrefix + ".10.1"}, + } + cacheWithNoEntry := ServiceEntryAddressStore{ + EntryAddresses: map[string]string{}, + Addresses: []string{}, + } + cacheWith255Entries := ServiceEntryAddressStore{ + EntryAddresses: map[string]string{}, + Addresses: []string{}, + } + + for i := 1; i <= 255; i++ { + address := common.LocalAddressPrefix + ".10." + strconv.Itoa(i) + cacheWith255Entries.EntryAddresses[strconv.Itoa(i) + ".mesh"] = address + cacheWith255Entries.Addresses = append(cacheWith255Entries.Addresses, address) + } + + emptyCacheController := test.FakeConfigMapController{ + GetError: nil, + PutError: nil, + ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithNoEntry, "123"), + } + + cacheController := test.FakeConfigMapController{ + GetError: nil, + PutError: nil, + ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithEntry, "123"), + } + + cacheControllerWith255Entries := test.FakeConfigMapController{ + GetError: nil, + PutError: nil, + ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWith255Entries, "123"), + } + + cacheControllerGetError := test.FakeConfigMapController{ + GetError: errors.New("BAD THINGS HAPPENED"), + PutError: nil, + ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithEntry, "123"), + } + + cacheControllerPutError := test.FakeConfigMapController{ + PutError: errors.New("BAD THINGS HAPPENED"), + GetError: nil, + ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithEntry, "123"), + } + + + testCases := []struct { + name string + seName string + seAddressCache ServiceEntryAddressStore + wantAddess string + cacheController admiral.ConfigMapControllerInterface + expectedCacheUpdate bool + wantedError error + }{ + { + name: "should return new available address", + seName: "e2e.a.mesh", + seAddressCache: cacheWithNoEntry, + wantAddess: common.LocalAddressPrefix + ".10.1", + cacheController: &emptyCacheController, + expectedCacheUpdate: true, + wantedError: nil, + }, + { + name: "should return address from map", + seName: "e2e.a.mesh", + seAddressCache: cacheWithEntry, + wantAddess: common.LocalAddressPrefix + ".10.1", + cacheController: &cacheController, + expectedCacheUpdate: false, + wantedError: nil, + }, + { + name: "should return new available address", + seName: "e2e.b.mesh", + seAddressCache: cacheWithEntry, + wantAddess: common.LocalAddressPrefix + ".10.2", + cacheController: &cacheController, + expectedCacheUpdate: true, + wantedError: nil, + }, + { + name: "should return new available address in higher subnet", + seName: "e2e.a.mesh", + seAddressCache: cacheWith255Entries, + wantAddess: common.LocalAddressPrefix + ".11.1", + cacheController: &cacheControllerWith255Entries, + expectedCacheUpdate: true, + wantedError: nil, + }, + { + name: "should gracefully propagate get error", + seName: "e2e.a.mesh", + seAddressCache: cacheWith255Entries, + wantAddess: "", + cacheController: &cacheControllerGetError, + expectedCacheUpdate: true, + wantedError: errors.New("BAD THINGS HAPPENED"), + }, + { + name: "Should not return address on put error", + seName: "e2e.abcdefghijklmnop.mesh", + seAddressCache: cacheWith255Entries, + wantAddess: "", + cacheController: &cacheControllerPutError, + expectedCacheUpdate: true, + wantedError: errors.New("BAD THINGS HAPPENED"), + }, + } + + for _, c := range testCases { + t.Run(c.name, func(t *testing.T) { + seAddress, needsCacheUpdate, err := GetLocalAddressForSe(c.seName, &c.seAddressCache, c.cacheController) + if c.wantAddess != "" { + if !reflect.DeepEqual(seAddress, c.wantAddess) { + t.Errorf("Wanted se address: %s, got: %s", c.wantAddess, seAddress) + } + if err==nil && c.wantedError==nil { + //we're fine + } else if err.Error() != c.wantedError.Error() { + t.Errorf("Error mismatch. Expected %v but got %v", c.wantedError, err) + } + if needsCacheUpdate != c.expectedCacheUpdate { + t.Errorf("Expected %v, got %v for needs cache update", c.expectedCacheUpdate, needsCacheUpdate) + } + } else { + if seAddress != "" { + t.Errorf("Unexpectedly found address: %s", seAddress) + } + } + }) + } + +} + +func TestMakeRemoteEndpointForServiceEntry(t *testing.T) { + address := "1.2.3.4" + locality := "us-west-2" + portName := "port" + + endpoint := makeRemoteEndpointForServiceEntry(address, locality, portName) + + if endpoint.Address != address { + t.Errorf("Address mismatch. Got: %v, expected: %v", endpoint.Address, address) + } + if endpoint.Locality != locality { + t.Errorf("Locality mismatch. Got: %v, expected: %v", endpoint.Locality, locality) + } + if endpoint.Ports[portName] != 15443 { + t.Errorf("Incorrect port found") + } +} + func buildFakeConfigMapFromAddressStore(addressStore *ServiceEntryAddressStore, resourceVersion string) *v1.ConfigMap { bytes, _ := yaml.Marshal(addressStore) diff --git a/admiral/pkg/controller/istio/destinationrule.go b/admiral/pkg/controller/istio/destinationrule.go index 457aaddd..44209630 100644 --- a/admiral/pkg/controller/istio/destinationrule.go +++ b/admiral/pkg/controller/istio/destinationrule.go @@ -26,7 +26,7 @@ type DestinationRuleEntry struct { } type DestinationRuleController struct { - IstioClient *versioned.Clientset + IstioClient versioned.Interface DestinationRuleHandler DestinationRuleHandler informer cache.SharedIndexInformer ctl *admiral.Controller diff --git a/admiral/pkg/controller/istio/serviceentry.go b/admiral/pkg/controller/istio/serviceentry.go index db769dbb..d1442023 100644 --- a/admiral/pkg/controller/istio/serviceentry.go +++ b/admiral/pkg/controller/istio/serviceentry.go @@ -26,7 +26,7 @@ type ServiceEntryEntry struct { } type ServiceEntryController struct { - IstioClient *versioned.Clientset + IstioClient versioned.Interface ServiceEntryHandler ServiceEntryHandler informer cache.SharedIndexInformer ctl *admiral.Controller From 94736741779718726d4660ef8b2f5ff396b27150 Mon Sep 17 00:00:00 2001 From: Joe Peacock Date: Tue, 31 Mar 2020 17:02:01 -0700 Subject: [PATCH 5/5] Uncommented/fixed more se tests Signed-off-by: Joe Peacock --- admiral/pkg/clusters/serviceentry_test.go | 140 ++++++++++++---------- 1 file changed, 79 insertions(+), 61 deletions(-) diff --git a/admiral/pkg/clusters/serviceentry_test.go b/admiral/pkg/clusters/serviceentry_test.go index 77f5dbec..46cac1a6 100644 --- a/admiral/pkg/clusters/serviceentry_test.go +++ b/admiral/pkg/clusters/serviceentry_test.go @@ -390,64 +390,82 @@ func TestModifyExistingSidecarForLocalClusterCommunication(t *testing.T) { } } -// -//func TestCreateServiceEntry(t *testing.T) { -// admiralCache := AdmiralCache{} -// -// localAddress := common.LocalAddressPrefix + ".10.1" -// -// cnameIdentityCache := sync.Map{} -// cnameIdentityCache.Store("dev.bar.global", "bar") -// admiralCache.CnameIdentityCache = &cnameIdentityCache -// -// admiralCache.ServiceEntryAddressStore = &ServiceEntryAddressStore{ -// EntryAddresses: map[string]string{"e2e.my-first-service.mesh-se":localAddress}, -// Addresses: []string{localAddress}, -// } -// -// admiralCache.CnameClusterCache = common.NewMapOfMaps() -// -// rc, _ := createMockRemoteController(func(i interface{}) { -// res := i.(istio.Config) -// se, ok := res.Spec.(*networking.ServiceEntry) -// if ok { -// if se.Hosts[0] != "dev.bar.global" { -// t.Errorf("Host mismatch. Expected dev.bar.global, got %v", se.Hosts[0]) -// } -// } -// }) -// -// params := AdmiralParams{ -// EnableSAN: true, -// SANPrefix: "prefix", -// LabelSet:&common.LabelSet{WorkloadIdentityLabel:"identity"}, -// HostnameSuffix: "mesh", -// } -// -// cacheWithEntry := ServiceEntryAddressStore{ -// EntryAddresses: map[string]string{"e2e.my-first-service.mesh": localAddress}, -// Addresses: []string{localAddress}, -// } -// -// cacheController := &test.FakeConfigMapController{ -// GetError: nil, -// PutError: nil, -// ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithEntry, "123"), -// } -// -// admiralCache.ConfigMapController = cacheController -// -// deployment := v12.Deployment{} -// deployment.Spec.Template.Labels = map[string]string{"env":"e2e", "identity":"my-first-service", } -// -// resultingEntry := createServiceEntry(rc, params, &admiralCache, &deployment, map[string]*networking.ServiceEntry{}) -// -// if resultingEntry.Hosts[0] != "e2e.my-first-service.mesh" { -// t.Errorf("Host mismatch. Got: %v, expected: e2e.my-first-service.mesh", resultingEntry.Hosts[0]) -// } -// -// if resultingEntry.Addresses[0] != localAddress { -// t.Errorf("Address mismatch. Got: %v, expected: " + localAddress, resultingEntry.Addresses[0]) -// } -// -//} + +func TestCreateServiceEntry(t *testing.T) { + + config := rest.Config{ + Host: "localhost", + } + stop := make(chan struct{}) + s, e := admiral.NewServiceController(stop, &test.MockServiceHandler{}, &config, time.Second*time.Duration(300)) + + if e != nil { + t.Fatalf("%v", e) + } + + admiralCache := AdmiralCache{} + + localAddress := common.LocalAddressPrefix + ".10.1" + + cnameIdentityCache := sync.Map{} + cnameIdentityCache.Store("dev.bar.global", "bar") + admiralCache.CnameIdentityCache = &cnameIdentityCache + + admiralCache.ServiceEntryAddressStore = &ServiceEntryAddressStore{ + EntryAddresses: map[string]string{"e2e.my-first-service.mesh-se":localAddress}, + Addresses: []string{localAddress}, + } + + admiralCache.CnameClusterCache = common.NewMapOfMaps() + + fakeIstioClient := istiofake.NewSimpleClientset() + rc := &RemoteController{ + ServiceEntryController: &istio.ServiceEntryController{ + IstioClient: fakeIstioClient, + }, + DestinationRuleController: &istio.DestinationRuleController{ + IstioClient: fakeIstioClient, + }, + NodeController: &admiral.NodeController{ + Locality: &admiral.Locality{ + Region: "us-west-2", + }, + }, + ServiceController: s, + } + + cacheWithEntry := ServiceEntryAddressStore{ + EntryAddresses: map[string]string{"e2e.my-first-service.mesh": localAddress}, + Addresses: []string{localAddress}, + } + + cacheController := &test.FakeConfigMapController{ + GetError: nil, + PutError: nil, + ConfigmapToReturn: buildFakeConfigMapFromAddressStore(&cacheWithEntry, "123"), + } + + admiralCache.ConfigMapController = cacheController + + deployment := v14.Deployment{} + deployment.Spec.Template.Labels = map[string]string{"env":"e2e", "identity":"my-first-service", } + + resultingEntry := createServiceEntry(rc, &admiralCache, &deployment, map[string]*istionetworkingv1alpha3.ServiceEntry{}) + + if resultingEntry.Hosts[0] != "e2e.my-first-service.mesh" { + t.Errorf("Host mismatch. Got: %v, expected: e2e.my-first-service.mesh", resultingEntry.Hosts[0]) + } + + if resultingEntry.Addresses[0] != localAddress { + t.Errorf("Address mismatch. Got: %v, expected: " + localAddress, resultingEntry.Addresses[0]) + } + + if resultingEntry.Endpoints[0].Address != "admiral_dummy.com" { + t.Errorf("Endpoint mismatch. Got %v, expected: %v", resultingEntry.Endpoints[0].Address, "admiral_dummy.com") + } + + if resultingEntry.Endpoints[0].Locality != "us-west-2" { + t.Errorf("Locality mismatch. Got %v, expected: %v", resultingEntry.Endpoints[0].Locality, "us-west-2") + } + +}