From d11568d539c1b6e8340b8971d9c43ff0d9a8e4e1 Mon Sep 17 00:00:00 2001 From: aattuluri Date: Mon, 4 Jul 2022 22:57:49 -0700 Subject: [PATCH] MESH-1970 Handle k8s service events and trigger SE generation (#238) (#136) --- admiral/pkg/clusters/handler.go | 27 ++---- admiral/pkg/clusters/handler_test.go | 81 +++++++++--------- admiral/pkg/clusters/registry.go | 66 +++++++-------- admiral/pkg/clusters/types.go | 48 ++++++++++- admiral/pkg/controller/admiral/controller.go | 3 +- admiral/pkg/controller/admiral/deployment.go | 63 +++----------- .../pkg/controller/admiral/deployment_test.go | 84 ++++--------------- admiral/pkg/controller/admiral/rollouts.go | 33 ++++---- .../pkg/controller/admiral/rollouts_test.go | 24 ++++-- admiral/pkg/controller/admiral/service.go | 46 +++++++--- .../pkg/controller/admiral/service_test.go | 61 ++++++++++++-- admiral/pkg/controller/common/common.go | 1 + admiral/pkg/test/mock.go | 4 + 13 files changed, 283 insertions(+), 258 deletions(-) diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go index 24503581..6e707c95 100644 --- a/admiral/pkg/clusters/handler.go +++ b/admiral/pkg/clusters/handler.go @@ -5,7 +5,6 @@ import ( "fmt" "net" "reflect" - "sort" "strings" "time" @@ -25,7 +24,6 @@ import ( ) const ( - ROLLOUT_POD_HASH_LABEL string = "rollouts-pod-template-hash" DefaultBaseEjectionTime int64 = 300 DefaultConsecutiveGatewayErrors uint32 = 50 DefaultInterval int64 = 60 @@ -774,13 +772,13 @@ func getServiceForDeployment(rc *RemoteController, deployment *k8sAppsV1.Deploym return nil } - cachedService := rc.ServiceController.Cache.Get(deployment.Namespace) + cachedServices := rc.ServiceController.Cache.Get(deployment.Namespace) - if cachedService == nil { + if cachedServices == nil { return nil } var matchedService *k8sV1.Service - for _, service := range cachedService.Service[deployment.Namespace] { + for _, service := range cachedServices { var match = true for lkey, lvalue := range service.Spec.Selector { value, ok := deployment.Spec.Selector.MatchLabels[lkey] @@ -846,9 +844,9 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin return nil } - cachedService := rc.ServiceController.Cache.Get(rollout.Namespace) + cachedServices := rc.ServiceController.Cache.Get(rollout.Namespace) - if cachedService == nil { + if cachedServices == nil { return nil } rolloutStrategy := rollout.Spec.Strategy @@ -926,18 +924,7 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin var matchedServices = make(map[string]*WeightedService) - //if we have more than one matching service we will pick the first one, for this to be deterministic we sort services - var servicesInNamespace = cachedService.Service[rollout.Namespace] - - servicesOrdered := make([]string, 0, len(servicesInNamespace)) - for k := range servicesInNamespace { - servicesOrdered = append(servicesOrdered, k) - } - - sort.Strings(servicesOrdered) - - for _, s := range servicesOrdered { - var service = servicesInNamespace[s] + for _, service := range cachedServices { var match = true //skip services that are not referenced in the rollout if len(blueGreenActiveService) > 0 && service.ObjectMeta.Name != blueGreenActiveService && service.ObjectMeta.Name != blueGreenPreviewService { @@ -948,7 +935,7 @@ func getServiceForRollout(rc *RemoteController, rollout *argo.Rollout) map[strin for lkey, lvalue := range service.Spec.Selector { // Rollouts controller adds a dynamic label with name rollouts-pod-template-hash to both active and passive replicasets. // This dynamic label is not available on the rollout template. Hence ignoring the label with name rollouts-pod-template-hash - if lkey == ROLLOUT_POD_HASH_LABEL { + if lkey == common.RolloutPodHashLabel { continue } value, ok := rollout.Spec.Selector.MatchLabels[lkey] diff --git a/admiral/pkg/clusters/handler_test.go b/admiral/pkg/clusters/handler_test.go index dca4109e..058ca239 100644 --- a/admiral/pkg/clusters/handler_test.go +++ b/admiral/pkg/clusters/handler_test.go @@ -658,10 +658,10 @@ func TestHandleVirtualServiceEvent(t *testing.T) { func TestGetServiceForRolloutCanary(t *testing.T) { //Struct of test case info. Name is required. - const NAMESPACE = "namespace" - const SERVICENAME = "serviceName" - const STABLESERVICENAME = "stableserviceName" - const CANARYSERVICENAME = "canaryserviceName" + const Namespace = "namespace" + const ServiceName = "serviceName" + const StableServiceName = "stableserviceName" + const CanaryServiceName = "canaryserviceName" const VS_NAME_1 = "virtualservice1" const VS_NAME_2 = "virtualservice2" const VS_NAME_3 = "virtualservice3" @@ -694,12 +694,11 @@ func TestGetServiceForRolloutCanary(t *testing.T) { selectorMap["app"] = "test" service := &coreV1.Service{ + ObjectMeta: v12.ObjectMeta{Name: ServiceName, Namespace: Namespace, CreationTimestamp: v12.NewTime(time.Now())}, Spec: coreV1.ServiceSpec{ Selector: selectorMap, }, } - service.Name = SERVICENAME - service.Namespace = NAMESPACE port1 := coreV1.ServicePort{ Port: 8080, } @@ -712,7 +711,7 @@ func TestGetServiceForRolloutCanary(t *testing.T) { service.Spec.Ports = ports stableService := &coreV1.Service{ - ObjectMeta: v12.ObjectMeta{Name: STABLESERVICENAME, Namespace: NAMESPACE}, + ObjectMeta: v12.ObjectMeta{Name: StableServiceName, Namespace: Namespace, CreationTimestamp: v12.NewTime(time.Now().Add(time.Duration(-15)))}, Spec: coreV1.ServiceSpec{ Selector: selectorMap, Ports: ports, @@ -720,7 +719,7 @@ func TestGetServiceForRolloutCanary(t *testing.T) { } canaryService := &coreV1.Service{ - ObjectMeta: v12.ObjectMeta{Name: CANARYSERVICENAME, Namespace: NAMESPACE}, + ObjectMeta: v12.ObjectMeta{Name: CanaryServiceName, Namespace: Namespace, CreationTimestamp: v12.NewTime(time.Now().Add(time.Duration(-15)))}, Spec: coreV1.ServiceSpec{ Selector: selectorMap, Ports: ports, @@ -778,38 +777,38 @@ func TestGetServiceForRolloutCanary(t *testing.T) { rcTemp.ServiceController.Cache.Put(canaryService) virtualService := &v1alpha32.VirtualService{ - ObjectMeta: v12.ObjectMeta{Name: VS_NAME_1, Namespace: NAMESPACE}, + ObjectMeta: v12.ObjectMeta{Name: VS_NAME_1, Namespace: Namespace}, Spec: v1alpha3.VirtualService{ Http: []*v1alpha3.HTTPRoute{{Route: []*v1alpha3.HTTPRouteDestination{ - {Destination: &v1alpha3.Destination{Host: STABLESERVICENAME}, Weight: 80}, - {Destination: &v1alpha3.Destination{Host: CANARYSERVICENAME}, Weight: 20}, + {Destination: &v1alpha3.Destination{Host: StableServiceName}, Weight: 80}, + {Destination: &v1alpha3.Destination{Host: CanaryServiceName}, Weight: 20}, }}}, }, } vsMutipleRoutesWithMatch := &v1alpha32.VirtualService{ - ObjectMeta: v12.ObjectMeta{Name: VS_NAME_2, Namespace: NAMESPACE}, + ObjectMeta: v12.ObjectMeta{Name: VS_NAME_2, Namespace: Namespace}, Spec: v1alpha3.VirtualService{ Http: []*v1alpha3.HTTPRoute{{Name: VS_ROUTE_PRIMARY, Route: []*v1alpha3.HTTPRouteDestination{ - {Destination: &v1alpha3.Destination{Host: STABLESERVICENAME}, Weight: 80}, - {Destination: &v1alpha3.Destination{Host: CANARYSERVICENAME}, Weight: 20}, + {Destination: &v1alpha3.Destination{Host: StableServiceName}, Weight: 80}, + {Destination: &v1alpha3.Destination{Host: CanaryServiceName}, Weight: 20}, }}}, }, } vsMutipleRoutesWithZeroWeight := &v1alpha32.VirtualService{ - ObjectMeta: v12.ObjectMeta{Name: VS_NAME_4, Namespace: NAMESPACE}, + ObjectMeta: v12.ObjectMeta{Name: VS_NAME_4, Namespace: Namespace}, Spec: v1alpha3.VirtualService{ Http: []*v1alpha3.HTTPRoute{{Name: "random", Route: []*v1alpha3.HTTPRouteDestination{ - {Destination: &v1alpha3.Destination{Host: STABLESERVICENAME}, Weight: 100}, - {Destination: &v1alpha3.Destination{Host: CANARYSERVICENAME}, Weight: 0}, + {Destination: &v1alpha3.Destination{Host: StableServiceName}, Weight: 100}, + {Destination: &v1alpha3.Destination{Host: CanaryServiceName}, Weight: 0}, }}}, }, } - rcTemp.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(NAMESPACE).Create(virtualService) - rcTemp.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(NAMESPACE).Create(vsMutipleRoutesWithMatch) - rcTemp.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(NAMESPACE).Create(vsMutipleRoutesWithZeroWeight) + rcTemp.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(Namespace).Create(virtualService) + rcTemp.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(Namespace).Create(vsMutipleRoutesWithMatch) + rcTemp.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(Namespace).Create(vsMutipleRoutesWithZeroWeight) canaryRollout := argo.Rollout{ Spec: argo.RolloutSpec{Template: coreV1.PodTemplateSpec{ @@ -823,7 +822,7 @@ func TestGetServiceForRolloutCanary(t *testing.T) { } canaryRollout.Spec.Selector = &labelSelector - canaryRollout.Namespace = NAMESPACE + canaryRollout.Namespace = Namespace canaryRollout.Spec.Strategy = argo.RolloutStrategy{ Canary: &argo.CanaryStrategy{}, } @@ -873,11 +872,11 @@ func TestGetServiceForRolloutCanary(t *testing.T) { }}} canaryRolloutIstioVs.Spec.Selector = &labelSelector - canaryRolloutIstioVs.Namespace = NAMESPACE + canaryRolloutIstioVs.Namespace = Namespace canaryRolloutIstioVs.Spec.Strategy = argo.RolloutStrategy{ Canary: &argo.CanaryStrategy{ - StableService: STABLESERVICENAME, - CanaryService: CANARYSERVICENAME, + StableService: StableServiceName, + CanaryService: CanaryServiceName, TrafficRouting: &argo.RolloutTrafficRouting{ Istio: &argo.IstioTrafficRouting{ VirtualService: argo.IstioVirtualService{Name: VS_NAME_1}, @@ -892,11 +891,11 @@ func TestGetServiceForRolloutCanary(t *testing.T) { }}} canaryRolloutIstioVsRouteMatch.Spec.Selector = &labelSelector - canaryRolloutIstioVsRouteMatch.Namespace = NAMESPACE + canaryRolloutIstioVsRouteMatch.Namespace = Namespace canaryRolloutIstioVsRouteMatch.Spec.Strategy = argo.RolloutStrategy{ Canary: &argo.CanaryStrategy{ - StableService: STABLESERVICENAME, - CanaryService: CANARYSERVICENAME, + StableService: StableServiceName, + CanaryService: CanaryServiceName, TrafficRouting: &argo.RolloutTrafficRouting{ Istio: &argo.IstioTrafficRouting{ VirtualService: argo.IstioVirtualService{Name: VS_NAME_2, Routes: []string{VS_ROUTE_PRIMARY}}, @@ -911,11 +910,11 @@ func TestGetServiceForRolloutCanary(t *testing.T) { }}} canaryRolloutIstioVsRouteMisMatch.Spec.Selector = &labelSelector - canaryRolloutIstioVsRouteMisMatch.Namespace = NAMESPACE + canaryRolloutIstioVsRouteMisMatch.Namespace = Namespace canaryRolloutIstioVsRouteMisMatch.Spec.Strategy = argo.RolloutStrategy{ Canary: &argo.CanaryStrategy{ - StableService: STABLESERVICENAME, - CanaryService: CANARYSERVICENAME, + StableService: StableServiceName, + CanaryService: CanaryServiceName, TrafficRouting: &argo.RolloutTrafficRouting{ Istio: &argo.IstioTrafficRouting{ VirtualService: argo.IstioVirtualService{Name: VS_NAME_3, Routes: []string{"random"}}, @@ -930,11 +929,11 @@ func TestGetServiceForRolloutCanary(t *testing.T) { }}} canaryRolloutIstioVsZeroWeight.Spec.Selector = &labelSelector - canaryRolloutIstioVsZeroWeight.Namespace = NAMESPACE + canaryRolloutIstioVsZeroWeight.Namespace = Namespace canaryRolloutIstioVsZeroWeight.Spec.Strategy = argo.RolloutStrategy{ Canary: &argo.CanaryStrategy{ - StableService: STABLESERVICENAME, - CanaryService: CANARYSERVICENAME, + StableService: StableServiceName, + CanaryService: CanaryServiceName, TrafficRouting: &argo.RolloutTrafficRouting{ Istio: &argo.IstioTrafficRouting{ VirtualService: argo.IstioVirtualService{Name: VS_NAME_4}, @@ -949,11 +948,11 @@ func TestGetServiceForRolloutCanary(t *testing.T) { }}} canaryRolloutIstioVsMimatch.Spec.Selector = &labelSelector - canaryRolloutIstioVsMimatch.Namespace = NAMESPACE + canaryRolloutIstioVsMimatch.Namespace = Namespace canaryRolloutIstioVsMimatch.Spec.Strategy = argo.RolloutStrategy{ Canary: &argo.CanaryStrategy{ - StableService: STABLESERVICENAME, - CanaryService: CANARYSERVICENAME, + StableService: StableServiceName, + CanaryService: CanaryServiceName, TrafficRouting: &argo.RolloutTrafficRouting{ Istio: &argo.IstioTrafficRouting{ VirtualService: argo.IstioVirtualService{Name: "random"}, @@ -964,14 +963,14 @@ func TestGetServiceForRolloutCanary(t *testing.T) { resultForDummy := map[string]*WeightedService{"dummy": {Weight: 1, Service: service1}} - resultForRandomMatch := map[string]*WeightedService{CANARYSERVICENAME: {Weight: 1, Service: canaryService}} + resultForRandomMatch := map[string]*WeightedService{CanaryServiceName: {Weight: 1, Service: canaryService}} - resultForStableServiceOnly := map[string]*WeightedService{STABLESERVICENAME: {Weight: 1, Service: stableService}} + resultForStableServiceOnly := map[string]*WeightedService{StableServiceName: {Weight: 1, Service: stableService}} - resultForCanaryWithIstio := map[string]*WeightedService{STABLESERVICENAME: {Weight: 80, Service: stableService}, - CANARYSERVICENAME: {Weight: 20, Service: canaryService}} + resultForCanaryWithIstio := map[string]*WeightedService{StableServiceName: {Weight: 80, Service: stableService}, + CanaryServiceName: {Weight: 20, Service: canaryService}} - resultForCanaryWithStableService := map[string]*WeightedService{STABLESERVICENAME: {Weight: 100, Service: stableService}} + resultForCanaryWithStableService := map[string]*WeightedService{StableServiceName: {Weight: 100, Service: stableService}} testCases := []struct { name string diff --git a/admiral/pkg/clusters/registry.go b/admiral/pkg/clusters/registry.go index 94517525..01a07ab2 100644 --- a/admiral/pkg/clusters/registry.go +++ b/admiral/pkg/clusters/registry.go @@ -21,7 +21,6 @@ const ( LogErrFormat = "op=%s type=%v name=%v cluster=%s, e=%v" ) - func InitAdmiral(ctx context.Context, params common.AdmiralParams) (*RemoteRegistry, error) { log.Infof("Initializing Admiral with params: %v", params) @@ -127,71 +126,72 @@ func (r *RemoteRegistry) createCacheController(clientConfig *rest.Config, cluste var err error - log.Infof("starting global traffic policy controller custerID: %v", clusterID) - - rc.GlobalTraffic, err = admiral.NewGlobalTrafficController(stop, &GlobalTrafficHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) + log.Infof("starting service controller clusterID: %v", clusterID) + rc.ServiceController, err = admiral.NewServiceController(stop, &ServiceHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, 0) if err != nil { - return fmt.Errorf(" Error with GlobalTrafficController controller init: %v", err) + return fmt.Errorf("error with ServiceController controller init: %v", err) } - log.Infof("starting deployment controller clusterID: %v", clusterID) - rc.DeploymentController, err = admiral.NewDeploymentController(stop, &DeploymentHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) + log.Infof("starting global traffic policy controller custerID: %v", clusterID) + + rc.GlobalTraffic, err = admiral.NewGlobalTrafficController(stop, &GlobalTrafficHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, 0) if err != nil { - return fmt.Errorf(" Error with DeploymentController controller init: %v", err) + return fmt.Errorf("error with GlobalTrafficController controller init: %v", err) } - if r.AdmiralCache == nil { - log.Warn("admiral cache was nil!") - } else if r.AdmiralCache.argoRolloutsEnabled { - log.Infof("starting rollout controller clusterID: %v", clusterID) - rc.RolloutController, err = admiral.NewRolloutsController(stop, &RolloutHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) - - if err != nil { - return fmt.Errorf(" Error with Rollout controller init: %v", err) - } - } log.Infof("starting node controller clusterID: %v", clusterID) rc.NodeController, err = admiral.NewNodeController(stop, &NodeHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig) if err != nil { - return fmt.Errorf(" Error with NodeController controller init: %v", err) + return fmt.Errorf("error with NodeController controller init: %v", err) } - log.Infof("starting service controller clusterID: %v", clusterID) - rc.ServiceController, err = admiral.NewServiceController(stop, &ServiceHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) + log.Infof("starting service entry controller for custerID: %v", clusterID) + rc.ServiceEntryController, err = istio.NewServiceEntryController(stop, &ServiceEntryHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, 0) if err != nil { - return fmt.Errorf(" Error with ServiceController controller init: %v", err) + return fmt.Errorf("error with ServiceEntryController init: %v", err) } - log.Infof("starting service entry controller for custerID: %v", clusterID) - rc.ServiceEntryController, err = istio.NewServiceEntryController(stop, &ServiceEntryHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) + log.Infof("starting destination rule controller for custerID: %v", clusterID) + rc.DestinationRuleController, err = istio.NewDestinationRuleController(stop, &DestinationRuleHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, 0) if err != nil { - return fmt.Errorf(" Error with ServiceEntryController init: %v", err) + return fmt.Errorf("error with DestinationRuleController init: %v", err) } - log.Infof("starting destination rule controller for custerID: %v", clusterID) - rc.DestinationRuleController, err = istio.NewDestinationRuleController(stop, &DestinationRuleHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) + log.Infof("starting virtual service controller for custerID: %v", clusterID) + rc.VirtualServiceController, err = istio.NewVirtualServiceController(stop, &VirtualServiceHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, 0) if err != nil { - return fmt.Errorf(" Error with DestinationRuleController init: %v", err) + return fmt.Errorf("error with VirtualServiceController init: %v", err) } - log.Infof("starting virtual service controller for custerID: %v", clusterID) - rc.VirtualServiceController, err = istio.NewVirtualServiceController(stop, &VirtualServiceHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) + rc.SidecarController, err = istio.NewSidecarController(stop, &SidecarHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, 0) if err != nil { - return fmt.Errorf(" Error with VirtualServiceController init: %v", err) + return fmt.Errorf("error with DestinationRuleController init: %v", err) } - rc.SidecarController, err = istio.NewSidecarController(stop, &SidecarHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) + log.Infof("starting deployment controller clusterID: %v", clusterID) + rc.DeploymentController, err = admiral.NewDeploymentController(stop, &DeploymentHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) if err != nil { - return fmt.Errorf(" Error with DestinationRuleController init: %v", err) + return fmt.Errorf("error with DeploymentController controller init: %v", err) + } + + if r.AdmiralCache == nil { + log.Warn("admiral cache was nil!") + } else if r.AdmiralCache.argoRolloutsEnabled { + log.Infof("starting rollout controller clusterID: %v", clusterID) + rc.RolloutController, err = admiral.NewRolloutsController(stop, &RolloutHandler{RemoteRegistry: r, ClusterID: clusterID}, clientConfig, resyncPeriod) + + if err != nil { + return fmt.Errorf("error with Rollout controller init: %v", err) + } } r.Lock() diff --git a/admiral/pkg/clusters/types.go b/admiral/pkg/clusters/types.go index c0693ca6..a874b0c1 100644 --- a/admiral/pkg/clusters/types.go +++ b/admiral/pkg/clusters/types.go @@ -15,6 +15,7 @@ import ( "github.com/istio-ecosystem/admiral/admiral/pkg/controller/secret" log "github.com/sirupsen/logrus" k8sAppsV1 "k8s.io/api/apps/v1" + k8sV1 "k8s.io/api/core/v1" k8s "k8s.io/client-go/kubernetes" ) @@ -155,6 +156,52 @@ type ServiceHandler struct { ClusterID string } +func (sh *ServiceHandler) Added(obj *k8sV1.Service) { + log.Infof(LogFormat, "Added", "service", obj.Name, sh.ClusterID, "received") + err := HandleEventForService(obj, sh.RemoteRegistry, sh.ClusterID) + if err != nil { + log.Errorf(LogErrFormat, "Error", "service", obj.Name, sh.ClusterID, err) + } +} + +func (sh *ServiceHandler) Updated(obj *k8sV1.Service) { + log.Infof(LogFormat, "Updated", "service", obj.Name, sh.ClusterID, "received") + err := HandleEventForService(obj, sh.RemoteRegistry, sh.ClusterID) + if err != nil { + log.Errorf(LogErrFormat, "Error", "service", obj.Name, sh.ClusterID, err) + } +} + +func (sh *ServiceHandler) Deleted(obj *k8sV1.Service) { + log.Infof(LogFormat, "Deleted", "service", obj.Name, sh.ClusterID, "received") + err := HandleEventForService(obj, sh.RemoteRegistry, sh.ClusterID) + if err != nil { + log.Errorf(LogErrFormat, "Error", "service", obj.Name, sh.ClusterID, err) + } +} + +func HandleEventForService(svc *k8sV1.Service, remoteRegistry *RemoteRegistry, clusterName string) error { + if svc.Spec.Selector == nil { + return errors.New("selector missing on service"); + } + matchingDeployements := remoteRegistry.RemoteControllers[clusterName].DeploymentController.GetDeploymentBySelectorInNamespace(svc.Spec.Selector, svc.Namespace) + if len(matchingDeployements) > 0 { + for _, deployment := range matchingDeployements { + HandleEventForDeployment(admiral.Update, &deployment, remoteRegistry, clusterName) + } + } + if common.GetAdmiralParams().ArgoRolloutsEnabled { + matchingRollouts := remoteRegistry.RemoteControllers[clusterName].RolloutController.GetRolloutBySelectorInNamespace(svc.Spec.Selector, svc.Namespace) + + if len(matchingRollouts) > 0 { + for _, rollout := range matchingRollouts { + HandleEventForRollout(admiral.Update, &rollout, remoteRegistry, clusterName) + } + } + } + return nil +} + func (dh *DependencyHandler) Added(obj *v1.Dependency) { log.Infof(LogFormat, "Add", "dependency-record", obj.Name, "", "Received=true namespace="+obj.Namespace) @@ -253,7 +300,6 @@ func HandleEventForRollout(event admiral.EventType, obj *argo.Rollout, remoteReg // helper function to handle add and delete for DeploymentHandler func HandleEventForDeployment(event admiral.EventType, obj *k8sAppsV1.Deployment, remoteRegistry *RemoteRegistry, clusterName string) { - log.Infof(LogFormat, event, "deployment", obj.Name, clusterName, "Received") globalIdentifier := common.GetDeploymentGlobalIdentifier(obj) diff --git a/admiral/pkg/controller/admiral/controller.go b/admiral/pkg/controller/admiral/controller.go index a6d32653..5275c106 100644 --- a/admiral/pkg/controller/admiral/controller.go +++ b/admiral/pkg/controller/admiral/controller.go @@ -100,7 +100,8 @@ func (c *Controller) Run(stopCh <-chan struct{}) { } log.Infof("Informer caches synced for controller=%v, current keys=%v", c.name, c.informer.GetStore().ListKeys()) - wait.Until(c.runWorker, 5*time.Second, stopCh) + //wait for 30 seconds for the first time (for all caches to sync) + wait.Until(c.runWorker, 30 * time.Second, stopCh) } func (c *Controller) runWorker() { diff --git a/admiral/pkg/controller/admiral/deployment.go b/admiral/pkg/controller/admiral/deployment.go index 20524607..02493966 100644 --- a/admiral/pkg/controller/admiral/deployment.go +++ b/admiral/pkg/controller/admiral/deployment.go @@ -6,6 +6,7 @@ import ( "github.com/istio-ecosystem/admiral/admiral/pkg/controller/util" "github.com/sirupsen/logrus" k8sAppsV1 "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/labels" k8sAppsinformers "k8s.io/client-go/informers/apps/v1" "k8s.io/client-go/rest" "time" @@ -79,45 +80,6 @@ func (p *deploymentCache) DeleteFromDeploymentClusterCache(key string, deploymen } } -func (d *DeploymentController) GetDeployments() ([]*k8sAppsV1.Deployment, error) { - - ns := d.K8sClient.CoreV1().Namespaces() - - namespaceSidecarInjectionLabelFilter := d.labelSet.NamespaceSidecarInjectionLabel + "=" + d.labelSet.NamespaceSidecarInjectionLabelValue - istioEnabledNs, err := ns.List(meta_v1.ListOptions{LabelSelector: namespaceSidecarInjectionLabelFilter}) - - if err != nil { - return nil, fmt.Errorf("error getting istio labled namespaces: %v", err) - } - - var res []*k8sAppsV1.Deployment - - for _, v := range istioEnabledNs.Items { - - deployments := d.K8sClient.AppsV1().Deployments(v.Name) - deploymentsList, err := deployments.List(meta_v1.ListOptions{}) - if err != nil { - return nil, fmt.Errorf("error listing deployments: %v", err) - } - var admiralDeployments []k8sAppsV1.Deployment - for _, deployment := range deploymentsList.Items { - if !d.shouldIgnoreBasedOnLabels(&deployment) { - admiralDeployments = append(admiralDeployments, deployment) - } - } - - if err != nil { - return nil, fmt.Errorf("error getting istio labled namespaces: %v", err) - } - - for _, pi := range admiralDeployments { - res = append(res, &pi) - } - } - - return res, nil -} - func NewDeploymentController(stopCh <-chan struct{}, handler DeploymentHandler, config *rest.Config, resyncPeriod time.Duration) (*DeploymentController, error) { deploymentController := DeploymentController{} @@ -212,27 +174,22 @@ func (d *DeploymentController) shouldIgnoreBasedOnLabels(deployment *k8sAppsV1.D return false //labels are fine, we should not ignore } -func (d *DeploymentController) GetDeploymentByLabel(labelValue string, namespace string) []k8sAppsV1.Deployment { - matchLabel := common.GetGlobalTrafficDeploymentLabel() - deploymentsInNamespace, err := d.K8sClient.AppsV1().Deployments(namespace).List(meta_v1.ListOptions{}) +func (d *DeploymentController) GetDeploymentBySelectorInNamespace(serviceSelector map[string]string, namespace string) []k8sAppsV1.Deployment { + + labelOptions := meta_v1.ListOptions{ + LabelSelector: labels.SelectorFromSet(serviceSelector).String(), + } + + matchedDeployments, err := d.K8sClient.AppsV1().Deployments(namespace).List(labelOptions) if err != nil { logrus.Errorf("Failed to list deployments in cluster, error: %v", err) return nil } - if deploymentsInNamespace.Items == nil { + if matchedDeployments.Items == nil { return []k8sAppsV1.Deployment{} } - var matchedDeployments = make([]k8sAppsV1.Deployment, 0) - - for _, deployment := range deploymentsInNamespace.Items { - if deployment.Spec.Template.Labels[matchLabel] == labelValue { - matchedDeployments = append(matchedDeployments, deployment) - log.Infof("Found matching GTP gtp with label=%s with value=%s for deployment=%s in namespace=%s", common.GetGlobalTrafficDeploymentLabel(), labelValue, deployment.Name, namespace) - } - } - - return matchedDeployments + return matchedDeployments.Items } diff --git a/admiral/pkg/controller/admiral/deployment_test.go b/admiral/pkg/controller/admiral/deployment_test.go index 79dc15f9..c3c5cb88 100644 --- a/admiral/pkg/controller/admiral/deployment_test.go +++ b/admiral/pkg/controller/admiral/deployment_test.go @@ -4,10 +4,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" "github.com/istio-ecosystem/admiral/admiral/pkg/test" - log "github.com/sirupsen/logrus" k8sAppsV1 "k8s.io/api/apps/v1" coreV1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/clientcmd" "sort" @@ -183,63 +182,6 @@ func TestDeploymentController_Deleted(t *testing.T) { } } -func TestDeploymentController_GetDeployments(t *testing.T) { - - depController := DeploymentController{ - labelSet: &common.LabelSet{ - DeploymentAnnotation: "sidecar.istio.io/inject", - NamespaceSidecarInjectionLabel: "istio-injection", - NamespaceSidecarInjectionLabelValue: "enabled", - AdmiralIgnoreLabel: "admiral-ignore", - }, - } - - client := fake.NewSimpleClientset() - - ns := coreV1.Namespace{} - ns.Labels = map[string]string{"istio-injection": "enabled"} - ns.Name = "test-ns" - - _, err := client.CoreV1().Namespaces().Create(&ns) - if err != nil { - t.Errorf("%v", err) - } - - deployment := k8sAppsV1.Deployment{} - deployment.Namespace = "test-ns" - deployment.Name = "deployment" - deployment.Spec.Template.Labels = map[string]string{"identity": "id", "istio-injected": "true"} - deployment.Spec.Template.Annotations = map[string]string{"sidecar.istio.io/inject": "true"} - deploymentWithBadLabels := k8sAppsV1.Deployment{} - deploymentWithBadLabels.Namespace = "test-ns" - deploymentWithBadLabels.Name = "deploymentWithBadLabels" - deploymentWithBadLabels.Spec.Template.Labels = map[string]string{"identity": "id", "random-label": "true"} - deploymentWithBadLabels.Spec.Template.Annotations = map[string]string{"woo": "yay"} - deploymentWithIgnoreLabels := k8sAppsV1.Deployment{} - deploymentWithIgnoreLabels.Namespace = "test-ns" - deploymentWithIgnoreLabels.Name = "deploymentWithIgnoreLabels" - deploymentWithIgnoreLabels.Spec.Template.Labels = map[string]string{"identity": "id", "istio-injected": "true", "admiral-ignore": "true"} - deploymentWithIgnoreLabels.Spec.Template.Annotations = map[string]string{"sidecar.istio.io/inject": "true"} - _, err = client.AppsV1().Deployments("test-ns").Create(&deployment) - _, err = client.AppsV1().Deployments("test-ns").Create(&deploymentWithBadLabels) - _, err = client.AppsV1().Deployments("test-ns").Create(&deploymentWithIgnoreLabels) - - if err != nil { - t.Errorf("%v", err) - } - - depController.K8sClient = client - resultingDeps, _ := depController.GetDeployments() - - if len(resultingDeps) != 1 { - t.Errorf("Get Deployments returned too many values. Expected 1, got %v", len(resultingDeps)) - } - if !cmp.Equal(resultingDeps[0], &deployment) { - log.Info("Object Diff: " + cmp.Diff(resultingDeps[0], &deployment)) - t.Errorf("Get Deployments returned the incorrect value. Got %v, expected %v", resultingDeps[0], deployment) - } -} - func TestNewDeploymentController(t *testing.T) { config, err := clientcmd.BuildConfigFromFlags("", "../../test/resources/admins@fake-cluster.k8s.local") if err != nil { @@ -255,52 +197,60 @@ func TestNewDeploymentController(t *testing.T) { } } -func TestDeploymentController_GetDeploymentByLabel(t *testing.T) { +func TestDeploymentController_GetDeploymentBySelectorInNamespace(t *testing.T) { deployment := k8sAppsV1.Deployment{} deployment.Namespace = "namespace" deployment.Name = "fake-app-deployment-qal" deployment.Spec = k8sAppsV1.DeploymentSpec{ + Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": "app1"},}, Template: coreV1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{"identity": "app1", "env": "qal"}, }, }, } + deployment.Labels = map[string]string{"identity": "app1"} deployment2 := k8sAppsV1.Deployment{} deployment2.Namespace = "namespace" deployment2.Name = "fake-app-deployment-e2e" deployment2.Spec = k8sAppsV1.DeploymentSpec{ + Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": "app1"},}, Template: coreV1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{"identity": "app1", "env": "e2e"}, }, }, } + deployment2.Labels = map[string]string{"identity": "app1"} deployment3 := k8sAppsV1.Deployment{} deployment3.Namespace = "namespace" deployment3.Name = "fake-app-deployment-prf-1" deployment3.CreationTimestamp = v1.Now() deployment3.Spec = k8sAppsV1.DeploymentSpec{ + Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": "app1"},}, Template: coreV1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{"identity": "app1", "env": "prf"}, }, }, } + deployment3.Labels = map[string]string{"identity": "app1"} deployment4 := k8sAppsV1.Deployment{} deployment4.Namespace = "namespace" deployment4.Name = "fake-app-deployment-prf-2" deployment4.CreationTimestamp = v1.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC) deployment4.Spec = k8sAppsV1.DeploymentSpec{ + Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": "app2"},}, Template: coreV1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{"identity": "app2", "env": "prf"}, }, }, } + deployment4.Labels = map[string]string{"identity": "app2"} oneDeploymentClient := fake.NewSimpleClientset(&deployment) @@ -315,37 +265,37 @@ func TestDeploymentController_GetDeploymentByLabel(t *testing.T) { name string expectedDeployments []k8sAppsV1.Deployment fakeClient *fake.Clientset - labelValue string + selector map[string]string }{ { name: "Get one", expectedDeployments: []k8sAppsV1.Deployment{deployment}, fakeClient: oneDeploymentClient, - labelValue: "app1", + selector: map[string]string{"identity": "app1"}, }, { name: "Get one from long list", expectedDeployments: []k8sAppsV1.Deployment{deployment4}, fakeClient: allDeploymentsClient, - labelValue: "app2", + selector: map[string]string{"identity": "app2"}, }, { name: "Get many from long list", expectedDeployments: []k8sAppsV1.Deployment{deployment, deployment3, deployment2}, fakeClient: allDeploymentsClient, - labelValue: "app1", + selector: map[string]string{"identity": "app1"}, }, { name: "Get none from long list", expectedDeployments: []k8sAppsV1.Deployment{}, fakeClient: allDeploymentsClient, - labelValue: "app3", + selector: map[string]string{"identity": "app3"}, }, { name: "Get none from empty list", expectedDeployments: []k8sAppsV1.Deployment{}, fakeClient: noDeploymentsClient, - labelValue: "app1", + selector: map[string]string{"identity": "app1"}, }, } @@ -353,7 +303,7 @@ func TestDeploymentController_GetDeploymentByLabel(t *testing.T) { for _, c := range testCases { t.Run(c.name, func(t *testing.T) { deploymentController.K8sClient = c.fakeClient - returnedDeployments := deploymentController.GetDeploymentByLabel(c.labelValue, "namespace") + returnedDeployments := deploymentController.GetDeploymentBySelectorInNamespace(c.selector, "namespace") sort.Slice(returnedDeployments, func(i, j int) bool { return returnedDeployments[i].Name > returnedDeployments[j].Name diff --git a/admiral/pkg/controller/admiral/rollouts.go b/admiral/pkg/controller/admiral/rollouts.go index e484b565..2be58bd7 100644 --- a/admiral/pkg/controller/admiral/rollouts.go +++ b/admiral/pkg/controller/admiral/rollouts.go @@ -2,6 +2,10 @@ package admiral import ( "fmt" + "k8s.io/apimachinery/pkg/labels" + "sync" + "time" + argo "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" argoclientset "github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned" argoprojv1alpha1 "github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/typed/rollouts/v1alpha1" @@ -14,8 +18,6 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" - "sync" - "time" ) // Handler interface contains the methods that are required @@ -200,27 +202,24 @@ func (roc *RolloutController) Deleted(ojb interface{}) { roc.RolloutHandler.Deleted(rollout) } -func (d *RolloutController) GetRolloutByLabel(labelValue string, namespace string) []argo.Rollout { - matchLabel := common.GetGlobalTrafficDeploymentLabel() - rolloutsInNamespace, err := d.RolloutClient.Rollouts(namespace).List(meta_v1.ListOptions{}) +func (d *RolloutController) GetRolloutBySelectorInNamespace(serviceSelector map[string]string, namespace string) []argo.Rollout { + + //Remove pod template hash from the service selector as that's not on the deployment + delete(serviceSelector, common.RolloutPodHashLabel) + + labelOptions := meta_v1.ListOptions{ + LabelSelector: labels.SelectorFromSet(serviceSelector).String(), + } + matchedRollouts, err := d.RolloutClient.Rollouts(namespace).List(labelOptions) if err != nil { logrus.Errorf("Failed to list rollouts in cluster, error: %v", err) return nil } - if rolloutsInNamespace.Items == nil { - return []argo.Rollout{} - } - - var matchedRollouts = make([]argo.Rollout, 0) - - for _, rollout := range rolloutsInNamespace.Items { - if rollout.Spec.Template.Labels[matchLabel] == labelValue { - matchedRollouts = append(matchedRollouts, rollout) - log.Infof("Found matching GTP gtp with label=%s with value=%s for rollout=%s in namespace=%s", common.GetGlobalTrafficDeploymentLabel(), labelValue, rollout.Name, namespace) - } + if matchedRollouts.Items == nil { + return make([]argo.Rollout,0) } - return matchedRollouts + return matchedRollouts.Items } diff --git a/admiral/pkg/controller/admiral/rollouts_test.go b/admiral/pkg/controller/admiral/rollouts_test.go index dad6e9d1..ffd024cc 100644 --- a/admiral/pkg/controller/admiral/rollouts_test.go +++ b/admiral/pkg/controller/admiral/rollouts_test.go @@ -195,52 +195,60 @@ func TestRolloutController_Deleted(t *testing.T) { } -func TestRolloutController_GetRolloutByLabel(t *testing.T) { +func TestRolloutController_GetRolloutBySelectorInNamespace(t *testing.T) { rollout := argo.Rollout{} rollout.Namespace = "namespace" rollout.Name = "fake-app-rollout-qal" rollout.Spec = argo.RolloutSpec{ + Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": "app1"},}, Template: coreV1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{"identity": "app1", "env": "qal"}, }, }, } + rollout.Labels = map[string]string{"identity": "app1"} rollout2 := argo.Rollout{} rollout2.Namespace = "namespace" rollout2.Name = "fake-app-rollout-e2e" rollout2.Spec = argo.RolloutSpec{ + Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": "app1"},}, Template: coreV1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{"identity": "app1", "env": "e2e"}, }, }, } + rollout2.Labels = map[string]string{"identity": "app1"} rollout3 := argo.Rollout{} rollout3.Namespace = "namespace" rollout3.Name = "fake-app-rollout-prf-1" rollout3.CreationTimestamp = v1.Now() rollout3.Spec = argo.RolloutSpec{ + Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": "app1"},}, Template: coreV1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{"identity": "app1", "env": "prf"}, }, }, } + rollout3.Labels = map[string]string{"identity": "app1"} rollout4 := argo.Rollout{} rollout4.Namespace = "namespace" rollout4.Name = "fake-app-rollout-prf-2" rollout4.CreationTimestamp = v1.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC) rollout4.Spec = argo.RolloutSpec{ + Selector: &v1.LabelSelector{MatchLabels: map[string]string{"identity": "app2"},}, Template: coreV1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{"identity": "app2", "env": "prf"}, }, }, } + rollout4.Labels = map[string]string{"identity": "app2"} oneRolloutClient := argofake.NewSimpleClientset(&rollout).ArgoprojV1alpha1() @@ -255,37 +263,37 @@ func TestRolloutController_GetRolloutByLabel(t *testing.T) { name string expectedRollouts []argo.Rollout fakeClient argoprojv1alpha1.ArgoprojV1alpha1Interface - labelValue string + selector map[string]string }{ { name: "Get one", expectedRollouts: []argo.Rollout{rollout}, fakeClient: oneRolloutClient, - labelValue: "app1", + selector: map[string]string {"identity": "app1", common.RolloutPodHashLabel: "random-hash"}, }, { name: "Get one from long list", expectedRollouts: []argo.Rollout{rollout4}, fakeClient: allRolloutsClient, - labelValue: "app2", + selector: map[string]string {"identity": "app2", common.RolloutPodHashLabel: "random-hash"}, }, { name: "Get many from long list", expectedRollouts: []argo.Rollout{rollout, rollout3, rollout2}, fakeClient: allRolloutsClient, - labelValue: "app1", + selector: map[string]string {"identity": "app1", common.RolloutPodHashLabel: "random-hash"}, }, { name: "Get none from long list", expectedRollouts: []argo.Rollout{}, fakeClient: allRolloutsClient, - labelValue: "app3", + selector: map[string]string {"identity": "app3", common.RolloutPodHashLabel: "random-hash"}, }, { name: "Get none from empty list", expectedRollouts: []argo.Rollout{}, fakeClient: noRolloutsClient, - labelValue: "app1", + selector: map[string]string {"identity": "app1"}, }, } @@ -293,7 +301,7 @@ func TestRolloutController_GetRolloutByLabel(t *testing.T) { for _, c := range testCases { t.Run(c.name, func(t *testing.T) { rolloutController.RolloutClient = c.fakeClient - returnedRollouts := rolloutController.GetRolloutByLabel(c.labelValue, "namespace") + returnedRollouts := rolloutController.GetRolloutBySelectorInNamespace(c.selector, "namespace") sort.Slice(returnedRollouts, func(i, j int) bool { return returnedRollouts[i].Name > returnedRollouts[j].Name diff --git a/admiral/pkg/controller/admiral/service.go b/admiral/pkg/controller/admiral/service.go index 2dc6a1e2..7c050640 100644 --- a/admiral/pkg/controller/admiral/service.go +++ b/admiral/pkg/controller/admiral/service.go @@ -2,6 +2,8 @@ package admiral import ( "fmt" + "github.com/prometheus/common/log" + "sort" "time" "github.com/istio-ecosystem/admiral/admiral/pkg/controller/common" @@ -19,6 +21,9 @@ import ( // Handler interface contains the methods that are required type ServiceHandler interface { + Added(obj *k8sV1.Service) + Updated(obj *k8sV1.Service) + Deleted(obj *k8sV1.Service) } type ServiceClusterEntry struct { @@ -70,10 +75,31 @@ func (s *serviceCache) getKey(service *k8sV1.Service) string { return service.Namespace } -func (s *serviceCache) Get(key string) *ServiceClusterEntry { +func (s *serviceCache) Get(key string) []*k8sV1.Service { s.mutex.Lock() defer s.mutex.Unlock() - return s.cache[key] + serviceClusterEntry := s.cache[key] + if serviceClusterEntry != nil { + return getOrderedServices (serviceClusterEntry.Service[key]) + } else { + return nil + } +} + +func getOrderedServices(serviceMap map[string]*k8sV1.Service) []*k8sV1.Service { + orderedServices := make([]*k8sV1.Service, 0, len(serviceMap)) + for _, value := range serviceMap { + orderedServices = append(orderedServices, value) + } + if len(orderedServices) > 1 { + sort.Slice(orderedServices, func(i, j int) bool { + iTime := orderedServices[i].CreationTimestamp + jTime := orderedServices[j].CreationTimestamp + log.Debugf("Service sorting name1=%s creationTime1=%v name2=%s creationTime2=%v", orderedServices[i].Name, iTime, orderedServices[j].Name, jTime) + return iTime.After(jTime.Time) + }) + } + return orderedServices } func (s *serviceCache) Delete(service *k8sV1.Service) { @@ -94,11 +120,11 @@ func (s *serviceCache) GetLoadBalancer(key string, namespace string) (string, in lb = "dummy.admiral.global" lbPort = common.DefaultMtlsPort ) - service := s.Get(namespace) - if service == nil || service.Service[namespace] == nil { + services := s.Get(namespace) + if len(services) == 0 { return lb, 0 } - for _, service := range service.Service[namespace] { + for _, service := range services { if service.Labels["app"] == key { loadBalancerStatus := service.Status.LoadBalancer.Ingress if len(loadBalancerStatus) > 0 { @@ -156,21 +182,21 @@ func NewServiceController(stopCh <-chan struct{}, handler ServiceHandler, config } func (s *ServiceController) Added(obj interface{}) { - HandleAddUpdateService(obj, s) + service := obj.(*k8sV1.Service) + s.Cache.Put(service) + s.ServiceHandler.Added(service) } func (s *ServiceController) Updated(obj interface{}, oldObj interface{}) { - HandleAddUpdateService(obj, s) -} - -func HandleAddUpdateService(obj interface{}, s *ServiceController) { service := obj.(*k8sV1.Service) s.Cache.Put(service) + s.ServiceHandler.Updated(service) } func (s *ServiceController) Deleted(obj interface{}) { service := obj.(*k8sV1.Service) s.Cache.Delete(service) + s.ServiceHandler.Deleted(service) } func (s *serviceCache) shouldIgnoreBasedOnLabels(service *k8sV1.Service) bool { diff --git a/admiral/pkg/controller/admiral/service_test.go b/admiral/pkg/controller/admiral/service_test.go index 01ee1f4c..c24fd813 100644 --- a/admiral/pkg/controller/admiral/service_test.go +++ b/admiral/pkg/controller/admiral/service_test.go @@ -75,22 +75,22 @@ func TestServiceCache_Put(t *testing.T) { if serviceCache.getKey(service) != "ns" { t.Errorf("Incorrect key. Got %v, expected ns", serviceCache.getKey(service)) } - if !cmp.Equal(serviceCache.Get("ns").Service["ns"][service.Name], service) { - t.Errorf("Incorrect service fount. Diff: %v", cmp.Diff(serviceCache.Get("ns").Service["ns"], service)) + if !cmp.Equal(serviceCache.Get("ns")[0], service) { + t.Errorf("Incorrect service found. Diff: %v", cmp.Diff(serviceCache.Get("ns")[0], service)) } - length := len(serviceCache.Get("ns").Service["ns"]) + length := len(serviceCache.Get("ns")) serviceCache.Put(service) if serviceCache.getKey(service) != "ns" { t.Errorf("Incorrect key. Got %v, expected ns", serviceCache.getKey(service)) } - if !cmp.Equal(serviceCache.Get("ns").Service["ns"][service.Name], service) { - t.Errorf("Incorrect service fount. Diff: %v", cmp.Diff(serviceCache.Get("ns").Service["ns"], service)) + if !cmp.Equal(serviceCache.Get("ns")[0], service) { + t.Errorf("Incorrect service found. Diff: %v", cmp.Diff(serviceCache.Get("ns")[0], service)) } - if (length) != len(serviceCache.Get("ns").Service["ns"]) { - t.Errorf("Re-added the same service. Cache length expected %v, got %v", length, len(serviceCache.Get("ns").Service["ns"])) + if (length) != len(serviceCache.Get("ns")) { + t.Errorf("Re-added the same service. Cache length expected %v, got %v", length, len(serviceCache.Get("ns"))) } serviceCache.Delete(service) @@ -299,3 +299,50 @@ func TestConcurrentGetAndPut(t *testing.T) { wg.Wait() } + +func TestGetOrderedServices(t *testing.T) { + + //Struct of test case info. Name is required. + testCases := []struct { + name string + services map[string]*v1.Service + expectedResult string + }{ + { + name: "Should return nil for nil input", + services: nil, + expectedResult: "", + }, + { + name: "Should return the only service", + services: map[string]*v1.Service { + "s1": {ObjectMeta: metaV1.ObjectMeta{Name: "s1", Namespace: "ns1", CreationTimestamp: metaV1.NewTime(time.Now())}}, + }, + expectedResult: "s1", + }, + { + name: "Should return the latest service by creationTime", + services: map[string]*v1.Service { + "s1": {ObjectMeta: metaV1.ObjectMeta{Name: "s1", Namespace: "ns1", CreationTimestamp: metaV1.NewTime(time.Now().Add(time.Duration(-15)))}}, + "s2": {ObjectMeta: metaV1.ObjectMeta{Name: "s2", Namespace: "ns1", CreationTimestamp: metaV1.NewTime(time.Now())}}, + }, + expectedResult: "s2", + }, + } + + //Run the test for every provided case + for _, c := range testCases { + t.Run(c.name, func(t *testing.T) { + result := getOrderedServices(c.services) + if c.expectedResult == "" && len(result) > 0 { + t.Errorf("Failed. Got %v, expected no service", result[0].Name) + } else if c.expectedResult != "" { + if len(result) > 0 && result[0].Name == c.expectedResult{ + //perfect + } else { + t.Errorf("Failed. Got %v, expected %v", result[0].Name, c.expectedResult) + } + } + }) + } +} diff --git a/admiral/pkg/controller/common/common.go b/admiral/pkg/controller/common/common.go index 94a6ae10..442a23b1 100644 --- a/admiral/pkg/controller/common/common.go +++ b/admiral/pkg/controller/common/common.go @@ -35,6 +35,7 @@ const ( AdmiralIgnoreAnnotation = "admiral.io/ignore" AdmiralCnameCaseSensitive = "admiral.io/cname-case-sensitive" BlueGreenRolloutPreviewPrefix = "preview" + RolloutPodHashLabel = "rollouts-pod-template-hash" ) type Event int diff --git a/admiral/pkg/test/mock.go b/admiral/pkg/test/mock.go index 3c231601..14a777d9 100644 --- a/admiral/pkg/test/mock.go +++ b/admiral/pkg/test/mock.go @@ -58,6 +58,10 @@ func (m *MockServiceHandler) Added(obj *k8sCoreV1.Service) { } +func (m *MockServiceHandler) Updated(obj *k8sCoreV1.Service) { + +} + func (m *MockServiceHandler) Deleted(obj *k8sCoreV1.Service) { }