From 5b9073678363b6ea319f919c63d4bdad33a009c8 Mon Sep 17 00:00:00 2001 From: Ryan Tay Date: Tue, 10 Sep 2024 12:33:43 -0700 Subject: [PATCH 1/3] fix casting of slices of services Signed-off-by: Ryan Tay --- admiral/pkg/controller/admiral/controller.go | 15 +++++++++- .../pkg/controller/admiral/controller_test.go | 30 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/admiral/pkg/controller/admiral/controller.go b/admiral/pkg/controller/admiral/controller.go index 97e3a338..28d0c651 100644 --- a/admiral/pkg/controller/admiral/controller.go +++ b/admiral/pkg/controller/admiral/controller.go @@ -3,6 +3,7 @@ package admiral import ( "context" "fmt" + v1 "k8s.io/api/core/v1" "reflect" "strings" "time" @@ -465,8 +466,11 @@ func shouldRetry(ctxLogger *logrus.Entry, ctx context.Context, obj interface{}, return true } latestObjMeta, latestOk := objFromCache.(metav1.Object) + if servicesSlice, servicesSliceOk := objFromCache.([]*v1.Service); servicesSliceOk { + latestObjMeta, latestOk = getMatchingServiceFromServiceSlice(objMeta, servicesSlice) + } if !latestOk || reflect.ValueOf(latestObjMeta).IsNil() { - ctxLogger.Errorf("task=shouldRetry message=unable to cast latest object from cache to metav1 object, obj received=%+v", objMeta) + ctxLogger.Errorf("task=shouldRetry message=unable to cast latest object from cache to metav1 object, obj is of type %+v,obj received=%+v", reflect.TypeOf(objFromCache), objMeta) return true } // event 1 ==> processed @@ -488,3 +492,12 @@ func shouldRetry(ctxLogger *logrus.Entry, ctx context.Context, obj interface{}, ctxLogger.Errorf("task=shouldRetry message=obj parsed=%v, retrying object, obj received=%+v", ok, objMeta) return true } + +func getMatchingServiceFromServiceSlice(service metav1.Object, services []*v1.Service) (metav1.Object, bool) { + for _, s := range services { + if s.Namespace == service.GetNamespace() && s.Name == service.GetName() { + return interface{}(s).(metav1.Object), true + } + } + return nil, false +} diff --git a/admiral/pkg/controller/admiral/controller_test.go b/admiral/pkg/controller/admiral/controller_test.go index c2817bf4..a7b716e5 100644 --- a/admiral/pkg/controller/admiral/controller_test.go +++ b/admiral/pkg/controller/admiral/controller_test.go @@ -2,6 +2,7 @@ package admiral import ( "context" + v1 "k8s.io/api/core/v1" "testing" log "github.com/sirupsen/logrus" @@ -166,6 +167,20 @@ func TestShouldRetry(t *testing.T) { ctx = context.Background() resourceName1 = "resource-name-1" resourceNameSpace1 = "resource-namespace-1" + serviceSlice = []*v1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName1 + "-canary", + Namespace: resourceNameSpace1, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName1, + Namespace: resourceNameSpace1, + }, + }, + } ) testCases := []struct { name string @@ -237,6 +252,21 @@ func TestShouldRetry(t *testing.T) { delegator: nil, expectedResult: true, }, + { + name: "Given an update event, " + + "When the controller is a service controller, " + + "When the controller returns a slice of services from the cache, " + + "Then the func should look through the cache for the matching service.", + obj: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName1, + Namespace: resourceNameSpace1, + }, + }, + delegator: NewMockDelegator(), + latestObjInKubernetes: interface{}(serviceSlice), + expectedResult: true, + }, } for _, c := range testCases { From 075ecd10c23949e92b024db8df8784a239471087 Mon Sep 17 00:00:00 2001 From: Ryan Tay Date: Tue, 10 Sep 2024 14:21:41 -0700 Subject: [PATCH 2/3] convert to case-switch Signed-off-by: Ryan Tay --- admiral/pkg/controller/admiral/controller.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/admiral/pkg/controller/admiral/controller.go b/admiral/pkg/controller/admiral/controller.go index 28d0c651..f0821c0e 100644 --- a/admiral/pkg/controller/admiral/controller.go +++ b/admiral/pkg/controller/admiral/controller.go @@ -455,6 +455,10 @@ func checkIfResourceVersionHasIncreased(ctxLogger *logrus.Entry, ctx context.Con } func shouldRetry(ctxLogger *logrus.Entry, ctx context.Context, obj interface{}, delegator Delegator) bool { + var ( + latestObjMeta metav1.Object + latestOk bool + ) objMeta, ok := obj.(metav1.Object) if ok { if reflect.ValueOf(objMeta).IsNil() || reflect.ValueOf(delegator).IsNil() { @@ -465,9 +469,12 @@ func shouldRetry(ctxLogger *logrus.Entry, ctx context.Context, obj interface{}, ctxLogger.Errorf("task=shouldRetry message=unable to fetch latest object from cache, obj received=%+v", objMeta) return true } - latestObjMeta, latestOk := objFromCache.(metav1.Object) - if servicesSlice, servicesSliceOk := objFromCache.([]*v1.Service); servicesSliceOk { + switch objFromCache.(type) { + case []*v1.Service: + servicesSlice, _ := objFromCache.([]*v1.Service) latestObjMeta, latestOk = getMatchingServiceFromServiceSlice(objMeta, servicesSlice) + default: + latestObjMeta, latestOk = objFromCache.(metav1.Object) } if !latestOk || reflect.ValueOf(latestObjMeta).IsNil() { ctxLogger.Errorf("task=shouldRetry message=unable to cast latest object from cache to metav1 object, obj is of type %+v,obj received=%+v", reflect.TypeOf(objFromCache), objMeta) From afbe1754ed994af3755dce4328c5d3f6f7d6259c Mon Sep 17 00:00:00 2001 From: Ryan Tay Date: Tue, 10 Sep 2024 14:24:04 -0700 Subject: [PATCH 3/3] signoff Signed-off-by: Ryan Tay --- admiral/pkg/controller/admiral/controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/admiral/pkg/controller/admiral/controller.go b/admiral/pkg/controller/admiral/controller.go index f0821c0e..81b24dad 100644 --- a/admiral/pkg/controller/admiral/controller.go +++ b/admiral/pkg/controller/admiral/controller.go @@ -471,6 +471,7 @@ func shouldRetry(ctxLogger *logrus.Entry, ctx context.Context, obj interface{}, } switch objFromCache.(type) { case []*v1.Service: + // ServiceController returns a slice of services from Get() which needs extra processing before casting servicesSlice, _ := objFromCache.([]*v1.Service) latestObjMeta, latestOk = getMatchingServiceFromServiceSlice(objMeta, servicesSlice) default: