diff --git a/test/e2e/cluster_status_spec.go b/test/e2e/cluster_status_spec.go index f17a138..c66c420 100644 --- a/test/e2e/cluster_status_spec.go +++ b/test/e2e/cluster_status_spec.go @@ -39,8 +39,8 @@ var _ = Describe("Cluster Status Reporter", Serial, utils.GinkgoLabelStatusReporter, utils.GinkgoLabelNoGPU, func() { const ( - // Allow a couple of resyncs plus startup grace and rollout slack. - emitTimeout = 3 * time.Minute + // The recovery check requires no fresh Warning over this window. + quietWindow = 3 * time.Minute clearTimeout = 5 * time.Minute bbrDeployment = "body-based-router" @@ -64,10 +64,17 @@ var _ = Describe("Cluster Status Reporter", gatewayAuthNamespace string localCSINamespace string kaitoNamespace string + // emitTimeout bounds the Warning for a Deployment scaled to zero. + // It follows the running reporter's resync and startup grace: with + // the chart's 60s grace, 3 minutes was only just enough. + emitTimeout time.Duration ) BeforeEach(func() { ctx = context.Background() + timings, err := utils.GetReporterTimings(ctx) + Expect(err).NotTo(HaveOccurred()) + emitTimeout = timings.DebouncedEmitTimeout() for _, target := range []struct { namespace *string deployment string @@ -123,7 +130,7 @@ var _ = Describe("Cluster Status Reporter", By("waiting for clusterBBRNotReady to stop firing once BBR is restored") recoverBaseline, err := utils.ReporterEventBaseline(ctx, "clusterBBRNotReady", "") Expect(err).NotTo(HaveOccurred()) - Expect(utils.EnsureNoReporterEventSince(ctx, "clusterBBRNotReady", "", recoverBaseline, emitTimeout)). + Expect(utils.EnsureNoReporterEventSince(ctx, "clusterBBRNotReady", "", recoverBaseline, quietWindow)). To(Succeed()) }) diff --git a/test/e2e/control_plane_error_spec.go b/test/e2e/control_plane_error_spec.go index f4f313f..2afd0e8 100644 --- a/test/e2e/control_plane_error_spec.go +++ b/test/e2e/control_plane_error_spec.go @@ -133,7 +133,11 @@ var _ = Describe("Control-plane Error Reporter", By("scaling the EPP Deployment to zero") Expect(utils.ScaleDeployment(ctx, ns, eppName, 0)).To(Succeed()) - ev, err := utils.WaitForReporterEventSince(ctx, "inferencesetEPPNotReady", ns, metav1Zero, emitTimeout) + // A Deployment scaled to zero is debounced by the reporter's + // startup grace, which an install may leave at the chart default. + timings, err := utils.GetReporterTimings(ctx) + Expect(err).NotTo(HaveOccurred()) + ev, err := utils.WaitForReporterEventSince(ctx, "inferencesetEPPNotReady", ns, metav1Zero, timings.DebouncedEmitTimeout()) Expect(err).NotTo(HaveOccurred()) Expect(ev.Message).To(ContainSubstring(isName)) Expect(ev.Message).To(ContainSubstring("scaled to zero")) diff --git a/test/e2e/utils/events.go b/test/e2e/utils/events.go index d16f3f9..d8e94e0 100644 --- a/test/e2e/utils/events.go +++ b/test/e2e/utils/events.go @@ -40,40 +40,98 @@ const ( // installed (kaito-system for the E2E install, kube-system on AI Manager). reporterSelector = "app.kubernetes.io/name=productionstack-status-reporter" - // defaultReporterEventRefresh is the chart's eventRefreshSeconds default, - // used when the reporter runs without --event-refresh-seconds. - defaultReporterEventRefresh = 300 * time.Second + // reporterEmitSlack covers what DebouncedEmitTimeout does not model: the + // scaled-down pod terminating before the Deployment counts as podless, the + // evaluation pass itself, and the Event write. + reporterEmitSlack = 90 * time.Second ) -// ReporterEventRefreshInterval returns how often the running reporter rewrites -// a still-active Warning (--event-refresh-seconds). The E2E install shortens it -// to 120s, while a managed install may keep the chart default. -func ReporterEventRefreshInterval(ctx context.Context) (time.Duration, error) { +// ReporterTimings are the running reporter's timing flags. Fields default to the +// chart values when the reporter runs without the flag. +type ReporterTimings struct { + // Resync is how often every reason is re-evaluated (--resync-seconds). + Resync time.Duration + // StartupGrace is how long a finding without a backing timestamp, such as + // a Deployment scaled to zero, must persist before it is published + // (--startup-grace-seconds). + StartupGrace time.Duration + // EventRefresh is how often a still-active Warning is rewritten + // (--event-refresh-seconds). + EventRefresh time.Duration +} + +// defaultReporterTimings are the productionstack-status-reporter chart defaults. +// The E2E install shortens StartupGrace to 30s and EventRefresh to 120s; other +// installs may keep these. +var defaultReporterTimings = ReporterTimings{ + Resync: 60 * time.Second, + StartupGrace: 60 * time.Second, + EventRefresh: 300 * time.Second, +} + +// DebouncedEmitTimeout bounds how long the reporter takes to publish a finding +// it debounces by first observation, such as a Deployment scaled to zero. It +// first sees the problem on the next resync, withholds it until a later resync +// finds it has persisted for StartupGrace, and when StartupGrace is a multiple +// of Resync, tick jitter can push that to one resync later still. +func (t ReporterTimings) DebouncedEmitTimeout() time.Duration { + graceTicks := (t.StartupGrace + t.Resync - 1) / t.Resync + return t.Resync + graceTicks*t.Resync + t.Resync + reporterEmitSlack +} + +// GetReporterTimings reads the timing flags of the running reporter Deployment. +func GetReporterTimings(ctx context.Context) (ReporterTimings, error) { cs, err := GetK8sClientset() if err != nil { - return 0, err + return ReporterTimings{}, err } list, err := cs.AppsV1().Deployments(metav1.NamespaceAll).List(ctx, metav1.ListOptions{LabelSelector: reporterSelector}) if err != nil { - return 0, fmt.Errorf("list reporter Deployments: %w", err) + return ReporterTimings{}, fmt.Errorf("list reporter Deployments: %w", err) } if len(list.Items) != 1 { - return 0, fmt.Errorf("expected one Deployment matching %q, found %d", reporterSelector, len(list.Items)) + return ReporterTimings{}, fmt.Errorf("expected one Deployment matching %q, found %d", reporterSelector, len(list.Items)) } + var args []string for _, c := range list.Items[0].Spec.Template.Spec.Containers { - for _, arg := range c.Args { - value, ok := strings.CutPrefix(arg, "--event-refresh-seconds=") + args = append(args, c.Args...) + } + return reporterTimingsFromArgs(args) +} + +func reporterTimingsFromArgs(args []string) (ReporterTimings, error) { + timings := defaultReporterTimings + flags := map[string]*time.Duration{ + "--resync-seconds=": &timings.Resync, + "--startup-grace-seconds=": &timings.StartupGrace, + "--event-refresh-seconds=": &timings.EventRefresh, + } + for _, arg := range args { + for prefix, field := range flags { + value, ok := strings.CutPrefix(arg, prefix) if !ok { continue } seconds, err := strconv.Atoi(value) - if err != nil || seconds <= 0 { - return 0, fmt.Errorf("invalid reporter flag %q", arg) + // The reporter accepts a zero startup grace; the other two must be positive. + if err != nil || seconds < 0 || (seconds == 0 && prefix != "--startup-grace-seconds=") { + return ReporterTimings{}, fmt.Errorf("invalid reporter flag %q", arg) } - return time.Duration(seconds) * time.Second, nil + *field = time.Duration(seconds) * time.Second } } - return defaultReporterEventRefresh, nil + return timings, nil +} + +// ReporterEventRefreshInterval returns how often the running reporter rewrites +// a still-active Warning (--event-refresh-seconds). The E2E install shortens it +// to 120s, while other installs may keep the chart default. +func ReporterEventRefreshInterval(ctx context.Context) (time.Duration, error) { + timings, err := GetReporterTimings(ctx) + if err != nil { + return 0, err + } + return timings.EventRefresh, nil } // ListReporterEvents returns every Event in kube-system emitted by the diff --git a/test/e2e/utils/events_test.go b/test/e2e/utils/events_test.go new file mode 100644 index 0000000..416bac0 --- /dev/null +++ b/test/e2e/utils/events_test.go @@ -0,0 +1,99 @@ +/* +Copyright 2026 The KAITO Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + "testing" + "time" +) + +func TestReporterTimingsFromArgs(t *testing.T) { + tests := []struct { + name string + args []string + want ReporterTimings + wantErr bool + }{ + { + name: "chart defaults when the flags are absent", + args: []string{"--leader-elect"}, + want: ReporterTimings{Resync: 60 * time.Second, StartupGrace: 60 * time.Second, EventRefresh: 300 * time.Second}, + }, + { + name: "E2E install", + args: []string{"--resync-seconds=60", "--startup-grace-seconds=30", "--event-refresh-seconds=120"}, + want: ReporterTimings{Resync: 60 * time.Second, StartupGrace: 30 * time.Second, EventRefresh: 120 * time.Second}, + }, + { + name: "zero startup grace is allowed", + args: []string{"--startup-grace-seconds=0"}, + want: ReporterTimings{Resync: 60 * time.Second, StartupGrace: 0, EventRefresh: 300 * time.Second}, + }, + {name: "zero resync", args: []string{"--resync-seconds=0"}, wantErr: true}, + {name: "negative grace", args: []string{"--startup-grace-seconds=-1"}, wantErr: true}, + {name: "not a number", args: []string{"--event-refresh-seconds=5m"}, wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := reporterTimingsFromArgs(tt.args) + if (err != nil) != tt.wantErr { + t.Fatalf("err = %v, wantErr %v", err, tt.wantErr) + } + if !tt.wantErr && got != tt.want { + t.Fatalf("got %+v, want %+v", got, tt.want) + } + }) + } +} + +func TestDebouncedEmitTimeout(t *testing.T) { + tests := []struct { + name string + timings ReporterTimings + want time.Duration + }{ + // Regression: with the chart's 60s grace, a fixed 3m wait timed out + // on clusterBBRNotReady. + { + name: "chart defaults", + timings: ReporterTimings{Resync: 60 * time.Second, StartupGrace: 60 * time.Second}, + want: 270 * time.Second, + }, + { + name: "E2E install rounds the grace up to one resync", + timings: ReporterTimings{Resync: 60 * time.Second, StartupGrace: 30 * time.Second}, + want: 270 * time.Second, + }, + { + name: "grace longer than one resync", + timings: ReporterTimings{Resync: 60 * time.Second, StartupGrace: 90 * time.Second}, + want: 330 * time.Second, + }, + { + name: "no grace", + timings: ReporterTimings{Resync: 30 * time.Second}, + want: 150 * time.Second, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.timings.DebouncedEmitTimeout(); got != tt.want { + t.Fatalf("got %s, want %s", got, tt.want) + } + }) + } +}