Skip to content

feat(hack): rewrite check-readiness as a Go command with test suite - #2755

Merged
Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
feat/check-readiness-go
Jun 9, 2026
Merged

Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
feat/check-readiness-go

Conversation

@myasnikovdaniil

@myasnikovdaniil myasnikovdaniil commented May 27, 2026 •

Copy link
Copy Markdown
Contributor

What this PR does

Replaces the hack/check-readiness.sh diagnostic with a Go command at
cmd/check-readiness, integrated into the build the same way as cozypkg:

  • make check-readiness builds _out/bin/check-readiness, with Version
    injected via -ldflags.
  • The tool shells out to kubectl and reports only non-ready cozystack /
    Flux / Kubernetes resources, parsed from status conditions (not column
    heuristics). Flags: --wait/--timeout, --watch, --core, --parallel,
    --verbose, -n/-l scoping, --no-color, --version.

Adds a black-box golden test suite under test/check-readiness/:

  • A mock kubectl serves canned fixtures; the suite diffs stdout/stderr/exit
    against golden files across 30 scenarios (arg parsing, condition/phase/suspend
    handling, CRD-not-installed, kubectl arg propagation, wait/timeout, edge
    cases) plus a PTY test for color output.
  • make test-check-readiness runs it; wired into make unit-tests.

Supersedes #2740, which added the same feature set to the bash script.

Release note

feat(hack): replace check-readiness.sh with a `check-readiness` Go command (build with `make check-readiness`); reports non-ready cozystack/Flux/Kubernetes resources with --wait/--watch/--core/--parallel/--verbose and namespace/label scoping.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added check-readiness command for verifying Kubernetes resource readiness status across namespaces
    • Supports multiple execution modes: one-shot, wait with timeout, and watch with configurable intervals
    • Includes namespace filtering, label selectors, parallel resource fetching, and color output control
  • Tests

    • Added comprehensive test suite validating command behavior across various scenarios

Review Change Stack

Replace hack/check-readiness.sh with a Go command at cmd/check-readiness,
wired into the build like cozypkg (`make check-readiness`, Version via
-ldflags). The tool shells out to kubectl and reports only non-ready
cozystack / Flux / Kubernetes resources parsed from status conditions,
supporting --wait/--timeout, --watch, --core, --parallel, --verbose,
-n/-l scoping, --no-color and --version.

Add a black-box golden test suite under test/check-readiness driven by a
mock kubectl over fixtures; `make test-check-readiness` runs it and it is
wired into `make unit-tests`.

Supersedes #2740, which added the same feature set to the bash script.

Assisted-By: Claude <[email protected]>
Signed-off-by: Myasnikov Daniil <[email protected]>
@github-actions github-actions Bot added area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review kind/feature Categorizes issue or PR as related to a new feature size/XXL This PR changes 1000+ lines, ignoring generated files labels May 27, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request migrates the check-readiness diagnostic tool from a bash script to a native Go implementation. This change improves reliability, maintainability, and performance by leveraging Go's concurrency and structured parsing capabilities. The new implementation is fully integrated into the project's build and test pipeline, supported by a new golden test suite that ensures consistent behavior across various cluster states and edge cases.

Highlights

  • Tool Migration: Replaced the bash-based hack/check-readiness.sh diagnostic script with a robust Go command located at cmd/check-readiness.
  • Improved Functionality: The new Go tool provides enhanced features including parallel fetching, configurable timeouts, namespace/label scoping, and improved status condition parsing.
  • Testing Infrastructure: Added a comprehensive black-box golden test suite in test/check-readiness/ that utilizes a mock kubectl to validate behavior across 30+ scenarios.
  • Build Integration: Integrated the new tool into the existing Makefile build system, ensuring version injection via -ldflags and inclusion in make unit-tests.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment Gemini (@gemini-code-assist) Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@dosubot dosubot Bot added area/build Issues or PRs related to image build infrastructure, multi-arch support area/testing Issues or PRs related to testing (e2e, bats, unit tests) labels May 27, 2026
@coderabbitai

coderabbitai Bot commented May 27, 2026 •

Copy link
Copy Markdown
Contributor

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

This PR replaces the Bash script hack/check-readiness.sh with a new Go CLI tool that detects non-ready Kubernetes resources by parsing status conditions instead of column heuristics. It includes build targets, the full CLI implementation, a comprehensive test suite with mocked kubectl, and 28+ golden output files validating all scenarios.

Changes

Check-readiness CLI Implementation

Layer / File(s) Summary
Build targets for check-readiness
Makefile
check-readiness binary build target with version embedding, test-check-readiness go test target, and integration into unit-tests aggregate.
Check-readiness CLI implementation
cmd/check-readiness/main.go
Full CLI parsing (--wait, --watch, --namespace, --selector, --timeout, --parallel, --core, --verbose, --no-color), kubectl api-resources caching for CRD detection, jsonpath-based condition/phase parsing, per-kind "not ready" output sections, colored labels, timeout handling, and exit codes.
Test framework, mock kubectl, and test infrastructure
test/check-readiness/main_test.go, test/check-readiness/fakekubectl/kubectl
Go test harness with table-driven test cases, fake kubectl mock script serving fixture-backed responses with <US> token rewriting, golden-file comparison with time normalization, environment isolation, subprocess execution, and PTY-based color verification.
Test fixtures and resource data
test/check-readiness/testdata/fixtures/_base/api-resources.txt, test/check-readiness/testdata/fixtures/crd-missing/api-resources.txt
Kubernetes API resource fixture tables defining resource kinds and versions (e.g., packages.cozystack.io, helmreleases.helm.toolkit.fluxcd.io, nodes, persistentvolumeclaims) for mock kubectl.
Golden output files for test validation
test/check-readiness/testdata/golden/*.stdout, test/check-readiness/testdata/golden/*.stderr
28+ golden files capturing expected CLI output across argument validation, readiness detection, wait/watch modes, namespace/selector flags, PVC phases, HelmRelease suspension, verbose conditions, timeouts, color output, and help text.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

kind/feature, size/L, area/ci

Suggested reviewers

  • kvaps
  • lllamnyp
  • androndo
  • lexfrei

Poem

🐰 A check-readiness tool hops into view,
With kubectl mocking and conditions anew,
Golden outputs dancing in test-driven grace,
Golang replaces bash in this cozy place! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.84% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the primary change: rewriting check-readiness from a Bash script to a Go command with comprehensive tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/check-readiness-go

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the legacy check-readiness.sh bash script with a new Go-based CLI tool cmd/check-readiness for diagnosing non-ready Cozystack, Flux, and Kubernetes resources. It also introduces a robust black-box testing suite under test/check-readiness/ using mock fixtures and golden files. The review feedback focuses on critical robustness improvements: checking for the existence of kubectl in PATH to prevent silent-success failures, explicitly handling errors from kubectl executions in fetchRows and runOnce (as mandated by the style guide), safely slicing strings using runes in cutChars to avoid malformed UTF-8 sequences, and tightening column checks when parsing api-resources output.

I am having trouble creating individual review comments. Click here to see my feedback.

cmd/check-readiness/main.go (205-206)

high

If kubectl is not installed or not available in the system's PATH, the tool will fail to run any queries, but because errors are currently ignored, it will silently report that all resources are ready and exit with 0. Checking if kubectl is available in PATH right after parsing arguments prevents this silent-success bug.

	cfg.setColors()

	if _, err := exec.LookPath(cfg.kubectl[0]); err != nil {
		fmt.Fprintf(os.Stderr, "Error: %s not found in PATH\n", cfg.kubectl[0])
		return 2
	}

cmd/check-readiness/main.go (377-409)

high

Currently, fetchRows completely discards any errors returned by cmd.Output(). If the API server is unreachable, or if there is an authentication/authorization failure, kubectl will fail, but fetchRows will silently return an empty string. This causes the tool to report that all resources are ready. Returning the error allows the caller to handle and report it properly.

// fetchRows runs kubectl get and returns raw jsonpath output.
func (cfg *config) fetchRows(e entry) (string, error) {
	scopeArgs := cfg.buildScopeArgs(e.scope)

	var jsonpath string
	if e.condType == "_phase" {
		jsonpath = `{range .items[*]}{.metadata.namespace}{"\x1f"}{.metadata.name}{"\x1f"}{.status.phase}{"\x1f"}{""}{"\x1f"}{""}{"\x1f"}{"false"}{"\n"}{end}`
	} else {
		suspendField := `{"false"}`
		if e.supportsSuspend {
			suspendField = `{.spec.suspend}`
		}
		var b strings.Builder
		b.WriteString(`{range .items[*]}`)
		b.WriteString(`{.metadata.namespace}{"\x1f"}`)
		b.WriteString(`{.metadata.name}{"\x1f"}`)
		b.WriteString(`{.status.conditions[?(@.type=="` + e.condType + `")].status}{"\x1f"}`)
		b.WriteString(`{.status.conditions[?(@.type=="` + e.condType + `")].reason}{"\x1f"}`)
		b.WriteString(`{.status.conditions[?(@.type=="` + e.condType + `")].message}{"\x1f"}`)
		b.WriteString(suspendField + `{"\n"}`)
		b.WriteString(`{end}`)
		jsonpath = b.String()
	}

	args := append([]string{}, cfg.kubectl[1:]...)
	args = append(args, "get", e.kind)
	args = append(args, scopeArgs...)
	args = append(args, "-o", "jsonpath="+jsonpath)

	cmd := exec.Command(cfg.kubectl[0], args...)
	var stderr strings.Builder
	cmd.Stderr = &stderr
	out, err := cmd.Output()
	if err != nil {
		if stderr.Len() > 0 {
			return "", fmt.Errorf("%s", strings.TrimSpace(stderr.String()))
		}
		return "", err
	}
	return string(out), nil
}
References
  1. Handle errors explicitly. Discarding meaningful errors with _ is a bug. (link)

cmd/check-readiness/main.go (537-592)

high

Update runOnce to capture and handle the errors returned by fetchRows. If any query fails, the error is printed to the output buffer and cfg.foundIssues is set to true, ensuring the tool exits with a non-zero code and reports the failure.

func (cfg *config) runOnce() string {
	cfg.foundIssues = false
	cfg.ensureAPIResources()

	entries := append([]entry{}, coreKinds...)
	if !cfg.coreOnly {
		entries = append(entries, fluxExtraKinds...)
		entries = append(entries, clusterKinds...)
		entries = append(entries, pvcKind)
	}

	rowsByIdx := make([]string, len(entries))
	errsByIdx := make([]error, len(entries))

	// Phase 1: fetch.
	if cfg.parallel {
		var wg sync.WaitGroup
		for idx := range entries {
			e := entries[idx]
			if !cfg.kindExists(e.kind) {
				continue
			}
			wg.Add(1)
			go func(idx int, e entry) {
				defer wg.Done()
				rowsByIdx[idx], errsByIdx[idx] = cfg.fetchRows(e)
			}(idx, e)
		}
		wg.Wait()
	} else {
		for idx := range entries {
			e := entries[idx]
			if !cfg.kindExists(e.kind) {
				continue
			}
			rowsByIdx[idx], errsByIdx[idx] = cfg.fetchRows(e)
		}
	}

	var b strings.Builder

	// Phase 2: process in catalog order.
	for idx := range entries {
		e := entries[idx]
		if !cfg.kindExists(e.kind) {
			fmt.Fprintf(&b, "%s--- %s: CRD not installed, skipping%s\n", cfg.dim, e.kind, cfg.reset)
			continue
		}
		if errsByIdx[idx] != nil {
			fmt.Fprintf(&b, "%sError fetching %s: %v%s\n", cfg.red, e.kind, errsByIdx[idx], cfg.reset)
			cfg.foundIssues = true
			continue
		}
		cfg.processRows(e, rowsByIdx[idx], &b)
	}

	if !cfg.foundIssues {
		fmt.Fprintf(&b, "%s%sAll resources are ready.%s\n", cfg.green, cfg.bold, cfg.reset)
	}

	return b.String()
}
References
  1. Handle errors explicitly. Discarding meaningful errors with _ is a bug. (link)

cmd/check-readiness/main.go (432-439)

medium

The comment states 'Use runes to be safe', but the implementation uses byte-based slicing (s[:n]). Slicing a string by bytes can cut in the middle of a multi-byte UTF-8 character, resulting in invalid UTF-8 strings. Converting the string to a slice of runes before slicing ensures safety and correctness.

// cutChars mimics `cut -c1-N` (rune-based to safely handle multi-byte UTF-8 characters).
func cutChars(s string, n int) string {
	runes := []rune(s)
	if len(runes) <= n {
		return s
	}
	return string(runes[:n])
}

cmd/check-readiness/main.go (359-361)

low

Since kubectl api-resources output always contains at least 4 columns (NAME, APIVERSION, NAMESPACED, KIND), checking len(fields) < 4 is more robust and prevents potential out-of-bounds or logical errors.

		if len(fields) < 4 {
			continue
		}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean Go rewrite of the readiness diagnostic with a thorough black-box golden suite; builds and vets clean, the parallel path is race-free, and there is no command-injection surface.

Business context: replace the hack/check-readiness.sh heuristic script with a cmd/check-readiness Go tool that reports non-ready cozystack/Flux/Kubernetes resources from status conditions, built and tested like the other in-repo commands.

Verified: go vet and go build are clean; kubectl is invoked via exec.Command(cfg.kubectl[0], args...) (arg slice, no shell), so the -n/-l passthrough is not an injection vector; the --parallel path fills a pre-sized rowsByIdx slice by unique index behind a WaitGroup barrier and mutates shared state (foundIssues) only in the sequential phase-2 pass, and go test -race ./test/check-readiness/ passes (30 golden scenarios plus a PTY color test). The Makefile wires check-readiness (with the Version ldflag) and test-check-readiness into unit-tests. The removed shell script has no remaining functional references — the only hits are historical v1.3.0 changelog entries, correctly left untouched.

Non-blocking: this supersedes #2740 (the bash-side version of the same feature set), so #2740 should be closed once this lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build Issues or PRs related to image build infrastructure, multi-arch support area/testing Issues or PRs related to testing (e2e, bats, unit tests) area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review kind/feature Categorizes issue or PR as related to a new feature size/XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants