Skip to content

Commit

Permalink
fix: do not reconcile gateways with unsupported gw class
Browse files Browse the repository at this point in the history
  • Loading branch information
@czeslavo
czeslavo committed Oct 7, 2024
1 parent 78d11d1 commit 4fd90da
Show file tree
Hide file tree
Showing 12 changed files with 286 additions and 95 deletions.
27 changes: 16 additions & 11 deletions controller/gateway/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/kong/gateway-operator/controller/pkg/watch"
operatorerrors "github.com/kong/gateway-operator/internal/errors"
gwtypes "github.com/kong/gateway-operator/internal/types"
"github.com/kong/gateway-operator/internal/utils/gatewayclass"
"github.com/kong/gateway-operator/pkg/consts"
gatewayutils "github.com/kong/gateway-operator/pkg/utils/gateway"
k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"
Expand Down Expand Up @@ -112,6 +113,20 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
return ctrl.Result{}, client.IgnoreNotFound(err)
}

log.Trace(logger, "checking gatewayclass", gateway)
gwc, err := gatewayclass.Get(ctx, r.Client, string(gateway.Spec.GatewayClassName))
if err != nil {
if errors.As(err, &operatorerrors.ErrUnsupportedGateway{}) {
log.Debug(logger, "resource not supported, ignoring", gateway,
"ExpectedGatewayClass", vars.ControllerName(),
"GatewayClass", gateway.Spec.GatewayClassName,
"Reason", err.Error(),
)
return ctrl.Result{}, nil
}
return ctrl.Result{}, err
}

log.Trace(logger, "managing cleanup for gateway resource", gateway)
if shouldReturnEarly, result, err := r.cleanup(ctx, logger, &gateway); err != nil || !result.IsZero() {
return result, err
Expand All @@ -122,7 +137,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
log.Trace(logger, "managing the gateway resource finalizers", gateway)
cpFinalizerSet := controllerutil.AddFinalizer(&gateway, string(GatewayFinalizerCleanupControlPlanes))
dpFinalizerSet := controllerutil.AddFinalizer(&gateway, string(GatewayFinalizerCleanupDataPlanes))
npFinalizerSet := controllerutil.AddFinalizer(&gateway, string(GatewayFinalizerCleanupNetworkpolicies))
npFinalizerSet := controllerutil.AddFinalizer(&gateway, string(GatewayFinalizerCleanupNetworkPolicies))
if cpFinalizerSet || dpFinalizerSet || npFinalizerSet {
log.Trace(logger, "Setting finalizers", gateway)
if err := r.Client.Update(ctx, &gateway); err != nil {
Expand All @@ -137,16 +152,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
return ctrl.Result{}, nil
}

log.Trace(logger, "checking gatewayclass", gateway)
gwc, err := r.verifyGatewayClassSupport(ctx, &gateway)
if err != nil {
if errors.Is(err, operatorerrors.ErrUnsupportedGateway) {
log.Debug(logger, "resource not supported, ignoring", gateway, "ExpectedGatewayClass", vars.ControllerName())
return ctrl.Result{}, nil
}
return ctrl.Result{}, err
}

if !gwc.IsAccepted() {
log.Debug(logger, "gatewayclass not accepted , ignoring", gateway)
return ctrl.Result{}, nil
Expand Down
2 changes: 1 addition & 1 deletion controller/gateway/controller_cleanup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (r *Reconciler) cleanup(
}
} else {
oldGateway := gateway.DeepCopy()
if controllerutil.RemoveFinalizer(gateway, string(GatewayFinalizerCleanupNetworkpolicies)) {
if controllerutil.RemoveFinalizer(gateway, string(GatewayFinalizerCleanupNetworkPolicies)) {
if err := r.Client.Patch(ctx, gateway, client.MergeFrom(oldGateway)); err != nil {
res, err := handleGatewayFinalizerPatchOrUpdateError(err, gateway, logger)
return true, res, err
Expand Down
4 changes: 2 additions & 2 deletions controller/gateway/controller_consts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ const (
GatewayFinalizerCleanupDataPlanes GatewayFinalizer = "gateway-operator.konghq.com/cleanup-dataplanes"
// GatewayFinalizerCleanupControlPlanes is the finalizer to cleanup owned controlplane resources.
GatewayFinalizerCleanupControlPlanes GatewayFinalizer = "gateway-operator.konghq.com/cleanup-controlplanes"
// GatewayFinalizerCleanupNetworkpolicies is the finalizer to cleanup owned network policies.
GatewayFinalizerCleanupNetworkpolicies GatewayFinalizer = "gateway-operator.konghq.com/cleanup-network-policies"
// GatewayFinalizerCleanupNetworkPolicies is the finalizer to cleanup owned network policies.
GatewayFinalizerCleanupNetworkPolicies GatewayFinalizer = "gateway-operator.konghq.com/cleanup-network-policies"
)
19 changes: 0 additions & 19 deletions controller/gateway/controller_reconciler_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ import (
"github.com/kong/gateway-operator/controller/pkg/secrets/ref"
operatorerrors "github.com/kong/gateway-operator/internal/errors"
gwtypes "github.com/kong/gateway-operator/internal/types"
"github.com/kong/gateway-operator/internal/utils/gatewayclass"
"github.com/kong/gateway-operator/pkg/consts"
gatewayutils "github.com/kong/gateway-operator/pkg/utils/gateway"
k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"
k8sreduce "github.com/kong/gateway-operator/pkg/utils/kubernetes/reduce"
k8sresources "github.com/kong/gateway-operator/pkg/utils/kubernetes/resources"
"github.com/kong/gateway-operator/pkg/vars"
)

// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -193,23 +191,6 @@ func gatewayAddressesFromService(svc corev1.Service) ([]gwtypes.GatewayStatusAdd
return addresses, nil
}

func (r *Reconciler) verifyGatewayClassSupport(ctx context.Context, gateway *gwtypes.Gateway) (*gatewayclass.Decorator, error) {
if gateway.Spec.GatewayClassName == "" {
return nil, operatorerrors.ErrUnsupportedGateway
}

gwc := gatewayclass.NewDecorator()
if err := r.Client.Get(ctx, client.ObjectKey{Name: string(gateway.Spec.GatewayClassName)}, gwc.GatewayClass); err != nil {
return nil, err
}

if string(gwc.Spec.ControllerName) != vars.ControllerName() {
return nil, operatorerrors.ErrUnsupportedGateway
}

return gwc, nil
}

func (r *Reconciler) getOrCreateGatewayConfiguration(ctx context.Context, gatewayClass *gatewayv1.GatewayClass) (*operatorv1beta1.GatewayConfiguration, error) {
gatewayConfig, err := r.getGatewayConfigForGatewayClass(ctx, gatewayClass)
if err != nil {
Expand Down
114 changes: 107 additions & 7 deletions controller/gateway/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,94 @@ func TestGatewayReconciler_Reconcile(t *testing.T) {
controlplaneSubResources []controllerruntimeclient.Object
testBody func(t *testing.T, reconciler Reconciler, gatewayReq reconcile.Request)
}{
{
name: "gateway class not found - gateway is ignored",
gatewayReq: reconcile.Request{
NamespacedName: types.NamespacedName{
Namespace: "test-namespace",
Name: "test-gateway",
},
},
gateway: &gwtypes.Gateway{
TypeMeta: metav1.TypeMeta{
APIVersion: "gateway.networking.k8s.io/v1beta1",
Kind: "Gateway",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-gateway",
Namespace: "test-namespace",
UID: types.UID(uuid.NewString()),
},
Spec: gatewayv1.GatewaySpec{
GatewayClassName: "not-existing-gatewayclass",
},
},
testBody: func(t *testing.T, r Reconciler, gatewayReq reconcile.Request) {
ctx := context.Background()
res, err := r.Reconcile(ctx, gatewayReq)
require.NoError(t, err, "reconciliation should not return an error")
require.Equal(t, res, reconcile.Result{}, "reconciliation should not return a requeue")

var gw gwtypes.Gateway
require.NoError(t, r.Client.Get(ctx, gatewayReq.NamespacedName, &gw))

require.Empty(t, gw.GetFinalizers(), "gateway should not have any finalizers as it's ignored")
},
},
{
name: "gateway class found, but controller name is not matching - gateway is ignored",
gatewayReq: reconcile.Request{
NamespacedName: types.NamespacedName{
Namespace: "test-namespace",
Name: "test-gateway",
},
},
gateway: &gwtypes.Gateway{
TypeMeta: metav1.TypeMeta{
APIVersion: "gateway.networking.k8s.io/v1beta1",
Kind: "Gateway",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-gateway",
Namespace: "test-namespace",
UID: types.UID(uuid.NewString()),
},
Spec: gatewayv1.GatewaySpec{
GatewayClassName: "test-gatewayclass",
},
},
gatewayClass: &gatewayv1.GatewayClass{
ObjectMeta: metav1.ObjectMeta{
Name: "test-gatewayclass",
},
Spec: gatewayv1.GatewayClassSpec{
ControllerName: gatewayv1.GatewayController("not-existing-controller"),
},
Status: gatewayv1.GatewayClassStatus{
Conditions: []metav1.Condition{
{
Type: string(gatewayv1.GatewayClassConditionStatusAccepted),
Status: metav1.ConditionTrue,
ObservedGeneration: 0,
LastTransitionTime: metav1.Now(),
Reason: string(gatewayv1.GatewayClassReasonAccepted),
Message: "the gatewayclass has been accepted by the controller",
},
},
},
},
testBody: func(t *testing.T, r Reconciler, gatewayReq reconcile.Request) {
ctx := context.Background()
res, err := r.Reconcile(ctx, gatewayReq)
require.NoError(t, err, "reconciliation should not return an error")
require.Equal(t, res, reconcile.Result{}, "reconciliation should not return a requeue")

var gw gwtypes.Gateway
require.NoError(t, r.Client.Get(ctx, gatewayReq.NamespacedName, &gw))

require.Empty(t, gw.GetFinalizers(), "gateway should not have any finalizers as it's ignored")
},
},
{
name: "service connectivity",
gatewayReq: reconcile.Request{
Expand Down Expand Up @@ -169,6 +257,16 @@ func TestGatewayReconciler_Reconcile(t *testing.T) {
// the dataplane service starts with no IP assigned, the gateway must be not ready
_, err := reconciler.Reconcile(ctx, gatewayReq)
require.NoError(t, err, "reconciliation returned an error")

t.Log("verifying the gateway gets finalizers assigned")
var gateway gwtypes.Gateway
require.NoError(t, reconciler.Client.Get(ctx, gatewayReq.NamespacedName, &gateway))
require.ElementsMatch(t, gateway.GetFinalizers(), []string{
string(GatewayFinalizerCleanupControlPlanes),
string(GatewayFinalizerCleanupDataPlanes),
string(GatewayFinalizerCleanupNetworkPolicies),
})

// need to trigger the Reconcile again because the first one only updated the finalizers
_, err = reconciler.Reconcile(ctx, gatewayReq)
require.NoError(t, err, "reconciliation returned an error")
Expand Down Expand Up @@ -301,17 +399,19 @@ func TestGatewayReconciler_Reconcile(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
ObjectsToAdd := []controllerruntimeclient.Object{
objectsToAdd := []controllerruntimeclient.Object{
tc.gateway,
tc.gatewayClass,
}
if tc.gatewayClass != nil {
objectsToAdd = append(objectsToAdd, tc.gatewayClass)
}
for _, gatewaySubResource := range tc.gatewaySubResources {
k8sutils.SetOwnerForObject(gatewaySubResource, tc.gateway)
gatewayutils.LabelObjectAsGatewayManaged(gatewaySubResource)
if gatewaySubResource.GetName() == "test-dataplane" {
for _, dataplaneSubresource := range tc.dataplaneSubResources {
k8sutils.SetOwnerForObject(dataplaneSubresource, gatewaySubResource)
ObjectsToAdd = append(ObjectsToAdd, dataplaneSubresource)
objectsToAdd = append(objectsToAdd, dataplaneSubresource)
}
}
if gatewaySubResource.GetName() == "test-controlplane" {
Expand All @@ -324,17 +424,17 @@ func TestGatewayReconciler_Reconcile(t *testing.T) {
})
for _, controlplaneSubResource := range tc.controlplaneSubResources {
k8sutils.SetOwnerForObject(controlplaneSubResource, gatewaySubResource)
ObjectsToAdd = append(ObjectsToAdd, controlplaneSubResource)
objectsToAdd = append(objectsToAdd, controlplaneSubResource)
}
}
ObjectsToAdd = append(ObjectsToAdd, gatewaySubResource)
objectsToAdd = append(objectsToAdd, gatewaySubResource)
}

fakeClient := fakectrlruntimeclient.
NewClientBuilder().
WithScheme(scheme.Scheme).
WithObjects(ObjectsToAdd...).
WithStatusSubresource(ObjectsToAdd...).
WithObjects(objectsToAdd...).
WithStatusSubresource(objectsToAdd...).
Build()

reconciler := Reconciler{
Expand Down
5 changes: 3 additions & 2 deletions controller/gateway/controller_watch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/kong/gateway-operator/controller/pkg/secrets/ref"
operatorerrors "github.com/kong/gateway-operator/internal/errors"
gwtypes "github.com/kong/gateway-operator/internal/types"
"github.com/kong/gateway-operator/internal/utils/gatewayclass"
"github.com/kong/gateway-operator/pkg/consts"
k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"
"github.com/kong/gateway-operator/pkg/utils/kubernetes/resources"
Expand All @@ -41,13 +42,13 @@ func (r *Reconciler) gatewayHasMatchingGatewayClass(obj client.Object) bool {
return false
}

_, err := r.verifyGatewayClassSupport(context.Background(), gateway)
_, err := gatewayclass.Get(context.Background(), r.Client, string(gateway.Spec.GatewayClassName))
if err != nil {
// filtering here is just an optimization, the reconciler will check the
// class as well. If we fail here it's most likely because of some failure
// of the Kubernetes API and it's technically better to enqueue the object
// than to drop it for eventual consistency during cluster outages.
return !errors.Is(err, operatorerrors.ErrUnsupportedGateway)
return !errors.As(err, &operatorerrors.ErrUnsupportedGateway{})
}

return true
Expand Down
5 changes: 3 additions & 2 deletions controller/specialized/aigateway_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/kong/gateway-operator/controller/pkg/log"
"github.com/kong/gateway-operator/controller/pkg/watch"
operatorerrors "github.com/kong/gateway-operator/internal/errors"
"github.com/kong/gateway-operator/internal/utils/gatewayclass"
k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"
"github.com/kong/gateway-operator/pkg/vars"
)
Expand Down Expand Up @@ -66,9 +67,9 @@ func (r *AIGatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
// and this check must be done here to ensure consistency.
//
// See: https://github.com/kubernetes-sigs/controller-runtime/issues/1996
gwc, err := r.verifyGatewayClassSupport(ctx, &aigateway)
gwc, err := gatewayclass.Get(ctx, r.Client, aigateway.Spec.GatewayClassName)
if err != nil {
if errors.Is(err, operatorerrors.ErrUnsupportedGateway) {
if errors.As(err, &operatorerrors.ErrUnsupportedGateway{}) {
log.Debug(logger, "resource not supported, ignoring", aigateway, "ExpectedGatewayClass", vars.ControllerName())
return ctrl.Result{}, nil
}
Expand Down
42 changes: 0 additions & 42 deletions controller/specialized/aigateway_controller_reconciler_utils.go
View file

This file was deleted.

5 changes: 3 additions & 2 deletions controller/specialized/aigateway_controller_watch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/kong/gateway-operator/api/v1alpha1"
operatorerrors "github.com/kong/gateway-operator/internal/errors"
"github.com/kong/gateway-operator/internal/utils/gatewayclass"
)

// -----------------------------------------------------------------------------
Expand All @@ -30,13 +31,13 @@ func (r *AIGatewayReconciler) aiGatewayHasMatchingGatewayClass(obj client.Object
return false
}

_, err := r.verifyGatewayClassSupport(context.Background(), aigateway)
_, err := gatewayclass.Get(context.Background(), r.Client, aigateway.Spec.GatewayClassName)
if err != nil {
// filtering here is just an optimization, the reconciler will check the
// class as well. If we fail here it's most likely because of some failure
// of the Kubernetes API and it's technically better to enqueue the object
// than to drop it for eventual consistency during cluster outages.
return !errors.Is(err, operatorerrors.ErrUnsupportedGateway)
return !errors.As(err, &operatorerrors.ErrUnsupportedGateway{})
}

return true
Expand Down
Loading

0 comments on commit 4fd90da

Please sign in to comment.