Skip to content

Adding SandboxTemplate and SandboxClaim CRDs #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 1, 2025
Merged

Adding SandboxTemplate and SandboxClaim CRDs #8

merged 1 commit into from
Oct 1, 2025

Conversation

@barney-s
Copy link
Contributor

@barney-s barney-s commented Sep 9, 2025 •
edited
Loading

For #17

Adding and extensions folder and creating the CRDs there.
Goal is to indicate this is not core part of sandbox but a useful pattern.

SandboxTemplate:

  • Config object no controller expected for this
  • Created by administrator to specify common parameters for a sandbox
  • example names: py3, java8, conda-teamA, ...
  • We use claims to create a Sandbox instance from the template

SandboxClaim:

  • Create an instance of sandbox from a given claim
  • ttl determines after how long the sandboxClaim is auto-deleted, thereby deleting the sandbox
  • Separation of concerns. User can be given access to create only claims.
image

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 9, 2025
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 9, 2025
vicentefb
vicentefb reviewed Sep 9, 2025
View reviewed changes
vicentefb
vicentefb reviewed Sep 9, 2025
View reviewed changes
Copy link
Contributor

@vicentefb vicentefb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a TTL related comment

janetkuo
janetkuo reviewed Sep 9, 2025
View reviewed changes
@barney-s barney-s mentioned this pull request Sep 9, 2025
Open
@tomergee
Copy link
Contributor

Sandbox template should also include the ttl field to be copied by the controller to the Sandbox?

maqiuyujoyce
maqiuyujoyce reviewed Sep 11, 2025
View reviewed changes
@matzew
Copy link

matzew commented Sep 11, 2025

Is there some background info on these new two CRDs?

@barney-s
Copy link
Contributor Author

Is there some background info on these new two CRDs?

#17 created for this.

@barney-s
Copy link
Contributor Author

Sandbox template should also include the ttl field to be copied by the controller to the Sandbox?

#18 created to track this

vicentefb
vicentefb suggested changes Sep 15, 2025
View reviewed changes
Copy link
Contributor

@vicentefb vicentefb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we expect a user to delete a sandbox? Is it via deleting the corresponding sandboxClaim?

What if a user kubectl delete sandbox directly, since sandbox is still a visible object to user. In this case, will the sandboxClaim controller try to create a new sandbox for this sandboxClaim?

@barney-s
Copy link
Contributor Author

When using claims, the user only interacts with claim.
They could delete the sandbox directly. That would be disruptive to the sandbox user.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2025
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2025
@barney-s barney-s mentioned this pull request Sep 16, 2025
Merged
@justinsb
Copy link
Contributor

Is there some background info on these new two CRDs?

#17 created for this.

I do agree with @matzew - it should be clear why we are doing these things (or at least what we are investigating - we are only in alpha). If they are going into examples/ I think the bar is lower, but I think this is proposed for the "main" API.

I added a comment on #17, thank you for opening that @barney-s, and let's continue the discussion there?

maqiuyujoyce
maqiuyujoyce reviewed Sep 18, 2025
View reviewed changes
@gongmax gongmax mentioned this pull request Sep 18, 2025
Open
janetkuo
janetkuo reviewed Sep 24, 2025
View reviewed changes
@barney-s barney-s force-pushed the template-claim branch 4 times, most recently from 72ac518 to fe60e98 Compare September 30, 2025 17:55
@barney-s barney-s mentioned this pull request Sep 30, 2025
Closed
@barney-s
Copy link
Contributor Author

Updated the PR to move the CRDs out of core. Moving them under extensions/ folder.
PTAL again @justinsb @janetkuo

janetkuo
janetkuo requested changes Sep 30, 2025
View reviewed changes
@barney-s
Adding SandboxTemplate and SandboxClaim CRDs
e3ec105 
SanboxTemplate:
* Config object no controller expected for this
* Created by administrator to specify common parameters for a sandbox
* example names: py3, java8, conda-teamA, ...
* We use claims to create a Sandbox instance from the template
* template is cluster scoped

SandboxClaim:
* Create an instance of sandbox from a given claim
* ttl determines after how long the sandboxClaim is auto-deleted,
  thereby deleting the sandbox
* Seperation of concerns. User can be given access to create only
  claims.
@barney-s barney-s requested a review from janetkuo September 30, 2025 22:58
justinsb
justinsb reviewed Oct 1, 2025
View reviewed changes
extensions/api/v1alpha1/sandboxclaim_types.go
@@ -0,0 +1,89 @@
// Copyright 2025 The Kubernetes Authors.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I need to improve my copyright checking script - two copyrights in this file :-)

@justinsb
Copy link
Contributor

justinsb commented Oct 1, 2025

As this is not on the same path as our "primary" API, I think putting it in extensions and iterating quickly sgtm!

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 1, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: barney-s, justinsb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 1, 2025
@k8s-ci-robot k8s-ci-robot merged commit 323740e into kubernetes-sigs:main Oct 1, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinsb justinsb justinsb left review comments

@soltysh soltysh Awaiting requested review from soltysh

@vicentefb vicentefb Awaiting requested review from vicentefb

@maqiuyujoyce maqiuyujoyce Awaiting requested review from maqiuyujoyce

@janetkuo janetkuo Awaiting requested review from janetkuo

Assignees

@justinsb justinsb

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@barney-s @tomergee @matzew @justinsb @k8s-ci-robot @janetkuo @maqiuyujoyce @vicentefb