feat: added shutdownTime to extensions CRs #124
Conversation
✅ Deploy Preview for agent-sandbox canceled.
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vicentefb The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
|
Hi @vicentefb. Thanks for your PR. I'm waiting for a github.com 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. 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. |
k8s-ci-robot
added
the
size/L
vicentefb
force-pushed
the
AddShutdownTimeToSandboxTemplate
branch
2 times, most recently
from
055781e to
d8d27ee
Compare
|
Thanks! Yes, there's this if statement in the sandbox controller that checks if the current time is after the shutdownTime https://github.com/kubernetes-sigs/agent-sandbox/blob/main/controllers/sandbox_controller.go#L448-L451 |
k8s-ci-robot
added
needs-rebase
k8s-ci-robot
added
the
lgtm
nit nit rebasing
vicentefb
force-pushed
the
AddShutdownTimeToSandboxTemplate
branch
from
71123b0 to
01f8f1d
Compare
k8s-ci-robot
removed
the
lgtm
|
New changes are detected. LGTM label has been removed. |
k8s-ci-robot
removed
the
needs-rebase
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
| // template should be deleted. | ||
| // +kubebuilder:validation:Format="date-time" | ||
| // +optional | ||
| ShutdownTime *metav1.Time `json:"shutdownTime,omitempty"` |
There was a problem hiding this comment.
I believe shutdownTime is better placed in the SandboxClaim rather than the template. This would give users more direct control over the lifecycle of each sandbox. Let me know if you have other use cases in mind.
| "time" // Added for shutdownTime | ||
|
|
||
| "github.com/google/go-cmp/cmp" | ||
| "github.com/google/go-cmp/cmp" // Added for ignoring fields |
There was a problem hiding this comment.
I don't think you need comments for imports
| "sigs.k8s.io/controller-runtime/pkg/reconcile" | ||
|
|
||
| "sigs.k8s.io/agent-sandbox/api/v1alpha1" | ||
| sandboxv1alpha1 "sigs.k8s.io/agent-sandbox/api/v1alpha1" |
There was a problem hiding this comment.
This seems repetitive, just keep the sandboxv1alpha1 one
| }, | ||
| Spec: extensionsv1alpha1.SandboxTemplateSpec{ | ||
| PodTemplate: sandboxv1alpha1.PodTemplate{ | ||
| PodTemplate: v1alpha1.PodTemplate{ |
There was a problem hiding this comment.
This change doesn't seem needed
5 participants
Fixes #108
This PR introduces
shutdownTimefeature to the sandbox extensions, allowing users to define an expiration time for their sandboxes. This feature is implemented with an override pattern:shutdownTimecan be set in the SandboxTemplate.shutdownTimein their SandboxClaim to override the template's default.This logic is implemented in the
sandboxclaim-controller, which copies the correct (overridden or default) shutdownTime to the Sandbox resource upon creation. The coresandbox-controlleris then responsible for watching this field and deleting the sandbox pod when the time is reached.