-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
feat(ws): add Workspace
and WorkspaceKind
CRD scaffolds
#6
feat(ws): add Workspace
and WorkspaceKind
CRD scaffolds
#6
Conversation
Workspace
and WorkspaceKind
CRD scaffolds
1bed8d1
to
b9fd481
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jiridanek I have made a large number of suggestions.
But the most important thing you need to do is regenerate the WorkspaceKind
resource as a Cluster-Scoped resource, its currently Namespace-scoped.
3ebcb5e
to
b019d0c
Compare
Co-authored-by: Mathew Wicks <[email protected]> Signed-off-by: Jiri Daněk <[email protected]>
b019d0c
to
9cdb82e
Compare
Signed-off-by: Jiri Daněk <[email protected]>
Signed-off-by: Mathew Wicks <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jiridanek I pushed a commit with some changes, I would appreciate you taking a look to see if I messed anything up: 4dc0c16
Also, I just want to clarify, in what situations are you using pointers (*
) in your type definitions, e.g. why are we using pointers for (I am forgetting when they are needed):
type WorkspaceKindActivityProbe struct {
// a "shell" command to run in the Workspace, if the Workspace had activity in the last 60 seconds, this command
// should return status 0, otherwise it should return status 1
Exec *WorkspaceKindActivityProbeExec `json:"exec,omitempty"`
// a Jupyter-specific probe will poll the /api/status endpoint of the Jupyter API, and use the last_activity field
// Users need to be careful that their other probes don't trigger a "last_activity" update,
// e.g. they should only check the health of Jupyter using the /api/status endpoint
Jupyter *WorkspaceKindActivityProbeJupyter `json:"jupyter,omitempty"`
}
// +kubebuilder:validation:XValidation:message="must specify exactly one of 'exec' or 'jupyter'",rule="!(has(self.exec) && has(self.jupyter)) && (has(self.exec) || has(self.jupyter))" | ||
type WorkspaceKindActivityProbe struct { | ||
// a "shell" command to run in the Workspace, if the Workspace had activity in the last 60 seconds, this command | ||
// should return status 0, otherwise it should return status 1 | ||
Exec *WorkspaceKindActivityProbeExec `json:"exec,omitempty"` | ||
|
||
// a Jupyter-specific probe will poll the /api/status endpoint of the Jupyter API, and use the last_activity field | ||
// Users need to be careful that their other probes don't trigger a "last_activity" update, | ||
// e.g. they should only check the health of Jupyter using the /api/status endpoint | ||
Jupyter *WorkspaceKindActivityProbeJupyter `json:"jupyter,omitempty"` | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I just want to clarify, in what situations are you using pointers (*) in your type definitions, e.g. why are we using pointers for (I am forgetting when they are needed):
That's for the has(self.exec)
. Pointers have the meaning of optionals in Go, and nil
means that value is not present. I understand it might be helpful not to need it (1B $$$ mistake and so on), but I did not manage to set up the XValidation
without it.
The other occurrence of pointers in the PR is the same case, also XValidation.
looks good to me, thanks! |
Signed-off-by: Mathew Wicks <[email protected]>
@jiridanek I did some small updates in bb2ca34 (mostly adding a few fields to the Workspace status that are needed for the UI), but I also added some configs to make the I am going to merge this now, so we can start the reconciliation loop PR (we can make any further changes in that PR). /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thesuperzapper 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 |
* feat(ws): add `Workspace` and `WorkspaceKind` CRD scaffolds Co-authored-by: Mathew Wicks <[email protected]> Signed-off-by: Jiri Daněk <[email protected]> * fixup, regenerate WOrkspaceKind to be cluster scope Signed-off-by: Jiri Daněk <[email protected]> * fixes to crd spec Signed-off-by: Mathew Wicks <[email protected]> * updates to CRD spec Signed-off-by: Mathew Wicks <[email protected]> --------- Signed-off-by: Jiri Daněk <[email protected]> Signed-off-by: Mathew Wicks <[email protected]> Co-authored-by: Mathew Wicks <[email protected]>
* feat(ws): add `Workspace` and `WorkspaceKind` CRD scaffolds Co-authored-by: Mathew Wicks <[email protected]> Signed-off-by: Jiri Daněk <[email protected]> * fixup, regenerate WOrkspaceKind to be cluster scope Signed-off-by: Jiri Daněk <[email protected]> * fixes to crd spec Signed-off-by: Mathew Wicks <[email protected]> * updates to CRD spec Signed-off-by: Mathew Wicks <[email protected]> --------- Signed-off-by: Jiri Daněk <[email protected]> Signed-off-by: Mathew Wicks <[email protected]> Co-authored-by: Mathew Wicks <[email protected]>
Followup on top of my:
Compared to the previous PR,
WorkspaceKind
must be created as cluster-wide resource, so(it has to be
--namespaced=false
;--namespaced false
is ignored, see kubernetes-sigs/kubebuilder#322 (comment))Current CRD spec:
Workspace
WorkspaceKind
Todo's from meeting https://docs.google.com/document/d/1SiWLah-U07hAc47sSsoI8-NkbkLnHl1YVzxw193QUKE/edit#heading=h.naudedl8te2q
image .... httpPort
into a list of ports (to allow multiple services per image)spec.spawner.deprecated: true/false
spec.spawner.deprecatedMessage: “xxxxx”