diff --git a/internal/k8s/reconciler/gatewayclass.go b/internal/k8s/reconciler/gatewayclass.go index b3d4b37ac..41c1194af 100644 --- a/internal/k8s/reconciler/gatewayclass.go +++ b/internal/k8s/reconciler/gatewayclass.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/consul-api-gateway/internal/k8s/gatewayclient" rstatus "github.com/hashicorp/consul-api-gateway/internal/k8s/reconciler/status" - "github.com/hashicorp/consul-api-gateway/internal/k8s/utils" apigwv1alpha1 "github.com/hashicorp/consul-api-gateway/pkg/apis/v1alpha1" ) @@ -51,8 +50,8 @@ func (g *K8sGatewayClasses) Upsert(ctx context.Context, class *K8sGatewayClass) g.mutex.Lock() defer g.mutex.Unlock() - if current, ok := g.gatewayClasses[class.class.Name]; ok { - if utils.ResourceVersionGreater(current.class.ResourceVersion, class.class.ResourceVersion) { + if cachedClass, ok := g.gatewayClasses[class.class.Name]; ok { + if cachedClass.class.ResourceVersion == class.class.ResourceVersion { // we have an old gatewayclass update ignore return nil } diff --git a/internal/k8s/reconciler/gatewayclass_test.go b/internal/k8s/reconciler/gatewayclass_test.go index dd2135a8b..0d0b3cae9 100644 --- a/internal/k8s/reconciler/gatewayclass_test.go +++ b/internal/k8s/reconciler/gatewayclass_test.go @@ -119,7 +119,7 @@ func TestGatewayClasses(t *testing.T) { gatewayClass = NewK8sGatewayClass(&gwv1beta1.GatewayClass{ ObjectMeta: meta.ObjectMeta{ - ResourceVersion: "0", + ResourceVersion: "1", }, Spec: gwv1beta1.GatewayClassSpec{ ParametersRef: &gwv1beta1.ParametersReference{ diff --git a/internal/k8s/reconciler/manager.go b/internal/k8s/reconciler/manager.go index 51defc338..023ae72b6 100644 --- a/internal/k8s/reconciler/manager.go +++ b/internal/k8s/reconciler/manager.go @@ -197,7 +197,7 @@ func (m *GatewayReconcileManager) UpsertGateway(ctx context.Context, g *gwv1beta if current == nil { return true } - return !utils.ResourceVersionGreater(current.(*K8sGateway).ResourceVersion, gateway.ResourceVersion) + return current.(*K8sGateway).ResourceVersion != gateway.ResourceVersion }) } @@ -235,7 +235,7 @@ func (m *GatewayReconcileManager) upsertRoute(ctx context.Context, r Route, pare if current == nil { return true } - return !utils.ResourceVersionGreater(current.(*K8sRoute).GetResourceVersion(), route.GetResourceVersion()) + return current.(*K8sRoute).GetResourceVersion() != route.GetResourceVersion() }) } diff --git a/internal/k8s/reconciler/manager_test.go b/internal/k8s/reconciler/manager_test.go index 0b221e6b4..29dbe2f0c 100644 --- a/internal/k8s/reconciler/manager_test.go +++ b/internal/k8s/reconciler/manager_test.go @@ -6,7 +6,10 @@ import ( "testing" "github.com/golang/mock/gomock" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -34,24 +37,34 @@ func TestUpsertGatewayClass(t *testing.T) { Logger: hclog.NewNullLogger(), }) - inner := &gwv1beta1.GatewayClass{} - expected := errors.New("expected") - client.EXPECT().UpdateStatus(gomock.Any(), inner).Return(expected) - require.Equal(t, expected, manager.UpsertGatewayClass(context.Background(), inner)) + t.Run("error validating GatewayClassConfig", func(t *testing.T) { + inner := &gwv1beta1.GatewayClass{ + Spec: gwv1beta1.GatewayClassSpec{ + ParametersRef: &gwv1beta1.ParametersReference{ + Group: apigwv1alpha1.Group, + Kind: apigwv1alpha1.GatewayClassConfigKind, + }, + }, + } + expected := errors.New("expected") + client.EXPECT().GetGatewayClassConfig(gomock.Any(), gomock.Any()).Return(nil, expected) - client.EXPECT().UpdateStatus(gomock.Any(), inner) - require.NoError(t, manager.UpsertGatewayClass(context.Background(), inner)) + err := manager.UpsertGatewayClass(context.Background(), inner) + assert.EqualError(t, err, expected.Error()) + }) - // validation - client.EXPECT().GetGatewayClassConfig(gomock.Any(), gomock.Any()).Return(nil, expected) - require.Equal(t, expected, manager.UpsertGatewayClass(context.Background(), &gwv1beta1.GatewayClass{ - Spec: gwv1beta1.GatewayClassSpec{ - ParametersRef: &gwv1beta1.ParametersReference{ - Group: apigwv1alpha1.Group, - Kind: apigwv1alpha1.GatewayClassConfigKind, - }, - }, - })) + t.Run("error updating status", func(t *testing.T) { + inner := &gwv1beta1.GatewayClass{ObjectMeta: v1.ObjectMeta{ResourceVersion: uuid.NewString()}} + expected := errors.New("expected") + client.EXPECT().UpdateStatus(gomock.Any(), inner).Return(expected) + assert.EqualError(t, manager.UpsertGatewayClass(context.Background(), inner), expected.Error()) + }) + + t.Run("successful update", func(t *testing.T) { + inner := &gwv1beta1.GatewayClass{ObjectMeta: v1.ObjectMeta{ResourceVersion: uuid.NewString()}} + client.EXPECT().UpdateStatus(gomock.Any(), inner) + assert.NoError(t, manager.UpsertGatewayClass(context.Background(), inner)) + }) } func TestUpsertGateway(t *testing.T) { diff --git a/internal/k8s/utils/versions.go b/internal/k8s/utils/versions.go deleted file mode 100644 index 561c20fd1..000000000 --- a/internal/k8s/utils/versions.go +++ /dev/null @@ -1,17 +0,0 @@ -package utils - -import "strconv" - -func ResourceVersionGreater(a, b string) bool { - aVal, err := strconv.Atoi(a) - if err != nil { - // a isn't numeric, return that b is greater - return false - } - bVal, err := strconv.Atoi(b) - if err != nil { - // b isn't numeric, return that a is greater - return true - } - return aVal > bVal -}