fix: 295: Shell access to a SHM mounted workload

[Zblocker64]

Closes akash-network/support#295

Rebased version of #310 onto current main. The original PR conflicted because main migrated its API imports from github.com/akash-network/akash-api / github.com/akash-network/node to pkg.akt.dev/go; the changes here are ported to the new packages (rtypes, attrtypes, pkg.akt.dev/go/sdl) and the tests were adjusted for the current ServiceStatus field types (int32) and the StatefulSet CurrentReplicas field.

This addresses an issue with the provider when attempting to SSH to a lease with a shared memory mount. It also corrects a websocket protocol violation on error (encountered while debugging the SHM issue). The core issue was that when the lease-shell command attempted to connect, it would try to do so on a StatefulSet due to detecting a volume on the deployment. Having it follow the convention of the Deploy() function lets it connect to the deployment properly.

1. StatefulSet Detection Fix

• Fixed incorrect logic in ServiceStatus that was checking for any mounted storage instead of persistent storage to determine workload type.
• The bug caused services with RAM volumes to incorrectly attempt StatefulSet operations, resulting in statefulsets.apps not found errors.
• Now correctly checks storage.Attributes.Find(sdl.StorageAttributePersistent) to match the deployment creation logic.

2. WebSocket Error Handling in lease-shell

• Fixed improper error handling when ServiceStatus fails during shell operations.
• Previously used http.Error() on WebSocket connections, causing protocol violations.
• Now properly uses the WebSocket writer with LeaseShellCodeFailure and logs errors.
• Prevents silent failures and improves debugging for shell access issues.

Tests

Added unit tests for StatefulSet detection logic covering:

• Services with persistent storage (should use StatefulSet)
• Services with non-persistent storage (should use Deployment)
• Services with no storage (should use Deployment)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2aafd18e-d38e-4da5-b82b-b888f4498c00

📥 Commits

Reviewing files that changed from the base of the PR and between 2d7f499 and 2e6fd08.

📒 Files selected for processing (3)

• cluster/kube/client.go
• cluster/kube/client_test.go
• gateway/rest/router.go

🚧 Files skipped from review as they are similar to previous changes (2)

• gateway/rest/router.go
• cluster/kube/client.go

---

Walkthrough

ServiceStatus now determines Kubernetes workload type from persistent storage attributes, with tests covering storage variants. Shell handling now exposes only approved errors and reports other service-status failures through a failure result.

Changes

Service status and shell handling

Layer / File(s)	Summary
Persistent storage workload detection cluster/kube/client.go, cluster/kube/client_test.go	ServiceStatus selects StatefulSets when storage is persistent and Deployments otherwise; table-driven tests validate replica counts for each storage configuration.
Shell failure response routing gateway/rest/router.go	leaseShellHandler sends safe errors in the response message and uses a failure websocket result without JSON encoding for other errors.

Estimated code review effort: 3 (Moderate) | ~15–30 minutes

Poem

A rabbit checks the pods at night,
Persistent paws choose StatefulSet right.
Safe errors hop to messages clear,
Hidden failures leave through a failure peer.
“Shells now burrow!” the bunny cheers.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the SHM shell-access fix reflected in the changes.
Description check	✅ Passed	The description accurately describes the SHM shell-access fix and related websocket error handling.
Linked Issues check	✅ Passed	The changes address #295 by correcting workload detection for SHM storage and improving shell error handling.
Out of Scope Changes check	✅ Passed	All changes relate to SHM shell access, tests, or the websocket error fix; no unrelated edits are evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-295-statefulset-shm-fix

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

cluster/kube/client.go (1)

1098-1105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify the loop and limit variable scope.

You can make the loop slightly more idiomatic by iterating over the values directly and using a short variable declaration (:=) inside the if statement to avoid declaring persistent in the outer scope.

♻️ Proposed refactor

-	persistent := false
-	for i := range svc.Resources.Storage {
-		attrVal := svc.Resources.Storage[i].Attributes.Find(sdl.StorageAttributePersistent)
-		if persistent, _ = attrVal.AsBool(); persistent {
+	for _, storage := range svc.Resources.Storage {
+		attrVal := storage.Attributes.Find(sdl.StorageAttributePersistent)
+		if persistent, _ := attrVal.AsBool(); persistent {
 			isDeployment = false
 			break
 		}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cluster/kube/client.go` around lines 1098 - 1105, Update the loop over
svc.Resources.Storage to iterate over storage values directly, and declare
persistent with := inside the if condition alongside the attribute lookup and
boolean conversion. Remove the outer persistent declaration while preserving the
existing isDeployment update and break behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cluster/kube/client.go`:
- Around line 1098-1105: Update the loop over svc.Resources.Storage to iterate
over storage values directly, and declare persistent with := inside the if
condition alongside the attribute lookup and boolean conversion. Remove the
outer persistent declaration while preserving the existing isDeployment update
and break behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6a0e8a96-9902-4c60-92c2-8b181d209793

📥 Commits

Reviewing files that changed from the base of the PR and between 5e9801e and 2d7f499.

📒 Files selected for processing (3)

• cluster/kube/client.go
• cluster/kube/client_test.go
• gateway/rest/router.go