From b57f54f312c729b79199a080ed0a286d7d5409c0 Mon Sep 17 00:00:00 2001 From: vgonuguntla Date: Tue, 19 Jul 2022 11:02:47 -0700 Subject: [PATCH] Updating return statements and logging --- admiral/pkg/clusters/registry_test.go | 30 ++++++++--------- admiral/pkg/clusters/types.go | 47 ++++++++++++++++++--------- 2 files changed, 47 insertions(+), 30 deletions(-) diff --git a/admiral/pkg/clusters/registry_test.go b/admiral/pkg/clusters/registry_test.go index 2e246138..2e82941a 100644 --- a/admiral/pkg/clusters/registry_test.go +++ b/admiral/pkg/clusters/registry_test.go @@ -187,25 +187,25 @@ func createMockRemoteController(f func(interface{})) (*RemoteController, error) Host: "localhost", } stop := make(chan struct{}) - d, e := admiral.NewDeploymentController("", stop, &test.MockDeploymentHandler{}, &config, time.Second*time.Duration(300)) - if e != nil { - return nil, e + d, err := admiral.NewDeploymentController("", stop, &test.MockDeploymentHandler{}, &config, time.Second*time.Duration(300)) + if err != nil { + return nil, err } - s, e := admiral.NewServiceController("test", stop, &test.MockServiceHandler{}, &config, time.Second*time.Duration(300)) - if e != nil { - return nil, e + s, err := admiral.NewServiceController("test", stop, &test.MockServiceHandler{}, &config, time.Second*time.Duration(300)) + if err != nil { + return nil, err } - n, e := admiral.NewNodeController("", stop, &test.MockNodeHandler{}, &config) - if e != nil { - return nil, e + n, err := admiral.NewNodeController("", stop, &test.MockNodeHandler{}, &config) + if err != nil { + return nil, err } - r, e := admiral.NewRolloutsController("test", stop, &test.MockRolloutHandler{}, &config, time.Second*time.Duration(300)) - if e != nil { - return nil, e + r, err := admiral.NewRolloutsController("test", stop, &test.MockRolloutHandler{}, &config, time.Second*time.Duration(300)) + if err != nil { + return nil, err } - rpc, e := admiral.NewRoutingPoliciesController(stop, &test.MockRoutingPolicyHandler{}, &config, time.Second*time.Duration(300)) - if e != nil { - return nil, e + rpc, err := admiral.NewRoutingPoliciesController(stop, &test.MockRoutingPolicyHandler{}, &config, time.Second*time.Duration(300)) + if err != nil { + return nil, err } deployment := k8sAppsV1.Deployment{ diff --git a/admiral/pkg/clusters/types.go b/admiral/pkg/clusters/types.go index fa55a496..0ab3e8b5 100644 --- a/admiral/pkg/clusters/types.go +++ b/admiral/pkg/clusters/types.go @@ -225,15 +225,19 @@ func (r RoutingPolicyHandler) Added(obj *v1.RoutingPolicy) { log.Infof(LogErrFormat, "success", "routingpolicy", obj.Name, obj.ClusterName, "Ignored the RoutingPolicy because of the annotation") return } - dependents, done := getDependents(obj, r) - if done { + dependents := getDependents(obj, r) + if len(dependents) == 0 { + log.Info("No dependents found for Routing Policy - ",obj.Name) return } - r.processroutingPolicy(dependents, obj, admiral.Add) + err := r.processroutingPolicy(dependents, obj, admiral.Add) + if err != nil { + log.Errorf(LogErrFormat, admiral.Add, "routingpolicy", obj.Name, "", err) + } log.Info("finished processing routing policy") } -func (r RoutingPolicyHandler) processroutingPolicy(dependents map[string]string, routingPolicy *v1.RoutingPolicy, eventType admiral.EventType ) { +func (r RoutingPolicyHandler) processroutingPolicy(dependents map[string]string, routingPolicy *v1.RoutingPolicy, eventType admiral.EventType ) error{ for _, remoteController := range r.RemoteRegistry.RemoteControllers { for _, dependent := range dependents { @@ -244,6 +248,7 @@ func (r RoutingPolicyHandler) processroutingPolicy(dependents map[string]string, filter, err := createOrUpdateEnvoyFilter(remoteController, routingPolicy, eventType, dependent, r.RemoteRegistry.AdmiralCache) if err != nil { log.Errorf(LogErrFormat, admiral.Add, "routingpolicy", routingPolicy.Name, remoteController.ClusterID, err) + return err } else { log.Infof("msg=%s name=%s cluster=%s", "created envoyfilter", filter.Name, remoteController.ClusterID) } @@ -251,6 +256,7 @@ func (r RoutingPolicyHandler) processroutingPolicy(dependents map[string]string, } } + return nil } func (r RoutingPolicyHandler) Updated(obj *v1.RoutingPolicy) { @@ -260,35 +266,44 @@ func (r RoutingPolicyHandler) Updated(obj *v1.RoutingPolicy) { r.Deleted(obj) return } - dependents, missingIdentityLabel := getDependents(obj, r) - if missingIdentityLabel { + dependents := getDependents(obj, r) + if len(dependents) == 0 { return } - r.processroutingPolicy(dependents, obj, admiral.Update) + err := r.processroutingPolicy(dependents, obj, admiral.Update) + if err != nil { + log.Errorf(LogErrFormat, admiral.Update, "routingpolicy", obj.Name,"", err) + } log.Info("updated routing policy") } -func getDependents(obj *v1.RoutingPolicy, r RoutingPolicyHandler) (map[string]string, bool) { +// getDependents - Returns the client dependents for the destination service with routing policy +// Returns a list of asset ID's of the client services or nil if no dependents are found +func getDependents(obj *v1.RoutingPolicy, r RoutingPolicyHandler) map[string]string { sourceIdentity := common.GetRoutingPolicyIdentity(obj) if len(sourceIdentity) == 0 { err := errors.New("identity label is missing") log.Warnf(LogErrFormat, "add", "RoutingPolicy", obj.Name, r.ClusterID, err) - return nil, true + return nil } dependents := r.RemoteRegistry.AdmiralCache.IdentityDependencyCache.Get(sourceIdentity).Copy() - return dependents, false + return dependents } func (r RoutingPolicyHandler) Deleted(obj *v1.RoutingPolicy) { - log.Info("deleted envoy filter for routing policy - ",obj.Name) - dependents, missingIdentityLabel := getDependents(obj, r) - if !missingIdentityLabel { - r.deleteEnvoyFilters(dependents, obj, admiral.Delete) + dependents := getDependents(obj, r) + if len(dependents) != 0 { + err := r.deleteEnvoyFilters(dependents, obj, admiral.Delete) + if err != nil { + log.Errorf(LogErrFormat, admiral.Delete, "envoyfilter", "", "", err) + }else{ + log.Info("deleted envoy filter for routing policy - ",obj.Name) + } } } -func (r RoutingPolicyHandler) deleteEnvoyFilters(dependents map[string]string, obj *v1.RoutingPolicy, eventType admiral.EventType) { +func (r RoutingPolicyHandler) deleteEnvoyFilters(dependents map[string]string, obj *v1.RoutingPolicy, eventType admiral.EventType) error { for _, dependent := range dependents { key := dependent + common.GetRoutingPolicyEnv(obj) clusterIdFilterMap := r.RemoteRegistry.AdmiralCache.RoutingPolicyFilterCache.Get(key) @@ -299,6 +314,7 @@ func (r RoutingPolicyHandler) deleteEnvoyFilters(dependents map[string]string, o err := rc.RoutingPolicyController.IstioClient.NetworkingV1alpha3().EnvoyFilters("istio-system").Delete(filter, &metaV1.DeleteOptions{}) if err != nil { log.Errorf(LogErrFormat, eventType, "envoyfilter", filter, rc.ClusterID, err) + return err } else { log.Infof(LogFormat, eventType, "envoyfilter", filter, rc.ClusterID, "deleting from cache") r.RemoteRegistry.AdmiralCache.RoutingPolicyFilterCache.Delete(key) @@ -307,6 +323,7 @@ func (r RoutingPolicyHandler) deleteEnvoyFilters(dependents map[string]string, o } } } + return nil } type DeploymentHandler struct {