fix: 295: Shell access to a SHM mounted workload #310
Conversation
akash-network#295) This commit addresses two critical issues in the Akash provider: 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 WebSocket writer with LeaseShellCodeFailure and logs errors - Prevents silent failures and improves debugging for shell access issues Added comprehensive 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) These fixes ensure proper workload type detection and improve error visibility for lease-shell operations, resolving issues with shell access to deployments using RAM volumes.
WalkthroughThe changes update the logic for detecting whether a Kubernetes service should be treated as a Deployment or StatefulSet based on persistent storage attributes, aligning it with deployment creation logic. A new unit test verifies this behavior. Additionally, error reporting in the WebSocket shell handler is adjusted to send errors over the WebSocket connection instead of HTTP. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST Gateway
participant KubeClient
Client->>REST Gateway: Open WebSocket (leaseShellHandler)
REST Gateway->>KubeClient: ServiceStatus()
KubeClient-->>REST Gateway: Service status or error
alt ServiceStatus error
REST Gateway->>Client: Send failure message over WebSocket
else Success
REST Gateway->>Client: Proceed with shell session
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.2.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/product/migration-guide for migration instructions Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🔇 Additional comments (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Closes akash-network/support#295
This commit 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 essentially was 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 let's it connect to the deployment properly.
StatefulSet Detection Fix:
WebSocket Error Handling in lease-shell:
Added comprehensive unit tests for StatefulSet detection logic covering:
These fixes ensure proper workload type detection and improve error visibility for lease-shell operations, resolving issues with shell access to deployments using RAM volumes.