Merge branch 'main' into add-external-traffic-policy-to-dataplanes-se…

…rvice-options

.github/workflows/tests.yaml

@@ -319,6 +319,7 @@ jobs:
       env:
         KONG_CONTROLLER_OUT: stdout
         GOTESTSUM_JUNITFILE: integration-tests-provision-dataplane-fail.xml
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
     - name: upload diagnostics
       if: always()
@@ -369,6 +370,7 @@ jobs:
       env:
         KONG_TEST_GATEWAY_OPERATOR_IMAGE_LOAD: gateway-operator:e2e-${{ github.sha }}
         GOTESTSUM_JUNITFILE: "e2e-tests.xml"
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
     - name: upload diagnostics
       if: always()

CHANGELOG.md

@@ -33,6 +33,12 @@
   `apis` -> `api` and `controllers` -> `controller`.
   [#84](https://github.com/Kong/gateway-operator/pull/84)
 
+### Changes
+
+- `Gateway` do not have their `Ready` status condition set anymore.
+  This aligns with Gateway API and its conformance test suite.
+  [#246](https://github.com/Kong/gateway-operator/pull/246)
+
 ### Fixes
 
 - Fix enforcing up to date `ControlPlane`'s `ValidatingWebhookConfiguration`

Dockerfile

@@ -2,7 +2,7 @@
 # Builder
 # ------------------------------------------------------------------------------
 
-FROM --platform=$BUILDPLATFORM golang:1.22.2 as builder
+FROM --platform=$BUILDPLATFORM golang:1.22.3 as builder
 
 WORKDIR /workspace
 ARG GOPATH

Makefile

@@ -62,15 +62,15 @@ YQ_VERSION = 4.43.1
 YQ = $(PROJECT_DIR)/bin/installs/yq/$(YQ_VERSION)/bin/yq
 .PHONY: yq
 yq: mise # Download yq locally if necessary.
-	$(MISE) plugin install --yes -q yq
-	$(MISE) install -q yq@$(YQ_VERSION)
+	@$(MISE) plugin install --yes -q yq
+	@$(MISE) install -q yq@$(YQ_VERSION)
 
 CONTROLLER_GEN_VERSION = $(shell $(YQ) -r '.controller-tools' < $(TOOLS_VERSIONS_FILE))
 CONTROLLER_GEN = $(PROJECT_DIR)/bin/installs/kube-controller-tools/$(CONTROLLER_GEN_VERSION)/bin/controller-gen
 .PHONY: controller-gen
 controller-gen: mise yq ## Download controller-gen locally if necessary.
-	$(MISE) plugin install --yes -q kube-controller-tools
-	$(MISE) install -q kube-controller-tools@$(CONTROLLER_GEN_VERSION)
+	@$(MISE) plugin install --yes -q kube-controller-tools
+	@$(MISE) install -q kube-controller-tools@$(CONTROLLER_GEN_VERSION)
 
 KUSTOMIZE_VERSION = $(shell $(YQ) -r '.kustomize' < $(TOOLS_VERSIONS_FILE))
 KUSTOMIZE = $(PROJECT_DIR)/bin/installs/kustomize/$(KUSTOMIZE_VERSION)/bin/kustomize

controller/controlplane/controller.go

@@ -228,10 +228,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	log.Trace(logger, "validating ControlPlane resource conditions", cp)
 	if r.ensureIsMarkedScheduled(cp) {
-		err := r.patchStatus(ctx, logger, cp)
+		res, err := r.patchStatus(ctx, logger, cp)
 		if err != nil {
+			log.Debug(logger, "unable to update ControlPlane resource", cp, "error", err)
+			return res, err
+		}
+		if res.Requeue {
 			log.Debug(logger, "unable to update ControlPlane resource", cp)
-			return ctrl.Result{}, err
+			return res, nil
 		}
 
 		log.Debug(logger, "ControlPlane resource now marked as scheduled", cp)
@@ -401,11 +405,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	if res != op.Noop {
 		if !dataplaneIsSet {
 			log.Debug(logger, "DataPlane not set, deployment for ControlPlane has been scaled down to 0 replicas", cp)
-			err := r.patchStatus(ctx, logger, cp)
+			res, err := r.patchStatus(ctx, logger, cp)
 			if err != nil {
-				log.Debug(logger, "unable to reconcile ControlPlane status", cp)
+				log.Debug(logger, "unable to reconcile ControlPlane status", cp, "error", err)
+				return ctrl.Result{}, err
 			}
-			return ctrl.Result{}, err
+			if res.Requeue {
+				log.Debug(logger, "unable to update ControlPlane resource", cp)
+				return res, nil
+			}
+			return ctrl.Result{}, nil
 		}
 		return ctrl.Result{}, nil // requeue will be triggered by the creation or update of the owned object
 	}
@@ -418,16 +427,31 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			k8sutils.NewCondition(k8sutils.ReadyType, metav1.ConditionFalse, k8sutils.WaitingToBecomeReadyReason, k8sutils.WaitingToBecomeReadyMessage),
 			cp,
 		)
-		return ctrl.Result{}, r.patchStatus(ctx, logger, cp)
+
+		res, err := r.patchStatus(ctx, logger, cp)
+		if err != nil {
+			log.Debug(logger, "unable to patch ControlPlane status", cp, "error", err)
+			return ctrl.Result{}, err
+		}
+		if res.Requeue {
+			log.Debug(logger, "unable to patch ControlPlane status", cp)
+			return res, nil
+		}
+		return ctrl.Result{}, nil
 	}
 
 	markAsProvisioned(cp)
 	k8sutils.SetReady(cp)
 
-	if err = r.patchStatus(ctx, logger, cp); err != nil {
-		log.Debug(logger, "unable to reconcile the ControlPlane resource", cp)
+	result, err := r.patchStatus(ctx, logger, cp)
+	if err != nil {
+		log.Debug(logger, "unable to patch ControlPlane status", cp, "error", err)
 		return ctrl.Result{}, err
 	}
+	if result.Requeue {
+		log.Debug(logger, "unable to patch ControlPlane status", cp)
+		return result, nil
+	}
 
 	log.Debug(logger, "reconciliation complete for ControlPlane resource", cp)
 	return ctrl.Result{}, nil
@@ -444,18 +468,25 @@ func validateControlPlane(controlPlane *operatorv1beta1.ControlPlane, devMode bo
 }
 
 // patchStatus Patches the resource status only when there are changes in the Conditions
-func (r *Reconciler) patchStatus(ctx context.Context, logger logr.Logger, updated *operatorv1beta1.ControlPlane) error {
+func (r *Reconciler) patchStatus(ctx context.Context, logger logr.Logger, updated *operatorv1beta1.ControlPlane) (ctrl.Result, error) {
 	current := &operatorv1beta1.ControlPlane{}
 
 	err := r.Client.Get(ctx, client.ObjectKeyFromObject(updated), current)
 	if err != nil && !k8serrors.IsNotFound(err) {
-		return err
+		return ctrl.Result{}, err
 	}
 
 	if k8sutils.NeedsUpdate(current, updated) {
 		log.Debug(logger, "patching ControlPlane status", updated, "status", updated.Status)
-		return r.Client.Status().Patch(ctx, updated, client.MergeFrom(current))
+		if err := r.Client.Status().Patch(ctx, updated, client.MergeFrom(current)); err != nil {
+			if k8serrors.IsConflict(err) {
+				log.Debug(logger, "conflict found when updating ControlPlane, retrying", current)
+				return ctrl.Result{Requeue: true, RequeueAfter: requeueWithoutBackoff}, nil
+			}
+			return ctrl.Result{}, fmt.Errorf("failed updating ControlPlane's status : %w", err)
+		}
+		return ctrl.Result{}, nil
 	}
 
-	return nil
+	return ctrl.Result{}, nil
 }

controller/gateway/controller.go

@@ -119,7 +119,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	if cpFinalizerSet || dpFinalizerSet || npFinalizerSet {
 		log.Trace(logger, "Setting finalizers", gateway)
 		if err := r.Client.Update(ctx, &gateway); err != nil {
-			return ctrl.Result{}, fmt.Errorf("failed updating Gateway's finalizers: %w", err)
+			res, err := handleGatewayFinalizerPatchOrUpdateError(err, &gateway, logger)
+			if err != nil {
+				return res, fmt.Errorf("failed updating Gateway's finalizers: %w", err)
+			}
+			if res.Requeue {
+				return res, nil
+			}
 		}
 		return ctrl.Result{}, nil
 	}
@@ -147,7 +153,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	gwConditionAware.initListenersStatus()
 	gwConditionAware.setConflicted()
 	gwConditionAware.setAccepted()
-	gwConditionAware.initReadyAndProgrammed()
+	gwConditionAware.initProgrammedAndListenersStatus()
 	if err := gwConditionAware.setResolvedRefsAndSupportedKinds(ctx, r.Client); err != nil {
 		return ctrl.Result{}, err
 	}
@@ -340,7 +346,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			gatewayConditionsAndListenersAware(&gateway))
 	}
 
-	gwConditionAware.setReadyAndProgrammed()
+	gwConditionAware.setProgrammedAndListenersConditions()
 	res, err := patch.ApplyGatewayStatusPatchIfNotEmpty(ctx, r.Client, logger, &gateway, oldGateway)
 	if err != nil {
 		return ctrl.Result{}, err

controller/gateway/controller_cleanup.go

@@ -5,6 +5,8 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -18,6 +20,8 @@ import (
 // Reconciler - Cleanup
 // ----------------------------------------------------------------------------
 
+const requeueWithoutBackoff = 200 * time.Millisecond
+
 // cleanup determines whether cleanup is needed/underway for a Gateway and
 // performs all necessary cleanup steps. Namely, it cleans up resources
 // managed on behalf of the Gateway and removes the finalizers once all
@@ -64,9 +68,9 @@ func (r *Reconciler) cleanup(
 	} else {
 		oldGateway := gateway.DeepCopy()
 		if controllerutil.RemoveFinalizer(gateway, string(GatewayFinalizerCleanupControlPlanes)) {
-			err := r.Client.Patch(ctx, gateway, client.MergeFrom(oldGateway))
-			if err != nil {
-				return true, ctrl.Result{}, err
+			if err := r.Client.Patch(ctx, gateway, client.MergeFrom(oldGateway)); err != nil {
+				res, err := handleGatewayFinalizerPatchOrUpdateError(err, gateway, logger)
+				return true, res, err
 			}
 			log.Debug(logger, "finalizer for cleaning up controlplanes removed", gateway)
 			return true, ctrl.Result{}, nil
@@ -91,9 +95,9 @@ func (r *Reconciler) cleanup(
 	} else {
 		oldGateway := gateway.DeepCopy()
 		if controllerutil.RemoveFinalizer(gateway, string(GatewayFinalizerCleanupDataPlanes)) {
-			err := r.Client.Patch(ctx, gateway, client.MergeFrom(oldGateway))
-			if err != nil {
-				return true, ctrl.Result{}, err
+			if err := r.Client.Patch(ctx, gateway, client.MergeFrom(oldGateway)); err != nil {
+				res, err := handleGatewayFinalizerPatchOrUpdateError(err, gateway, logger)
+				return true, res, err
 			}
 			log.Debug(logger, "finalizer for cleaning up dataplanes removed", gateway)
 			return true, ctrl.Result{}, nil
@@ -117,9 +121,9 @@ func (r *Reconciler) cleanup(
 	} else {
 		oldGateway := gateway.DeepCopy()
 		if controllerutil.RemoveFinalizer(gateway, string(GatewayFinalizerCleanupNetworkpolicies)) {
-			err := r.Client.Patch(ctx, gateway, client.MergeFrom(oldGateway))
-			if err != nil {
-				return true, ctrl.Result{}, err
+			if err := r.Client.Patch(ctx, gateway, client.MergeFrom(oldGateway)); err != nil {
+				res, err := handleGatewayFinalizerPatchOrUpdateError(err, gateway, logger)
+				return true, res, err
 			}
 			log.Debug(logger, "finalizer for cleaning up network policies removed", gateway)
 			return true, ctrl.Result{}, nil
@@ -129,3 +133,32 @@ func (r *Reconciler) cleanup(
 	log.Debug(logger, "owned resources cleanup completed", gateway)
 	return true, ctrl.Result{}, nil
 }
+
+func handleGatewayFinalizerPatchOrUpdateError(err error, gateway *gatewayv1.Gateway, logger logr.Logger) (ctrl.Result, error) {
+	// Short cirtcuit.
+	if err == nil {
+		return ctrl.Result{}, nil
+	}
+
+	// If the Gateway is not found, then requeue without an error.
+	if k8serrors.IsNotFound(err) {
+		return ctrl.Result{
+			Requeue:      true,
+			RequeueAfter: requeueWithoutBackoff,
+		}, nil
+	}
+	// Since controllers use cached clients, it's possible that the Gateway is out of sync with what
+	// is in the API server and this causes:
+	// Forbidden: no new finalizers can be added if the object is being deleted, found new finalizers []string{...}
+	// Code below handles that gracefully to not show users the errors that are not actionable.
+	if cause, ok := k8serrors.StatusCause(err, metav1.CauseTypeForbidden); k8serrors.IsInvalid(err) && ok {
+		log.Debug(logger, "failed to delete a finalizer on Gateway, requeueing request", gateway, "cause", cause)
+		return ctrl.Result{
+			Requeue:      true,
+			RequeueAfter: requeueWithoutBackoff,
+		}, nil
+	}
+
+	// Return the error as is.
+	return ctrl.Result{}, err
+}

controller/gateway/controller_reconciler_utils.go

@@ -536,12 +536,11 @@ func supportedRoutesByProtocol() map[gatewayv1.ProtocolType]map[gatewayv1.Kind]s
 	}
 }
 
-// initReadyAndProgrammed initializes the gateway Programmed and Ready conditions
-// by setting the underlying Gateway Programmed and Ready status to false.
-// Furthermore, it sets the supportedKinds and initializes the readiness to false with reason
-// Pending for each Gateway listener.
-func (g *gatewayConditionsAndListenersAwareT) initReadyAndProgrammed() {
-	k8sutils.InitReady(g)
+// initProgrammedAndListenersStatus initializes the gateway Programmed condition
+// by setting the underlying Gateway Programmed status to false.
+// It also sets the listeners Programmed condition by setting the underlying
+// Listener Programmed status to false.
+func (g *gatewayConditionsAndListenersAwareT) initProgrammedAndListenersStatus() {
 	k8sutils.InitProgrammed(g)
 	for i := range g.Spec.Listeners {
 		lStatus := listenerConditionsAware(&g.Status.Listeners[i])
@@ -635,12 +634,11 @@ func (g *gatewayConditionsAndListenersAwareT) setConflicted() {
 	}
 }
 
-// setReadyAndProgrammed sets the gateway Programmed and Ready conditions by
-// setting the underlying Gateway Programmed and Ready status to true.
-// Furthermore, it sets the supportedKinds and initializes the readiness to true with reason
-// Ready or false with reason Invalid for each Gateway listener.
-func (g *gatewayConditionsAndListenersAwareT) setReadyAndProgrammed() {
-	k8sutils.SetReady(g)
+// setProgrammedAndListenersConditions sets the gateway Programmed condition by setting the underlying
+// Gateway Programmed status to true.
+// It also sets the listeners Programmed condition by setting the underlying
+// Listener Programmed status to true.
+func (g *gatewayConditionsAndListenersAwareT) setProgrammedAndListenersConditions() {
 	k8sutils.SetProgrammed(g)
 
 	for i := range g.Spec.Listeners {

controller/gateway/controller_test.go

@@ -181,7 +181,7 @@ func TestGatewayReconciler_Reconcile(t *testing.T) {
 
 				var currentGateway gwtypes.Gateway
 				require.NoError(t, reconciler.Client.Get(ctx, gatewayReq.NamespacedName, &currentGateway))
-				require.False(t, k8sutils.IsReady(gatewayConditionsAndListenersAware(&currentGateway)))
+				require.False(t, k8sutils.IsProgrammed(gatewayConditionsAndListenersAware(&currentGateway)))
 				condition, found := k8sutils.GetCondition(GatewayServiceType, gatewayConditionsAndListenersAware(&currentGateway))
 				require.True(t, found)
 				require.Equal(t, condition.Status, metav1.ConditionFalse)
@@ -200,7 +200,7 @@ func TestGatewayReconciler_Reconcile(t *testing.T) {
 				require.NoError(t, err, "reconciliation returned an error")
 				// the dataplane service now has a clusterIP assigned, the gateway must be ready
 				require.NoError(t, reconciler.Client.Get(ctx, gatewayReq.NamespacedName, &currentGateway))
-				require.True(t, k8sutils.IsReady(gatewayConditionsAndListenersAware(&currentGateway)))
+				require.True(t, k8sutils.IsProgrammed(gatewayConditionsAndListenersAware(&currentGateway)))
 				condition, found = k8sutils.GetCondition(GatewayServiceType, gatewayConditionsAndListenersAware(&currentGateway))
 				require.True(t, found)
 				require.Equal(t, condition.Status, metav1.ConditionTrue)
@@ -234,7 +234,7 @@ func TestGatewayReconciler_Reconcile(t *testing.T) {
 				_, err = reconciler.Reconcile(ctx, gatewayReq)
 				require.NoError(t, err, "reconciliation returned an error")
 				require.NoError(t, reconciler.Client.Get(ctx, gatewayReq.NamespacedName, &currentGateway))
-				require.True(t, k8sutils.IsReady(gatewayConditionsAndListenersAware(&currentGateway)))
+				require.True(t, k8sutils.IsProgrammed(gatewayConditionsAndListenersAware(&currentGateway)))
 				condition, found = k8sutils.GetCondition(GatewayServiceType, gatewayConditionsAndListenersAware(&currentGateway))
 				require.True(t, found)
 				require.Equal(t, condition.Status, metav1.ConditionTrue)
@@ -267,7 +267,7 @@ func TestGatewayReconciler_Reconcile(t *testing.T) {
 				_, err = reconciler.Reconcile(ctx, gatewayReq)
 				require.NoError(t, err, "reconciliation returned an error")
 				require.NoError(t, reconciler.Client.Get(ctx, gatewayReq.NamespacedName, &currentGateway))
-				require.True(t, k8sutils.IsReady(gatewayConditionsAndListenersAware(&currentGateway)))
+				require.True(t, k8sutils.IsProgrammed(gatewayConditionsAndListenersAware(&currentGateway)))
 				condition, found = k8sutils.GetCondition(GatewayServiceType, gatewayConditionsAndListenersAware(&currentGateway))
 				require.True(t, found)
 				require.Equal(t, condition.Status, metav1.ConditionTrue)
@@ -289,7 +289,7 @@ func TestGatewayReconciler_Reconcile(t *testing.T) {
 				require.NoError(t, reconciler.Client.Get(ctx, gatewayReq.NamespacedName, &currentGateway))
 				// the dataplane service has no clusterIP assigned, the gateway must be not ready
 				// and no addresses must be assigned
-				require.False(t, k8sutils.IsReady(gatewayConditionsAndListenersAware(&currentGateway)))
+				require.False(t, k8sutils.IsProgrammed(gatewayConditionsAndListenersAware(&currentGateway)))
 				condition, found = k8sutils.GetCondition(GatewayServiceType, gatewayConditionsAndListenersAware(&currentGateway))
 				require.True(t, found)
 				require.Equal(t, condition.Status, metav1.ConditionFalse)

debug.Dockerfile

@@ -2,7 +2,7 @@
 # Debug image
 # ------------------------------------------------------------------------------
 
-FROM --platform=$BUILDPLATFORM golang:1.22.2 as debug
+FROM --platform=$BUILDPLATFORM golang:1.22.3 as debug
 
 ARG GOPATH
 ARG GOCACHE