Feature: Pause/resume sandbox (delete/restore Pod with no persistancy)#299
Feature: Pause/resume sandbox (delete/restore Pod with no persistancy)#299tomergee wants to merge 3 commits intokubernetes-sigs:mainfrom
Conversation
… be terminated and resumed pod will be recreated
… be terminated and resumed pod will be recreated
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tomergee The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @tomergee. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
✅ Deploy Preview for agent-sandbox canceled.
|
|
/retest |
|
@tomergee: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| f"Failed to communicate with the sandbox via the gateway at {url}.") from e | ||
|
|
||
| @trace_span("pause") | ||
| def pause(self): |
There was a problem hiding this comment.
Can we have a response objects for both methods with the status of the pause?
It might happen that the scaling down might not succeed. To report such case it would be nice to store the response.
| wait: If True (default), blocks until the sandbox pod is ready. | ||
| If False, returns immediately after patching replicas. | ||
| """ | ||
| if not self.sandbox_name: |
There was a problem hiding this comment.
Can we ever have this case based on the current logic?
This seems to be set when we first initialize the client, if the name is not set we throw an error. Were you thinking to eventually move from this model and use the create and terminate methods?
| """ | ||
| if not self.sandbox_name: | ||
| raise RuntimeError("Cannot resume; no sandbox has been created.") | ||
|
|
There was a problem hiding this comment.
can we add a validation check to see if the replica count is 0 before resuming?
| **Data Persistence:** Pausing deletes the pod, so any data on ephemeral storage (container | ||
| filesystem, `emptyDir` volumes) is **lost**. To persist data across pause/resume, the `Sandbox` CR | ||
| supports `spec.volumeClaimTemplates` for PVC-backed volumes. Note that this field is on the `Sandbox` | ||
| spec directly and is not currently exposed through `SandboxTemplate`/`SandboxClaim`. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@tomergee: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Feature: Support Sandbox Pause/Resume capabilities, on pause pod will be terminated and resumed pod will be recreated