From a258f674ea41126adeea7f26d82dc6458ab998c0 Mon Sep 17 00:00:00 2001 From: Ditto P S Date: Tue, 15 Sep 2026 02:02:14 +0530 Subject: [PATCH 1/2] fix: stop reporting false blockers on clusters that work A readiness run against a k3s cluster Bud was already serving reported three blockers, and none of them was stopping anything. This corrects the checks and the catalogue behind each false finding, and keeps the real ones. storage - csi-healthy matched every rancher/* image to rancher.io/local-path, so a Pending k3s ServiceLB pod read as a broken storage driver. The vendor label is now a last resort, used only when the provisioner has no more specific name (driver.longhorn.io, openebs.io/local). - csi-healthy no longer reports "registered on 0/N nodes" for a provisioner that is not a CSI driver. - expansion no longer asks node-local classes for allowVolumeExpansion: local-path ignores the requested size and has no resizer, so the node's disk is the ceiling and the suggested patch changed nothing. egress and charts - The prometheus-community, bitnami, CloudNativePG, Altinity, Percona, SeaweedFS and OpenTelemetry repos are optional: every ApplicationSet installs those charts from registry.bud.studio as packaged OCI charts with the subcharts inside, so the cluster never fetches them. dapr stays install-time. - Let's Encrypt moves to a new egress.acme check, skipped unless TLS is obtained through ACME, and it states the real consequence (no certificate) instead of "the sync stops at the first image or chart". - egress.proxy no longer treats k3s's NO_PROXY on helm-install pods as a proxy. argocd - chart-repo-credential tries an anonymous pull before predicting a 401: a charts project served to anonymous clients needs no repository Secret. gpu - The functional probe defaults to debian:trixie-slim. HAMi preloads its vGPU library into every GPU container and it needs libdl.so.2, so busybox's shell died in the loader. A loader error is now named, with the flag to change it. registry - budimages.azurecr.io is retired and removed from the inventory. report - Reports saved from the interactive view carry the budctl version, catalogue, cluster and domain. Report.Stamp is shared by both output paths; the TUI used to save before main stamped the report, so the file said "cluster unreachable". Co-Authored-By: Claude Opus 5 --- README.md | 6 +- cmd/budctl/flags.go | 2 +- cmd/budctl/main.go | 9 +-- internal/checks/argocd.go | 39 +++++++++- internal/checks/argocd_test.go | 42 +++++++++- internal/checks/charts.go | 6 +- internal/checks/charts_test.go | 34 ++++---- internal/checks/egress.go | 46 +++++++++-- internal/checks/egress_test.go | 91 +++++++++++++++++++++- internal/checks/gpu.go | 44 +++++++++-- internal/checks/gpu_test.go | 27 +++++-- internal/checks/harness_test.go | 1 - internal/checks/registry.go | 4 - internal/checks/registry_test.go | 1 - internal/checks/storage.go | 77 ++++++++++++++++--- internal/checks/storage_test.go | 128 +++++++++++++++++++++++++++++++ internal/engine/context.go | 3 + internal/engine/model.go | 14 ++++ internal/intake/defaults.yaml | 52 ++++++++----- internal/tui/app.go | 4 +- internal/tui/report_test.go | 62 +++++++++++++++ 21 files changed, 604 insertions(+), 88 deletions(-) create mode 100644 internal/tui/report_test.go diff --git a/README.md b/README.md index 765accb..564c21e 100644 --- a/README.md +++ b/README.md @@ -204,10 +204,12 @@ a closed output pipe (`budctl check | head`), which kills a Go program outright unless it says otherwise. `--no-probe` is read-only; `budctl cleanup` sweeps leftovers from a killed run. -The GPU probe requests `nvidia.com/gpu: 1` on a **busybox-class image**, not a +The GPU probe requests `nvidia.com/gpu: 1` on a **slim Debian image**, not a CUDA image: asserting the injected device node is present proves allocation, device-plugin injection and the runtime-hook chain for megabytes instead of -gigabytes. +gigabytes. It is not busybox because HAMi preloads its vGPU library into every +GPU container, and that library needs glibc's `libdl.so.2`; point +`--gpu-probe-image` at a mirror of any glibc image on an air-gapped cluster. ## SKIP is never a pass diff --git a/cmd/budctl/flags.go b/cmd/budctl/flags.go index 3c6a445..07bbc6e 100644 --- a/cmd/budctl/flags.go +++ b/cmd/budctl/flags.go @@ -141,7 +141,7 @@ func parseFlags(argv []string) (config, error) { fs.BoolVar(&c.keepProbes, "keep", false, "keep the probe namespace for debugging") fs.StringVar(&c.probeNamespace, "probe-namespace", "", "namespace for probe objects") fs.StringVar(&c.probeImage, "probe-image", "curlimages/curl:8.10.1", "image for network probes") - fs.StringVar(&c.gpuProbeImage, "gpu-probe-image", "busybox:1.36", "image for the GPU probe (deliberately not a CUDA image)") + fs.StringVar(&c.gpuProbeImage, "gpu-probe-image", "debian:trixie-slim", "image for the GPU probe (a slim glibc image, deliberately not a CUDA image)") fs.StringVar(&c.egressFrom, "egress-from", "cluster", "where egress is tested: cluster | workstation | both") fs.BoolVar(&c.hfThroughput, "hf-throughput", false, "sample Hugging Face download throughput") fs.StringVar(&c.argocdNamespace, "argocd-namespace", "argocd", "namespace ArgoCD is (or will be) installed in") diff --git a/cmd/budctl/main.go b/cmd/budctl/main.go index 9e0640c..028ac2f 100644 --- a/cmd/budctl/main.go +++ b/cmd/budctl/main.go @@ -121,6 +121,7 @@ func run() int { ArgoCDNamespace: cfg.argocdNamespace, ArgoCDEnabled: !cfg.noArgoCD, RegistryCreds: map[string]adapters.Credential{}, + BudctlVersion: Version, } if answers.RegistryUser != "" { c.Opts.RegistryCreds["registry.bud.studio"] = adapters.Credential{ @@ -178,13 +179,7 @@ func run() int { } else { rep = engine.Summarize(engine.Run(ctx, c, sel, nil)) } - rep.Platform = string(c.Platform.Distribution) - rep.Meta = map[string]string{ - "budctlVersion": Version, - "catalogVersion": profile.CatalogVersion, - "serverVersion": c.Platform.Version, - "domain": answers.Domain, - } + rep.Stamp(c) switch cfg.format { case "json": diff --git a/internal/checks/argocd.go b/internal/checks/argocd.go index 355292c..b52ef18 100644 --- a/internal/checks/argocd.go +++ b/internal/checks/argocd.go @@ -372,10 +372,33 @@ func init() { Output: strings.Join(Sorted(seen), "\n"), } if match == nil { + // With no matching Secret ArgoCD pulls anonymously, and a charts + // project the registry serves to anonymous clients needs no + // Secret at all. Ask the registry the same question before + // predicting a 401 that the cluster may never see. + var anonEv []engine.Evidence + refused := "" + if c.OCI != nil { + ref, version := argocdUmbrellaChartRef(c) + status, note := c.OCI.ChartManifest(ctx, ref, version, nil) + anonEv = append(anonEv, engine.Evidence{ + What: "HEAD " + ref + " at " + version + " with no credential (anonymous token)", + Output: string(status) + " — " + note, + }) + if status == adapters.ManifestOK { + return ch.Pass(fmt.Sprintf("no repository Secret covers %s/%s, and none is needed: the registry serves the %s chart to an anonymous pull, which is how ArgoCD fetches it without one", + argocdChartRegistry, argocdChartRepo, argocdUmbrellaChart)). + WithEvidence(append([]engine.Evidence{ev}, anonEv...)...). + Bounds("anonymous access is a setting on the registry's charts project, not on this cluster: if that project is made private, every Application's next pull fails with 401 until a repository Secret exists — and this pull came from this workstation, so whether the repo-server can reach the registry is argocd.chart-repo-reachable's question") + } + if status == adapters.ManifestUnauthorized { + refused = ", and the registry refuses an anonymous pull" + } + } return ch.Fail( - fmt.Sprintf("no ArgoCD repository Secret covers %s/%s, so every Application sourcing the %s chart fails its first pull with 401 unauthorized", argocdChartRegistry, argocdChartRepo, argocdUmbrellaChart), + fmt.Sprintf("no ArgoCD repository Secret covers %s/%s%s, so every Application sourcing the %s chart fails its first pull with 401 unauthorized", argocdChartRegistry, argocdChartRepo, refused, argocdUmbrellaChart), "kubectl apply -n "+inst.namespace+" -f - <<'EOF'\napiVersion: v1\nkind: Secret\nmetadata:\n name: bud-charts-repo\n labels:\n argocd.argoproj.io/secret-type: repository\nstringData:\n type: helm\n url: "+argocdChartRegistry+"/"+argocdChartRepo+"\n enableOCI: \"true\"\n username: robot$yourname\n password: \nEOF"). - WithEvidence(ev) + WithEvidence(append([]engine.Evidence{ev}, anonEv...)...) } name := match.Name() @@ -784,6 +807,18 @@ func argocdSecretField(o adapters.Object, key string) string { return strings.TrimSpace(string(dec)) } +// argocdUmbrellaChartRef is the chart and version an Application sources: the +// pin charts.oci resolves, or the version in --chart-dir when one was given. +func argocdUmbrellaChartRef(c *engine.Ctx) (ref, version string) { + version = argocdTargetChartVersion(c) + for _, e := range chartsOCICatalogue { + if e.name == argocdUmbrellaChart && version == "" { + version = e.version + } + } + return "oci://" + argocdChartRegistry + "/" + argocdChartRepo + "/" + argocdUmbrellaChart, version +} + // argocdRepoHost extracts the registry host from an ArgoCD repository URL. OCI // repository Secrets carry a bare "registry.bud.studio/charts" with no scheme, // which url.Parse reads as a path rather than a host. diff --git a/internal/checks/argocd_test.go b/internal/checks/argocd_test.go index 0f8bf10..81f84b9 100644 --- a/internal/checks/argocd_test.go +++ b/internal/checks/argocd_test.go @@ -729,6 +729,27 @@ func TestArgoCDVersionSkipsWhenNotInstalled(t *testing.T) { // argocd.chart-repo-credential // --------------------------------------------------------------------------- +// argocdTestRegistry scripts registry.bud.studio's token flow. anonymous decides +// whether the token issued to a client with no credential can read the charts +// project — the one registry setting that decides whether ArgoCD needs a +// repository Secret at all. +func argocdTestRegistry(anonymous bool) func(*http.Request) (*http.Response, error) { + return func(r *http.Request) (*http.Response, error) { + switch { + case strings.HasSuffix(r.URL.Path, "/service/token"): + resp := argocdReply(http.StatusOK) + resp.Body = io.NopCloser(strings.NewReader(`{"token":"anon"}`)) + return resp, nil + case anonymous && strings.Contains(r.URL.Path, "/manifests/") && r.Header.Get("Authorization") == "Bearer anon": + return argocdReply(http.StatusOK), nil + default: + resp := argocdReply(http.StatusUnauthorized) + resp.Header.Set("WWW-Authenticate", `Bearer realm="https://registry.bud.studio/service/token",service="harbor-registry"`) + return resp, nil + } + } +} + func TestArgoCDChartRepoCredentialRiskWhenNoSecretCoversTheRegistry(t *testing.T) { cases := []struct { name string @@ -760,13 +781,30 @@ func TestArgoCDChartRepoCredentialRiskWhenNoSecretCoversTheRegistry(t *testing.T for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { f := argocdTestInstall(argocdTestCluster()).with("secrets", "argocd", tc.secrets...) - r := run(t, f, "argocd.chart-repo-credential") + r := argocdRunHTTP(t, f, "argocd.chart-repo-credential", argocdTestRegistry(false)) assertStatus(t, r, "RISK") - argocdAssertMentions(t, r, tc.claims...) + argocdAssertMentions(t, r, append(tc.claims, "refuses an anonymous pull")...) }) } } +// A charts project the registry serves to anonymous clients needs no Secret: +// ArgoCD pulls anonymously when none matches. The tcs-vmware cluster synced +// every chart that way while this check predicted a 401 on the first pull. +func TestArgoCDChartRepoCredentialPassesWhenTheChartsProjectAllowsAnonymousPulls(t *testing.T) { + f := argocdTestInstall(argocdTestCluster()).with("secrets", "argocd", + argocdTestSecret("argocd", "bud-config-repo", "repository", map[string]string{ + "type": "git", "url": "https://github.com/bud/config.git", + })) + r := argocdRunHTTP(t, f, "argocd.chart-repo-credential", argocdTestRegistry(true)) + + assertStatus(t, r, "PASS") + argocdAssertMentions(t, r, "anonymous pull", "none is needed") + if !strings.Contains(r.DoesNotProve, "made private") { + t.Fatalf("a pass that rests on a registry setting must say the setting can change: %q", r.DoesNotProve) + } +} + // enableOCI is the single field that decides whether ArgoCD speaks the OCI API // or asks a registry for index.yaml — and asking yields a confusing 404, not an // error naming the setting. diff --git a/internal/checks/charts.go b/internal/checks/charts.go index ec292f4..068ac1e 100644 --- a/internal/checks/charts.go +++ b/internal/checks/charts.go @@ -253,11 +253,11 @@ func init() { switch { case len(blockers) > 0: return ch.Fail( - fmt.Sprintf("%d chart %s in scope %s serve index.yaml, so the dependencies pinned against %s never resolve and those charts do not install", + fmt.Sprintf("%d chart %s an ApplicationSet installs from directly %s serve index.yaml, so the charts sourced from %s do not install", len(blockers), Plural(len(blockers), "repository", "repositories"), Plural(len(blockers), "does not", "do not"), Plural(len(blockers), "it", "them")), - "allowlist these hosts on :443 from whatever runs the dependency resolve — this workstation for a direct `helm install`, the ArgoCD repo-server for a synced install — or mirror each repo and repoint the chart dependencies", + "allowlist these hosts on :443 from whatever fetches the chart — the ArgoCD repo-server for a synced install, this workstation for a direct `helm install` — or mirror each repo and repoint the Application's repoURL", append(Sorted(blockers), Sorted(risks)...)..., ).WithEvidence(evidence...) case len(risks) > 0: @@ -435,7 +435,7 @@ func chartsClassicSeverity(c *engine.Ctx, t intake.EgressTarget) (engine.Severit if t.When == "install" { return engine.Block, "" } - return engine.Risk, "optional: needed only if the feature behind it is enabled" + return engine.Risk, "optional: not fetched by the default install" } func chartsIsDataStoreRepo(u string) bool { diff --git a/internal/checks/charts_test.go b/internal/checks/charts_test.go index 9ae6255..dd19cdb 100644 --- a/internal/checks/charts_test.go +++ b/internal/checks/charts_test.go @@ -326,14 +326,15 @@ func TestChartsClassicSkipsWhenTheInventoryNamesNoRepositories(t *testing.T) { assertSkipHasReason(t, r) } -// One repo down blocks the install, and the finding has to name the chart that -// needs it: "bitnami unreachable" and "the common library subchart is absent" -// are not the same problem to the person holding the ticket. +// A repo an ApplicationSet installs from directly blocks the install when it is +// down, and the finding has to name what stops working: "dapr unreachable" and +// "the Dapr control plane cannot be installed" are not the same problem to the +// person holding the ticket. func TestChartsClassicBlocksWhenAnInstallTimeRepoIsUnreachable(t *testing.T) { r := chartsRun(t, vanilla(), "charts.classic", - chartsAllOK(chartsBlock("charts.bitnami.com")), nil) + chartsAllOK(chartsBlock("dapr.github.io")), nil) assertStatus(t, r, "BLOCK") - chartsMentions(t, r, "bitnami charts", "common library subchart") + chartsMentions(t, r, "dapr charts", "Dapr control plane") } // index.yaml is a document, not a registry endpoint: the "401 proves the host is @@ -355,25 +356,28 @@ func TestChartsClassicRisksWhenOnlyAnOptionalRepoIsUnreachable(t *testing.T) { chartsMentions(t, r, "SigNoz charts") } -// The same dead host is a blocker or a risk depending on whether the operator -// asked for in-cluster data stores. Reporting a repo nobody will contact as a -// blocker is how a report trains people to ignore it. -func TestChartsClassicScopesDataStoreReposToTheIntakeAnswer(t *testing.T) { +// The data-store operator charts are subcharts of the postgres, clickhouse and +// mongodb charts, and ArgoCD installs those from registry.bud.studio as packaged +// OCI charts with the subchart inside. A tcs-vmware cluster with +// docs.altinity.com blocked synced ClickHouse anyway. So a dead upstream repo +// is a risk for whoever builds the chart from source, never a blocker — and the +// note still says why when the operator chose external data stores. +func TestChartsClassicNeverBlocksOnAReposBundledIntoThePublishedCharts(t *testing.T) { cases := []struct { name string inClusterData bool - want string + note string }{ - {"in-cluster data stores: no CNPG operator means no database for any service", true, "BLOCK"}, - {"external data stores: the CNPG operator is never installed", false, "RISK"}, + {"in-cluster data stores: the operator subchart ships inside the OCI chart", true, "bundled in the published OCI chart"}, + {"external data stores: the operator is never installed", false, "external data stores selected"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { f := vanilla().withAnswers(func(a *intake.Answers) { a.InClusterData = tc.inClusterData }) r := chartsRun(t, f, "charts.classic", - chartsAllOK(chartsBlock("cloudnative-pg.github.io")), nil) - assertStatus(t, r, tc.want) - chartsMentions(t, r, "CloudNativePG charts") + chartsAllOK(chartsBlock("cloudnative-pg.github.io"), chartsBlock("docs.altinity.com")), nil) + assertStatus(t, r, "RISK") + chartsMentions(t, r, "CloudNativePG charts", "Altinity ClickHouse charts", tc.note) }) } } diff --git a/internal/checks/egress.go b/internal/checks/egress.go index 4549ec1..f8b78f5 100644 --- a/internal/checks/egress.go +++ b/internal/checks/egress.go @@ -86,6 +86,27 @@ func init() { }, }) + // The ACME directory is not an install-time fetch: the sync completes + // without it and cert-manager simply never issues. It is on the path only + // when the operator chose ACME — a provided certificate is a Secret, and + // 'none' publishes plain HTTP — so it gets its own check and its own gate. + engine.Register(&engine.Check{ + ID: "egress.acme", Group: "egress", Severity: engine.Block, + DependsOn: []string{"platform"}, Probe: true, + Run: func(ctx context.Context, c *engine.Ctx) engine.Result { + ch := engine.Lookup("egress.acme") + if !strings.HasPrefix(string(c.Answers.TLS), "acme") { + return ch.Skip("TLS will be obtained by " + domainsTLSMethod(c) + ", so cert-manager never calls an ACME directory") + } + r := egressEvaluate(ctx, c, ch, "acme", + "cert-manager cannot register an ACME account, so no certificate is issued and every hostname serves an untrusted one") + if r.State == engine.StateFail { + r.Remedy += " — or answer TLS as 'provided' and supply the certificate, which takes ACME off the path" + } + return r + }, + }) + // Port 22, not 443. An egress allowlist written as "HTTPS to the internet" // covers every other check in this group and still breaks every ArgoCD // Application whose repoURL is ssh:// — and it breaks them quietly, as a @@ -642,8 +663,11 @@ func egressEvaluate(ctx context.Context, c *engine.Ctx, ch *engine.Check, when, // egressNoun names a catalogue slice the way the report should read it: // "install-time", not "install", and never "runtime-time". func egressNoun(when string) string { - if when == "install" { + switch when { + case "install": return "install-time" + case "acme": + return "ACME" } return when } @@ -875,6 +899,10 @@ func egressReportProxy(ctx context.Context, c *engine.Ctx, ch *engine.Check) eng // egressNodeProxyHints looks for proxy environment variables on workloads that are // already running. It is a heuristic, not a reading of the node environment — // which is why the result is INFO and says so. +// +// NO_PROXY on its own is not a hint: k3s's helm controller sets it on every +// helm-install pod whether or not a proxy exists, so it is reported only beside +// an HTTP_PROXY or HTTPS_PROXY on the same container. func egressNodeProxyHints(ctx context.Context, c *engine.Ctx) []string { if c.Kube == nil { return nil @@ -886,6 +914,7 @@ func egressNodeProxyHints(ctx context.Context, c *engine.Ctx) []string { if !ok { continue } + var proxies, noProxy []string for _, e := range envs { em, ok := e.(map[string]any) if !ok { @@ -893,13 +922,20 @@ func egressNodeProxyHints(ctx context.Context, c *engine.Ctx) []string { } name, _ := em["name"].(string) value, _ := em["value"].(string) + if value == "" { + continue + } + line := fmt.Sprintf("pod/%s: %s=%s", p.Name(), strings.ToUpper(name), value) switch strings.ToUpper(name) { - case "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY": - if value != "" { - out = append(out, fmt.Sprintf("pod/%s: %s=%s", p.Name(), strings.ToUpper(name), value)) - } + case "HTTP_PROXY", "HTTPS_PROXY": + proxies = append(proxies, line) + case "NO_PROXY": + noProxy = append(noProxy, line) } } + if len(proxies) > 0 { + out = append(append(out, proxies...), noProxy...) + } } } return Sorted(out) diff --git a/internal/checks/egress_test.go b/internal/checks/egress_test.go index 6da3de5..8e1f29a 100644 --- a/internal/checks/egress_test.go +++ b/internal/checks/egress_test.go @@ -39,6 +39,8 @@ const ( egressTestDaprCharts = "https://dapr.github.io/helm-charts/index.yaml" egressTestHFAPI = "https://huggingface.co/api/models/gpt2" egressTestKyverno = "https://reg.kyverno.io/v2/" + egressTestAltinity = "https://docs.altinity.com/clickhouse-operator/index.yaml" + egressTestACME = "https://acme-v02.api.letsencrypt.org/directory" egressTestSSH = "github.com:22" ) @@ -200,7 +202,7 @@ func egressAssertNoVerdictEffect(t *testing.T, r engine.Result) { // three checks into permanent skips, and every fixture below into a tautology. func TestEgressCatalogueFillsEveryBucket(t *testing.T) { p := egressTestProfile(t) - for _, when := range []string{"install", "runtime", "optional"} { + for _, when := range []string{"install", "runtime", "optional", "acme"} { if len(egressTargetsWhen(p.Egress, when)) == 0 { t.Fatalf("the embedded catalogue lists no %q endpoints, so egress.%s can only ever SKIP", when, when) } @@ -338,7 +340,7 @@ func TestEgressInstallFollowsPodWhenWorkstationDisagrees(t *testing.T) { // the brief is to expose implementation problems, not to edit egress.go). // // Under --egress-from both, a node that could not pull the probe image produces -// egress.install PASS — "all 15 install-time endpoints answered from this +// egress.install PASS — "all 7 install-time endpoints answered from this // workstation" — because the ImagePull branch is guarded by (ws == nil || // !ws.Ran) and the workstation sweep then carries the verdict. The identical // cluster is a BLOCK under the default --egress-from cluster. That is the exact @@ -373,7 +375,7 @@ func TestEgressInstallGoesGreenOnAnUnpullableProbeImageUnderBothVantages(t *test // checks before the body runs; the body must reach the same verdict on its own, // because "we did not look" is not "we looked and it was fine". func TestEgressSkipsWithReasonWhenThereIsNoProbeRunner(t *testing.T) { - for _, id := range []string{"egress.install", "egress.runtime", "egress.optional", "egress.ssh"} { + for _, id := range []string{"egress.install", "egress.runtime", "egress.optional", "egress.acme", "egress.ssh"} { t.Run(id, func(t *testing.T) { f := egressCluster().withOpts(func(o *engine.Options) { o.NoProbe = true }) r := egressRun(t, f, id) @@ -433,6 +435,68 @@ func TestEgressOptionalPassesWhenAddonEndpointsAnswer(t *testing.T) { egressSeedCluster(egressTestVantage(t, egressVantageCluster, nil))), "PASS") } +// --------------------------------------------------------------------------- +// egress.acme +// --------------------------------------------------------------------------- + +// The shape of a real cluster Bud synced onto: Let's Encrypt reset the +// connection and the upstream ClickHouse chart repo was blocked. Neither is +// fetched by the sync, so the install set passes; the directory is egress.acme's +// to report and the Altinity repo only egress.optional's. +func TestEgressInstallPassesWhenOnlyACMEAndBundledChartReposAreBlocked(t *testing.T) { + v := egressTestVantage(t, egressVantageCluster, map[string]string{ + egressTestACME: "000", + egressTestAltinity: "000", + }) + assertStatus(t, egressRun(t, egressCluster(), "egress.install", egressSeedCluster(v)), "PASS") + + opt := egressRun(t, egressCluster(), "egress.optional", egressSeedCluster(v)) + assertStatus(t, opt, "INFO") + egressAssertNoVerdictEffect(t, opt) + egressAssertDetail(t, opt, "bundled in the published OCI chart") +} + +// Under an ACME answer an unreachable directory blocks: no certificate is ever +// issued. The summary has to say that, and not that the sync stops at a chart. +func TestEgressACMEBlocksWhenTheDirectoryIsUnreachableUnderAnACMEAnswer(t *testing.T) { + for _, method := range []intake.TLSMethod{intake.TLSACMEHTTP01, intake.TLSACMEDNS01} { + t.Run(string(method), func(t *testing.T) { + f := egressCluster().withAnswers(func(a *intake.Answers) { a.TLS = method }) + v := egressTestVantage(t, egressVantageCluster, map[string]string{egressTestACME: "000"}) + r := egressRun(t, f, "egress.acme", egressSeedCluster(v)) + + assertStatus(t, r, "BLOCK") + egressAssertContains(t, "summary", r.Summary, "Let's Encrypt ACME") + egressAssertContains(t, "summary", r.Summary, "no certificate is issued") + if strings.Contains(r.Summary, "image or chart") { + t.Fatalf("an ACME directory is neither an image nor a chart: %s", r.Summary) + } + egressAssertContains(t, "remedy", r.Remedy, "acme-v02.api.letsencrypt.org") + egressAssertContains(t, "remedy", r.Remedy, "'provided'") + }) + } +} + +// A provided certificate or plain HTTP never calls the directory, so the same +// blocked host is not this install's problem: a skip that says why. +func TestEgressACMESkipsWhenTLSIsNotObtainedThroughACME(t *testing.T) { + for _, method := range []intake.TLSMethod{intake.TLSProvided, intake.TLSNone} { + t.Run(string(method), func(t *testing.T) { + f := egressCluster().withAnswers(func(a *intake.Answers) { a.TLS = method }) + v := egressTestVantage(t, egressVantageCluster, map[string]string{egressTestACME: "000"}) + r := egressRun(t, f, "egress.acme", egressSeedCluster(v)) + + assertSkipHasReason(t, r) + egressAssertContains(t, "skip reason", r.Summary, "never calls an ACME directory") + }) + } +} + +func TestEgressACMEPassesWhenTheDirectoryAnswers(t *testing.T) { + assertStatus(t, egressRun(t, egressCluster(), "egress.acme", + egressSeedCluster(egressTestVantage(t, egressVantageCluster, nil))), "PASS") +} + // --------------------------------------------------------------------------- // egress.ssh // --------------------------------------------------------------------------- @@ -607,6 +671,27 @@ func TestEgressProxyReportsNodeEnvironmentHintsOnVanilla(t *testing.T) { egressAssertDetail(t, r, "pod/kube-proxy-abcde: HTTPS_PROXY=http://proxy.corp.example.com:3128") } +// k3s's helm controller sets NO_PROXY on every helm-install pod, proxy or not. +// Reading that alone as a hint explained a direct-connection cluster's egress +// failures as a proxy's. +func TestEgressProxyIgnoresNoProxyWithoutAProxyBesideIt(t *testing.T) { + egressClearProxyEnv(t) + f := egressCluster().with("pods", "kube-system", adapters.Object{ + "apiVersion": "v1", "kind": "Pod", + "metadata": map[string]any{"name": "helm-install-traefik-tqqgj", "namespace": "kube-system"}, + "spec": map[string]any{"containers": []any{map[string]any{ + "name": "helm", "image": "rancher/klipper-helm:v0.13.3", + "env": []any{map[string]any{"name": "NO_PROXY", "value": ".svc,.cluster.local,10.42.0.0/16,10.43.0.0/16"}}, + }}}, + "status": map[string]any{"phase": "Running"}, + }) + + r := run(t, f, "egress.proxy") + + assertStatus(t, r, "INFO") + egressAssertContains(t, "summary", r.Summary, "no egress proxy detected") +} + // §5.9's promise that "every egress result records it": with a cluster-wide // proxy in play, a node firewall rule is the wrong remedy, and a failure that // sent the operator to one would waste the outage. diff --git a/internal/checks/gpu.go b/internal/checks/gpu.go index 565ffa6..1123c40 100644 --- a/internal/checks/gpu.go +++ b/internal/checks/gpu.go @@ -33,9 +33,19 @@ const ( // Deliberately NOT a CUDA image (FRD-020 D10). Requesting nvidia.com/gpu: 1 // makes the device plugin inject the device nodes whatever the image is, so - // asserting /dev/nvidiactl inside busybox proves allocation, injection and - // the runtime-hook chain for megabytes instead of gigabytes. - gpuDefaultProbeImage = "busybox:1.36" + // asserting /dev/nvidiactl inside a slim image proves allocation, injection + // and the runtime-hook chain for megabytes instead of gigabytes. + // + // Slim, but glibc with its compatibility libraries — not busybox. HAMi + // preloads its vGPU interceptor into every container that asks for a GPU, + // and that library needs libdl.so.2: busybox's shell then dies in the + // loader before it prints a line. budcluster's own GPU canary uses a Debian + // slim image for the same reason. + gpuDefaultProbeImage = "debian:trixie-slim" + + // gpuLoaderError is what the dynamic loader prints when an injected library + // needs something the probe image does not ship. + gpuLoaderError = "error while loading shared libraries" ) // gpuVendor is one accelerator family: what the kubelet advertises, what the @@ -531,7 +541,7 @@ func init() { case gpuImagePullFailure(outcome.Reason, outcome.Events): // Not a GPU answer: the node could not fetch a few megabytes of - // busybox. registry.from-cluster and egress.install own that + // base image. registry.from-cluster and egress.install own that // blocker; reporting it here as a GPU fault would send the // operator to the wrong stack. return ch.Skip(fmt.Sprintf( @@ -556,7 +566,7 @@ func init() { case strings.Contains(outcome.Logs, gpuDevicePresent): return ch.Pass(fmt.Sprintf("a pod requesting %s: 1 scheduled, started, and found %s inside the container", v.Resource, strings.Join(v.Devices, " or "))). - With(fmt.Sprintf("probe image %s — a busybox-class image, not CUDA: the device plugin injects the device nodes whatever the image is (FRD-020 D10)", image)). + With(fmt.Sprintf("probe image %s — a slim base image, not CUDA: the device plugin injects the device nodes whatever the image is (FRD-020 D10)", image)). WithEvidence(ev...). Bounds("the probe runs under the default scheduler with no runtimeClassName and no toleration, while budcluster renders model pods with runtimeClassName: nvidia and schedulerName: hami-scheduler; and it says nothing about whether a specific model fits in the GPU's memory or whether MIG partitioning matches the deployment profile — budsim answers those at deploy time") @@ -574,6 +584,18 @@ func init() { With("the container listed /dev; the output is in the evidence below"). WithEvidence(ev...) + case strings.Contains(outcome.Logs, gpuLoaderError): + // Started, and the shell died in the dynamic loader before the + // script ran: a library the GPU stack injected (HAMi's vGPU + // interceptor, or the NVIDIA hook's driver libraries) needs one + // the probe image lacks. Allocation and container creation + // worked; the device assertion never ran, so it is unverified + // rather than failed — and the image is the thing to change. + return ch.Skip(fmt.Sprintf( + "the GPU probe started, but the probe image %s could not load a library the GPU stack injected into it (%s), so device injection is unverified; re-run with --gpu-probe-image set to a glibc image such as %s", + image, gpuFirstLine(outcome.Logs, gpuLoaderError), gpuDefaultProbeImage)). + WithEvidence(ev...) + default: // Started, exited, and said neither. Reporting a device fault // here would be inventing a finding out of a missing log. @@ -980,7 +1002,7 @@ func gpuOrNone(s, fallback string) string { return s } -// gpuImagePullFailure separates "the node could not fetch busybox" from "the +// gpuImagePullFailure separates "the node could not fetch the image" from "the // runtime refused to create the container". Both leave the pod scheduled and // not started, and their remedies have nothing in common. func gpuImagePullFailure(reason string, events []string) bool { @@ -992,3 +1014,13 @@ func gpuImagePullFailure(reason string, events []string) bool { } return false } + +// gpuFirstLine returns the first output line containing needle, trimmed. +func gpuFirstLine(logs, needle string) string { + for _, line := range strings.Split(logs, "\n") { + if strings.Contains(line, needle) { + return strings.TrimSpace(line) + } + } + return needle +} diff --git a/internal/checks/gpu_test.go b/internal/checks/gpu_test.go index 25449a0..8a6b51b 100644 --- a/internal/checks/gpu_test.go +++ b/internal/checks/gpu_test.go @@ -732,7 +732,7 @@ func TestGPUOperatorFunctionalPassesWhenDeviceIsPresentInContainer(t *testing.T) // FRD-020 D10: the probe must cost megabytes, not gigabytes. The recorded // evidence is where a regression to a CUDA image would show up. -func TestGPUOperatorFunctionalProbesWithBusyboxNotCUDA(t *testing.T) { +func TestGPUOperatorFunctionalProbesWithASlimImageNotCUDA(t *testing.T) { f := vanilla().with("nodes", "", node("gpu-1", withGPU("4"))) r := gpuTestRunWithProbes(t, f, 10*time.Second, gpuTestPodReactor("gpu-1", gpuDevicePresent, gpuTestTerminated("Succeeded", "Completed", 0))) @@ -740,8 +740,8 @@ func TestGPUOperatorFunctionalProbesWithBusyboxNotCUDA(t *testing.T) { // Asserted on the recorded request itself, not on the prose: the detail line // legitimately contains the word CUDA to explain why it is NOT used. asked := strings.ToLower(r.Evidence[0].What) - if !strings.Contains(asked, "image busybox:1.36, requesting nvidia.com/gpu: 1") { - t.Fatalf("probe transcript does not show the busybox-class image: %q", r.Evidence[0].What) + if !strings.Contains(asked, "image "+gpuDefaultProbeImage+", requesting nvidia.com/gpu: 1") { + t.Fatalf("probe transcript does not show the default slim image: %q", r.Evidence[0].What) } if strings.Contains(asked, "cuda") || strings.Contains(asked, "nvcr.io") { t.Fatalf("the default GPU probe pulled a CUDA-class image (FRD-020 D10): %q", r.Evidence[0].What) @@ -759,14 +759,14 @@ func TestGPUOperatorFunctionalHonoursExplicitProbeImage(t *testing.T) { gpuTestAssertMentions(t, r, "image nvcr.io/nvidia/cuda:12.4.1-base-ubi9") } -// A node that cannot pull a few megabytes of busybox has told us nothing about +// A node that cannot pull a few megabytes of base image has told us nothing about // its GPU. Reporting that as a GPU fault would send the operator to the wrong // stack entirely — it belongs to registry/egress. func TestGPUOperatorFunctionalSkipsAndRedirectsOnImagePullFailure(t *testing.T) { f := vanilla().with("nodes", "", node("gpu-1", withGPU("4"))) r := gpuTestRunWithProbes(t, f, 10*time.Second, gpuTestPodReactor("gpu-1", "", gpuTestWaiting("Failed", "ImagePullBackOff", - "Back-off pulling image busybox:1.36"))) + "Back-off pulling image "+gpuDefaultProbeImage))) assertSkipHasReason(t, r) gpuTestAssertMentions(t, r, "registry.from-cluster", "egress.install", "was not exercised") gpuTestAssertSilentOn(t, r, "stage:") @@ -785,6 +785,23 @@ func TestGPUOperatorFunctionalSkipsWhenProbePodCannotBeCreated(t *testing.T) { gpuTestAssertMentions(t, r, "could not be created", "was not exercised") } +// The tcs-vmware GPU node: HAMi scheduled the probe, the container started, and +// busybox's shell died loading HAMi's preloaded vGPU library. That is the image +// failing, not the GPU — a skip that names the loader error and the flag, never +// a device fault and never the generic "unreadable output". +func TestGPUOperatorFunctionalSkipsAndNamesTheImageWhenTheLoaderFails(t *testing.T) { + f := vanilla(). + with("nodes", "", node("gpu-1", withGPU("4"))). + withOpts(func(o *engine.Options) { o.GPUProbeImage = "busybox:1.36" }) + r := gpuTestRunWithProbes(t, f, 10*time.Second, + gpuTestPodReactor("gpu-1", + "/bin/sh: error while loading shared libraries: libdl.so.2: cannot open shared object file: No such file or directory\n", + gpuTestTerminated("Failed", "Error", 127))) + assertSkipHasReason(t, r) + gpuTestAssertMentions(t, r, "busybox:1.36", "libdl.so.2", "--gpu-probe-image", gpuDefaultProbeImage) + gpuTestAssertSilentOn(t, r, "stage:") +} + // Ran, exited, and asserted nothing readable. Inferring a device fault from a // missing log line would be inventing a finding. func TestGPUOperatorFunctionalSkipsWhenProbeOutputIsUnreadable(t *testing.T) { diff --git a/internal/checks/harness_test.go b/internal/checks/harness_test.go index a459932..8932cc0 100644 --- a/internal/checks/harness_test.go +++ b/internal/checks/harness_test.go @@ -368,7 +368,6 @@ var probeOnly = map[string]string{ "charts.runtime": "needs real chart repositories", "charts.aibrix": "needs GitHub release assets", "storage.provision": "provisions a real PVC", - "storage.csi-healthy": "needs real CSI driver pods", "gpu.operator-functional": "schedules a pod requesting a real GPU", "toolchain.clock": "needs a real reference clock", "toolchain.exec-plugin": "needs a kubeconfig with an exec stanza on disk", diff --git a/internal/checks/registry.go b/internal/checks/registry.go index aa0d3f1..6823bde 100644 --- a/internal/checks/registry.go +++ b/internal/checks/registry.go @@ -185,10 +185,6 @@ func regFeatureActive(ctx context.Context, c *engine.Ctx, feature string) bool { return known && len(nodes) > 0 case "opensandbox": return c.Answers.OpenSandbox - case "model-serving": - // Serving models is the product. Only an install that intends to hold - // no models at all can treat the vLLM runtime registry as optional. - return c.Answers.ModelCount > 0 || c.Answers.Deployments > 0 case "": return true default: diff --git a/internal/checks/registry_test.go b/internal/checks/registry_test.go index d8264c3..abcb87d 100644 --- a/internal/checks/registry_test.go +++ b/internal/checks/registry_test.go @@ -654,7 +654,6 @@ func registryEgressLogs(codes map[string]string, order ...string) string { var registryRequiredHosts = []string{ "registry.bud.studio", "docker.io", "quay.io", "ghcr.io", "registry.k8s.io", "ecr-public.aws.com", "reg.kyverno.io", - "budimages.azurecr.io", } func registryAllReached(code string) map[string]string { diff --git a/internal/checks/storage.go b/internal/checks/storage.go index a386a1c..f8107ab 100644 --- a/internal/checks/storage.go +++ b/internal/checks/storage.go @@ -203,6 +203,12 @@ func init() { broken = append(broken, fmt.Sprintf("%s (%s): %s", sc.Name, prov, strings.Join(bad, "; "))) case len(matched) == 0 && !drivers[prov] && !strings.HasPrefix(prov, "kubernetes.io/"): orphaned = append(orphaned, fmt.Sprintf("%s (%s)", sc.Name, prov)) + case !drivers[prov]: + // No CSIDriver object: an external provisioner such as + // local-path, which never registers on a CSINode. "0/2 nodes" + // would read as a driver missing from every node. + detail = append(detail, fmt.Sprintf("%s: %s — %d/%d %s Running; not a CSI driver, so there is no per-node registration to count", + sc.Name, prov, len(matched), len(matched), Plural(len(matched), "pod", "pods"))) default: detail = append(detail, fmt.Sprintf("%s: %s — %d/%d %s Running, registered on %d/%d nodes", sc.Name, prov, len(matched), len(matched), Plural(len(matched), "pod", "pods"), @@ -238,7 +244,7 @@ func init() { } return ch.Pass( fmt.Sprintf("the %s behind %s healthy", - Plural(len(detail), "CSI driver", "CSI drivers"), Plural(len(cands), "the candidate class is", "every candidate class is")), + Plural(len(detail), "provisioner", "provisioners"), Plural(len(cands), "the candidate class is", "every candidate class is")), append(detail, "candidates: "+why)...). WithEvidence(evidence...). Bounds("that the driver provisions — a Running pod with a registered CSINode entry still fails on a full backend, a missing volume group or a bad secret; storage.provision is what binds a claim") @@ -264,7 +270,18 @@ func init() { fixed := []string{} ok := []string{} + nodeLocal := []string{} for _, sc := range growth { + // A node-local class carves each volume out of one node's + // filesystem. There is no resizer to call and local-path ignores + // the requested size outright, so allowVolumeExpansion neither + // helps nor limits: the ceiling is that node's free space, which + // storage.capacity measures. Asking for the flag here sends the + // operator to a patch that changes nothing. + if stIsNodeLocal(sc) { + nodeLocal = append(nodeLocal, fmt.Sprintf("%s (%s) backs %s", sc.Name, sc.Provisioner, strings.Join(Sorted(d.Growth[sc.Name]), ", "))) + continue + } if sc.Expansion { ok = append(ok, fmt.Sprintf("%s (%s)", sc.Name, sc.Provisioner)) continue @@ -272,15 +289,28 @@ func init() { fixed = append(fixed, fmt.Sprintf("%s (%s) backs %s", sc.Name, sc.Provisioner, strings.Join(Sorted(d.Growth[sc.Name]), ", "))) } ev := engine.Evidence{What: "kubectl get storageclass -o custom-columns=NAME:.metadata.name,EXPANSION:.allowVolumeExpansion", Output: stClassTable(classes)} + nodeLocalNote := func() []string { + if len(nodeLocal) == 0 { + return nil + } + return append([]string{"node-local, where allowVolumeExpansion does not apply — each volume is bounded by the disk of the one node it landed on, not by a size that can be resized (storage.capacity measures that disk):"}, nodeLocal...) + } if len(fixed) > 0 { return ch.Fail( - strings.Join(stClassNamesByExpansion(growth, false), ", ")+" has allowVolumeExpansion unset, so the model registry and the ClickHouse volume can only be grown by deleting the PVC and re-downloading every model", + strings.Join(stClassNamesByExpansion(stNotNodeLocal(growth), false), ", ")+" has allowVolumeExpansion unset, so the model registry and the ClickHouse volume can only be grown by deleting the PVC and re-downloading every model", "set it before the volumes have data in them: kubectl patch storageclass -p '{\"allowVolumeExpansion\":true}' — the field is mutable, but it only affects claims resized after the change", - append(fixed, "growth-prone volumes: "+why)...). + append(append(fixed, nodeLocalNote()...), "growth-prone volumes: "+why)...). WithEvidence(ev) } - return ch.Pass("allowVolumeExpansion is set on "+strings.Join(ok, ", "), "growth-prone volumes: "+why). + if len(ok) == 0 { + return ch.Pass( + "expansion does not apply: every class backing a growth-prone volume is node-local, where the node's disk — not a resizable volume — is the ceiling", + append(nodeLocalNote(), "growth-prone volumes: "+why)...). + WithEvidence(ev). + Bounds("that the volumes have room to grow — a node-local volume never moves to a roomier node, and it shares that node's filesystem with the image store; storage.capacity is what measures the free space") + } + return ch.Pass("allowVolumeExpansion is set on "+strings.Join(ok, ", "), append(nodeLocalNote(), "growth-prone volumes: "+why)...). WithEvidence(ev). Bounds("that expansion works — the driver must also advertise the EXPAND_VOLUME capability, and an offline-only driver still needs the pod restarted to finish a resize") }, @@ -792,6 +822,16 @@ func stClassNames(classes []stClass) []string { return out } +func stNotNodeLocal(classes []stClass) []stClass { + out := []stClass{} + for _, sc := range classes { + if !stIsNodeLocal(sc) { + out = append(out, sc) + } + } + return out +} + func stClassNamesByExpansion(classes []stClass, expansion bool) []string { out := []string{} for _, sc := range classes { @@ -1118,14 +1158,33 @@ var stGenericTokens = map[string]bool{ "provisioner": true, "dev": true, "local": true, "cloud": true, } +// stDistinctiveTokens keeps the vendor's domain label as a last resort rather +// than a peer of the driver's own name. "rancher" in rancher.io/local-path is +// also in every image k3s ships — CoreDNS, Traefik, klipper-lb — so matching on +// it reads any Pending system pod as a broken storage driver. The vendor label +// is used only when nothing more specific is left, as in driver.longhorn.io or +// openebs.io/local, where the vendor is the driver. func stDistinctiveTokens(provisioner string) []string { - out := []string{} - for _, part := range strings.FieldsFunc(provisioner, func(r rune) bool { return r == '.' || r == '/' }) { - if p := strings.ToLower(part); !stGenericTokens[p] && len(p) >= 3 { - out = append(out, p) + host, path, _ := strings.Cut(provisioner, "/") + labels := strings.Split(host, ".") + vendor := "" + if len(labels) >= 2 { + vendor = labels[len(labels)-2] + labels = append(labels[:len(labels)-2:len(labels)-2], labels[len(labels)-1]) + } + distinctive := func(parts []string) []string { + out := []string{} + for _, part := range parts { + if p := strings.ToLower(part); !stGenericTokens[p] && len(p) >= 3 { + out = append(out, p) + } } + return out } - return out + if out := distinctive(append(labels, strings.Split(path, "/")...)); len(out) > 0 { + return out + } + return distinctive([]string{vendor}) } func stPodMentions(p adapters.Object, needle string) bool { diff --git a/internal/checks/storage_test.go b/internal/checks/storage_test.go index bb1f8b2..26d7e95 100644 --- a/internal/checks/storage_test.go +++ b/internal/checks/storage_test.go @@ -417,3 +417,131 @@ func TestStorageRWXPassesWhenTheChartAsksForNoRWX(t *testing.T) { storagePVC("private", "openebs-lvm", "ReadWriteOnce")) assertStatus(t, r, "PASS") } + +// --------------------------------------------------------------------------- +// storage.csi-healthy +// --------------------------------------------------------------------------- + +// storageDriverPod is a kube-system pod in one of the three states the check +// distinguishes: Running, Pending (never scheduled), or crashlooping. +func storageDriverPod(name, image, phase, waiting string) adapters.Object { + status := map[string]any{"phase": phase} + if waiting != "" { + status["containerStatuses"] = []any{map[string]any{ + "name": "c", "ready": false, + "state": map[string]any{"waiting": map[string]any{"reason": waiting, "message": "seeded"}}, + }} + } + return adapters.Object{ + "apiVersion": "v1", "kind": "Pod", + "metadata": map[string]any{"name": name, "namespace": "kube-system"}, + "spec": map[string]any{"containers": []any{map[string]any{"name": "c", "image": image}}}, + "status": status, + } +} + +// storageK3sSystemPods is a stock k3s kube-system: every image is rancher/*, +// and the ServiceLB pod for a second LoadBalancer on port 80 sits Pending +// because Traefik's already holds the host port. +func storageK3sSystemPods(provisioner adapters.Object) []adapters.Object { + return []adapters.Object{ + provisioner, + storageDriverPod("coredns-54996dc9b4-tm2p7", "rancher/mirrored-coredns-coredns:1.14.6", "Running", ""), + storageDriverPod("traefik-59b7647586-mhpb2", "rancher/mirrored-library-traefik:3.7.8", "Running", ""), + storageDriverPod("svclb-envoy-aibrix-eg-903790dc-jbl4w", "rancher/klipper-lb:v0.4.17", "Pending", ""), + } +} + +// k3s ships local-path and a dozen rancher/* system images. The vendor label +// "rancher" is not the driver's name, and matching on it reported a Pending +// ServiceLB pod as a broken storage driver while every PVC was Bound. +func TestStorageCSIHealthyIgnoresUnrelatedPodsFromTheProvisionersVendor(t *testing.T) { + f := vanilla(). + with("storageclasses.storage.k8s.io", "", storageClass("local-path", "rancher.io/local-path", true, false, nil)). + with("pods", "", storageK3sSystemPods( + storageDriverPod("local-path-provisioner-58d557dc48-sl4f8", "rancher/local-path-provisioner:v0.0.36", "Running", ""))...) + + r := run(t, f, "storage.csi-healthy") + + assertStatus(t, r, "PASS") + if detail := strings.Join(r.Detail, "\n"); strings.Contains(detail, "registered on 0/") { + t.Fatalf("local-path is not a CSI driver and never registers on a node; a 0/N count reads as a missing driver:\n%s", detail) + } + for _, ev := range r.Evidence { + for _, unrelated := range []string{"svclb", "coredns", "traefik"} { + if strings.Contains(ev.Output, unrelated) { + t.Fatalf("%s was counted as a pod implementing rancher.io/local-path:\n%s", unrelated, ev.Output) + } + } + } +} + +// The same cluster with the provisioner itself crashlooping is still a blocker: +// narrowing the match must not stop it from finding the real driver pod. +func TestStorageCSIHealthyBlocksWhenTheLocalPathProvisionerIsCrashlooping(t *testing.T) { + f := vanilla(). + with("storageclasses.storage.k8s.io", "", storageClass("local-path", "rancher.io/local-path", true, false, nil)). + with("pods", "", storageK3sSystemPods( + storageDriverPod("local-path-provisioner-58d557dc48-sl4f8", "rancher/local-path-provisioner:v0.0.36", "Running", "CrashLoopBackOff"))...) + + r := run(t, f, "storage.csi-healthy") + + assertStatus(t, r, "BLOCK") + detail := strings.Join(r.Detail, "\n") + if !strings.Contains(detail, "local-path-provisioner") || strings.Contains(detail, "svclb") { + t.Fatalf("the blocker must name the provisioner pod and only it:\n%s", detail) + } +} + +// Where the vendor IS the driver there is nothing more specific to match on, +// and the vendor label has to keep working. +func TestStorageDistinctiveTokensFallBackToTheVendorOnlyWhenNothingElseIsLeft(t *testing.T) { + for prov, want := range map[string]string{ + "rancher.io/local-path": "local-path", + "ebs.csi.aws.com": "ebs", + "csi.vsphere.vmware.com": "vsphere", + "nfs.csi.k8s.io": "nfs", + "driver.longhorn.io": "longhorn", + "local.csi.openebs.io": "openebs", + "openebs.io/local": "openebs", + "pd.csi.storage.gke.io": "gke", + "com.nutanix.csi": "nutanix", + "rook-ceph.rbd.csi.ceph.com": "rook-ceph rbd", + } { + if got := strings.Join(stDistinctiveTokens(prov), " "); got != want { + t.Errorf("stDistinctiveTokens(%q) = %q, want %q", prov, got, want) + } + } +} + +// local-path ignores the requested size and has no resizer, so a RISK asking +// for allowVolumeExpansion sent the operator to a patch that changes nothing, +// and "grow by deleting the PVC" described a limit the volume does not have. +func TestStorageExpansionDoesNotAskANodeLocalClassToExpand(t *testing.T) { + f := vanilla().with("storageclasses.storage.k8s.io", "", + storageClass("local-path", "rancher.io/local-path", true, false, nil)) + r := run(t, f, "storage.expansion") + + assertStatus(t, r, "PASS") + if !strings.Contains(r.Summary, "node-local") || strings.Contains(r.Remedy, "kubectl patch") { + t.Fatalf("a node-local class must be explained, not patched:\n summary: %s\n remedy: %s", r.Summary, r.Remedy) + } + if !strings.Contains(r.DoesNotProve, "storage.capacity") { + t.Fatalf("the pass must point at the check that measures the real ceiling: %q", r.DoesNotProve) + } +} + +// A node-local class beside a network class does not excuse the network one: +// the RISK still names the class that cannot grow, and only that class. +func TestStorageExpansionStillRisksOnANonLocalClassBesideANodeLocalOne(t *testing.T) { + f := vanilla().with("storageclasses.storage.k8s.io", "", + storageClass("local-path", "rancher.io/local-path", true, false, nil), + storageClass("fixed", "ebs.csi.aws.com", false, false, nil)) + r := storageRunRendered(t, f, "storage.expansion", + storagePVC("bud-models-registry", "fixed", "ReadWriteOnce")) + + assertStatus(t, r, "RISK") + if !strings.HasPrefix(r.Summary, "fixed has allowVolumeExpansion unset") { + t.Fatalf("the RISK must name only the class that can be resized: %s", r.Summary) + } +} diff --git a/internal/engine/context.go b/internal/engine/context.go index 67500f0..a419efe 100644 --- a/internal/engine/context.go +++ b/internal/engine/context.go @@ -64,6 +64,9 @@ type Options struct { ArgoCDNamespace string ArgoCDEnabled bool RegistryCreds map[string]adapters.Credential + + // BudctlVersion is the build that ran, recorded in every report it writes. + BudctlVersion string } // Ctx is handed to every check. Anything a check discovers that a later check diff --git a/internal/engine/model.go b/internal/engine/model.go index 6e31042..7dff5c8 100644 --- a/internal/engine/model.go +++ b/internal/engine/model.go @@ -135,6 +135,20 @@ type Report struct { Meta map[string]string `json:"meta,omitempty"` } +// Stamp records what a report was checked against. Every surface that writes a +// report calls it once the run has finished: the platform and server version +// are only known after the platform checks have run, and a report saved from +// the interactive view without them reads as a run against no cluster at all. +func (r *Report) Stamp(c *Ctx) { + r.Platform = string(c.Platform.Distribution) + r.Meta = map[string]string{ + "budctlVersion": c.Opts.BudctlVersion, + "catalogVersion": c.Profile.CatalogVersion, + "serverVersion": c.Platform.Version, + "domain": c.Answers.Domain, + } +} + func Summarize(results []Result) Report { rep := Report{Counts: map[string]int{}, Results: results, Verdict: Ready} for _, r := range results { diff --git a/internal/intake/defaults.yaml b/internal/intake/defaults.yaml index cc3c7b8..081bdc9 100644 --- a/internal/intake/defaults.yaml +++ b/internal/intake/defaults.yaml @@ -1,7 +1,7 @@ # Floors and inventories budctl embeds at build time (FRD-020 §7, §9.3). # These are the values an operator cannot reasonably be asked for. Capacity # requirements do NOT live here — they derive from the intake answers. -catalogVersion: "2026-09-13" +catalogVersion: "2026-09-15" # 80 GiB, not 60. The 14 first-party images at 1.2.8 measure 19.8 GiB compressed # from registry manifests, inferring to ~40 GiB extracted; novu x4, otel, @@ -75,6 +75,10 @@ appsetComponents: # Verified by fetching the upstream charts and cross-checking against the images # running in a live cluster. Four of these appear NOWHERE in the bud-runtime # repository: they are upstream chart defaults that Bud installs unmodified. +# +# budimages.azurecr.io is retired and deliberately absent. Leftover references +# still name it — budcluster's NODE_INFO_* image env in the bud chart, which no +# code reads — but nothing an install or a deployment runs pulls from it. registries: - host: registry.bud.studio pulledBy: every first-party image, and the OCI charts @@ -98,10 +102,6 @@ registries: pulledBy: Kyverno, all five images, via the chart's defaultRegistry requirement: required feature: kyverno - - host: budimages.azurecr.io - pulledBy: vLLM runtime and node-info-collector images, at model-deploy time - requirement: conditional - feature: model-serving - host: nvcr.io pulledBy: NVIDIA GPU operator and dcgm-exporter requirement: conditional @@ -150,42 +150,50 @@ egress: label: Kyverno registry when: optional consequence: the Kyverno addon cannot pull its five images + # prometheus-community, bitnami, CloudNativePG, Altinity, Percona and SeaweedFS + # are dependencies of wrapper charts, not install sources. Every ApplicationSet + # in infra/appsets installs those charts from oci://registry.bud.studio/charts, + # and ArgoCD pulls the packaged chart with its charts/ directory already inside + # — the repo-server never resolves them. A cluster that cannot reach + # docs.altinity.com syncs ClickHouse regardless. They matter only to whoever + # builds a chart from source, so they are optional; dapr, which an + # ApplicationSet sources directly, stays install. - url: https://prometheus-community.github.io/helm-charts/index.yaml label: prometheus-community charts - when: install - consequence: the umbrella chart's prometheus and prometheus-adapter dependencies + when: optional + consequence: the prometheus and prometheus-adapter subcharts of the bud chart, bundled in the published OCI chart; fetched only when that chart is built from source - url: https://charts.bitnami.com/bitnami/index.yaml label: bitnami charts - when: install - consequence: the common library subchart + when: optional + consequence: the common library subchart of the bud chart, bundled in the published OCI chart; fetched only when that chart is built from source - url: https://dapr.github.io/helm-charts/index.yaml label: dapr charts when: install consequence: the Dapr control plane cannot be installed - url: https://cloudnative-pg.github.io/charts/index.yaml label: CloudNativePG charts - when: install - consequence: no PostgreSQL operator, so every service's database is absent + when: optional + consequence: the CloudNativePG operator subchart of the postgres chart, bundled in the published OCI chart; fetched only when that chart is built from source - url: https://docs.altinity.com/clickhouse-operator/index.yaml label: Altinity ClickHouse charts - when: install - consequence: no ClickHouse operator, so budmetrics and gateway analytics have nowhere to write + when: optional + consequence: the ClickHouse operator subchart of the clickhouse chart, bundled in the published OCI chart; fetched only when that chart is built from source - url: https://percona.github.io/percona-helm-charts/index.yaml label: Percona charts - when: install - consequence: no MongoDB operator, so Novu will not start + when: optional + consequence: the MongoDB operator subchart of the mongodb chart, bundled in the published OCI chart; fetched only when that chart is built from source - url: https://seaweedfs.github.io/seaweedfs-operator/index.yaml label: SeaweedFS charts - when: install - consequence: no S3 object store for the model registry + when: optional + consequence: the SeaweedFS operator subchart of the seaweedfs chart, bundled in the published OCI chart; fetched only when that chart is built from source - url: https://openebs.github.io/openebs/index.yaml label: OpenEBS charts when: optional consequence: only if OpenEBS is the chosen storage provisioner - url: https://open-telemetry.github.io/opentelemetry-helm-charts/index.yaml label: OpenTelemetry charts - when: install - consequence: no OTel operator, so trace collection is absent + when: optional + consequence: the OpenTelemetry operator subchart of the otel chart, which no ApplicationSet installs; fetched only when that chart is built from source - url: https://argoproj.github.io/argo-helm/index.yaml label: Argo charts when: optional @@ -230,10 +238,12 @@ egress: label: PyPI when: optional consequence: sandboxed code execution and eval harnesses that install packages at runtime + # Its own bucket, not install: the sync completes without it, and only an + # ACME TLS answer puts the directory on the path (egress.acme). - url: https://acme-v02.api.letsencrypt.org/directory label: Let's Encrypt ACME - when: install - consequence: certificate issuance for every ingress host; without it TLS stays self-signed + when: acme + consequence: cert-manager cannot register an ACME account, so no certificate is issued and every hostname serves an untrusted one egressTcp: - host: github.com diff --git a/internal/tui/app.go b/internal/tui/app.go index e66e556..d181425 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -144,7 +144,9 @@ func (m *Model) startRun() tea.Cmd { m.prog = newProgress() go func() { results := engine.Run(m.runCtx, m.ctx, m.sel, observer{ch: m.statusCh}) - m.statusCh <- doneMsg{report: engine.Summarize(results)} + rep := engine.Summarize(results) + rep.Stamp(m.ctx) + m.statusCh <- doneMsg{report: rep} }() return tea.Batch(m.waitForMsg(), m.spin.Tick) } diff --git a/internal/tui/report_test.go b/internal/tui/report_test.go new file mode 100644 index 0000000..4cf5c84 --- /dev/null +++ b/internal/tui/report_test.go @@ -0,0 +1,62 @@ +package tui + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/BudEcosystem/budctl/internal/engine" +) + +// A report saved from the interactive view is the one an operator attaches to a +// ticket. It once went out with no budctl version, no catalogue and "cluster: +// unreachable" over a run that had just passed 48 checks against that cluster, +// because only the non-interactive path stamped the report, and it did so after +// the interactive view had already written the file. +func TestInteractiveRunReportNamesTheBuildAndTheCluster(t *testing.T) { + c := testCtx(t) + c.Opts.BudctlVersion = "0.3.1-test" + c.Answers.Domain = "bud.example.com" + c.Platform.Distribution = engine.DistVanilla + c.Platform.Version = "v1.36.3+k3s1" + + // One offline check with no dependencies: the run finishes at once and + // needs neither a cluster nor a network. + m := New(context.Background(), c, engine.Selection{Only: []string{"toolchain.age-identity"}}, true, "") + _ = m.startRun() + + var rep engine.Report + deadline := time.After(30 * time.Second) + for done := false; !done; { + select { + case msg := <-m.statusCh: + if d, ok := msg.(doneMsg); ok { + rep, done = d.report, true + } + case <-deadline: + t.Fatal("the run never finished") + } + } + + for key, want := range map[string]string{ + "budctlVersion": "0.3.1-test", + "catalogVersion": c.Profile.CatalogVersion, + "serverVersion": "v1.36.3+k3s1", + "domain": "bud.example.com", + } { + if got := rep.Meta[key]; got != want { + t.Errorf("report meta %s = %q, want %q", key, got, want) + } + } + + md := markdownReport(rep, c.Answers) + for _, want := range []string{"0.3.1-test", c.Profile.CatalogVersion, "v1.36.3+k3s1"} { + if !strings.Contains(md, want) { + t.Errorf("saved markdown does not mention %q", want) + } + } + if strings.Contains(md, "unreachable") { + t.Errorf("saved markdown calls a reachable cluster unreachable:\n%s", md) + } +} From 3e054c496aa847b5b61fb30e6e54e0bbf39541d7 Mon Sep 17 00:00:00 2001 From: Ditto P S Date: Tue, 15 Sep 2026 02:02:14 +0530 Subject: [PATCH 2/2] budctl 0.3.1 Co-Authored-By: Claude Opus 5 --- install.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install.sh b/install.sh index 2035fa6..ed65e6a 100755 --- a/install.sh +++ b/install.sh @@ -16,7 +16,7 @@ set -eu # tag that disagrees with it. Pinned rather than resolved from the GitHub # API at runtime: the unauthenticated API allows 60 requests per hour per IP, # which one NATed customer site can exhaust between two engineers. -VERSION="${BUDCTL_VERSION:-0.3.0}" +VERSION="${BUDCTL_VERSION:-0.3.1}" REPO="BudEcosystem/budctl" die() { echo "install.sh: $*" >&2; exit 1; }