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/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; } 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) + } +}