Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix casting of slices of services #340

Merged
merged 3 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions admiral/pkg/controller/admiral/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package admiral
import (
"context"
"fmt"
v1 "k8s.io/api/core/v1"
"reflect"
"strings"
"time"
Expand Down Expand Up @@ -454,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() {
Expand All @@ -464,9 +469,16 @@ 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)
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:
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 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
Expand All @@ -488,3 +500,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
}
30 changes: 30 additions & 0 deletions admiral/pkg/controller/admiral/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package admiral

import (
"context"
v1 "k8s.io/api/core/v1"
"testing"

log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down