diff --git a/config/deployment.yaml b/config/deployment.yaml index 393d912c6..c17b48193 100644 --- a/config/deployment.yaml +++ b/config/deployment.yaml @@ -46,8 +46,11 @@ spec: - --author-attribution-grace=10s # A short refresh interval so the e2e suite can watch an idle GitTarget notice a branch # somebody moved from outside, without waiting out the 10m default. The effective - # granularity is still the 5m control-plane reconcile. - - --git-refresh-interval=30s + # granularity is still the 5m control-plane reconcile. Kept above the 1m status.remote + # publication floor, as the default is: below it, a reconcile inside the floor refreshes + # an observation that has not been published yet, and our own push is published as a + # fetch. + - --git-refresh-interval=90s # The push receiver, so the e2e suite can deliver a Git host's push webhook through the # gitops-reverser-receiver Service (receiver-service.yaml) instead of waiting for a refresh. - --receiver-bind-address=:9292 diff --git a/docs/spec/e2e-bi-directional-corner.md b/docs/spec/e2e-bi-directional-corner.md index ddb801bc7..3843f6db0 100644 --- a/docs/spec/e2e-bi-directional-corner.md +++ b/docs/spec/e2e-bi-directional-corner.md @@ -178,6 +178,25 @@ dedicated CI leg: | Task | `task test-e2e-bi-directional` | | CI leg | `bi-directional` | +### The timing spec: `bi_directional_timing_e2e_test.go` + +The other specs prove what the loop does and only bound how long it may take. +[`bi_directional_timing_e2e_test.go`](../../test/e2e/bi_directional_timing_e2e_test.go) +(label `timing`) measures it. It mirrors a labeled ConfigMap into the folder a Flux +Kustomization or an Argo CD Application applies, then times each sample from the stimulus +(an API edit, or a Git-side push landing on Gitea) to the effect an operator would look for: + +- the commit on the branch; +- Flux applying it after the default 30s quiet period; +- the trigger holding its applier again; +- the live object changing after a Git-side push through the receiver. + +Each duration goes into a `gmeasure` experiment, which Ginkgo prints as an N/min/median/max +table and writes into the JSON report. `task allure-e2e-report` attaches it to the spec as +"Measurements". Every sample also has a budget, and a sample over it fails the spec. The +budgets catch a path that fell back to the 5-minute steady tick, not a slow runner. +`E2E_TIMING_SAMPLES` sets the sample count (default 3). + ### The third spec: `provenance_markers_e2e_test.go` The corner hosts a third spec, and it is the reason the corner is worth more than the @@ -605,4 +624,6 @@ namespaces and resource names, and none is `Serial`. - [`test/e2e/flux_bi_directional_e2e_test.go`](../../test/e2e/flux_bi_directional_e2e_test.go) and [`test/e2e/argocd_bi_directional_e2e_test.go`](../../test/e2e/argocd_bi_directional_e2e_test.go): the two specs in this corner +- [`test/e2e/bi_directional_timing_e2e_test.go`](../../test/e2e/bi_directional_timing_e2e_test.go): + the loop's latency, as numbers - [`internal/sanitize/types.go`](../../internal/sanitize/types.go): the strip lists diff --git a/docs/spec/e2e-serial-registry.md b/docs/spec/e2e-serial-registry.md index fdaa36e84..211524bd5 100644 --- a/docs/spec/e2e-serial-registry.md +++ b/docs/spec/e2e-serial-registry.md @@ -29,17 +29,22 @@ row below. ## Serial containers +A Serial container runs alone, after every parallel spec of its CI leg has finished, so its whole +runtime adds to that leg's wall clock. Place it on a leg with time to spare: `CRD Lifecycle` +carries the `crd-lifecycle` label rather than `manager` for exactly that reason, which puts it in +the `full-core` leg beside the restart specs instead of at the end of the longest leg. + | File | Container | Shared state it touches | Why name isolation can't fix it | |---|---|---|---| | [test/e2e/restart_reconcile_e2e_test.go](../../test/e2e/restart_reconcile_e2e_test.go) | `Restart Reconcile Safety` | Rollout-restarts the controller deployment. | The controller is a singleton; restarting it disrupts in-flight reconciles/commits for every other spec. | | [test/e2e/flux_restart_recovery_e2e_test.go](../../test/e2e/flux_restart_recovery_e2e_test.go) | `Flux Restart Recovery` | Rollout-restarts the controller deployment while a Flux apply runs, and labels a node so that apply's Deployment can schedule. | The controller is a singleton, as for `Restart Reconcile Safety`; a node label is cluster-wide. | | [test/e2e/image_refresh_test.go](../../test/e2e/image_refresh_test.go) | `image refresh dependency chain` | Changes the controller image / redeploys the controller. | Same singleton controller; an image swap perturbs all concurrent specs. | -| [test/e2e/crd_lifecycle_e2e_test.go](../../test/e2e/crd_lifecycle_e2e_test.go) | `Manager CRD Lifecycle` | Installs/deletes a cluster-scoped CRD and watches all `customresourcedefinitions` cluster-wide, asserting exact Git file presence/absence. | A concurrent CRD install/delete bumps the global discovery-catalog generation and re-resolves every GitTarget's watched-type tables, delaying this spec's reconcile (CRD file appears late / post-delete sweep lags — the 649↔673 flake). A cluster-wide catalog bump cannot be scoped by name. | +| [test/e2e/crd_lifecycle_e2e_test.go](../../test/e2e/crd_lifecycle_e2e_test.go) | `CRD Lifecycle` | Installs/deletes a cluster-scoped CRD and watches all `customresourcedefinitions` cluster-wide, asserting exact Git file presence/absence. | A concurrent CRD install/delete bumps the global discovery-catalog generation and re-resolves every GitTarget's watched-type tables, delaying this spec's reconcile (CRD file appears late / post-delete sweep lags — the 649↔673 flake). A cluster-wide catalog bump cannot be scoped by name. | | [test/e2e/tilt_playground_e2e_test.go](../../test/e2e/tilt_playground_e2e_test.go) | `playground` | Reusable manual-playground fixture: fixed (non-randomized) `tilt-playground` namespace and `playground` repo/provider names, preserved across runs. | A singleton by construction — its resource names are intentionally stable so `task playground-*` can re-attach, so per-run name isolation does not apply. | ### De-serialized -`Manager CRD Lifecycle` ([crd_lifecycle_e2e_test.go](../../test/e2e/crd_lifecycle_e2e_test.go)) +`CRD Lifecycle` ([crd_lifecycle_e2e_test.go](../../test/e2e/crd_lifecycle_e2e_test.go)) **was de-serialized and then re-serialized** — it is in the Serial table above, not here. Finding #2's fix ([manager.go:1191](../../internal/watch/manager.go#L1191)) — resnapshot a target only when its *resolved* plan hash changes — removed the worst diff --git a/internal/controller/gittarget_controller.go b/internal/controller/gittarget_controller.go index 9bdb80600..2ba1f5501 100644 --- a/internal/controller/gittarget_controller.go +++ b/internal/controller/gittarget_controller.go @@ -203,9 +203,10 @@ func (r *GitTargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, providerErr } - // awaitingAnswer: this reconcile is about to ask the remote where the branch is, so the answer - // lands after the status write below. See requeueForRemoteAnswer. - awaitingAnswer := r.publishGitObservations(st, &target, providerNS, repoIdentityOf(gitProvider)) + // remoteWake: whether this reconcile is about to ask the remote where the branch is, so the + // answer lands after the status write below, and how long the floor holds back an answer it + // already has. See requeueForRemoteAnswer. + remoteWake := r.publishGitObservations(st, &target, providerNS, repoIdentityOf(gitProvider)) // Ahead of every gate, for the reason the layout stanza above is: the join series has to carry // the targets that are NOT working. git_pushes_total names a branch that stopped advancing and @@ -304,7 +305,7 @@ func (r *GitTargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // requeueAfter shortens the cadence when the status write lost a race: the object then holds // the WINNER's older answer and nothing re-enqueues it, because the For() predicate filters // status-only updates by design. - requeue := st.requeueAfter(requeueForRemoteAnswer(gitTargetRequeue(rd), awaitingAnswer)) + requeue := st.requeueAfter(requeueForRemoteAnswer(gitTargetRequeue(rd), remoteWake)) // TEMPORARY at Info, while Failure A is open. The data plane can converge and this status not // follow it: a run has shown the render gate reaching True and both this GitTarget and every // WatchRule on it still publishing "Rechecking" two minutes later, with no dropped reconcile @@ -921,20 +922,31 @@ func (r *GitTargetReconciler) publishGitObservations( target *configbutleraiv1alpha3.GitTarget, providerNS string, repo git.RepoIdentity, -) bool { +) remoteWake { layout, scanned := r.observeLayout(target) publishLayout(st, target, layout, scanned) now := r.clockNow() remote, remoteSeen := r.observeRemote(target, providerNS) - r.publishRemote(st, target, remote, remoteSeen, repo, now) - // Whether the refresh this reconcile goes on to enqueue will actually reach the remote: the - // worker spends a connection only when what it holds is older than the interval configured. - return r.GitRefreshInterval > 0 && (!remoteSeen || remote.Age(now) >= r.GitRefreshInterval) + heldFor := r.publishRemote(st, target, remote, remoteSeen, repo, now) + return remoteWake{ + // Whether the refresh this reconcile goes on to enqueue will actually reach the remote: the + // worker spends a connection only when what it holds is older than the interval configured. + asked: r.GitRefreshInterval > 0 && (!remoteSeen || remote.Age(now) >= r.GitRefreshInterval), + heldFor: heldFor, + } +} + +// remoteWake is what status.remote needs from the next reconcile's timing. +type remoteWake struct { + // asked: this reconcile asked the remote where the branch is, and the answer lands after it. + asked bool + // heldFor: the floor holds back an answer this target already has, for this long. + heldFor time.Duration } // requeueForRemoteAnswer shortens the cadence for a target that has just ASKED the remote where -// its branch is. +// its branch is, or that holds an answer the floor has not let out yet. // // It is the one piece of scheduling this status field gets, and it exists because the refresh is // enqueued during the reconcile: the worker answers moments later, after the status write, so the @@ -942,13 +954,25 @@ func (r *GitTargetReconciler) publishGitObservations( // spend a connection every interval and publish what it learned a whole tick late — visible to an // operator as a target that follows a foreign push ten minutes after noticing it. // -// Nothing else is threaded anywhere. A reconcile that asked nothing keeps its own cadence, and no -// early return has to carry a deadline. -func requeueForRemoteAnswer(cadence time.Duration, asked bool) time.Duration { - if asked && RemotePublicationInterval < cadence { - return RemotePublicationInterval +// Asking is not enough on its own, because the requeue it returns can be lost. A refresh that +// finds the branch moved re-enqueues the target while the asking reconcile is still running, and +// the queue folds a delayed requeue into an item that is already waiting (controller-runtime's +// priority queue keeps the earlier time). The reconcile that then runs holds a fresh answer, so it +// asks nothing, and the floor stops it publishing: without heldFor it returned the steady cadence, +// and the answer waited the whole tick after all. So whichever reconcile meets the held answer +// comes back when the floor lifts. +// +// Nothing else is threaded anywhere. A reconcile that asked nothing and holds nothing keeps its +// own cadence, and no early return has to carry a deadline. +func requeueForRemoteAnswer(cadence time.Duration, wake remoteWake) time.Duration { + next := cadence + if wake.asked && RemotePublicationInterval < next { + next = RemotePublicationInterval + } + if wake.heldFor > 0 && wake.heldFor < next { + next = wake.heldFor } - return cadence + return next } // ensureEventStream wires this GitTarget to the branch worker for the repository its GitProvider diff --git a/internal/controller/gittarget_remote.go b/internal/controller/gittarget_remote.go index ee9e45f01..41846d1a6 100644 --- a/internal/controller/gittarget_remote.go +++ b/internal/controller/gittarget_remote.go @@ -118,6 +118,9 @@ func (r *GitTargetReconciler) observeRemote( // Two things ignore the floor, because they are correctness rather than throughput: the FIRST // observation, which is the difference between "nothing has looked" and an answer, and any // correction that involves the repository changing under the target. +// +// It returns how long the floor is holding news back, zero when nothing is held. A held answer +// must not wait for whatever reconcile happens to come next: see requeueForRemoteAnswer. func (r *GitTargetReconciler) publishRemote( st *reconcileStatus, target *configbutleraiv1alpha3.GitTarget, @@ -125,7 +128,7 @@ func (r *GitTargetReconciler) publishRemote( seen bool, repo git.RepoIdentity, now time.Time, -) { +) time.Duration { ref := types.NewResourceReference(target.Name, target.Namespace) published, everPublished := r.remotePublications.last(ref, target.UID) // What this target PUBLISHED was proved against a repository it no longer points at: its @@ -137,12 +140,12 @@ func (r *GitTargetReconciler) publishRemote( // published with, and it has to go rather than go stale. target.Status.Remote = nil st.afterPersist(func() { r.remotePublications.forget(ref) }) - return + return 0 } if !seen { // Nothing has looked at this branch since the process started. What is published is the // last thing anyone proved about it, and there is nothing better to replace it with. - return + return 0 } next := &configbutleraiv1alpha3.GitTargetRemoteStatus{ Commit: observed.Commit, @@ -151,11 +154,11 @@ func (r *GitTargetReconciler) publishRemote( Parent: parentStatusOf(observed), } if !remoteStatusIsNews(target.Status.Remote, next) { - return + return 0 } // The floor, skipped for the two corrections above and for the first publication. if everPublished && !staleRepository && now.Sub(published.at) < RemotePublicationInterval { - return + return published.at.Add(RemotePublicationInterval).Sub(now) } target.Status.Remote = next // Only once the write has landed, and dated THEN rather than now. A patch refused by the @@ -166,6 +169,7 @@ func (r *GitTargetReconciler) publishRemote( // so on a slow reconcile a cooldown dated from it can be half spent before the write lands, // and a reconcile queued behind it would publish again straight away. st.afterPersist(func() { r.remotePublications.record(ref, target.UID, r.clockNow(), observed.Repo) }) + return 0 } // withdrawRemote decides whether what is published has to be taken back rather than replaced. diff --git a/internal/controller/gittarget_remote_test.go b/internal/controller/gittarget_remote_test.go index c9b95c0e4..a0da683d2 100644 --- a/internal/controller/gittarget_remote_test.go +++ b/internal/controller/gittarget_remote_test.go @@ -417,12 +417,55 @@ func TestPublishRemote_ADeletedTargetReleasesItsLedgerEntry(t *testing.T) { // it, or an idle one spends a connection every interval and publishes what it learned a whole tick // late. A target that asked nothing keeps its own cadence. func TestRequeueForRemoteAnswer_ShortensOnlyWhenSomethingWasAsked(t *testing.T) { - assert.Equal(t, RemotePublicationInterval, requeueForRemoteAnswer(RequeueSteadyInterval, true)) - assert.Equal(t, RequeueSteadyInterval, requeueForRemoteAnswer(RequeueSteadyInterval, false)) - assert.Equal(t, time.Second, requeueForRemoteAnswer(time.Second, true), + assert.Equal(t, RemotePublicationInterval, + requeueForRemoteAnswer(RequeueSteadyInterval, remoteWake{asked: true})) + assert.Equal(t, RequeueSteadyInterval, requeueForRemoteAnswer(RequeueSteadyInterval, remoteWake{})) + assert.Equal(t, time.Second, requeueForRemoteAnswer(time.Second, remoteWake{asked: true}), "a target already on a faster loop is not slowed down to the publication interval") } +// TestRequeueForRemoteAnswer_ComesBackWhenTheFloorLifts is the reconcile after a lost requeue. The +// asking reconcile's one-minute requeue is folded away when the refresh's answer re-enqueues the +// target while it runs; the reconcile that follows holds that answer fresh, asks nothing, and is +// stopped by the floor. It must come back when the floor lifts, not a whole steady tick later. +func TestRequeueForRemoteAnswer_ComesBackWhenTheFloorLifts(t *testing.T) { + held := 59 * time.Second + assert.Equal(t, held, requeueForRemoteAnswer(RequeueSteadyInterval, remoteWake{heldFor: held})) + assert.Equal(t, held, requeueForRemoteAnswer(RequeueSteadyInterval, remoteWake{asked: true, heldFor: held}), + "the earlier of the two wakes wins") + assert.Equal(t, time.Second, requeueForRemoteAnswer(time.Second, remoteWake{heldFor: held}), + "a target already on a faster loop is not slowed down to the floor") +} + +// TestPublishRemote_ReportsHowLongTheFloorHoldsAnAnswer: the floor delays news, and says for how +// long, so the reconcile can come back for it. What is not news, and what is published, holds +// nothing back. +func TestPublishRemote_ReportsHowLongTheFloorHoldsAnAnswer(t *testing.T) { + r := &GitTargetReconciler{} + target := remoteTestTarget() + start := time.Now() + previous := r.clock + r.clock = func() time.Time { return start } + defer func() { r.clock = previous }() + + none := git.RepoIdentity{} + later := start.Add(2 * time.Second) + + st := &reconcileStatus{} + first := observation("aaaa", start, git.ObservedByPush, none) + assert.Zero(t, r.publishRemote(st, target, first, true, none, start), + "the first observation is published, so nothing is held") + st.runPersisted() + + moved := observation("bbbb", later, git.ObservedByFetch, none) + assert.Equal(t, 58*time.Second, r.publishRemote(&reconcileStatus{}, target, moved, true, none, later), + "news two seconds after a write waits out the rest of the floor") + assert.Equal(t, "aaaa", target.Status.Remote.Commit) + + assert.Zero(t, r.publishRemote(&reconcileStatus{}, target, first, true, none, later), + "what is already published is not news, so the floor holds nothing back") +} + // remoteLedgerFixture drives publishRemote through the REAL status session, so what advances the // ledger is a patch the API server accepted rather than a callback a test chose to run. type remoteLedgerFixture struct { diff --git a/test/e2e/bi_directional_timing_e2e_test.go b/test/e2e/bi_directional_timing_e2e_test.go new file mode 100644 index 000000000..f9f7657d5 --- /dev/null +++ b/test/e2e/bi_directional_timing_e2e_test.go @@ -0,0 +1,433 @@ +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "fmt" + "os" + "regexp" + "strconv" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/onsi/gomega/gmeasure" +) + +// How long the bi-directional loop takes, as numbers. +// +// The other bi-directional specs prove WHAT happens and only bound how long it may take. These +// time it: each sample starts at the stimulus (an API edit, or a Git-side push landing on the +// server) and stops at the effect an operator would look for, and every duration is recorded in +// a gmeasure experiment that Ginkgo prints as a table (N, min, median, mean, stddev, max) and +// writes into the JSON report. +// +// Each duration also has a budget, and a sample over it fails the spec. The budgets are loose on +// purpose: they catch a path that silently fell back to a periodic tick (minutes), not a slow CI +// runner (seconds). The table is the measurement; the budget is the guarantee. +// +// The folder is SHARED, which is the case a trigger exists for: GitOps Reverser mirrors a +// labeled ConfigMap into the folder the applier applies. The WatchRule selects only that label, +// so the namespace's kube-root-ca.crt never reaches the folder and the applier applies exactly one +// object. +// +// Resolution: the probes poll every timingPoll and each probe is a kubectl or git round trip, so +// a sample reads about a second long at worst. E2E_TIMING_SAMPLES sets the sample count. + +const ( + timingPoll = 500 * time.Millisecond + + // An API edit is on the remote branch. The e2e commit window is 0s, so this is the watch + // event, the commit and the push. + timingCommitBudget = 45 * time.Second + // A Git-side push the receiver hears of has reached the live object, through the applier. + timingReceiverBudget = 90 * time.Second + // An apply the trigger started has finished, and the trigger holds its applier again. + timingHeldBudget = 60 * time.Second + + timingDefaultSamples = 3 + timingLabelKey = "e2e.configbutler.ai/timing" + timingConfigMap = "bi-timing" +) + +// Measurement names, shared by both appliers so the two tables read side by side. +const ( + measureAPIToCommit = "API edit → commit on the branch" + measureCommitToApplied = "commit on the branch → Flux applied it" + measureAPIToApplied = "API edit → Flux applied it (includes the quiet period)" + measureAppliedToHeld = "applied → trigger holds the applier again" + measurePushToLive = "Git-side push → live object (receiver)" + measurePushToSyncDone = "Git-side push → Argo CD sync finished (receiver)" + measurePushToFluxHeld = "Git-side push → Flux applied it and is held again (receiver)" + timingSampleValueFormat = "%s-%d" +) + +// timingSamples is how many samples each measurement takes. +func timingSamples() int { + if raw := strings.TrimSpace(os.Getenv("E2E_TIMING_SAMPLES")); raw != "" { + n, err := strconv.Atoi(raw) + if err == nil && n > 0 { + return n + } + } + return timingDefaultSamples +} + +// timedUntil polls probe until it passes, at most budget after start, records the time from start +// to the first passing probe under name, and returns that duration. +func timedUntil( + experiment *gmeasure.Experiment, name string, start time.Time, budget time.Duration, probe func(g Gomega), +) time.Duration { + GinkgoHelper() + remaining := budget - time.Since(start) + Expect(remaining).To(BeNumerically(">", 0), "%s: the budget of %s was spent before the probe began", name, budget) + Eventually(probe, remaining, timingPoll).Should(Succeed(), "%s: over its budget of %s", name, budget) + elapsed := time.Since(start) + experiment.RecordDuration(name, elapsed, gmeasure.Precision(100*time.Millisecond)) + return elapsed +} + +// timingPipeline is one GitTarget mirroring the labeled ConfigMap into the folder an applier +// applies. +type timingPipeline struct { + testNs string + repo *RepoArtifacts + providerName string + targetName string + ruleName string + gitPath string + receiverName string + hookID int64 +} + +func newTimingPipeline(testNs, prefix string, repo *RepoArtifacts) *timingPipeline { + return &timingPipeline{ + testNs: testNs, + repo: repo, + providerName: prefix + "-provider", + targetName: prefix + "-target", + ruleName: prefix + "-rule", + gitPath: "timing/" + prefix, + receiverName: prefix + "-receiver", + } +} + +// filePath is where the GitTarget writes the ConfigMap, relative to the repository root. +func (p *timingPipeline) filePath() string { + return fmt.Sprintf("%s/%s/configmaps/%s.yaml", p.gitPath, p.testNs, timingConfigMap) +} + +// start brings up GitProvider (with a receiver Gitea pushes to), GitTarget and WatchRule, creates +// the ConfigMap, and waits for its first commit, so the folder exists before an applier names it. +func (p *timingPipeline) start() { + GinkgoHelper() + _, err := kubectlRunInNamespace(p.testNs, "apply", "-f", p.repo.SecretsYAML) + Expect(err).NotTo(HaveOccurred(), "failed to apply git secrets to the test namespace") + applySOPSAgeKeyToNamespace(p.testNs) + configureRepoOriginWithCredentials(p.repo, p.testNs) + + createGitProviderWithURLInNamespace(p.providerName, p.testNs, p.repo.GitSecretHTTP, p.repo.RepoURLHTTP) + verifyResourceStatus("gitprovider", p.providerName, p.testNs, "True", "Succeeded", "") + p.hookID = registerOperatorReceiver(p.testNs, p.providerName, p.receiverName, p.repo.RepoName) + + createGitTarget(p.targetName, p.testNs, p.providerName, p.gitPath, "main") + _, err = kubectlRunWithStdin(p.testNs, fmt.Sprintf(`apiVersion: configbutler.ai/v1alpha3 +kind: WatchRule +metadata: + name: %s + namespace: %s +spec: + gitTargetRef: + name: %s + rules: + - resources: ["configmaps"] + objectSelector: + matchLabels: + %s: "true" +`, p.ruleName, p.testNs, p.targetName, timingLabelKey), "apply", "-f", "-") + Expect(err).NotTo(HaveOccurred(), "failed to apply the WatchRule") + verifyResourceStatus("gittarget", p.targetName, p.testNs, "True", "Succeeded", "") + waitForStreamsRunning(p.targetName, p.testNs) + + _, err = kubectlRunWithStdin(p.testNs, fmt.Sprintf(`apiVersion: v1 +kind: ConfigMap +metadata: + name: %s + namespace: %s + labels: + %s: "true" +data: + value: seed +`, timingConfigMap, p.testNs, timingLabelKey), "apply", "-f", "-") + Expect(err).NotTo(HaveOccurred(), "failed to create the timed ConfigMap") + Eventually(func(g Gomega) { + g.Expect(p.committedValue(g)).To(Equal("seed")) + }, timingCommitBudget, timingPoll).Should(Succeed(), "the ConfigMap must be in Git before an applier applies it") +} + +func (p *timingPipeline) cleanup() { + deleteRepoHook(p.repo.RepoName, p.hookID) + cleanupWatchRule(p.ruleName, p.testNs) + cleanupGitTarget(p.targetName, p.testNs) + cleanupNamespacedResource(p.testNs, "gitprovider", p.providerName) + cleanupNamespacedResource(p.testNs, "secret", p.receiverName) +} + +var timingValueLine = regexp.MustCompile(`(?m)^([ \t]+value:[ \t]*).*$`) + +// committedFile is the ConfigMap's file on the remote branch. +func (p *timingPipeline) committedFile(g Gomega) string { + _, err := gitRun(p.repo.CheckoutDir, "fetch", "origin", "main") + g.Expect(err).NotTo(HaveOccurred()) + out, err := gitRun(p.repo.CheckoutDir, "show", "origin/main:"+p.filePath()) + g.Expect(err).NotTo(HaveOccurred(), "the ConfigMap is not at %s on main", p.filePath()) + return out +} + +// committedValue is data.value of the ConfigMap as the remote branch holds it. +func (p *timingPipeline) committedValue(g Gomega) string { + match := timingValueLine.FindStringSubmatch(p.committedFile(g)) + g.Expect(match).NotTo(BeNil(), "the committed ConfigMap has no data.value") + return strings.TrimSpace(strings.TrimPrefix(match[0], match[1])) +} + +// liveValue is data.value of the live ConfigMap. +func (p *timingPipeline) liveValue(g Gomega) string { + out, err := kubectlRunInNamespace(p.testNs, "get", "configmap", timingConfigMap, "-o", "jsonpath={.data.value}") + g.Expect(err).NotTo(HaveOccurred()) + return strings.TrimSpace(out) +} + +// editThroughAPI sets data.value through the Kubernetes API and returns when the request returned. +func (p *timingPipeline) editThroughAPI(value string) time.Time { + GinkgoHelper() + start := time.Now() + _, err := kubectlRunInNamespace(p.testNs, "patch", "configmap", timingConfigMap, "--type=merge", "-p", + fmt.Sprintf(`{"data":{"value":%q}}`, value)) + Expect(err).NotTo(HaveOccurred(), "failed to edit the ConfigMap through the API") + return start +} + +// pushFromGitSide rewrites data.value in the committed file, byte for byte otherwise, so the +// applied object mirrors back as no change, and pushes it as another writer would. It returns the +// commit and the moment the push returned, which is when the commit is on the server. +func (p *timingPipeline) pushFromGitSide(value string) (string, time.Time) { + GinkgoHelper() + content := timingValueLine.ReplaceAllString(p.committedFile(Default), "${1}"+value) + head := pushFileFromOutside(p.repo, p.filePath(), content, "e2e: a Git-side timing change to "+value) + return head, time.Now() +} + +// timingSettle outlasts the apply window's grace after an apply finishes (docs/design/apply-trigger.md), +// the time the applier's own write to the object can still be on its way back through the mirror. +const timingSettle = 15 * time.Second + +// settled holds, for timingSettle, that the branch is still at head and the live object still +// holds value: the apply mirrored back as no change. A sample starts only once the previous one +// has settled, so each times one change through a quiet loop. +// +// It is not only pacing. Pushing again while the previous apply's own write is still pending is +// a race this spec found with Argo CD: that write, replayed onto the newer push, reverted it. Here +// it would show as a commit after head. +func (p *timingPipeline) settled(head, value string) { + GinkgoHelper() + Consistently(func(g Gomega) { + g.Expect(p.remoteBranchHead(g)).To(Equal(head), "the apply must mirror back as no change") + g.Expect(p.liveValue(g)).To(Equal(value)) + }, timingSettle, time.Second).Should(Succeed()) +} + +// remoteBranchHead is where main is on the server. +func (p *timingPipeline) remoteBranchHead(g Gomega) string { + _, err := gitRun(p.repo.CheckoutDir, "fetch", "origin", "main") + g.Expect(err).NotTo(HaveOccurred()) + out, err := gitRun(p.repo.CheckoutDir, "rev-parse", "origin/main") + g.Expect(err).NotTo(HaveOccurred()) + return strings.TrimSpace(out) +} + +var _ = Describe("Bi Directional timing (Flux)", Label("bi-directional", "flux", "timing"), Ordered, + SpecPriority(10), func() { + var ( + testNs, fluxName string + pipeline *timingPipeline + ) + + BeforeAll(func() { + skipUnlessBiDirectionalEnabled() + allowCrossNamespaceAppliers() + testNs = testNamespaceFor("bi-timing-flux") + _, _ = kubectlRun("create", "namespace", testNs) + fluxName = "bi-timing-" + testNs + + repo := SetupRepo(resolveE2EContext(), testNs, fmt.Sprintf("e2e-bi-timing-flux-%d", GinkgoRandomSeed())) + pipeline = newTimingPipeline(testNs, "flux-timing", repo) + pipeline.start() + + By("letting Flux apply the shared folder once, then naming a trigger with the default quiet period") + applyFluxApplier(testNs, repo, fluxName, fluxName+"-git", "main", "./"+pipeline.gitPath) + createFluxTrigger(testNs, fluxName, fluxName, "") + nameApplyTrigger(testNs, pipeline.targetName, "FluxTrigger", fluxName, "") + verifyResourceCondition("gittarget", pipeline.targetName, testNs, + "ApplyTriggerReady", "True", "Succeeded", "") + verifyResourceCondition(fluxTriggerResource, fluxName, testNs, "Held", "True", "Suspended", "") + }) + + AfterAll(func() { + if !biDirectionalEnabled() { + return + } + if pipeline != nil { + // nil when BeforeAll failed before building it. + pipeline.cleanup() + } + cleanupNamespacedResource(testNs, fluxTriggerResource, fluxName) + cleanupNamespacedResource("flux-system", "kustomization", fluxName) + cleanupNamespacedResource("flux-system", "gitrepository", fluxName) + cleanupNamespacedResource("flux-system", "secret", fluxName+"-git") + cleanupNamespace(testNs) + }) + + // heldAgain passes once the trigger reports the operation for commit Succeeded, with the + // Kustomization suspended again. + heldAgain := func(g Gomega, commit string) { + operation := readTriggerOperation(g, testNs, fluxTriggerResource, fluxName) + g.Expect(operation.Commit).To(Equal(commit)) + g.Expect(operation.Phase).To(Equal("Succeeded")) + g.Expect(kustomizationSuspended(g, fluxName)).To(BeTrue()) + } + + // The API side: an edit is mirrored, the branch goes quiet, and the trigger asks Flux to catch + // up. The quiet period is the default, because that is the latency a user gets; it is part of + // the total by design, so the spec also proves it is honored: no apply lands before it ends. + It("times an API edit through Git to a Flux apply", func() { + experiment := gmeasure.NewExperiment("Flux: API edit round trip") + AddReportEntry(experiment.Name, experiment) + quiet := 30 * time.Second // DefaultApplierQuietPeriod; the trigger leaves spec.quietPeriod empty. + experiment.RecordNote(fmt.Sprintf("quietPeriod %s (the default), commit window 0s", quiet)) + + for i := range timingSamples() { + value := fmt.Sprintf(timingSampleValueFormat, "api", i) + By("sample " + strconv.Itoa(i) + ": editing the ConfigMap through the API") + start := pipeline.editThroughAPI(value) + + var head string + timedUntil(experiment, measureAPIToCommit, start, timingCommitBudget, func(g Gomega) { + g.Expect(pipeline.committedValue(g)).To(Equal(value)) + head = pipeline.remoteBranchHead(g) + }) + committedAt := time.Now() + + applied := timedUntil(experiment, measureAPIToApplied, start, timingCommitBudget+quiet+timingHeldBudget, + func(g Gomega) { + g.Expect(fluxLastAppliedRevision(g, fluxName)).To(ContainSubstring(head)) + }) + experiment.RecordDuration(measureCommitToApplied, time.Since(committedAt), + gmeasure.Precision(100*time.Millisecond)) + Expect(applied).To(BeNumerically(">=", quiet), + "Flux applied %s after the edit, before the branch had been quiet for %s", applied, quiet) + appliedAt := time.Now() + + timedUntil(experiment, measureAppliedToHeld, appliedAt, timingHeldBudget, func(g Gomega) { + heldAgain(g, head) + }) + Expect(pipeline.liveValue(Default)).To(Equal(value), "the catch-up must not revert the edit it applies") + } + }) + + // The Git side: another writer pushes, Gitea tells the receiver, and the trigger asks Flux to + // apply that commit. A ForeignCommit waits for no quiet period. + It("times a Git-side push through the receiver to the live object", func() { + experiment := gmeasure.NewExperiment("Flux: Git-side push") + AddReportEntry(experiment.Name, experiment) + + for i := range timingSamples() { + value := fmt.Sprintf(timingSampleValueFormat, "git", i) + By("sample " + strconv.Itoa(i) + ": pushing a Git-side change") + head, pushed := pipeline.pushFromGitSide(value) + + timedUntil(experiment, measurePushToLive, pushed, timingReceiverBudget, func(g Gomega) { + g.Expect(pipeline.liveValue(g)).To(Equal(value)) + }) + timedUntil(experiment, measurePushToFluxHeld, pushed, timingReceiverBudget+timingHeldBudget, + func(g Gomega) { + g.Expect(fluxLastAppliedRevision(g, fluxName)).To(ContainSubstring(head)) + heldAgain(g, head) + }) + pipeline.settled(head, value) + } + }) + }) + +var _ = Describe("Bi Directional timing (Argo CD)", Label("bi-directional", "argocd", "timing"), Ordered, + SpecPriority(10), func() { + var ( + testNs string + pipeline *timingPipeline + run argoBiDirectionalRun + ) + + BeforeAll(func() { + skipUnlessBiDirectionalEnabled() + requireArgoCDInstalled() + allowCrossNamespaceAppliers() + testNs = testNamespaceFor("bi-timing-argo") + _, _ = kubectlRun("create", "namespace", testNs) + + repo := SetupRepo(resolveE2EContext(), testNs, fmt.Sprintf("e2e-bi-timing-argo-%d", GinkgoRandomSeed())) + pipeline = newTimingPipeline(testNs, "argo-timing", repo) + run = newArgoBiDirectionalRun(testNs, repo, "argo-timing") + pipeline.start() + + By("registering the repository with Argo CD and naming the Application, automated sync off") + run.applyArgoRepoSecret() + cfg := run.baseAppConfig() + cfg.Path = pipeline.gitPath + run.applyArgoApp(cfg) + createArgoCDTrigger(testNs, run.appName, run.argoNs, run.appName) + nameApplyTrigger(testNs, pipeline.targetName, "ArgoCDTrigger", run.appName, "") + verifyResourceCondition("gittarget", pipeline.targetName, testNs, + "ApplyTriggerReady", "True", "Succeeded", "") + }) + + AfterAll(func() { + if !biDirectionalEnabled() { + return + } + cleanupNamespacedResource(run.argoNs, "application.argoproj.io", run.appName) + cleanupNamespacedResource(run.argoNs, "secret", run.repoSecretName) + cleanupNamespacedResource(testNs, argoCDTriggerResource, run.appName) + if pipeline != nil { + // nil when BeforeAll failed before building it. + pipeline.cleanup() + } + cleanupNamespace(testNs) + }) + + // Argo CD is never asked to catch up with GitOps Reverser's own commits, so the API side has + // no apply to time: its latency is the commit, which the Flux table already measures. The Git + // side is a ForeignCommit sync pinned to the pushed commit. + It("times a Git-side push through the receiver to the live object", func() { + experiment := gmeasure.NewExperiment("Argo CD: Git-side push") + AddReportEntry(experiment.Name, experiment) + + for i := range timingSamples() { + value := fmt.Sprintf(timingSampleValueFormat, "git", i) + By("sample " + strconv.Itoa(i) + ": pushing a Git-side change") + head, pushed := pipeline.pushFromGitSide(value) + + timedUntil(experiment, measurePushToLive, pushed, timingReceiverBudget, func(g Gomega) { + g.Expect(pipeline.liveValue(g)).To(Equal(value)) + }) + timedUntil(experiment, measurePushToSyncDone, pushed, timingReceiverBudget+timingHeldBudget, + func(g Gomega) { + operation := run.triggerOperation(g) + g.Expect(operation.Reason).To(Equal("ForeignCommit")) + g.Expect(operation.Commit).To(Equal(head)) + g.Expect(operation.Phase).To(Equal("Succeeded")) + }) + pipeline.settled(head, value) + } + }) + }) diff --git a/test/e2e/crd_lifecycle_e2e_test.go b/test/e2e/crd_lifecycle_e2e_test.go index 6216c56bf..e418bf923 100644 --- a/test/e2e/crd_lifecycle_e2e_test.go +++ b/test/e2e/crd_lifecycle_e2e_test.go @@ -5,7 +5,6 @@ package e2e import ( "fmt" "os" - "os/exec" "path/filepath" "time" @@ -21,7 +20,12 @@ import ( // GitTarget's watched-type tables, delaying this spec's reconcile so the CRD file appeared late // (verifyFileExists) or its post-delete sweep lagged (verifyFileDeleted) — the 649↔673 flake. // Serial pins it to the isolated serial phase, where no other spec churns cluster-scoped CRD state. -var _ = Describe("Manager CRD Lifecycle", Label("manager"), Serial, Ordered, func() { +// +// Not labeled manager: a Serial spec runs alone after every parallel spec has finished, so its +// whole runtime (about 3.5 min) adds to its CI leg's wall clock. The manager leg is the longest, +// the core leg has minutes to spare, and restart_reconcile's serial phase there is no CRD churn +// running alongside it. +var _ = Describe("CRD Lifecycle", Label("crd-lifecycle"), Serial, Ordered, func() { var ( testNs string crdLifecycleRepo *RepoArtifacts @@ -334,6 +338,10 @@ var _ = Describe("Manager CRD Lifecycle", Label("manager"), Serial, Ordered, fun } Eventually(verifyCRDInstanceExists, 15*time.Second, time.Second).Should(Succeed()) + // Read where the branch is BEFORE the patch, from the remote: a status-only change must not + // move it. Read after the patch, a status commit that landed fast would be the baseline. + headBeforeStatus := remoteHead(Default, crdLifecycleRepo.CheckoutDir) + By("applying status update to the IceCreamOrder CR") statusPatch := `{"status":{"phase":"Preparing","cost":12.5,"message":"Queued for pickup"}}` statusOutput, statusErr := kubectlRunInNamespace( @@ -349,41 +357,23 @@ var _ = Describe("Manager CRD Lifecycle", Label("manager"), Serial, Ordered, fun Expect(statusErr).NotTo(HaveOccurred(), "Status subresource patch should succeed") By(fmt.Sprintf("status patched successfully: %s", statusOutput)) - By("getting current git commit hash") - gitRevCmd := exec.Command("git", "rev-parse", "HEAD") - gitRevCmd.Dir = crdLifecycleRepo.CheckoutDir - beforeStatusCommit, _ := gitRevCmd.Output() - - By("waiting to ensure no new commit is created from status update") - time.Sleep(10 * time.Second) - - By("verifying no new commit was created and status is not in Git") - verifyStatusNotCommitted := func(g Gomega) { - pullLatestRepoState(g, crdLifecycleRepo.CheckoutDir) - - // Check that commit hash hasn't changed - gitRevCmd := exec.Command("git", "rev-parse", "HEAD") - gitRevCmd.Dir = crdLifecycleRepo.CheckoutDir - afterStatusCommit, err := gitRevCmd.Output() - g.Expect(err).NotTo(HaveOccurred()) - - By(fmt.Sprintf("Commit before status: %s", string(beforeStatusCommit))) - By(fmt.Sprintf("Commit after status: %s", string(afterStatusCommit))) - - // Read the file again to ensure status is still not present - expectedFile := filepath.Join(crdLifecycleRepo.CheckoutDir, - "e2e/icecream-test", - iceCreamInstancePath(crdGroupCRDLifecycle, testNs, crdInstanceName)) - content, readErr := os.ReadFile(expectedFile) - g.Expect(readErr).NotTo(HaveOccurred()) - g.Expect(string(content)).NotTo(ContainSubstring("status:"), - "CRD instance file should still NOT contain status field after status update") - g.Expect(string(content)).NotTo(ContainSubstring("phase:"), - "CRD instance file should NOT contain status phase content") - g.Expect(string(content)).NotTo(ContainSubstring("Queued for pickup"), - "CRD instance file should NOT contain status content") - } - Eventually(verifyStatusNotCommitted).Should(Succeed()) + By("verifying no new commit is created and status is not in Git") + Consistently(func(g Gomega) { + g.Expect(remoteHead(g, crdLifecycleRepo.CheckoutDir)).To(Equal(headBeforeStatus), + "a status-only update must not produce a commit") + }, 10*time.Second, time.Second).Should(Succeed()) + pullLatestRepoState(Default, crdLifecycleRepo.CheckoutDir) + expectedFile := filepath.Join(crdLifecycleRepo.CheckoutDir, + "e2e/icecream-test", + iceCreamInstancePath(crdGroupCRDLifecycle, testNs, crdInstanceName)) + content, readErr := os.ReadFile(expectedFile) + Expect(readErr).NotTo(HaveOccurred()) + Expect(string(content)).NotTo(ContainSubstring("status:"), + "CRD instance file should still NOT contain status field after status update") + Expect(string(content)).NotTo(ContainSubstring("phase:"), + "CRD instance file should NOT contain status phase content") + Expect(string(content)).NotTo(ContainSubstring("Queued for pickup"), + "CRD instance file should NOT contain status content") By("✅ Status update verified - no Git commit created and status not in file") diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 7e4526bf7..fc148c309 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -63,6 +63,7 @@ var _ = SynchronizedBeforeSuite(func() []byte { By("preparing e2e cluster prerequisites via Task target") prepareE2EClusterOnce() + waitForPreviousRunNamespaces() // After prepare, while this process still holds the exclusive cluster lock, so every spec runs // against a Prometheus with no retained samples and a controller whose counters start at zero. @@ -72,6 +73,28 @@ var _ = SynchronizedBeforeSuite(func() []byte { configureE2EProcess() }) +// waitForPreviousRunNamespaces waits out test namespaces an earlier run with this seed is still +// deleting. cleanupNamespace does not wait for a namespace to finalize: every namespace is named +// for the run's seed (testNamespaceFor), so within one run nothing reads it again. A rerun with the +// SAME seed (reproducing a failure with --seed) is the exception, and would find its namespaces +// still terminating, where the specs' create ignores AlreadyExists and the first write is refused. +// Within one run there is nothing to wait for yet: this runs before any spec. +func waitForPreviousRunNamespaces() { + out, err := kubectlRun("get", "namespaces", "-o", + `jsonpath={range .items[?(@.status.phase=="Terminating")]}{.metadata.name}{"\n"}{end}`) + Expect(err).NotTo(HaveOccurred(), "failed to list terminating namespaces") + prefix := fmt.Sprintf("%d-test-", GinkgoRandomSeed()) + pinned := strings.TrimSpace(os.Getenv("TESTNAMESPACE")) + for _, name := range strings.Fields(out) { + if !strings.HasPrefix(name, prefix) && name != pinned { + continue + } + By(fmt.Sprintf("waiting for namespace %s, left by an earlier run with this seed, to finish deleting", name)) + _, err := kubectlRun("wait", "--for=delete", "namespace/"+name, "--timeout=180s") + Expect(err).NotTo(HaveOccurred(), "namespace %s is still terminating", name) + } +} + // resetE2EObservability discards Prometheus's retained samples and restarts the controller so its // in-process counters start from zero. Both halves are needed: resetting Prometheus alone leaves // the controller ready to republish its old totals on the very next scrape. diff --git a/test/e2e/flux_applier_e2e_test.go b/test/e2e/flux_applier_e2e_test.go index ec695fb65..a9439cac7 100644 --- a/test/e2e/flux_applier_e2e_test.go +++ b/test/e2e/flux_applier_e2e_test.go @@ -23,7 +23,7 @@ import ( // spec's API edit or this spec's Git-side push. Once the trigger holds it, the Kustomization is // suspended, and no Flux Receiver is wired to the repository, so any apply inside the spec is one // GitOps Reverser started. -var _ = Describe("Manager Flux Applier", Label("manager", "flux-applier"), Ordered, func() { +var _ = Describe("Manager Flux Applier", Label("manager", "flux-applier"), Ordered, SpecPriority(90), func() { const ( providerName = "flux-applier-provider" destName = "flux-applier-dest" @@ -178,7 +178,7 @@ var _ = Describe("Manager Flux Applier", Label("manager", "flux-applier"), Order // With no receiver and no API edit, the only thing that can find a Git-side push is the // refresher on the GitTarget's steady tick. This is the guarantee that a missing webhook costs // latency and never an apply (docs/design/apply-trigger.md). - // The e2e install refreshes after 30s, so the wait is the control plane's reconcile cadence of + // The e2e install refreshes after 90s, so the wait is the control plane's reconcile cadence of // about five minutes, as in the refresh-remote spec. It("applies a Git-side push the refresher finds, with no receiver and no API edit", func() { Eventually(func(g Gomega) { diff --git a/test/e2e/helpers.go b/test/e2e/helpers.go index 81d74f2a2..2aa474cdb 100644 --- a/test/e2e/helpers.go +++ b/test/e2e/helpers.go @@ -356,7 +356,9 @@ func cleanupNamespace(name string) { return } By(fmt.Sprintf("deleting test namespace '%s'", name)) - _, _ = kubectlRun("delete", "namespace", name, "--ignore-not-found=true") + // --wait=false: finalizing a namespace takes 5-10s and nothing reads it again, since every + // test namespace is named for this run's seed. Waiting held the spec's process for it. + _, _ = kubectlRun("delete", "namespace", name, "--ignore-not-found=true", "--wait=false") } func cleanupNamespacedResource(namespace, resource, name string) { diff --git a/test/e2e/kcp_workspace_test.go b/test/e2e/kcp_workspace_test.go index e7257cb30..ab5d470ad 100644 --- a/test/e2e/kcp_workspace_test.go +++ b/test/e2e/kcp_workspace_test.go @@ -88,6 +88,17 @@ func startKcpTunnel() *kcpTunnel { t := &kcpTunnel{cmd: cmd, cancel: cancel, admin: admin, workdir: workdir} t.rootKubeconfig = t.writeDerivedKubeconfig("root", fmt.Sprintf("https://127.0.0.1:%d/clusters/root", kcpLocalPort), kcpFrontProxySNI) + + // A port that accepts connections is not a front-proxy that accepts the admin's credentials. + // setup-kcp.sh waits for the Deployments and the admin kubeconfig Secret, and one CI run still + // had its first authenticated request refused ("the server has asked for the client to provide + // credentials") a minute after both. Wait for an authenticated read through the tunnel, so a + // startup that is still settling is not reported as a failing spec. + Eventually(func() error { + _, err := kubectlWithKubeconfig(t.rootKubeconfig, "", "get", "workspaces") + return err + }).WithTimeout(90*time.Second).WithPolling(3*time.Second). + Should(Succeed(), "kcp never accepted the admin credentials through the front-proxy") return t } diff --git a/test/e2e/object_selector_e2e_test.go b/test/e2e/object_selector_e2e_test.go index 851c454bb..573e64407 100644 --- a/test/e2e/object_selector_e2e_test.go +++ b/test/e2e/object_selector_e2e_test.go @@ -22,7 +22,7 @@ import ( // Two targets watch the same ConfigMaps with the same selector: one Always, one on the default // (OnEvent). Each retention claim is made only after the Always target has visibly acted on the // same snapshot, so "still present" means "decided to keep", not "not processed yet". -var _ = Describe("Manager WatchRule objectSelector", Label("manager"), Ordered, func() { +var _ = Describe("Manager WatchRule objectSelector", Label("manager"), Ordered, SpecPriority(50), func() { const ( providerName = "gitprovider-selector" alwaysTarget = "selector-always-target" diff --git a/test/e2e/refresh_remote_e2e_test.go b/test/e2e/refresh_remote_e2e_test.go index 2937845ef..a56f77fe2 100644 --- a/test/e2e/refresh_remote_e2e_test.go +++ b/test/e2e/refresh_remote_e2e_test.go @@ -26,18 +26,21 @@ import ( // one that writer left behind. // // Timing. The refresh rides the GitTarget reconcile, and a converged target requeues on the -// 5-minute steady interval, so the bound here is one steady tick plus the refresh itself — not -// the 30s --git-refresh-interval config/deployment.yaml sets, which only decides how old an -// observation may be when that tick arrives. +// 5-minute steady interval, so the bound on noticing the move is one steady tick plus the refresh +// itself — not the 90s --git-refresh-interval config/deployment.yaml sets, which only decides how +// old an observation may be when that tick arrives. What the refresh learns is published in the +// same round: the reconcile that asked comes back for the answer, and one the publication floor +// stops comes back when it lifts (requeueForRemoteAnswer). // -// status.remote is SAMPLED onto that same tick: proving where the branch is and publishing it are -// two different rates, and nothing in the data plane wakes a target because a branch moved. So -// every wait on that stanza below is one steady tick wide, including the one after our OWN push. -// That is the contract, not slack: see docs/spec/status-conditions-guide.md. +// status.remote is SAMPLED: proving where the branch is and publishing it are two different +// rates, and nothing in the data plane wakes a target because a branch moved. Our OWN push is +// published once RemotePublicationInterval has passed since the last write, which was the unborn +// branch moments before; a minute, not a tick. That is the contract, not slack: see +// docs/spec/status-conditions-guide.md. // // No reconcile-request annotation is used anywhere in this spec: an operator asking by hand is // precisely the case this feature exists to remove. -var _ = Describe("Manager Remote Refresh", Label("manager", "refresh"), Ordered, func() { +var _ = Describe("Manager Remote Refresh", Label("manager", "refresh"), Ordered, SpecPriority(100), func() { var ( testNs string repo *RepoArtifacts @@ -104,7 +107,8 @@ var _ = Describe("Manager Remote Refresh", Label("manager", "refresh"), Ordered, g.Expect(pushedCommit).NotTo(BeEmpty(), "a target that has pushed knows where its branch is") g.Expect(gitTargetRemoteField(g, destName, testNs, "verifiedBy")).To(Equal("Push")) g.Expect(pushedCommit).To(Equal(remoteHeadSHA(g, repo.CheckoutDir))) - }, 7*time.Minute, 5*time.Second).Should(Succeed()) + }, 3*time.Minute, 5*time.Second).Should(Succeed(), + "published when the floor lifts, well before a steady tick") verifiedBefore := gitTargetRemoteField(Default, destName, testNs, "lastVerifiedAt") Expect(verifiedBefore).NotTo(BeEmpty(), "the observation is dated, or nothing can report its own staleness") diff --git a/test/e2e/refusal_reapply_e2e_test.go b/test/e2e/refusal_reapply_e2e_test.go index 65aae2dc2..5058a582b 100644 --- a/test/e2e/refusal_reapply_e2e_test.go +++ b/test/e2e/refusal_reapply_e2e_test.go @@ -32,7 +32,7 @@ import ( // Flux applies the overlay when its Kustomization is created, and the trigger then holds it // suspended, so a revert of the drifted env var inside this spec is an apply GitOps Reverser started, // and nothing else: a resume of the unchanged commit, which is all a Refusal needs. -var _ = Describe("Manager Refusal Reapply", Label("manager", "refusal-reapply"), Ordered, func() { +var _ = Describe("Manager Refusal Reapply", Label("manager", "refusal-reapply"), Ordered, SpecPriority(80), func() { var ( testNs string repo *RepoArtifacts diff --git a/test/e2e/tools/ginkgo-allure/main.go b/test/e2e/tools/ginkgo-allure/main.go index e76f11a72..139a9ae9a 100644 --- a/test/e2e/tools/ginkgo-allure/main.go +++ b/test/e2e/tools/ginkgo-allure/main.go @@ -12,6 +12,7 @@ import ( "io" "os" "path/filepath" + "regexp" "strconv" "strings" "time" @@ -116,7 +117,10 @@ func convertReports(outputDir, path string, reports []types.Report) (int, error) continue } result := convertSpec(path, reportIndex, specIndex, report, spec) - if err := writeResult(outputDir, result, spec.CombinedOutput()); err != nil { + if err := writeResult(outputDir, result, + textAttachment{name: "Ginkgo output", suffix: "attachment", body: spec.CombinedOutput()}, + textAttachment{name: "Measurements", suffix: "measurements", body: measurements(spec)}, + ); err != nil { return count, err } count++ @@ -169,14 +173,50 @@ func convertSpec(path string, reportIndex, specIndex int, report types.Report, s return result } -func writeResult(outputDir string, result allureResult, combinedOutput string) error { - if strings.TrimSpace(combinedOutput) != "" { - source := result.UUID + "-attachment.txt" - if err := os.WriteFile(filepath.Join(outputDir, source), []byte(combinedOutput), 0o600); err != nil { +// textAttachment is a plain-text file attached to an Allure result; an empty body attaches nothing. +type textAttachment struct { + name string + suffix string + body string +} + +// ginkgoStyle matches the {{bold}}-style console markup Ginkgo keeps in a report entry's text. +var ginkgoStyle = regexp.MustCompile(`\{\{[^}]*\}\}`) + +// measurements renders a spec's report entries (AddReportEntry, such as a gmeasure experiment's +// table) as plain text, so the numbers a spec records sit beside its result in Allure. +func measurements(spec types.SpecReport) string { + var out strings.Builder + for _, entry := range spec.ReportEntries { + // A report read back from JSON keeps the text Ginkgo rendered when it wrote the file in + // Representation; StringRepresentation is the in-process fallback. + text := entry.Value.Representation + if text == "" { + text = entry.StringRepresentation() + } + text = strings.TrimSpace(ginkgoStyle.ReplaceAllString(text, "")) + if text == "" { + continue + } + if !strings.HasPrefix(text, entry.Name) { + out.WriteString(entry.Name + "\n") + } + out.WriteString(text + "\n\n") + } + return out.String() +} + +func writeResult(outputDir string, result allureResult, attachments ...textAttachment) error { + for _, attachment := range attachments { + if strings.TrimSpace(attachment.body) == "" { + continue + } + source := result.UUID + "-" + attachment.suffix + ".txt" + if err := os.WriteFile(filepath.Join(outputDir, source), []byte(attachment.body), 0o600); err != nil { return fmt.Errorf("write attachment %s: %w", source, err) } result.Attachments = append(result.Attachments, allureAttachment{ - Name: "Ginkgo output", + Name: attachment.name, Source: source, Type: "text/plain", }) diff --git a/test/e2e/tools/ginkgo-allure/main_test.go b/test/e2e/tools/ginkgo-allure/main_test.go index 49600f475..80ca791ae 100644 --- a/test/e2e/tools/ginkgo-allure/main_test.go +++ b/test/e2e/tools/ginkgo-allure/main_test.go @@ -320,7 +320,8 @@ func TestWriteResult_NoAttachment(t *testing.T) { Status: "passed", Stage: "finished", } - if err := writeResult(outputDir, result, " \n\t"); err != nil { + blank := textAttachment{name: "Ginkgo output", suffix: "attachment", body: " \n\t"} + if err := writeResult(outputDir, result, blank); err != nil { t.Fatalf("writeResult: %v", err) } @@ -333,6 +334,58 @@ func TestWriteResult_NoAttachment(t *testing.T) { } } +// styledTable stands in for a gmeasure experiment: a value Ginkgo renders through ColorableString. +type styledTable struct{} + +func (styledTable) ColorableString() string { + return "{{bold}}Flux: Git-side push\n{{/}}push → live | 3 | 2.2s | 2.7s" +} + +func (styledTable) String() string { return "unused" } + +func TestRun_AttachesReportEntriesAsMeasurements(t *testing.T) { + reports := fixtureReport() + spec := &reports[0].SpecReports[1] + spec.ReportEntries = []types.ReportEntry{ + {Name: "Flux: Git-side push", Value: types.WrapEntryValue(styledTable{})}, + {Name: "empty", Value: types.WrapEntryValue(nil)}, + {Name: "gitea-commit-verification", Value: types.WrapEntryValue("verified: true")}, + } + reportPath := writeFixtureReport(t, reports) + outputDir := filepath.Join(t.TempDir(), "allure-results") + if _, err := run(outputDir, []string{reportPath}); err != nil { + t.Fatalf("run: %v", err) + } + + result := findResult(t, readAllureResults(t, outputDir), "passes") + var source string + for _, attachment := range result.Attachments { + if attachment.Name == "Measurements" { + source = attachment.Source + } + } + if source == "" { + t.Fatalf("attachments: got %#v, want one named Measurements", result.Attachments) + } + data, err := os.ReadFile(filepath.Join(outputDir, source)) + if err != nil { + t.Fatalf("read measurements: %v", err) + } + want := "Flux: Git-side push\npush → live | 3 | 2.2s | 2.7s\n\n" + + "gitea-commit-verification\nverified: true\n\n" + if string(data) != want { + t.Errorf("measurements:\n got %q\nwant %q", string(data), want) + } + + for _, other := range readAllureResults(t, outputDir) { + for _, attachment := range other.Attachments { + if other.Name != "passes" && attachment.Name == "Measurements" { + t.Errorf("%s: a spec without report entries got a Measurements attachment", other.Name) + } + } + } +} + func TestStatusMappings(t *testing.T) { cases := map[types.SpecState]string{ types.SpecStatePassed: "passed",