From cc874eb52b94e20377f619559c0f9efb42346a20 Mon Sep 17 00:00:00 2001 From: ahmadalguydi Date: Tue, 28 Jul 2026 19:12:45 +0300 Subject: [PATCH 1/6] feat(nomos): poll status until repositories complete --- cmd/nomos/status/cluster_state.go | 20 ++++++ cmd/nomos/status/poll_until_test.go | 71 +++++++++++++++++++++ cmd/nomos/status/status.go | 33 ++++++++-- docs/design-docs/03-poll-until-status.md | 79 ++++++++++++++++++++++++ 4 files changed, 197 insertions(+), 6 deletions(-) create mode 100644 cmd/nomos/status/poll_until_test.go create mode 100644 docs/design-docs/03-poll-until-status.md diff --git a/cmd/nomos/status/cluster_state.go b/cmd/nomos/status/cluster_state.go index 02880ca9ff..3a1d1acbe1 100644 --- a/cmd/nomos/status/cluster_state.go +++ b/cmd/nomos/status/cluster_state.go @@ -46,6 +46,26 @@ type ClusterState struct { isMulti *bool } +// allSynced reports whether every reachable cluster has at least one repository +// and every repository has completed successfully. A cluster or repository +// error keeps polling active so callers can observe a later successful state. +func allSynced(states map[string]*ClusterState) bool { + if len(states) == 0 { + return false + } + for _, state := range states { + if state == nil || state.Error != "" || len(state.repos) == 0 { + return false + } + for _, repo := range state.repos { + if repo == nil || repo.status != syncedMsg || len(repo.errors) > 0 { + return false + } + } + } + return true +} + func (c *ClusterState) printRows(writer io.Writer) { util.MustFprintf(writer, "\n") util.MustFprintf(writer, "%s\n", c.Ref) diff --git a/cmd/nomos/status/poll_until_test.go b/cmd/nomos/status/poll_until_test.go new file mode 100644 index 0000000000..28c62b41a7 --- /dev/null +++ b/cmd/nomos/status/poll_until_test.go @@ -0,0 +1,71 @@ +// Copyright 2026 Google LLC +// +// 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 status + +import "testing" + +func TestAllSynced(t *testing.T) { + tests := []struct { + name string + states map[string]*ClusterState + want bool + }{ + { + name: "all repositories synced", + states: map[string]*ClusterState{ + "cluster": {repos: []*RepoState{{status: syncedMsg}}}, + }, + want: true, + }, + { + name: "pending repository", + states: map[string]*ClusterState{ + "cluster": {repos: []*RepoState{{status: pendingMsg}}}, + }, + }, + { + name: "cluster error", + states: map[string]*ClusterState{ + "cluster": {Error: "unavailable", repos: []*RepoState{{status: syncedMsg}}}, + }, + }, + { + name: "empty state", + states: map[string]*ClusterState{ + "cluster": {}, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := allSynced(test.states); got != test.want { + t.Fatalf("allSynced() = %v, want %v", got, test.want) + } + }) + } +} + +func TestValidatePollUntil(t *testing.T) { + if err := validatePollUntil(""); err != nil { + t.Fatalf("validatePollUntil(\"\") returned error: %v", err) + } + if err := validatePollUntil(pollUntilComplete); err != nil { + t.Fatalf("validatePollUntil(%q) returned error: %v", pollUntilComplete, err) + } + if err := validatePollUntil("ready"); err == nil { + t.Fatal("validatePollUntil(\"ready\") returned nil") + } +} diff --git a/cmd/nomos/status/status.go b/cmd/nomos/status/status.go index c56db29a70..8b69b8ddb6 100644 --- a/cmd/nomos/status/status.go +++ b/cmd/nomos/status/status.go @@ -34,14 +34,17 @@ import ( ) const ( - pendingMsg = "PENDING" - syncedMsg = "SYNCED" - stalledMsg = "STALLED" - reconcilingMsg = "RECONCILING" + pendingMsg = "PENDING" + syncedMsg = "SYNCED" + stalledMsg = "STALLED" + reconcilingMsg = "RECONCILING" + pollUntilComplete = "complete" + defaultPollInterval = 5 * time.Second ) var ( pollingInterval time.Duration + pollUntil string namespace string resourceStatus bool name string @@ -51,6 +54,7 @@ func init() { flags.AddContexts(Cmd) Cmd.Flags().DurationVar(&flags.ClientTimeout, "timeout", restconfig.DefaultTimeout, "Sets the timeout for connecting to each cluster. Defaults to 15 seconds. Example: --timeout=30s") Cmd.Flags().DurationVar(&pollingInterval, "poll", 0*time.Second, "Continuously polls for status updates at the specified interval. If not provided, the command runs only once. Example: --poll=30s for polling every 30 seconds") + Cmd.Flags().StringVar(&pollUntil, "poll-until", "", "Continues polling until the requested state is reached. Supported value: complete. Defaults to a 5-second polling interval when --poll is not set.") Cmd.Flags().StringVar(&namespace, "namespace", "", "Filters the status output by the specified RootSync or RepoSync namespace. If not provided, displays status for all RootSync and RepoSync objects.") Cmd.Flags().BoolVar(&resourceStatus, "resources", true, "Displays detailed status for individual resources managed by RootSync or RepoSync objects. Defaults to true.") Cmd.Flags().StringVar(&name, "name", "", "Filters the status output by the specified RootSync or RepoSync name.") @@ -97,6 +101,9 @@ var Cmd = &cobra.Command{ RunE: func(cmd *cobra.Command, _ []string) error { // Don't show usage on error, as argument validation passed. cmd.SilenceUsage = true + if err := validatePollUntil(pollUntil); err != nil { + return err + } fmt.Println("Connecting to clusters...") @@ -116,9 +123,15 @@ var Cmd = &cobra.Command{ names := clusterNames(clientMap) writer := util.NewWriter(os.Stdout) + if pollUntil != "" && pollingInterval == 0 { + pollingInterval = defaultPollInterval + } if pollingInterval > 0 { for { - printStatus(cmd.Context(), writer, clientMap, names) + complete := printStatus(cmd.Context(), writer, clientMap, names) + if pollUntil == pollUntilComplete && complete { + return nil + } time.Sleep(pollingInterval) } } else { @@ -161,7 +174,7 @@ func clusterStates(ctx context.Context, clientMap map[string]*ClusterClient) (ma // and then prints a formatted status row for each one. If there are any errors reported by either // object, those are printed in a second table under the status table. // nolint:errcheck -func printStatus(ctx context.Context, writer *tabwriter.Writer, clientMap map[string]*ClusterClient, names []string) { +func printStatus(ctx context.Context, writer *tabwriter.Writer, clientMap map[string]*ClusterClient, names []string) bool { // First build up a map of all the states to display. stateMap, monoRepoClusters := clusterStates(ctx, clientMap) @@ -192,6 +205,14 @@ func printStatus(ctx context.Context, writer *tabwriter.Writer, clientMap map[st } writer.Flush() + return allSynced(stateMap) +} + +func validatePollUntil(value string) error { + if value != "" && value != pollUntilComplete { + return fmt.Errorf("unsupported --poll-until value %q; supported value is %q", value, pollUntilComplete) + } + return nil } // clearTerminal executes an OS-specific command to clear all output on the terminal. diff --git a/docs/design-docs/03-poll-until-status.md b/docs/design-docs/03-poll-until-status.md new file mode 100644 index 0000000000..170a7191d4 --- /dev/null +++ b/docs/design-docs/03-poll-until-status.md @@ -0,0 +1,79 @@ +# Poll until status is complete + +* Author(s): ahmadalguydi +* Approver: +* Status: provisional + +## Summary + +Add `--poll-until=complete` to `nomos status` so automation can wait for every +reachable RootSync and RepoSync to finish successfully. The command prints the +initial status immediately, then refreshes it until all repositories report +`SYNCED` or the caller's external timeout terminates the process. + +## Motivation + +Issue [#2006](https://github.com/GoogleContainerTools/config-sync/issues/2006) +requests a bounded way to wait for eventual reconciliation without requiring +scripts to parse repeated status output. + +## Design Overview + +The existing `--poll` loop is extended with an optional completion predicate. +The predicate is true only when every discovered cluster is reachable, has at +least one RootSync or RepoSync, and every repository is `SYNCED` without +reported errors. A stalled, pending, reconciling, unavailable, or errored +repository keeps polling. The new flag accepts only `complete`; invalid values +fail before any cluster request. + +`--poll-until=complete` uses a five-second interval when `--poll` is omitted. +An explicit `--poll` interval continues to take precedence. Existing commands +without `--poll-until` retain their current one-shot or indefinite polling +behavior. + +## User Guide + +Run a status check once as before: + +```shell +nomos status +``` + +Wait for all repositories to become synchronized, refreshing every 10 seconds: + +```shell +timeout 2m nomos status --poll=10s --poll-until=complete +``` + +When all repositories are `SYNCED`, the command exits successfully after +printing that status. If the external timeout expires first, the process is +terminated and the caller can treat that as a reconciliation timeout. + +## Risks and Mitigations + +| Risk | Mitigation | +| --- | --- | +| Polling can increase API traffic. | Require an explicit completion mode and use a conservative five-second default; callers can choose a longer interval. | +| A cluster with no sync objects could be mistaken for completion. | Treat empty or unavailable cluster states as incomplete. | +| Existing users depend on indefinite `--poll`. | Preserve the existing loop unless `--poll-until` is provided. | + +## Test Plan + +Unit tests cover complete, pending, errored, and empty cluster states, plus +validation of the supported flag value. Existing status tests continue to +exercise rendering and cluster-state collection. + +## Open Issues/Questions + +### Should more completion states be supported? + +Resolution: Not yet resolved. The first implementation intentionally supports +only `complete`, matching issue #2006 and leaving room for future predicates. + +## Alternatives Considered + +### Shell-side output parsing + +Parsing repeated `nomos status` output is fragile and requires callers to know +the output format. A structured completion predicate keeps the behavior inside +the CLI while preserving its human-readable output. From 64ff1554803df13cd433cf09600cc835403a71ed Mon Sep 17 00:00:00 2001 From: ahmadalguydi Date: Wed, 29 Jul 2026 07:06:59 +0300 Subject: [PATCH 2/6] fix(status): wait for managed resources to be current Treat poll-until complete as full resource readiness, not only repository apply completion. Keep polling when a reported managed resource is not Current and document the behavior.\n\nAddresses maintainer review on #2006. --- cmd/nomos/status/cluster_state.go | 5 +++++ cmd/nomos/status/poll_until_test.go | 16 +++++++++++++++- docs/design-docs/03-poll-until-status.md | 7 ++++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/cmd/nomos/status/cluster_state.go b/cmd/nomos/status/cluster_state.go index 3a1d1acbe1..fb52095d2c 100644 --- a/cmd/nomos/status/cluster_state.go +++ b/cmd/nomos/status/cluster_state.go @@ -61,6 +61,11 @@ func allSynced(states map[string]*ClusterState) bool { if repo == nil || repo.status != syncedMsg || len(repo.errors) > 0 { return false } + for _, resource := range repo.resources { + if resource.Status != kptv1alpha1.Current { + return false + } + } } } return true diff --git a/cmd/nomos/status/poll_until_test.go b/cmd/nomos/status/poll_until_test.go index 28c62b41a7..a87af1a735 100644 --- a/cmd/nomos/status/poll_until_test.go +++ b/cmd/nomos/status/poll_until_test.go @@ -14,7 +14,11 @@ package status -import "testing" +import ( + "testing" + + kptv1alpha1 "github.com/GoogleContainerTools/config-sync/pkg/api/kpt.dev/v1alpha1" +) func TestAllSynced(t *testing.T) { tests := []struct { @@ -35,6 +39,16 @@ func TestAllSynced(t *testing.T) { "cluster": {repos: []*RepoState{{status: pendingMsg}}}, }, }, + { + name: "repository with non-current resource", + states: map[string]*ClusterState{ + "cluster": {repos: []*RepoState{{ + status: syncedMsg, + resources: []kptv1alpha1.ResourceStatus{{ + Status: kptv1alpha1.Failed, + }}, + }}}, + }, { name: "cluster error", states: map[string]*ClusterState{ diff --git a/docs/design-docs/03-poll-until-status.md b/docs/design-docs/03-poll-until-status.md index 170a7191d4..7261e95ae2 100644 --- a/docs/design-docs/03-poll-until-status.md +++ b/docs/design-docs/03-poll-until-status.md @@ -9,7 +9,8 @@ Add `--poll-until=complete` to `nomos status` so automation can wait for every reachable RootSync and RepoSync to finish successfully. The command prints the initial status immediately, then refreshes it until all repositories report -`SYNCED` or the caller's external timeout terminates the process. +`SYNCED` and every reported managed resource is `Current`, or the caller's +external timeout terminates the process. ## Motivation @@ -31,6 +32,10 @@ An explicit `--poll` interval continues to take precedence. Existing commands without `--poll-until` retain their current one-shot or indefinite polling behavior. +Completion checks managed-resource status independently of the `--resources` +display flag. A repository with no managed resources is complete once its sync +status is `SYNCED`; any reported resource in a non-`Current` state keeps polling. + ## User Guide Run a status check once as before: From 8603456afac23790348083b69b6accf8cd9d5c04 Mon Sep 17 00:00:00 2001 From: ahmadalguydi Date: Fri, 11 Sep 2026 12:12:17 +0300 Subject: [PATCH 3/6] fix(status): repair poll-until test literals --- cmd/nomos/status/poll_until_test.go | 38 +++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/cmd/nomos/status/poll_until_test.go b/cmd/nomos/status/poll_until_test.go index a87af1a735..54d0b63de4 100644 --- a/cmd/nomos/status/poll_until_test.go +++ b/cmd/nomos/status/poll_until_test.go @@ -29,36 +29,54 @@ func TestAllSynced(t *testing.T) { { name: "all repositories synced", states: map[string]*ClusterState{ - "cluster": {repos: []*RepoState{{status: syncedMsg}}}, + "cluster": &ClusterState{ + repos: []*RepoState{ + &RepoState{status: syncedMsg}, + }, + }, }, want: true, }, { name: "pending repository", states: map[string]*ClusterState{ - "cluster": {repos: []*RepoState{{status: pendingMsg}}}, + "cluster": &ClusterState{ + repos: []*RepoState{ + &RepoState{status: pendingMsg}, + }, + }, }, }, { name: "repository with non-current resource", states: map[string]*ClusterState{ - "cluster": {repos: []*RepoState{{ - status: syncedMsg, - resources: []kptv1alpha1.ResourceStatus{{ - Status: kptv1alpha1.Failed, - }}, - }}}, + "cluster": &ClusterState{ + repos: []*RepoState{ + &RepoState{ + status: syncedMsg, + resources: []kptv1alpha1.ResourceStatus{ + {Status: kptv1alpha1.Failed}, + }, + }, + }, + }, + }, }, { name: "cluster error", states: map[string]*ClusterState{ - "cluster": {Error: "unavailable", repos: []*RepoState{{status: syncedMsg}}}, + "cluster": &ClusterState{ + Error: "unavailable", + repos: []*RepoState{ + &RepoState{status: syncedMsg}, + }, + }, }, }, { name: "empty state", states: map[string]*ClusterState{ - "cluster": {}, + "cluster": &ClusterState{}, }, }, } From b47f2b635e5bd78df1f70ad41c1bc4099c7f6f3c Mon Sep 17 00:00:00 2001 From: ahmadalguydi Date: Fri, 11 Sep 2026 12:25:10 +0300 Subject: [PATCH 4/6] gofmt status polling changes --- cmd/nomos/status/status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/nomos/status/status.go b/cmd/nomos/status/status.go index 8b69b8ddb6..7e9c6c38a9 100644 --- a/cmd/nomos/status/status.go +++ b/cmd/nomos/status/status.go @@ -44,7 +44,7 @@ const ( var ( pollingInterval time.Duration - pollUntil string + pollUntil string namespace string resourceStatus bool name string From 20db06ab90c02b1a66aa78e7a781b3a0f3d1e456 Mon Sep 17 00:00:00 2001 From: ahmadalguydi Date: Fri, 11 Sep 2026 12:42:02 +0300 Subject: [PATCH 5/6] simplify status test literals --- cmd/nomos/status/poll_until_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/nomos/status/poll_until_test.go b/cmd/nomos/status/poll_until_test.go index 54d0b63de4..b4aa5c8ba3 100644 --- a/cmd/nomos/status/poll_until_test.go +++ b/cmd/nomos/status/poll_until_test.go @@ -29,9 +29,9 @@ func TestAllSynced(t *testing.T) { { name: "all repositories synced", states: map[string]*ClusterState{ - "cluster": &ClusterState{ + "cluster": { repos: []*RepoState{ - &RepoState{status: syncedMsg}, + {status: syncedMsg}, }, }, }, @@ -40,9 +40,9 @@ func TestAllSynced(t *testing.T) { { name: "pending repository", states: map[string]*ClusterState{ - "cluster": &ClusterState{ + "cluster": { repos: []*RepoState{ - &RepoState{status: pendingMsg}, + {status: pendingMsg}, }, }, }, @@ -50,9 +50,9 @@ func TestAllSynced(t *testing.T) { { name: "repository with non-current resource", states: map[string]*ClusterState{ - "cluster": &ClusterState{ + "cluster": { repos: []*RepoState{ - &RepoState{ + { status: syncedMsg, resources: []kptv1alpha1.ResourceStatus{ {Status: kptv1alpha1.Failed}, @@ -65,10 +65,10 @@ func TestAllSynced(t *testing.T) { { name: "cluster error", states: map[string]*ClusterState{ - "cluster": &ClusterState{ + "cluster": { Error: "unavailable", repos: []*RepoState{ - &RepoState{status: syncedMsg}, + {status: syncedMsg}, }, }, }, @@ -76,7 +76,7 @@ func TestAllSynced(t *testing.T) { { name: "empty state", states: map[string]*ClusterState{ - "cluster": &ClusterState{}, + "cluster": {}, }, }, } From 13cf2d1fea8149609b81eadb6f2dad5e77e1e4b2 Mon Sep 17 00:00:00 2001 From: ahmadalguydi Date: Wed, 16 Sep 2026 21:22:47 +0300 Subject: [PATCH 6/6] feat(nomos): add sync and readiness polling targets --- cmd/nomos/status/client.go | 3 + cmd/nomos/status/cluster_state.go | 50 +++++--- cmd/nomos/status/poll_until_test.go | 139 +++++++++++++++++++++-- cmd/nomos/status/status.go | 39 ++++--- cmd/nomos/status/status_test.go | 8 +- docs/design-docs/03-poll-until-status.md | 73 +++++++----- 6 files changed, 241 insertions(+), 71 deletions(-) diff --git a/cmd/nomos/status/client.go b/cmd/nomos/status/client.go index 290ed3727f..ec0e9bc2ac 100644 --- a/cmd/nomos/status/client.go +++ b/cmd/nomos/status/client.go @@ -168,15 +168,18 @@ func (c *ClusterClient) clusterStatus(ctx context.Context, cluster, namespace st if namespace == configsync.ControllerNamespace { if err := c.rootRepoClusterStatus(ctx, cs); err != nil { + cs.noSyncObjects = errors.Is(err, ErrNoRootSyncsFound) cs.Error = err.Error() } } else if namespace != "" { if err := c.namespaceRepoClusterStatus(ctx, cs, namespace); err != nil { + cs.noSyncObjects = errors.Is(err, ErrNoRepoSyncsFound) cs.Error = err.Error() } } else if isOss || (cs.isMulti != nil && *cs.isMulti) { if err := c.multiRepoClusterStatus(ctx, cs); err != nil { + cs.noSyncObjects = errors.Is(err, ErrNoRootSyncsFound) && errors.Is(err, ErrNoRepoSyncsFound) cs.Error = err.Error() } } else { diff --git a/cmd/nomos/status/cluster_state.go b/cmd/nomos/status/cluster_state.go index fb52095d2c..320aa1366b 100644 --- a/cmd/nomos/status/cluster_state.go +++ b/cmd/nomos/status/cluster_state.go @@ -41,34 +41,48 @@ type ClusterState struct { Ref string status string // Error represents the sync errors - Error string - repos []*RepoState - isMulti *bool + Error string + noSyncObjects bool + repos []*RepoState + isMulti *bool } -// allSynced reports whether every reachable cluster has at least one repository -// and every repository has completed successfully. A cluster or repository -// error keeps polling active so callers can observe a later successful state. -func allSynced(states map[string]*ClusterState) bool { - if len(states) == 0 { - return false - } +// pollUntilReached checks the selected repositories against the requested sync +// or resource-readiness target. Empty clusters do not block polling, but a +// requested name must appear before it can be considered complete. +func pollUntilReached(states map[string]*ClusterState, syncName, target string) (bool, error) { + matchedRepo := false for _, state := range states { - if state == nil || state.Error != "" || len(state.repos) == 0 { - return false + if state == nil || (state.Error != "" && !state.noSyncObjects) { + return false, nil + } + if state.noSyncObjects { + continue } for _, repo := range state.repos { - if repo == nil || repo.status != syncedMsg || len(repo.errors) > 0 { - return false + if repo == nil { + return false, nil + } + if syncName != "" && repo.syncName != syncName { + continue + } + matchedRepo = true + if repo.status != syncedMsg || len(repo.errors) > 0 { + return false, nil } - for _, resource := range repo.resources { - if resource.Status != kptv1alpha1.Current { - return false + if target != pollUntilSynced { + for _, resource := range repo.resources { + if resource.Status == kptv1alpha1.Failed { + return false, fmt.Errorf("resource %s managed by %s:%s has status %q", resourceStatusToString(resource), repo.scope, repo.syncName, resource.Status) + } + if resource.Status != kptv1alpha1.Current { + return false, nil + } } } } } - return true + return syncName == "" || matchedRepo, nil } func (c *ClusterState) printRows(writer io.Writer) { diff --git a/cmd/nomos/status/poll_until_test.go b/cmd/nomos/status/poll_until_test.go index b4aa5c8ba3..d14f5e84db 100644 --- a/cmd/nomos/status/poll_until_test.go +++ b/cmd/nomos/status/poll_until_test.go @@ -15,16 +15,18 @@ package status import ( + "strings" "testing" kptv1alpha1 "github.com/GoogleContainerTools/config-sync/pkg/api/kpt.dev/v1alpha1" ) -func TestAllSynced(t *testing.T) { +func TestPollUntilReached(t *testing.T) { tests := []struct { - name string - states map[string]*ClusterState - want bool + name string + states map[string]*ClusterState + want bool + wantErr bool }{ { name: "all repositories synced", @@ -61,6 +63,7 @@ func TestAllSynced(t *testing.T) { }, }, }, + wantErr: true, }, { name: "cluster error", @@ -74,22 +77,139 @@ func TestAllSynced(t *testing.T) { }, }, { - name: "empty state", + name: "empty reachable cluster has nothing to wait for", states: map[string]*ClusterState{ "cluster": {}, }, + want: true, + }, + { + name: "cluster without sync objects does not block another cluster", + states: map[string]*ClusterState{ + "empty-cluster": {Error: "No RootSync resources found\nNo RepoSync resources found", noSyncObjects: true}, + "active-cluster": {repos: []*RepoState{{status: syncedMsg}}}, + }, + want: true, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - if got := allSynced(test.states); got != test.want { - t.Fatalf("allSynced() = %v, want %v", got, test.want) + got, err := pollUntilReached(test.states, "", pollUntilCurrent) + if (err != nil) != test.wantErr { + t.Fatalf("pollUntilReached() error = %v, want error %v", err, test.wantErr) + } + if got != test.want { + t.Fatalf("pollUntilReached() = %v, want %v", got, test.want) } }) } } +func TestPollUntilReachedRequiresCurrentResourceForCurrentMode(t *testing.T) { + tests := []struct { + name string + resourceState kptv1alpha1.Status + want bool + wantErr bool + }{ + {name: "current resource", resourceState: kptv1alpha1.Current, want: true}, + {name: "unknown resource", resourceState: kptv1alpha1.Unknown, want: false}, + {name: "failed resource", resourceState: kptv1alpha1.Failed, want: false, wantErr: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + states := map[string]*ClusterState{ + "cluster": {repos: []*RepoState{{ + status: syncedMsg, + resources: []kptv1alpha1.ResourceStatus{{Status: test.resourceState}}, + }}}, + } + got, err := pollUntilReached(states, "", pollUntilCurrent) + if (err != nil) != test.wantErr { + t.Fatalf("pollUntilReached() error = %v, want error %v", err, test.wantErr) + } + if got != test.want { + t.Fatalf("pollUntilReached() = %v, want %v", got, test.want) + } + }) + } +} + +func TestPollUntilReachedFailsForFailedResourceInCurrentMode(t *testing.T) { + states := map[string]*ClusterState{ + "cluster": {repos: []*RepoState{{ + scope: "apps", + syncName: "payments", + status: syncedMsg, + resources: []kptv1alpha1.ResourceStatus{{ + ObjMetadata: kptv1alpha1.ObjMetadata{ + GroupKind: kptv1alpha1.GroupKind{Group: "apps", Kind: "Deployment"}, + Name: "checkout", + }, + Status: kptv1alpha1.Failed, + }}, + }}}, + } + + done, err := pollUntilReached(states, "", pollUntilCurrent) + if err == nil { + t.Fatal("pollUntilReached() returned no error for a failed resource") + } + if done { + t.Fatal("pollUntilReached() reported completion for a failed resource") + } + if !strings.Contains(err.Error(), "deployment.apps/checkout") { + t.Fatalf("pollUntilReached() error = %q, want the failed resource identified", err) + } +} + +func TestPollUntilReachedUsesNameFilter(t *testing.T) { + states := map[string]*ClusterState{ + "cluster": { + repos: []*RepoState{ + {syncName: "selected", status: syncedMsg}, + {syncName: "other", status: pendingMsg}, + }, + }, + } + got, err := pollUntilReached(states, "selected", pollUntilCurrent) + if err != nil { + t.Fatalf("pollUntilReached() returned unexpected error: %v", err) + } + if !got { + t.Fatal("pollUntilReached() = false, want true when the selected repo is synced") + } +} + +func TestPollUntilReachedWaitsForNamedRepoToAppear(t *testing.T) { + states := map[string]*ClusterState{"cluster": {repos: []*RepoState{{syncName: "other", status: syncedMsg}}}} + if got, err := pollUntilReached(states, "selected", pollUntilCurrent); err != nil || got { + t.Fatalf("pollUntilReached() = (%v, %v), want (false, nil) when no repo matches", got, err) + } +} + +func TestPollUntilReachedSyncedTargetIgnoresResourceReadiness(t *testing.T) { + states := map[string]*ClusterState{ + "cluster": { + repos: []*RepoState{{ + syncName: "selected", + status: syncedMsg, + resources: []kptv1alpha1.ResourceStatus{ + {Status: kptv1alpha1.Unknown}, + }, + }}, + }, + } + got, err := pollUntilReached(states, "", pollUntilSynced) + if err != nil { + t.Fatalf("pollUntilReached() returned unexpected error: %v", err) + } + if !got { + t.Fatal("pollUntilReached() = false, want true when sync completed and mode is synced") + } +} + func TestValidatePollUntil(t *testing.T) { if err := validatePollUntil(""); err != nil { t.Fatalf("validatePollUntil(\"\") returned error: %v", err) @@ -97,6 +217,11 @@ func TestValidatePollUntil(t *testing.T) { if err := validatePollUntil(pollUntilComplete); err != nil { t.Fatalf("validatePollUntil(%q) returned error: %v", pollUntilComplete, err) } + for _, value := range []string{pollUntilSynced, pollUntilCurrent} { + if err := validatePollUntil(value); err != nil { + t.Errorf("validatePollUntil(%q) returned error: %v", value, err) + } + } if err := validatePollUntil("ready"); err == nil { t.Fatal("validatePollUntil(\"ready\") returned nil") } diff --git a/cmd/nomos/status/status.go b/cmd/nomos/status/status.go index 7e9c6c38a9..53b06cd2fe 100644 --- a/cmd/nomos/status/status.go +++ b/cmd/nomos/status/status.go @@ -38,6 +38,8 @@ const ( syncedMsg = "SYNCED" stalledMsg = "STALLED" reconcilingMsg = "RECONCILING" + pollUntilSynced = "synced" + pollUntilCurrent = "current" pollUntilComplete = "complete" defaultPollInterval = 5 * time.Second ) @@ -54,7 +56,7 @@ func init() { flags.AddContexts(Cmd) Cmd.Flags().DurationVar(&flags.ClientTimeout, "timeout", restconfig.DefaultTimeout, "Sets the timeout for connecting to each cluster. Defaults to 15 seconds. Example: --timeout=30s") Cmd.Flags().DurationVar(&pollingInterval, "poll", 0*time.Second, "Continuously polls for status updates at the specified interval. If not provided, the command runs only once. Example: --poll=30s for polling every 30 seconds") - Cmd.Flags().StringVar(&pollUntil, "poll-until", "", "Continues polling until the requested state is reached. Supported value: complete. Defaults to a 5-second polling interval when --poll is not set.") + Cmd.Flags().StringVar(&pollUntil, "poll-until", "", "Continues polling until the requested state is reached. Supported values: synced, current (complete is an alias for current). Defaults to a 5-second polling interval when --poll is not set.") Cmd.Flags().StringVar(&namespace, "namespace", "", "Filters the status output by the specified RootSync or RepoSync namespace. If not provided, displays status for all RootSync and RepoSync objects.") Cmd.Flags().BoolVar(&resourceStatus, "resources", true, "Displays detailed status for individual resources managed by RootSync or RepoSync objects. Defaults to true.") Cmd.Flags().StringVar(&name, "name", "", "Filters the status output by the specified RootSync or RepoSync name.") @@ -78,7 +80,8 @@ func SaveToTempFile(ctx context.Context, contexts []string) (*os.File, error) { } names := clusterNames(clientMap) - printStatus(ctx, writer, clientMap, names) + stateMap, monoRepoClusters := clusterStates(ctx, clientMap) + printStatus(writer, stateMap, monoRepoClusters, names) err = tmpFile.Close() if err != nil { return tmpFile, fmt.Errorf("failed to close status file writer with error: %w", err) @@ -128,14 +131,22 @@ var Cmd = &cobra.Command{ } if pollingInterval > 0 { for { - complete := printStatus(cmd.Context(), writer, clientMap, names) - if pollUntil == pollUntilComplete && complete { - return nil + stateMap, monoRepoClusters := clusterStates(cmd.Context(), clientMap) + printStatus(writer, stateMap, monoRepoClusters, names) + if pollUntil != "" { + done, completionErr := pollUntilReached(stateMap, name, pollUntil) + if completionErr != nil { + return completionErr + } + if done { + return nil + } } time.Sleep(pollingInterval) } } else { - printStatus(cmd.Context(), writer, clientMap, names) + stateMap, monoRepoClusters := clusterStates(cmd.Context(), clientMap) + printStatus(writer, stateMap, monoRepoClusters, names) } return nil }, @@ -170,14 +181,11 @@ func clusterStates(ctx context.Context, clientMap map[string]*ClusterClient) (ma return stateMap, monoRepoClusters } -// printStatus fetches ConfigManagementStatus and/or RepoStatus from each cluster in the given map -// and then prints a formatted status row for each one. If there are any errors reported by either -// object, those are printed in a second table under the status table. +// printStatus writes the collected cluster states in a formatted table. If +// there are any errors reported by either object, those are printed in a +// second table under the status table. // nolint:errcheck -func printStatus(ctx context.Context, writer *tabwriter.Writer, clientMap map[string]*ClusterClient, names []string) bool { - // First build up a map of all the states to display. - stateMap, monoRepoClusters := clusterStates(ctx, clientMap) - +func printStatus(writer *tabwriter.Writer, stateMap map[string]*ClusterState, monoRepoClusters, names []string) { // Log a notice for the detected clusters that are running in the mono-repo mode. util.MonoRepoNotice(writer, monoRepoClusters...) @@ -205,12 +213,11 @@ func printStatus(ctx context.Context, writer *tabwriter.Writer, clientMap map[st } writer.Flush() - return allSynced(stateMap) } func validatePollUntil(value string) error { - if value != "" && value != pollUntilComplete { - return fmt.Errorf("unsupported --poll-until value %q; supported value is %q", value, pollUntilComplete) + if value != "" && value != pollUntilSynced && value != pollUntilCurrent && value != pollUntilComplete { + return fmt.Errorf("unsupported --poll-until value %q; supported values are %q, %q, and %q", value, pollUntilSynced, pollUntilCurrent, pollUntilComplete) } return nil } diff --git a/cmd/nomos/status/status_test.go b/cmd/nomos/status/status_test.go index f1f32d0f92..f9b7ecd0b1 100644 --- a/cmd/nomos/status/status_test.go +++ b/cmd/nomos/status/status_test.go @@ -361,8 +361,9 @@ func TestClusterStates(t *testing.T) { }, wantStateMap: map[string]*ClusterState{ "multi-repo-cluster": { - Ref: "multi-repo-cluster", - Error: "No RootSync resources found; No RepoSync resources found", + Ref: "multi-repo-cluster", + Error: "No RootSync resources found; No RepoSync resources found", + noSyncObjects: true, }, }, wantMonoRepoClusters: nil, @@ -572,7 +573,8 @@ func TestPrintStatus(t *testing.T) { var buf bytes.Buffer writer := tabwriter.NewWriter(&buf, 0, 0, 5, ' ', 0) - printStatus(context.Background(), writer, tc.clientMap, tc.names) + stateMap, monoRepoClusters := clusterStates(context.Background(), tc.clientMap) + printStatus(writer, stateMap, monoRepoClusters, tc.names) got := buf.String() if diff := cmp.Diff(tc.want, got); diff != "" { diff --git a/docs/design-docs/03-poll-until-status.md b/docs/design-docs/03-poll-until-status.md index 7261e95ae2..5c94160865 100644 --- a/docs/design-docs/03-poll-until-status.md +++ b/docs/design-docs/03-poll-until-status.md @@ -6,11 +6,11 @@ ## Summary -Add `--poll-until=complete` to `nomos status` so automation can wait for every -reachable RootSync and RepoSync to finish successfully. The command prints the -initial status immediately, then refreshes it until all repositories report -`SYNCED` and every reported managed resource is `Current`, or the caller's -external timeout terminates the process. +Add `--poll-until` targets to `nomos status` so automation can wait either for +RootSync and RepoSync objects to finish syncing or for their reported managed +resources to become current. The command prints the initial status immediately, +then refreshes it until the selected target is reached or the caller's external +timeout terminates the process. ## Motivation @@ -20,21 +20,30 @@ scripts to parse repeated status output. ## Design Overview -The existing `--poll` loop is extended with an optional completion predicate. -The predicate is true only when every discovered cluster is reachable, has at -least one RootSync or RepoSync, and every repository is `SYNCED` without -reported errors. A stalled, pending, reconciling, unavailable, or errored -repository keeps polling. The new flag accepts only `complete`; invalid values -fail before any cluster request. - -`--poll-until=complete` uses a five-second interval when `--poll` is omitted. +The existing `--poll` loop is extended with optional completion targets: +`--poll-until=synced` waits until each selected repository reports `SYNCED` +without errors; `--poll-until=current` adds the requirement that every reported +managed resource is `Current`. `complete` remains an alias for `current`. +A stalled, pending, reconciling, unavailable, or errored repository keeps +polling. A `Failed` managed resource stops `current` polling with an error +instead of waiting indefinitely. An `Unknown` resource does not count as +current; callers that only need successful Config Sync application can use +`synced` instead. Invalid values fail before any cluster request. + +The `--name` filter also scopes the completion check. A requested name must be +present in at least one queried cluster before polling can succeed. Reachable +clusters with no sync objects do not block other clusters from completing. + +Either completion target uses a five-second interval when `--poll` is omitted. An explicit `--poll` interval continues to take precedence. Existing commands without `--poll-until` retain their current one-shot or indefinite polling behavior. -Completion checks managed-resource status independently of the `--resources` -display flag. A repository with no managed resources is complete once its sync -status is `SYNCED`; any reported resource in a non-`Current` state keeps polling. +The `current` target checks managed-resource status independently of the +`--resources` display flag. A repository with no managed resources is complete +once its sync status is `SYNCED`. `Unknown`, `InProgress`, and other non-current +states keep polling, while `Failed` returns an error. The `synced` target does +not wait for managed-resource status. ## User Guide @@ -44,36 +53,46 @@ Run a status check once as before: nomos status ``` -Wait for all repositories to become synchronized, refreshing every 10 seconds: +Wait until all repositories have finished syncing, refreshing every 10 seconds: + +```shell +timeout 2m nomos status --poll=10s --poll-until=synced +``` + +Wait for all reported managed resources to reach `Current`: ```shell -timeout 2m nomos status --poll=10s --poll-until=complete +timeout 2m nomos status --poll=10s --poll-until=current ``` -When all repositories are `SYNCED`, the command exits successfully after -printing that status. If the external timeout expires first, the process is -terminated and the caller can treat that as a reconciliation timeout. +When the requested state is reached, the command exits successfully after +printing that status. `current` exits with an error if any resource reports +`Failed`. If the external timeout expires first, the process is terminated and +the caller can treat that as a reconciliation timeout. ## Risks and Mitigations | Risk | Mitigation | | --- | --- | | Polling can increase API traffic. | Require an explicit completion mode and use a conservative five-second default; callers can choose a longer interval. | -| A cluster with no sync objects could be mistaken for completion. | Treat empty or unavailable cluster states as incomplete. | +| A cluster with no sync objects could hold multi-cluster polling open. | Ignore empty clusters while checking other clusters; require a requested `--name` to appear before success. | +| A resource may never report kstatus `Current`. | Offer `synced` for apply completion and `current` for resource readiness; fail fast for `Failed` resources. | | Existing users depend on indefinite `--poll`. | Preserve the existing loop unless `--poll-until` is provided. | ## Test Plan -Unit tests cover complete, pending, errored, and empty cluster states, plus -validation of the supported flag value. Existing status tests continue to -exercise rendering and cluster-state collection. +Unit tests cover `synced` and `current` targets, name filtering, empty clusters, +current, unknown, and failed resource states, plus validation of supported flag +values. Existing status tests continue to exercise rendering and cluster-state +collection. ## Open Issues/Questions ### Should more completion states be supported? -Resolution: Not yet resolved. The first implementation intentionally supports -only `complete`, matching issue #2006 and leaving room for future predicates. +Resolution: Yes. `synced` covers Config Sync application, and `current` covers +managed-resource readiness. `complete` is retained as a compatibility alias for +`current`. ## Alternatives Considered