feat(gateway-provisioning): Fixes the intermittent "Degraded" status bug

components/control-plane/internal/reconciler/health.go

@@ -30,6 +30,8 @@ const defaultHealthInterval = 30 * time.Second
 // specs/platform/openshell-gateway-routing.spec.md § Gateway Exposure Configuration.
 const defaultRouteReadyTimeout = 10 * time.Minute
 
+const defaultDeploymentReadyTimeout = 10 * time.Minute
+
 // routeVerifyInterval is the minimum time between residual route/console
 // absence re-checks for a settled (torn-down, addressless) gateway.
 //
@@ -52,15 +54,16 @@ type GatewayHealthReconciler struct {
 	// clusterID scopes the health sweep to this managed cluster's gateways. When
 	// non-empty the fleet list is filtered server-side so a spoke never stamps
 	// (Degraded/Running) a gateway owned by another cluster. Empty sweeps all.
-	clusterID           string
-	interval            time.Duration
-	exposure            exposure.Port
-	routeReadyTimeout   time.Duration
-	keycloakConfig      *gateway.KeycloakConfig
-	isOpenShift         bool
-	hasGatewayAPI       bool
-	ingressMode         string
-	skipNetworkPolicies bool
+	clusterID              string
+	interval               time.Duration
+	exposure               exposure.Port
+	routeReadyTimeout      time.Duration
+	deploymentReadyTimeout time.Duration
+	keycloakConfig         *gateway.KeycloakConfig
+	isOpenShift            bool
+	hasGatewayAPI          bool
+	ingressMode            string
+	skipNetworkPolicies    bool
 
 	// consoleClientChecker is a single, long-lived Keycloak client reused across
 	// every tick's residual-absence checks. Constructed once (when Keycloak is
@@ -71,8 +74,8 @@ type GatewayHealthReconciler struct {
 	// client needs no additional synchronization.
 	consoleClientChecker gateway.ConsoleClientChecker
 
-	// now is the clock, overridable in tests.
-	now func() time.Time
+	now                   func() time.Time
+	deploymentReadinessFn func(ctx context.Context, clientset kubernetes.Interface, namespace, name string) (bool, string, error)
 
 	// routeNotReadySince records, per gateway, when its Deployment first became
 	// Ready while its external exposure was not, so the route-readiness grace
@@ -94,10 +97,11 @@ type GatewayHealthReconciler struct {
 	// settled gateway keeps being re-verified forever at that low cadence, because
 	// elapsed wall-clock time is not proof that a stale provisioning pass cannot
 	// still resurrect resources (see routeVerifyInterval).
-	mu                 sync.Mutex
-	routeNotReadySince map[string]time.Time
-	routeTornDown      map[string]bool
-	routeVerifiedAt    map[string]time.Time
+	mu                      sync.Mutex
+	routeNotReadySince      map[string]time.Time
+	deploymentNotReadySince map[string]time.Time
+	routeTornDown           map[string]bool
+	routeVerifiedAt         map[string]time.Time
 }
 
 func NewGatewayHealthReconciler(clientset *kubernetes.Clientset, dynamicClient dynamic.Interface, grpcConn *grpc.ClientConn, exposurePort exposure.Port, keycloakConfig *gateway.KeycloakConfig, clusterID string) *GatewayHealthReconciler {
@@ -118,23 +122,26 @@ func NewGatewayHealthReconciler(clientset *kubernetes.Clientset, dynamicClient d
 	hasGatewayAPI := gateway.DetectGatewayAPI(clientset)
 	ingressMode := gateway.IngressMode(hasGatewayAPI, isOpenShift)
 	return &GatewayHealthReconciler{
-		clientset:            clientset,
-		dynamicClient:        dynamicClient,
-		grpcConn:             grpcConn,
-		clusterID:            clusterID,
-		interval:             defaultHealthInterval,
-		exposure:             exposurePort,
-		routeReadyTimeout:    routeReadyTimeout(),
-		keycloakConfig:       keycloakConfig,
-		consoleClientChecker: consoleClientChecker,
-		isOpenShift:          isOpenShift,
-		hasGatewayAPI:        hasGatewayAPI,
-		ingressMode:          ingressMode,
-		skipNetworkPolicies:  os.Getenv("GATEWAY_SKIP_NETWORK_POLICIES") == "true",
-		now:                  time.Now,
-		routeNotReadySince:   make(map[string]time.Time),
-		routeTornDown:        make(map[string]bool),
-		routeVerifiedAt:      make(map[string]time.Time),
+		clientset:               clientset,
+		dynamicClient:           dynamicClient,
+		grpcConn:                grpcConn,
+		clusterID:               clusterID,
+		interval:                defaultHealthInterval,
+		exposure:                exposurePort,
+		routeReadyTimeout:       routeReadyTimeout(),
+		deploymentReadyTimeout:  deploymentReadyTimeout(),
+		keycloakConfig:          keycloakConfig,
+		consoleClientChecker:    consoleClientChecker,
+		isOpenShift:             isOpenShift,
+		hasGatewayAPI:           hasGatewayAPI,
+		ingressMode:             ingressMode,
+		skipNetworkPolicies:     os.Getenv("GATEWAY_SKIP_NETWORK_POLICIES") == "true",
+		now:                     time.Now,
+		deploymentReadinessFn:   gateway.DeploymentReadiness,
+		routeNotReadySince:      make(map[string]time.Time),
+		deploymentNotReadySince: make(map[string]time.Time),
+		routeTornDown:           make(map[string]bool),
+		routeVerifiedAt:         make(map[string]time.Time),
 	}
 }
 
@@ -150,6 +157,16 @@ func routeReadyTimeout() time.Duration {
 	return defaultRouteReadyTimeout
 }
 
+func deploymentReadyTimeout() time.Duration {
+	if v := os.Getenv("GATEWAY_DEPLOYMENT_READY_TIMEOUT"); v != "" {
+		if d, err := time.ParseDuration(v); err == nil && d > 0 {
+			return d
+		}
+		log.Printf("WARN invalid GATEWAY_DEPLOYMENT_READY_TIMEOUT %q; using default %s", v, defaultDeploymentReadyTimeout)
+	}
+	return defaultDeploymentReadyTimeout
+}
+
 // Run drives the health reconciliation loop until the context is cancelled.
 func (h *GatewayHealthReconciler) Run(ctx context.Context) error {
 	log.Printf("INFO gateway health reconciler started (interval=%s routeReadyTimeout=%s ingressMode=%s)", h.interval, h.routeReadyTimeout, h.ingressMode)
@@ -233,7 +250,7 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl
 		log.Printf("WARN gateway health: %s: %v", gatewayID, err)
 		return
 	}
-	ready, reason, err := gateway.DeploymentReadiness(ctx, h.clientset, namespace, gateway.GatewayDeploymentName)
+	ready, reason, err := h.deploymentReadinessFn(ctx, h.clientset, namespace, gateway.GatewayDeploymentName)
 	if err != nil {
 		log.Printf("WARN gateway health: %s: %v", gatewayID, err)
 		return
@@ -242,23 +259,30 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl
 	var desiredPhase, desiredStatus string
 	switch {
 	case !ready:
-		// The Deployment has not been created yet; the provisioning path still
-		// owns this gateway. Leave its phase untouched.
 		if reason == "deployment not found" {
 			return
 		}
 		h.clearRouteTimer(gatewayID)
-		desiredPhase, desiredStatus = string(gatewayhealth.PhaseDegraded), reason
+		if phase == string(gatewayhealth.PhaseProvisioning) {
+			since := h.markDeploymentNotReady(gatewayID)
+			if h.now().Sub(since) >= h.deploymentReadyTimeout {
+				h.clearDeploymentTimer(gatewayID)
+				desiredPhase, desiredStatus = string(gatewayhealth.PhaseDegraded), fmt.Sprintf("deployment not ready after %s: %s", h.deploymentReadyTimeout, reason)
+			} else {
+				desiredPhase, desiredStatus = string(gatewayhealth.PhaseProvisioning), reason
+			}
+		} else {
+			h.clearDeploymentTimer(gatewayID)
+			desiredPhase, desiredStatus = string(gatewayhealth.PhaseDegraded), reason
+		}
 	case h.exposure != nil && isRoutedGateway(gw):
-		// Deployment is Ready; a routed gateway additionally requires its external
-		// exposure to be observed Ready before it can be Running.
+		h.clearDeploymentTimer(gatewayID)
 		desiredPhase, desiredStatus = h.evaluateRouteReadiness(ctx, gatewayID, namespace, phase)
 		if desiredPhase == "" {
-			// Transient error observing the exposure; leave the phase untouched
-			// rather than flap the gateway.
 			return
 		}
 	default:
+		h.clearDeploymentTimer(gatewayID)
 		h.clearRouteTimer(gatewayID)
 		desiredPhase, desiredStatus = string(gatewayhealth.PhaseRunning), gatewayhealth.StatusHealthy
 	}
@@ -575,3 +599,20 @@ func (h *GatewayHealthReconciler) clearRouteTimer(gatewayID string) {
 	defer h.mu.Unlock()
 	delete(h.routeNotReadySince, gatewayID)
 }
+
+func (h *GatewayHealthReconciler) markDeploymentNotReady(gatewayID string) time.Time {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	if t, ok := h.deploymentNotReadySince[gatewayID]; ok {
+		return t
+	}
+	t := h.now()
+	h.deploymentNotReadySince[gatewayID] = t
+	return t
+}
+
+func (h *GatewayHealthReconciler) clearDeploymentTimer(gatewayID string) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	delete(h.deploymentNotReadySince, gatewayID)
+}

components/control-plane/internal/reconciler/health_test.go

@@ -16,6 +16,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	dynamicfake "k8s.io/client-go/dynamic/fake"
+	"k8s.io/client-go/kubernetes"
 	k8sfake "k8s.io/client-go/kubernetes/fake"
 )
 
@@ -36,10 +37,12 @@ func (f fakeExposure) ObserveReadiness(context.Context, exposure.Request) (expos
 
 func newHealthRec(exp exposure.Port, now func() time.Time, timeout time.Duration) *GatewayHealthReconciler {
 	return &GatewayHealthReconciler{
-		exposure:           exp,
-		routeReadyTimeout:  timeout,
-		now:                now,
-		routeNotReadySince: make(map[string]time.Time),
+		exposure:                exp,
+		routeReadyTimeout:       timeout,
+		deploymentReadyTimeout:  timeout,
+		now:                     now,
+		routeNotReadySince:      make(map[string]time.Time),
+		deploymentNotReadySince: make(map[string]time.Time),
 	}
 }
 
@@ -676,3 +679,95 @@ func TestListAllGateways_Empty(t *testing.T) {
 		t.Fatalf("expected 1 call, got %d", callCount)
 	}
 }
+
+func fakeDeploymentReadiness(ready bool, reason string) func(context.Context, kubernetes.Interface, string, string) (bool, string, error) {
+	return func(context.Context, kubernetes.Interface, string, string) (bool, string, error) {
+		return ready, reason, nil
+	}
+}
+
+func newHealthRecForDeploymentTest(now func() time.Time, timeout time.Duration, readyFn func(context.Context, kubernetes.Interface, string, string) (bool, string, error)) *GatewayHealthReconciler {
+	h := newHealthRec(nil, now, timeout)
+	h.deploymentReadinessFn = readyFn
+	h.routeTornDown = map[string]bool{"gw-1": true}
+	h.routeVerifiedAt = map[string]time.Time{"gw-1": now()}
+	return h
+}
+
+func TestReconcileGatewayHealth_ProvisioningDeploymentNotReadyWithinGraceStaysProvisioning(t *testing.T) {
+	var gotPhase string
+	client := &fakeGatewayClient{updateFn: func(_ context.Context, req *pb.UpdateGatewayRequest, _ ...grpc.CallOption) (*pb.UpdateGatewayResponse, error) {
+		if req.Phase != nil {
+			gotPhase = *req.Phase
+		}
+		return &pb.UpdateGatewayResponse{}, nil
+	}}
+
+	phase := "Provisioning"
+	gw := &pb.Gateway{
+		Metadata:  &pb.ObjectReference{Id: "gw-1"},
+		Phase:     &phase,
+		Namespace: "openshell-abc",
+	}
+
+	h := newHealthRecForDeploymentTest(fixedClock(time.Unix(1000, 0)), 10*time.Minute, fakeDeploymentReadiness(false, "0/1 replicas ready"))
+	h.reconcileGatewayHealth(context.Background(), client, gw)
+
+	if gotPhase != "Provisioning" {
+		t.Fatalf("got phase %q, want Provisioning (within grace window)", gotPhase)
+	}
+}
+
+func TestReconcileGatewayHealth_ProvisioningDeploymentNotReadyBeyondGraceBecomesDegraded(t *testing.T) {
+	var gotPhase string
+	client := &fakeGatewayClient{updateFn: func(_ context.Context, req *pb.UpdateGatewayRequest, _ ...grpc.CallOption) (*pb.UpdateGatewayResponse, error) {
+		if req.Phase != nil {
+			gotPhase = *req.Phase
+		}
+		return &pb.UpdateGatewayResponse{}, nil
+	}}
+
+	phase := "Provisioning"
+	gw := &pb.Gateway{
+		Metadata:  &pb.ObjectReference{Id: "gw-1"},
+		Phase:     &phase,
+		Namespace: "openshell-abc",
+	}
+
+	cur := time.Unix(1000, 0)
+	h := newHealthRecForDeploymentTest(func() time.Time { return cur }, 10*time.Minute, fakeDeploymentReadiness(false, "0/1 replicas ready"))
+
+	h.reconcileGatewayHealth(context.Background(), client, gw)
+
+	cur = cur.Add(11 * time.Minute)
+	h.routeVerifiedAt["gw-1"] = cur
+	h.reconcileGatewayHealth(context.Background(), client, gw)
+
+	if gotPhase != "Degraded" {
+		t.Fatalf("got phase %q, want Degraded after grace window expired", gotPhase)
+	}
+}
+
+func TestReconcileGatewayHealth_RunningLosesDeploymentReadinessBecomesDegradedImmediately(t *testing.T) {
+	var gotPhase string
+	client := &fakeGatewayClient{updateFn: func(_ context.Context, req *pb.UpdateGatewayRequest, _ ...grpc.CallOption) (*pb.UpdateGatewayResponse, error) {
+		if req.Phase != nil {
+			gotPhase = *req.Phase
+		}
+		return &pb.UpdateGatewayResponse{}, nil
+	}}
+
+	phase := "Running"
+	gw := &pb.Gateway{
+		Metadata:  &pb.ObjectReference{Id: "gw-1"},
+		Phase:     &phase,
+		Namespace: "openshell-abc",
+	}
+
+	h := newHealthRecForDeploymentTest(fixedClock(time.Unix(1000, 0)), 10*time.Minute, fakeDeploymentReadiness(false, "0/1 replicas ready"))
+	h.reconcileGatewayHealth(context.Background(), client, gw)
+
+	if gotPhase != "Degraded" {
+		t.Fatalf("got phase %q, want Degraded immediately (no grace for Running)", gotPhase)
+	}
+}

specs/platform/openshell-gateway-health.spec.md

@@ -121,6 +121,17 @@ non-empty `.status.addresses`.
 - AND it SHALL set the `phase` to `Degraded`
 - AND it SHALL record the reason in `status`
 
+### Requirement: Deployment Readiness Grace Configuration
+
+| Variable | Default | Description |
+|---|---|---|
+| `GATEWAY_DEPLOYMENT_READY_TIMEOUT` | `10m` | Grace window a Provisioning gateway's Deployment may remain not-Ready before the control plane transitions the `phase` to `Degraded`. A gateway that had already reached `Running` and then loses Deployment readiness is moved to `Degraded` immediately, with no grace. |
+
+The grace window governs only the `Provisioning -> Degraded` transition for a
+Deployment that never becomes Ready; it is not a hard deadline that stops
+observation. The control plane SHALL keep observing the Deployment after the
+window elapses, so a gateway that eventually becomes ready returns to `Running`.
+
 #### Scenario: Provisioning fails to apply
 
 - GIVEN a Gateway being reconciled