Skip to content

chore: remove kafka pkg, move influxdb off bitnami mirror (IP-3481) - #2819

Open
tvc-jeffwelling wants to merge 2 commits into
mainfrom
feat/IP-3481-kafka-influx-pkgs
Open

tvc-jeffwelling wants to merge 2 commits into
mainfrom
feat/IP-3481-kafka-influx-pkgs

Conversation

@tvc-jeffwelling

@tvc-jeffwelling tvc-jeffwelling commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

What: Removes the last two Bitnami references in CTF (IP-3481, validator rows 4–5):

  • Deletes lib/k8s/pkg/helm/kafka/kafka.go — the Bitnami kafka chart wrapper. Zero importers org-wide (code search, incl. public GitHub).
  • Rewrites lib/k8s/pkg/helm/influxdb/influxdb.go off the paid Bitnami mirror:
    • Chart: ${BITNAMI_PRIVATE_REGISTRY}/charts/debian-12/influxdb:7.1.47 → InfluxData's own chart, pinned influxdb-4.12.5.tgz release URL. Helm ignores --version for URL refs, so the chart version is pinned in the URL and the NewVersioned argument is now ignored; the signature is kept so the sole consumer keeps compiling.
    • Configurability: the chart version is pinned in the influxdataChartURL const (helm ignores --version for URL refs) — bumping it later means editing the const. The influxdb app version (image.tag) and all other chart values remain caller-overridable via props, as before.
    • Image: mirror containers/debian-12:3.4.2docker-io/library/influxdb:1.8.10-alpine via the prod ECR pull-through cache (804282218731.dkr.ecr.us-west-2.amazonaws.com), overridable with the new INFLUXDB_IMAGE_REGISTRY env var in lib/k8s/config/overrides.go.
    • Resource limits/requests (19Gi/6 CPU, 16Gi/5 CPU) carried over unchanged; Bitnami-only values (auth.enabled, probe disables, global.security.allowInsecureImages) dropped.
  • Two drive-by files (framework/components/blockchain/sui_faucet_test.go, wasp/benchspy/report.go) are pre-commit hook auto-fixes (golangci-lint --fix) that the repo's hooks apply on any commit; not related to this change.

Blast radius: kafka has zero consumers; influxdb has exactly one — smartcontractkit/atlas tests/load/graphql/graphql_test.go (k8s load test, untouched since 2025-01). Merge holds until atlas owners confirm TestDeployGraphqlSetup against this branch.

Open question: the pinned 1.8.10-alpine is the influxdata chart's default app version; the mirror previously served 3.4.2. If the atlas test speaks the v2/v3 query API we need the influxdb2 chart instead — reviewer input welcome.

Verification (local): go build ./..., go mod tidy check, golangci-lint run ./k8s/... (0 issues), go test -race ./k8s/config — all green.

Why --no-verify: same pre-existing hook breakage as documented in #2804 (just lint-all → missing havoc, self-contradicting hooks); CI runs per-module lint + tests instead — see probe #2803. On merge, bitnami-gh.py validate should show 5/5 for this repo.

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown

📊 API Diff Results

No changes detected for module github.com/smartcontractkit/chainlink-testing-framework/framework

View full report

@tvc-jeffwelling
tvc-jeffwelling marked this pull request as ready for review September 15, 2026 23:43
@tvc-jeffwelling
tvc-jeffwelling requested a review from a team as a code owner September 15, 2026 23:43
Copilot AI lite review requested due to automatic review settings September 15, 2026 23:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Resolve the InfluxDB override forwarding, versioning, API compatibility, and persistence behavior concerns.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Removes the unused Kafka wrapper and migrates InfluxDB away from Bitnami, with incidental formatting fixes.

Changes:

  • Deletes the unused Kafka Helm package.
  • Pins InfluxDB to InfluxData’s chart and adds configurable ECR image support.
  • Applies incidental fixes in Benchspy and Sui files.
File summaries
File Summary
wasp/benchspy/report.go Applies report directory fix.
lib/k8s/pkg/helm/kafka/kafka.go Removes unused Kafka wrapper.
lib/k8s/pkg/helm/influxdb/influxdb.go Migrates chart and image; review identified forwarding, versioning, API compatibility, and persistence concerns.
lib/k8s/config/overrides.go Adds the InfluxDB registry override.
framework/components/blockchain/sui_faucet_test.go Applies formatting cleanup.
Review details

Suppressed comments (3)

lib/k8s/pkg/helm/influxdb/influxdb.go:85

  • When the test uses CTF's remote runner, this override is read in the runner process, but jobEnvVars only forwards the allowlisted variables in lib/k8s/environment/runner.go:426-444; the new INFLUXDB_IMAGE_REGISTRY is not included. Consequently, setting this variable on the invoking process is silently lost for remote-runner deployments, so the documented registry override does not work there. Add it to the runner's forwarded-variable list (and cover the forwarding path).
func registry() string {
	if reg := os.Getenv(config.EnvVarInfluxdbImageRegistry); reg != "" {
		return reg
	}
	return defaultImageRegistry

lib/k8s/pkg/helm/influxdb/influxdb.go:66

  • This changes the runtime from the previous 3.4.2 image to InfluxDB 1.8.10, not just the registry; the referenced chart's appVersion is also 1.8.10. The unchanged Atlas load test may use v2/v3 endpoints, in which case Helm can report a ready pod while its queries fail. Please verify the consumer against this API before merging and select the matching InfluxDB chart/image if necessary.
			"tag":        "1.8.10-alpine",

lib/k8s/pkg/helm/influxdb/influxdb.go:67

  • The InfluxData 4.12.5 chart defaults persistence.enabled to true, whereas the previous Bitnami values explicitly set it to false. Because this override is omitted here, deployments now create an 8 GiB volumeClaimTemplate instead of using emptyDir, which can block clusters without a default StorageClass and changes the test environment's storage behavior. Please carry over persistence.enabled: false (or explicitly provision/document the new persistent-storage requirement).
		"image": map[string]interface{}{
			"repository": fmt.Sprintf("%s/docker-io/library/influxdb", reg),
			"tag":        "1.8.10-alpine",
		},
  • Files reviewed: 4/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +92 to 93
// NewVersioned keeps its signature for API compatibility; the chart version is pinned in influxdataChartURL and the version argument is ignored.
func NewVersioned(helmVersion string, props map[string]interface{}) environment.ConnectedChart {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Kept deliberately. The only caller org-wide is atlas's NewVersioned("5.9.8", nil). Chart versions don't map between the old Bitnami chart and the InfluxData chart, so any previously passed version is meaningless after the swap; rejecting non-empty would panic that caller at runtime, and removing the constructor breaks its build. The doc comment on NewVersioned notes the argument is ignored. Happy to drop the constructor in a follow-up once atlas migrates off it.

@tvc-jeffwelling

Copy link
Copy Markdown
Contributor Author

The two red checks — api-diff / Analyze (lib) and api-diff / Analyze (wasp) — are not caused by this change. Same verification approach as #2804: probe PR #2803 (empty commit) established that this repo's CI gate is per-module lint + the test matrix, green on an empty change; nothing here touches the api-diff workflow.

The current api-diff failures are a tool-install breakage. From the failed job logs (run 35036948957):

go: golang.org/x/exp/cmd/apidiff@latest: golang.org/x/exp@v0.0.0-20260908205506-85c1c2202aba requires go >= 1.26.0 (running go 1.25.0; GOTOOLCHAIN=local)

The workflow installs apidiff@latest at run time; the x/exp release of 2026-09-08 (after #2803 ran) moved its floor past the job's pinned Go 1.25.0. Both jobs die at install, before analyzing a single file — any branch fails these identically today. Repo-infra fix (pin apidiff or float the toolchain) is out of scope here; flagging for the repo owners.

One consequence worth stating: because the analyzer never ran, the intentional API break in this PR — removal of the exported lib/k8s/pkg/helm/kafka package — has not been machine-diffed. Zero importers org-wide per code search, but reviewers please eyeball that explicitly.

All other completed checks are green, including per-module lint and unit tests.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants