diff --git a/README.md b/README.md index 1b242309..d0af76f5 100644 --- a/README.md +++ b/README.md @@ -181,6 +181,8 @@ The API server exposes Prometheus metrics on its metrics port (default `:8080/me |---|---|---| | `hypershell_gateways_total{phase="Running"|"Provisioning"|"Degraded"|"Failed"}` | Gauge | Number of gateways by phase. Queried live from the database on each scrape. | +The control plane also exports `gateway.provision.duration` through OTLP. This histogram measures the time in seconds from Gateway creation to its first successful `Running` phase. A standard Prometheus conversion exposes it as `gateway_provision_duration_seconds`. + ### Grafana dashboard A pre-built Grafana dashboard is provided at `dashboards/hypershell-dashboard.yml`. It is packaged as a Kubernetes ConfigMap with the `grafana_dashboard: "true"` label so it is picked up automatically by the Grafana sidecar. diff --git a/components/control-plane/internal/otel/metrics.go b/components/control-plane/internal/otel/metrics.go index 3384f70d..48345381 100644 --- a/components/control-plane/internal/otel/metrics.go +++ b/components/control-plane/internal/otel/metrics.go @@ -10,9 +10,10 @@ import ( ) var ( - reconcileDuration metric.Int64Histogram - reconcileErrors metric.Int64Counter - watchReconnects metric.Int64Counter + reconcileDuration metric.Int64Histogram + gatewayProvisionDuration metric.Float64Histogram + reconcileErrors metric.Int64Counter + watchReconnects metric.Int64Counter ) func registerMetrics() error { @@ -28,6 +29,16 @@ func registerMetrics() error { return err } + gatewayProvisionDuration, err = meter.Float64Histogram( + "gateway.provision.duration", + metric.WithUnit("s"), + metric.WithDescription("Time from Gateway creation until its first successful transition to Running"), + metric.WithExplicitBucketBoundaries(1, 5, 10, 15, 30, 45, 60, 90, 120, 180, 300, 600, 900), + ) + if err != nil { + return err + } + reconcileErrors, err = meter.Int64Counter( "reconcile.errors", metric.WithUnit("{error}"), @@ -56,6 +67,15 @@ func RecordReconcileDuration(ctx context.Context, kind, eventType string, start )) } +// RecordGatewayProvisionDuration records one successful create-to-Running +// duration. The caller owns the one-observation rule for each Gateway. +func RecordGatewayProvisionDuration(ctx context.Context, duration time.Duration) { + if gatewayProvisionDuration == nil || duration < 0 { + return + } + gatewayProvisionDuration.Record(ctx, duration.Seconds()) +} + // RecordReconcileError increments the reconcile error counter. func RecordReconcileError(ctx context.Context, kind string) { if reconcileErrors == nil { diff --git a/components/control-plane/internal/otel/metrics_test.go b/components/control-plane/internal/otel/metrics_test.go new file mode 100644 index 00000000..cc14a0fa --- /dev/null +++ b/components/control-plane/internal/otel/metrics_test.go @@ -0,0 +1,83 @@ +package otel + +import ( + "context" + "reflect" + "testing" + "time" + + "go.opentelemetry.io/otel" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" +) + +func TestRecordGatewayProvisionDuration(t *testing.T) { + previousProvider := otel.GetMeterProvider() + previousReconcileDuration := reconcileDuration + previousGatewayProvisionDuration := gatewayProvisionDuration + previousReconcileErrors := reconcileErrors + previousWatchReconnects := watchReconnects + + reader := sdkmetric.NewManualReader() + provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + otel.SetMeterProvider(provider) + reconcileDuration = nil + gatewayProvisionDuration = nil + reconcileErrors = nil + watchReconnects = nil + t.Cleanup(func() { + otel.SetMeterProvider(previousProvider) + reconcileDuration = previousReconcileDuration + gatewayProvisionDuration = previousGatewayProvisionDuration + reconcileErrors = previousReconcileErrors + watchReconnects = previousWatchReconnects + _ = provider.Shutdown(context.Background()) + }) + + if err := registerMetrics(); err != nil { + t.Fatalf("registerMetrics() returned an error: %v", err) + } + + RecordGatewayProvisionDuration(context.Background(), 37_500*time.Millisecond) + RecordGatewayProvisionDuration(context.Background(), -1*time.Second) + + var collected metricdata.ResourceMetrics + if err := reader.Collect(context.Background(), &collected); err != nil { + t.Fatalf("Collect() returned an error: %v", err) + } + + for _, scope := range collected.ScopeMetrics { + for _, gotMetric := range scope.Metrics { + if gotMetric.Name != "gateway.provision.duration" { + continue + } + if gotMetric.Unit != "s" { + t.Fatalf("metric unit = %q, want s", gotMetric.Unit) + } + histogram, ok := gotMetric.Data.(metricdata.Histogram[float64]) + if !ok { + t.Fatalf("metric data type = %T, want float64 histogram", gotMetric.Data) + } + if len(histogram.DataPoints) != 1 { + t.Fatalf("data point count = %d, want 1", len(histogram.DataPoints)) + } + point := histogram.DataPoints[0] + if point.Count != 1 { + t.Fatalf("sample count = %d, want 1", point.Count) + } + if point.Sum != 37.5 { + t.Fatalf("sample sum = %v, want 37.5", point.Sum) + } + if point.Attributes.Len() != 0 { + t.Fatalf("metric attributes = %v, want none", point.Attributes) + } + wantBounds := []float64{1, 5, 10, 15, 30, 45, 60, 90, 120, 180, 300, 600, 900} + if !reflect.DeepEqual(point.Bounds, wantBounds) { + t.Fatalf("bucket bounds = %v, want %v", point.Bounds, wantBounds) + } + return + } + } + + t.Fatal("gateway.provision.duration metric was not collected") +} diff --git a/components/control-plane/internal/reconciler/gateway_vocabulary.go b/components/control-plane/internal/reconciler/gateway_vocabulary.go new file mode 100644 index 00000000..e4c19799 --- /dev/null +++ b/components/control-plane/internal/reconciler/gateway_vocabulary.go @@ -0,0 +1,11 @@ +package reconciler + +// These values are the shared Gateway health vocabulary for the reconciler and +// health paths. Keep phase comparisons and phase updates on this vocabulary. +const ( + gatewayPhaseProvisioning = "Provisioning" + gatewayPhaseRunning = "Running" + gatewayPhaseDegraded = "Degraded" + gatewayPhaseFailed = "Failed" + gatewayStatusHealthy = "Healthy" +) diff --git a/components/control-plane/internal/reconciler/health.go b/components/control-plane/internal/reconciler/health.go index 351f2c3a..7de61b5c 100644 --- a/components/control-plane/internal/reconciler/health.go +++ b/components/control-plane/internal/reconciler/health.go @@ -223,7 +223,7 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl // observable workload. Leave Pending gateways to the provisioning path and // Failed gateways to a subsequent spec change. switch phase { - case "Running", "Degraded", "Provisioning": + case gatewayPhaseRunning, gatewayPhaseDegraded, gatewayPhaseProvisioning: default: return } @@ -273,7 +273,7 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl return } h.clearRouteTimer(gatewayID) - desiredPhase, desiredStatus = "Degraded", reason + desiredPhase, desiredStatus = gatewayPhaseDegraded, 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. @@ -285,7 +285,7 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl } default: h.clearRouteTimer(gatewayID) - desiredPhase, desiredStatus = "Running", "Healthy" + desiredPhase, desiredStatus = gatewayPhaseRunning, gatewayStatusHealthy } // active_sandbox_count is maintained independently by the event-driven @@ -297,10 +297,14 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl return } - if _, err := client.UpdateGateway(ctx, update); err != nil { + response, err := client.UpdateGateway(ctx, update) + if err != nil { log.Printf("WARN gateway health: update %s to %s: %v", gatewayID, desiredPhase, err) return } + if isGatewayProvisionCompletion(phase, desiredPhase) { + observeGatewayProvisionDuration(ctx, response.GetGateway()) + } log.Printf("INFO gateway health: %s %s -> %s (%s)", gatewayID, phase, desiredPhase, desiredStatus) } @@ -314,7 +318,7 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl // observations retain normal ownership of phase and status so operational // failures remain visible. func observedGatewayHealthUpdate(gatewayID, currentPhase, currentStatus, desiredPhase, desiredStatus string, keycloakConfigured bool) *pb.UpdateGatewayRequest { - if keycloakConfigured && isGatewayKeycloakClientStatus(currentStatus) && desiredPhase == "Running" && desiredStatus == "Healthy" { + if keycloakConfigured && isGatewayKeycloakClientStatus(currentStatus) && desiredPhase == gatewayPhaseRunning && desiredStatus == gatewayStatusHealthy { if currentPhase == desiredPhase { return nil } @@ -560,21 +564,21 @@ func (h *GatewayHealthReconciler) evaluateRouteReadiness(ctx context.Context, ga } if rr.Ready { h.clearRouteTimer(gatewayID) - return "Running", "Healthy" + return gatewayPhaseRunning, gatewayStatusHealthy } - if currentPhase == "Provisioning" { + if currentPhase == gatewayPhaseProvisioning { since := h.markRouteNotReady(gatewayID) if h.now().Sub(since) >= h.routeReadyTimeout { h.clearRouteTimer(gatewayID) - return "Degraded", fmt.Sprintf("route not ready after %s: %s", h.routeReadyTimeout, rr.Reason) + return gatewayPhaseDegraded, fmt.Sprintf("route not ready after %s: %s", h.routeReadyTimeout, rr.Reason) } - return "Provisioning", rr.Reason + return gatewayPhaseProvisioning, rr.Reason } // currentPhase is Running (lost readiness) or Degraded (still unhealthy). h.clearRouteTimer(gatewayID) - return "Degraded", rr.Reason + return gatewayPhaseDegraded, rr.Reason } // markRouteNotReady records the first time the gateway's Deployment was observed diff --git a/components/control-plane/internal/reconciler/metrics.go b/components/control-plane/internal/reconciler/metrics.go new file mode 100644 index 00000000..3028f473 --- /dev/null +++ b/components/control-plane/internal/reconciler/metrics.go @@ -0,0 +1,82 @@ +package reconciler + +import ( + "context" + "sync" + "time" + + pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + cpotel "github.com/openshift-online/hypershell/components/control-plane/internal/otel" +) + +// observedGatewayProvisions coordinates the event-driven and health reconcile +// paths. These paths can observe the same first Running transition at the same +// time. The Gateway identifier stays in process memory and is not an OTel +// attribute. +var observedGatewayProvisions sync.Map + +// observeGatewayProvisionDuration records the time from API creation until the +// first successful transition to Running. It ignores incomplete or invalid +// timestamps so telemetry cannot change reconciliation behavior. +func observeGatewayProvisionDuration(ctx context.Context, gw *pb.Gateway) { + duration, ok := gatewayProvisionDuration(gw) + if !ok { + return + } + gatewayID := gw.GetMetadata().GetId() + if !claimGatewayProvisionObservation(gatewayID) { + return + } + cpotel.RecordGatewayProvisionDuration(ctx, duration) +} + +func claimGatewayProvisionObservation(gatewayID string) bool { + if gatewayID == "" { + return false + } + _, loaded := observedGatewayProvisions.LoadOrStore(gatewayID, struct{}{}) + return !loaded +} + +func forgetGatewayProvisionObservation(gatewayID string) { + observedGatewayProvisions.Delete(gatewayID) +} + +// suppressGatewayProvisionObservation prevents work on a previously Running or +// Degraded Gateway from producing a new provision observation. The caller uses +// the stored phase for a normal event. The retry adapter supplies the phase that +// it cleared when it bypasses the phase gate. +func suppressGatewayProvisionObservation(gatewayID, previousPhase string) { + if previousPhase == gatewayPhaseRunning || previousPhase == gatewayPhaseDegraded { + claimGatewayProvisionObservation(gatewayID) + } +} + +func gatewayProvisionDuration(gw *pb.Gateway) (time.Duration, bool) { + if gw == nil || gw.GetMetadata() == nil { + return 0, false + } + createdAt := gw.GetMetadata().GetCreatedAt() + runningAt := gw.GetMetadata().GetUpdatedAt() + if createdAt == nil || runningAt == nil { + return 0, false + } + if err := createdAt.CheckValid(); err != nil { + return 0, false + } + if err := runningAt.CheckValid(); err != nil { + return 0, false + } + duration := runningAt.AsTime().Sub(createdAt.AsTime()) + if duration < 0 { + return 0, false + } + return duration, true +} + +// isGatewayProvisionCompletion excludes later recovery transitions. A new +// gateway can stay in Provisioning after the first reconcile while its route +// becomes ready. A Running gateway that fails moves through Degraded instead. +func isGatewayProvisionCompletion(currentPhase, desiredPhase string) bool { + return currentPhase == gatewayPhaseProvisioning && desiredPhase == gatewayPhaseRunning +} diff --git a/components/control-plane/internal/reconciler/metrics_test.go b/components/control-plane/internal/reconciler/metrics_test.go new file mode 100644 index 00000000..0c3aa4a8 --- /dev/null +++ b/components/control-plane/internal/reconciler/metrics_test.go @@ -0,0 +1,153 @@ +package reconciler + +import ( + "sync" + "sync/atomic" + "testing" + "time" + + pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + "google.golang.org/protobuf/types/known/timestamppb" +) + +func TestGatewayProvisionDuration(t *testing.T) { + runningAt := time.Date(2026, time.September, 3, 20, 0, 0, 0, time.UTC) + + tests := []struct { + name string + gateway *pb.Gateway + want time.Duration + wantValid bool + }{ + { + name: "valid creation time", + gateway: &pb.Gateway{Metadata: &pb.ObjectReference{ + CreatedAt: timestamppb.New(runningAt.Add(-2 * time.Minute)), + UpdatedAt: timestamppb.New(runningAt), + }}, + want: 2 * time.Minute, + wantValid: true, + }, + {name: "missing gateway", gateway: nil}, + {name: "missing metadata", gateway: &pb.Gateway{}}, + { + name: "missing creation time", + gateway: &pb.Gateway{Metadata: &pb.ObjectReference{ + UpdatedAt: timestamppb.New(runningAt), + }}, + }, + { + name: "missing update time", + gateway: &pb.Gateway{Metadata: &pb.ObjectReference{ + CreatedAt: timestamppb.New(runningAt.Add(-2 * time.Minute)), + }}, + }, + { + name: "invalid creation time", + gateway: &pb.Gateway{Metadata: &pb.ObjectReference{ + CreatedAt: ×tamppb.Timestamp{Seconds: 253402300800}, + UpdatedAt: timestamppb.New(runningAt), + }}, + }, + { + name: "invalid update time", + gateway: &pb.Gateway{Metadata: &pb.ObjectReference{ + CreatedAt: timestamppb.New(runningAt.Add(-2 * time.Minute)), + UpdatedAt: ×tamppb.Timestamp{Seconds: 253402300800}, + }}, + }, + { + name: "update before creation", + gateway: &pb.Gateway{Metadata: &pb.ObjectReference{ + CreatedAt: timestamppb.New(runningAt.Add(time.Minute)), + UpdatedAt: timestamppb.New(runningAt), + }}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, valid := gatewayProvisionDuration(test.gateway) + if valid != test.wantValid || got != test.want { + t.Fatalf("gatewayProvisionDuration() = (%s, %v), want (%s, %v)", got, valid, test.want, test.wantValid) + } + }) + } +} + +func TestIsGatewayProvisionCompletion(t *testing.T) { + tests := []struct { + current string + desired string + want bool + }{ + {current: "Provisioning", desired: "Running", want: true}, + {current: "Degraded", desired: "Running", want: false}, + {current: "Running", desired: "Running", want: false}, + {current: "Provisioning", desired: "Degraded", want: false}, + } + + for _, test := range tests { + if got := isGatewayProvisionCompletion(test.current, test.desired); got != test.want { + t.Errorf("isGatewayProvisionCompletion(%q, %q) = %v, want %v", test.current, test.desired, got, test.want) + } + } +} + +func TestClaimGatewayProvisionObservation(t *testing.T) { + const gatewayID = "gateway-provision-observation-test" + forgetGatewayProvisionObservation(gatewayID) + t.Cleanup(func() { forgetGatewayProvisionObservation(gatewayID) }) + + var claims atomic.Int32 + var group sync.WaitGroup + for range 100 { + group.Add(1) + go func() { + defer group.Done() + if claimGatewayProvisionObservation(gatewayID) { + claims.Add(1) + } + }() + } + group.Wait() + + if got := claims.Load(); got != 1 { + t.Fatalf("successful claims = %d, want 1", got) + } + if claimGatewayProvisionObservation("") { + t.Fatal("empty Gateway identifier was accepted") + } + + forgetGatewayProvisionObservation(gatewayID) + if !claimGatewayProvisionObservation(gatewayID) { + t.Fatal("claim after Gateway deletion was rejected") + } +} + +func TestSuppressGatewayProvisionObservation(t *testing.T) { + tests := []struct { + phase string + suppressed bool + }{ + {phase: "Running", suppressed: true}, + {phase: "Degraded", suppressed: true}, + {phase: "Provisioning", suppressed: false}, + {phase: "Failed", suppressed: false}, + {phase: "", suppressed: false}, + } + + for _, test := range tests { + t.Run(test.phase, func(t *testing.T) { + gatewayID := "gateway-recovery-" + test.phase + forgetGatewayProvisionObservation(gatewayID) + t.Cleanup(func() { forgetGatewayProvisionObservation(gatewayID) }) + + suppressGatewayProvisionObservation(gatewayID, test.phase) + claimed := claimGatewayProvisionObservation(gatewayID) + if got := !claimed; got != test.suppressed { + t.Fatalf("suppressed = %v, want %v", got, test.suppressed) + } + }) + } +} diff --git a/components/control-plane/internal/reconciler/reconciler.go b/components/control-plane/internal/reconciler/reconciler.go index 20aec087..c7bc5af8 100644 --- a/components/control-plane/internal/reconciler/reconciler.go +++ b/components/control-plane/internal/reconciler/reconciler.go @@ -1252,6 +1252,11 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. log.Printf("WARN gateway event %s has nil resource, skipping", event.ResourceID) return nil } + previousPhase := gw.GetPhase() + if event.PhaseBeforeRetry != "" { + previousPhase = event.PhaseBeforeRetry + } + suppressGatewayProvisionObservation(event.ResourceID, previousPhase) ctx, endSpan := cpotel.StartReconcileSpan(ctx, "Gateway", event.Type.String()) span := trace.SpanFromContext(ctx) @@ -1260,6 +1265,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. defer func() { endSpan(reconcileErr) }() if event.Type == watcher.EventDeleted { + forgetGatewayProvisionObservation(event.ResourceID) var deleteDBConfig databaseConfig var deleteErrs []error if gw.DatabaseId != "" { @@ -1358,7 +1364,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. // gated. In particular, controller startup seeds existing Running gateways; // reconciling before the return below lets newly introduced client settings // converge without forcing a full gateway rollout. - if gw.Phase != nil && (*gw.Phase == "Running" || *gw.Phase == "Provisioning" || *gw.Phase == "Degraded") { + if gw.Phase != nil && (*gw.Phase == gatewayPhaseRunning || *gw.Phase == gatewayPhaseProvisioning || *gw.Phase == gatewayPhaseDegraded) { if err := r.reconcileExistingGatewayKeycloakClient(ctx, event.ResourceID, gw); err != nil { var identityErr *gatewayKeycloakClientIdentityError if errors.As(err, &identityErr) { @@ -1498,10 +1504,10 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. RouteStillDesired: r.makeRouteStillDesired(event.ResourceID), } - r.updateGatewayPhase(ctx, event.ResourceID, "Provisioning") + r.updateGatewayPhase(ctx, event.ResourceID, gatewayPhaseProvisioning) if err := gateway.ReconcileGateway(ctx, r.dynamicClient, r.clientset, nsConfig, r.manifests, opts); err != nil { - r.updateGatewayPhase(ctx, event.ResourceID, "Failed") + r.updateGatewayPhase(ctx, event.ResourceID, gatewayPhaseFailed) reconcileErr = fmt.Errorf("reconcile gateway %s: %w", gw.Name, err) return reconcileErr } @@ -1511,7 +1517,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. // Deployment never becomes ready, set Degraded and record why. ready, reason := gateway.WaitForGatewayReady(ctx, r.clientset, namespace, 2*time.Minute) if !ready { - r.updateGatewayHealth(ctx, event.ResourceID, "Degraded", reason) + r.updateGatewayHealth(ctx, event.ResourceID, gatewayPhaseDegraded, reason) log.Printf("WARN gateway %s applied but not ready in namespace %s: %s", gw.Name, namespace, reason) return nil } @@ -1530,14 +1536,20 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. routed := isRoutedGateway(gw) if r.exposure != nil && routed { if r.waitForRouteReady(ctx, namespace) { - r.updateGatewayHealth(ctx, event.ResourceID, "Running", "Healthy") + // The observation guard rejects work that started in Running or Degraded. + if runningGateway := r.updateGatewayHealth(ctx, event.ResourceID, gatewayPhaseRunning, gatewayStatusHealthy); runningGateway != nil { + observeGatewayProvisionDuration(ctx, runningGateway) + } log.Printf("INFO gateway %s provisioned and route ready in namespace %s", gw.Name, namespace) } else { - r.updateGatewayHealth(ctx, event.ResourceID, "Provisioning", "Deployment ready; awaiting route readiness") + r.updateGatewayHealth(ctx, event.ResourceID, gatewayPhaseProvisioning, "Deployment ready; awaiting route readiness") log.Printf("INFO gateway %s deployment ready in namespace %s; awaiting route readiness", gw.Name, namespace) } } else { - r.updateGatewayHealth(ctx, event.ResourceID, "Running", "Healthy") + // The observation guard rejects work that started in Running or Degraded. + if runningGateway := r.updateGatewayHealth(ctx, event.ResourceID, gatewayPhaseRunning, gatewayStatusHealthy); runningGateway != nil { + observeGatewayProvisionDuration(ctx, runningGateway) + } log.Printf("INFO gateway %s provisioned and ready in namespace %s", gw.Name, namespace) } @@ -1901,17 +1913,20 @@ func listAllGateways(ctx context.Context, client pb.GatewayServiceClient) ([]*pb // updateGatewayHealth sets the Gateway `phase` and `status` together in a single // gRPC update so the console and CLI observe a consistent lifecycle state and -// health descriptor. -func (r *GatewayReconciler) updateGatewayHealth(ctx context.Context, gatewayID, phase, status string) { +// health descriptor. It returns the stored Gateway on success so callers can +// use the API server timestamps. It returns nil if the update fails. +func (r *GatewayReconciler) updateGatewayHealth(ctx context.Context, gatewayID, phase, status string) *pb.Gateway { client := pb.NewGatewayServiceClient(r.grpcConn) - _, err := client.UpdateGateway(ctx, &pb.UpdateGatewayRequest{ + response, err := client.UpdateGateway(ctx, &pb.UpdateGatewayRequest{ Id: gatewayID, Phase: &phase, Status: &status, }) if err != nil { log.Printf("WARN failed to update gateway %s health to %s (%s): %v", gatewayID, phase, status, err) + return nil } + return response.GetGateway() } func (r *GatewayReconciler) updateGatewayPhase(ctx context.Context, gatewayID string, phase string) { diff --git a/components/control-plane/internal/watcher/seed_test.go b/components/control-plane/internal/watcher/seed_test.go index 1dc98332..255eb8da 100644 --- a/components/control-plane/internal/watcher/seed_test.go +++ b/components/control-plane/internal/watcher/seed_test.go @@ -385,3 +385,23 @@ func TestSeedGateways_KeepsAbsentWhenConfirmFails(t *testing.T) { t.Fatalf("pruned %v, want none (absence unproven when confirmation fails)", sink.pruned) } } + +func TestClearGatewayPhaseForRetryPreservesOriginalPhase(t *testing.T) { + original := gw("gateway-1", "Degraded") + event := Event[*pb.Gateway]{ + Type: EventUpdated, + ResourceID: "gateway-1", + Resource: original, + } + + got := clearGatewayPhaseForRetry(event) + if got.PhaseBeforeRetry != "Degraded" { + t.Fatalf("phase before retry = %q, want Degraded", got.PhaseBeforeRetry) + } + if got.Resource.GetPhase() != "" { + t.Fatalf("retry phase = %q, want empty", got.Resource.GetPhase()) + } + if original.GetPhase() != "Degraded" { + t.Fatalf("source phase = %q, want Degraded", original.GetPhase()) + } +} diff --git a/components/control-plane/internal/watcher/watcher.go b/components/control-plane/internal/watcher/watcher.go index 0fbe677d..79562510 100644 --- a/components/control-plane/internal/watcher/watcher.go +++ b/components/control-plane/internal/watcher/watcher.go @@ -43,6 +43,9 @@ type Event[T any] struct { Type EventType ResourceID string Resource T + // PhaseBeforeRetry contains the Gateway phase that the retry adapter + // cleared. Other resource types leave this field empty. + PhaseBeforeRetry string } type Handler[T any] interface { @@ -487,13 +490,16 @@ func WatchGateways(ctx context.Context, conn *grpc.ClientConn, handler Handler[* // record a terminal phase. Clearing the phase -- and only on retries -- restores // the gate-bypassing recovery the watch stream cannot provide, while the rest of // the payload still reflects the latest observed spec so an un-routed gateway is -// torn down, not resurrected. proto.Clone avoids mutating the shared latest entry -// (and copying the message value, which vet forbids). +// torn down, not resurrected. The event keeps the original phase so the +// reconciler can distinguish a recovery from a first provision. proto.Clone +// avoids mutating the shared latest entry (and copying the message value, which +// vet forbids). func clearGatewayPhaseForRetry(ev Event[*pb.Gateway]) Event[*pb.Gateway] { if ev.Resource == nil { return ev } clone := proto.Clone(ev.Resource).(*pb.Gateway) + ev.PhaseBeforeRetry = clone.GetPhase() clone.Phase = nil ev.Resource = clone return ev diff --git a/skills/RECONCILE.md b/skills/RECONCILE.md index 780ded52..bc9b28db 100644 --- a/skills/RECONCILE.md +++ b/skills/RECONCILE.md @@ -48,9 +48,9 @@ skills/ ## Reconciliation State -**Last analyzed**: 2026-08-31 (Keycloak event-storm KC-ES-W1 complete; OpenShift local-dev lifecycle and manual e2e driver features rebased in from 2026-08-25/27) +**Last analyzed**: 2026-09-03 (scoped analysis of the CP-OBS-07 Gateway provision-duration changes; the last full-corpus analysis remains 2026-08-31) **Spec corpus**: 40 spec files; the coverage table tracks 32 analyzed feature/spec groups after adding OpenShell Gateway Console and OpenShift Development -**Codebase commit**: working tree (Keycloak event-storm KC-ES-W1 + OpenShift local-dev) +**Codebase commit**: `b97a99d` (CP-OBS-GPD-W1 complete) ### Coverage Summary @@ -100,6 +100,26 @@ Layer 7: web-console/architecture (depends on data-model, security, UI ## Gap Table +### control-plane-observability.spec.md (CP-OBS-07 Gateway provision-duration delta) + +| # | Requirement | Status | Gap | Code Location | Wave | +|---|-------------|--------|-----|---------------|------| +| CP-OBS-07a | Export `gateway.provision.duration` as a histogram in seconds, with explicit buckets from 1 second through 15 minutes | Present | - | `components/control-plane/internal/otel/metrics.go`, `metrics_test.go` | CP-OBS-GPD-W1 | +| CP-OBS-07b | Use the stored Gateway `created_at` and `updated_at` values, and ignore missing, invalid, or reversed values | Present | - | `components/control-plane/internal/reconciler/metrics.go`, `metrics_test.go`; `components/api-server/plugins/gateways/grpc_presenter.go` | CP-OBS-GPD-W1 | +| CP-OBS-07c | Record only after a successful direct update to `Running` | Present | - | `components/control-plane/internal/reconciler/reconciler.go` | CP-OBS-GPD-W1 | +| CP-OBS-07d | Record a delayed `Provisioning` to `Running` transition, but do not record a `Degraded` to `Running` recovery | Present | - | `components/control-plane/internal/reconciler/health.go`, `metrics.go`, `gateway_vocabulary.go`; `components/control-plane/internal/watcher/watcher.go` | CP-OBS-GPD-W1 | +| CP-OBS-07e | Record at most one observation for one Gateway and do not export a Gateway identifier as a metric attribute | Present | - | `components/control-plane/internal/reconciler/metrics.go`, `metrics_test.go`; `components/control-plane/internal/otel/metrics_test.go` | CP-OBS-GPD-W1 | + +**Scoped coverage:** 5 of 5 changed CP-OBS-07 fields are present. This scoped run does not change the full-corpus coverage table. + +**Direction checks:** + +- Spec to code: The metric contract, both promotion paths, timestamp rules, recovery rule, single-observation rule, and attribute rule are present. +- Code to spec: All new provision-duration behavior is in CP-OBS-07. The exact intermediate bucket values are implementation details inside the specified range. +- OpenAPI to spec: The metric does not add a public API field. The existing `ObjectReference` contract supplies `created_at` and `updated_at`, and the gRPC presenter returns both fields after an update. + +The first gap analysis found a race between the event-driven reconciler and the health reconciler. Both paths could record the first `Running` transition. CP-OBS-GPD-W1 adds one process-wide claim per Gateway. The delete path removes the claim. A normal reconcile uses the stored phase before work starts. A forced retry keeps the phase that existed before the retry bypass. These checks prevent work on a `Running` or `Degraded` Gateway from producing a new observation. + ### openshell-gateway-console.spec.md | # | Requirement | Status | Gap | Code Location | Wave | @@ -479,6 +499,19 @@ The OpenShift e2e driver (`tests/e2e/drivers/openshift.sh`) remains a gap for HY ## Wave Plan +### CP-OBS-GPD-W1: Reconcile Gateway provision-duration metrics ✅ + +**Scope:** Changed CP-OBS-07 fields only | **Status:** Complete + +1. Add the histogram name, unit, description, and explicit buckets. +2. Use API server timestamps from the successful `Running` update response. +3. Record direct and delayed first promotions. +4. Exclude `Degraded` recoveries, including forced recovery retries. +5. Coordinate the two promotion paths so only one path records the observation. +6. Verify that the metric has no Gateway identifier attribute. + +**CP-OBS-GPD-W1 summary:** The first implementation met the metric, timestamp, and transition contracts. The scoped reconcile found and closed one concurrent-recording gap. A per-Gateway claim now coordinates the event-driven and health paths. The direct path checks the stored phase before work starts. The retry adapter keeps the phase that existed before it bypasses the phase gate. Recovery and later desired-state work cannot look like a new provision. Package constants now keep the Gateway phase and healthy-status values consistent. Focused race tests pass. + ### KC-ES-W1: Stop the Keycloak event storm **Scope:** OI-7, SA-14 @@ -739,6 +772,7 @@ label-selected pod informer. | Date | Commit | Action | Coverage | Notes | |------|--------|--------|----------|-------| +| 2026-09-03 | `b97a99d` | Reconciled the CP-OBS-07 Gateway provision-duration delta | 5/5 scoped fields present | Found and closed a duplicate-observation race between the event-driven and health promotion paths. Added a concurrent claim, stored-phase and forced-recovery checks, shared package constants, delete cleanup, timestamp tests, bucket tests, and a no-attribute test. The full-corpus percentage is unchanged. | | 2026-08-31 | working tree | Completed Keycloak event-storm KC-ES-W1 | 82% | Corrected the token lifetime unit, reused tokens until the 80 percent threshold, accepted the provider-managed service-account scope, rejected all other client scopes, and added regression tests. OI-7 and SA-14 are present. | | 2026-08-31 | 9ac4354 | Keycloak event-storm scoped gap analysis | 82% | Found two partial requirements: the token cache uses nanoseconds for `expires_in`, and service-account convergence rejects Keycloak's built-in scope. Planned control-plane wave KC-ES-W1. | | 2026-08-27 | 9984ed0 | Completed Gateway Console GC-W1 | 82% | Added mode-selected Route exposure, admission readiness, lifecycle cleanup, custom-host RBAC, and tests. All nine console requirements are present. | diff --git a/specs/platform/control-plane-observability.spec.md b/specs/platform/control-plane-observability.spec.md index b1064deb..456f065f 100644 --- a/specs/platform/control-plane-observability.spec.md +++ b/specs/platform/control-plane-observability.spec.md @@ -199,9 +199,12 @@ The control plane SHALL export OpenTelemetry metrics for reconciliation and watc | Metric | Type | Unit | Description | |--------|------|------|-------------| | `reconcile.duration` | Histogram | `ms` | Latency of a single resource reconciliation | +| `gateway.provision.duration` | Histogram | `s` | Time from Gateway creation until its first successful transition to `Running` | | `reconcile.errors` | Counter | `{error}` | Count of failed reconciliations | | `watch.reconnects` | Counter | `{reconnect}` | Count of watch stream reconnections | +The control plane SHALL record one `gateway.provision.duration` observation only after the Gateway phase update to `Running` succeeds. The initial reconcile path SHALL record a direct transition to `Running`. The health reconcile path SHALL record a delayed transition from `Provisioning` to `Running`. It SHALL NOT record a later recovery from `Degraded` to `Running` as a new provision. The duration SHALL use the `created_at` and `updated_at` values in the stored Gateway that the API server returns. It SHALL ignore missing, invalid, or reversed timestamps. The metric SHALL NOT contain a Gateway identifier. Its explicit bucket boundaries SHALL cover 1 second through 15 minutes. + Metrics SHALL complement any future Prometheus metrics endpoint and SHALL NOT prevent adding one later. **Verification:** Trigger reconciliations and watch reconnections; confirm the duration histogram records reconcile latency, the error counter increments on failure, and the reconnect counter increments on watch stream reconnection, all labeled by resource kind. @@ -220,6 +223,15 @@ Metrics SHALL complement any future Prometheus metrics endpoint and SHALL NOT pr - THEN `reconcile.errors` SHALL be incremented - AND the sample SHALL be labeled with the resource kind +#### Scenario: Gateway provision duration recorded + +- GIVEN a Gateway has a valid creation time +- AND the Gateway has not reached `Running` +- WHEN the control plane successfully changes its phase to `Running` +- THEN `gateway.provision.duration` SHALL record the time from creation to that phase change in seconds +- AND a later recovery from `Degraded` to `Running` SHALL NOT record another observation +- AND the metric SHALL NOT contain the Gateway identifier + #### Scenario: Watch reconnect metric incremented - GIVEN the OTel SDK is initialized