fix: address linting alerts

[kristina-solovyova]

No description provided.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[kristina-solovyova]

• fix: upgrade go to 1.26.2 to address govulncheck issues #2473
• fix: remove deadcode #2471
• fix: address linting alerts #2393 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add the label main-merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Claude finished @kristina-solovyova's task in 1m 34s —— View job

---

Code Review

• Gather context
• Review changed files
• Post feedback

---

Overall this is a solid linting cleanup. Most changes are style improvements or correct patterns. A few items worth noting:

Correctness fixes (most important)

internal/services/discovery/discovery.go contains two important bug fixes for loop variable capture:

• GetClusterByUID (line ~317): for i := range clustersList.Items + &clustersList.Items[i] instead of &cluster from a range variable — correct fix, the old code would yield dangling/aliased pointers
• GetWekaClientsForCluster (line ~492): same pattern, &clientsList.Items[i] — correct fix

Error propagation improvement

GetClusterContainers now returns (error) instead of silently swallowing it. All 4 callers have been updated to handle the error. This is a meaningful behavioral improvement — previously, k8s API failures would silently produce empty container lists.

Concerns

1. .golangci.yaml — broad test file suppressions (errcheck, gocritic, ineffassign, staticcheck, unused suppressed for all _test.go files). This is quite broad and could hide real issues like unchecked errors in test setup code. It would be safer to fix the specific test file issues rather than suppress wholesale. Fix this →

2. internal/services/discovery/discovery.go:322 — errors.New("Cluster not found") still uses a capital C, inconsistent with the error casing fix in pod.go ("weka container not found in pod" was lowercased). Minor, but worth making consistent.

3. ComputeCapacityBasedHugepages (hugepages.go:75) — ctx is discarded (_, logger, end := ...). This is fine since ctx isn't passed to any child calls in this function, but it means the span won't be linked as a parent if any downstream calls are added. Low risk given the current function body, just worth being aware of.

Clean removals confirmed

• getBoolEnv, getFloatEnvOrDefault, getDurationEnv — confirmed unused across the codebase, safe to remove
• getHugePagesOffset method on PodFactory — confirmed unused, safe to remove

Minor style improvements are all correct:

• else after return removed ✓
• if/else if/else → switch ✓
• Multiple append calls merged ✓
• strings.Join([]string{a, b}, "") → a + b ✓
• container.ObjectMeta.Labels → container.Labels ✓
• Import grouping (stdlib / k8s / internal) ✓

[Copilot]

Pull request overview

This PR addresses Go lint warnings across the operator by tightening up API/error handling, fixing common Go pitfalls (range-variable pointers, shadowing), and applying small refactors for cleaner code.

Changes:

• Fix pointer-to-range-variable patterns and variable shadowing in several controllers/services.
• Make discovery.GetClusterContainers return an error (instead of silently discarding it) and update call sites accordingly.
• Refactor a few blocks to satisfy linters (ineffassign/unused, simplified returns, import grouping) and update golangci-lint exclusions.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
internal/services/discovery/discovery.go	Removes duplicate util import, simplifies logic, fixes pointer-to-range-variable bug, and propagates container-listing errors.
internal/controllers/wekacontainer/funcs_getters.go	Handles new GetClusterContainers error return.
internal/controllers/wekacluster/funcs_helpers.go	Handles new GetClusterContainers error return.
internal/controllers/wekaclient/client_reconciler_loop.go	Fetches cluster containers with error handling; fixes err shadowing; removes now-unneeded helper.
internal/controllers/resources/pod.go	Refactors slice appends and minor cleanups to satisfy lint.
internal/controllers/operations/trace_session.go	Groups imports; handles new GetClusterContainers error return.
internal/controllers/allocator/hugepages.go	Simplifies branching and avoids ineffectual ctx assignment from instrumentation span helper.
internal/config/env.go	Removes unused env helpers and minor signature cleanup.
.golangci.yaml	Adds exclusions for several linters in _test.go files.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[graphite-app]

Graphite Automations

"Add anton/matt/sergey/kristina as reviwers on operator PRs" took an action on this PR • (03/24/26)

2 reviewers were added to this PR based on Anton Bykov's automation.

[claude]

Claude finished @kristina-solovyova's task in 46s —— View job

---

No changes since the last review (tip commit fec9efd6 is unchanged). See the previous review below for full feedback.

---

Code Review

Overall this is a solid linting cleanup. Most changes are style improvements or correct patterns. A few items worth noting:

Correctness fixes (most important)

internal/services/discovery/discovery.go contains two important bug fixes for loop variable capture:

• GetClusterByUID (line ~317): for i := range clustersList.Items + &clustersList.Items[i] instead of &cluster from a range variable — correct fix, the old code would yield dangling/aliased pointers
• GetWekaClientsForCluster (line ~492): same pattern, &clientsList.Items[i] — correct fix

Error propagation improvement

GetClusterContainers now returns (error) instead of silently swallowing it. All 4 callers have been updated to handle the error. This is a meaningful behavioral improvement — previously, k8s API failures would silently produce empty container lists.

Concerns

1. .golangci.yaml — broad test file suppressions (errcheck, gocritic, ineffassign, staticcheck, unused suppressed for all _test.go files). This is quite broad and could hide real issues like unchecked errors in test setup code. It would be safer to fix the specific test file issues rather than suppress wholesale. Fix this →

2. internal/services/discovery/discovery.go:322 — errors.New("Cluster not found") still uses a capital C, inconsistent with the error casing fix in pod.go ("weka container not found in pod" was lowercased). Minor, but worth making consistent.

3. ComputeCapacityBasedHugepages (hugepages.go:75) — ctx is discarded (_, logger, end := ...). This is fine since ctx isn't passed to any child calls in this function, but it means the span won't be linked as a parent if any downstream calls are added. Low risk given the current function body, just worth being aware of.

Clean removals confirmed

• getBoolEnv, getFloatEnvOrDefault, getDurationEnv — confirmed unused across the codebase, safe to remove
• getHugePagesOffset method on PodFactory — confirmed unused, safe to remove

Minor style improvements are all correct:

• else after return removed ✓
• if/else if/else → switch ✓
• Multiple append calls merged ✓
• strings.Join([]string{a, b}, "") → a + b ✓
• container.ObjectMeta.Labels → container.Labels ✓
• Import grouping (stdlib / k8s / internal) ✓

[claude]

Claude finished @kristina-solovyova's task in 1m 57s —— View job

---

Code Review

• Gather context
• Check for new commits since last review
• Review changes
• Post feedback

---

The branch was rebased since the last review (old tip fec9efd6 → new tip 55936e74). The updated commit adds 12 previously unreviewed files. The changes in the already-reviewed 100 files are unchanged.

New files reviewed in this pass

internal/services/weka.go

• ConfigureNfs interface changed from value NFSParams → pointer *NFSParams — caller in funcs_nfs.go correctly updated to &services.NFSParams{...} ✓
• SupportsFlag: old code created a local cmd variable then immediately used it; new code in-place appends to cmdParts — functionally equivalent, just removes the unnecessary intermediate. ✓
• GetCapacity loop: for fsIdx := range filesystems { fs := &filesystems[fsIdx] — same correct loop-variable-capture fix pattern as discovery.go. ✓
• EnsureNoUser refactored to early-continue pattern — more idiomatic. ✓
• RemoveDrive error string: "\"..." → %q — equivalent. ✓
• EnsureNfsInterfaceGroupPorts / EnsureNfsIpRanges: err renamed to parseErr/unmarshalErr — avoids shadowing outer err, correct fix. ✓

pkg/metrics/prommetrics.go

Changed fmt.Sprintf("%s=\"%s\"", k, v) → fmt.Sprintf("%s=%q", k, v) for Prometheus label values.

Concern: %q uses Go string quoting semantics — for values containing \n, \\, or non-ASCII, the output will differ from the original. For the common case of plain strings this is identical, but for edge-case label values this is a subtle behavior change in the metrics exposition format. In practice %q is more correct here (Prometheus text format requires these escapes), but it's worth verifying this doesn't break any scraper that was tolerating the old unescaped output. Low risk, just noting it.

pkg/util/maps.go:63,70

return value.(V), true //nolint:errcheck // error return value intentionally not checked

The nolint directive is wrong — errcheck lints for unchecked error returns, not type assertions. The correct directive (if needed) would be //nolint:forcetypeassert. This won't cause a build or runtime issue, but the comment is misleading. Minor. Fix this →

pkg/util/http.go

nil → http.NoBody for GET request body — correct idiomatic fix. ✓

pkg/util/kubernetes.go

Parameter renames (client → restClient, k8sClient) to avoid shadowing the client package import — correct fix throughout. ✓

scripts/gen-api-docs.go

• Removed dead isCRD function (appears unused in this file after the refactor). ✓
• switch node := n.(type) { case *ast.TypeSpec: → if node, ok := n.(*ast.TypeSpec); ok { — linter simplification, correct. ✓
• //nolint:errcheck on all fmt.Fprintf calls in a script file — acceptable, this is a dev tool. ✓

---

Carry-over from prior reviews (still unaddressed)

1. .golangci.yaml — broad test file suppressions — errcheck, gocritic, ineffassign, staticcheck, unused suppressed for all _test.go files. Still preferable to fix specific issues over wholesale suppression. Fix this →
2. internal/services/discovery/discovery.go:322 — errors.New("Cluster not found") still uses a capital C. Minor consistency issue.
3. ComputeCapacityBasedHugepages (hugepages.go:75) — ctx is discarded — low risk, noted for awareness.

---

Overall: the new additions are clean. The only new actionable item is the wrong nolint directive in maps.go. Everything else looks correct.

[Copilot]

Pull request overview

Copilot reviewed 111 out of 112 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/controllers/wekacluster/reconciler_loop.go:89

• The manual pause flow is no longer added to the step list. GetAllSteps now has a comment describing the manual pause path, but there is no SimpleStep that calls HandleManualPause, so overrides.paused=true will never transition the cluster to Paused / pause containers.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[graphite-app]

Merge activity

• Apr 20, 9:39 AM UTC: kristina-solovyova added this pull request to the Graphite merge queue.
• Apr 20, 9:39 AM UTC: CI is running for this pull request on a draft pull request (#2472) due to your merge queue CI optimization settings.
• Apr 20, 10:15 AM UTC: Merged by the Graphite merge queue via draft PR: #2472.