feat(ws): Notebooks 2.0 // Backend // Create mocked resources #345
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Signed-off-by: Yehudit Kerido <[email protected]>
yehudit1987
force-pushed
the
create-mocked-resources
branch
from
6a1933b to
4261b5a
Compare
| YQ_VERSION = v4.30.6 | ||
| YQ_BINARY = /usr/local/bin/yq | ||
| KUBECTL_VERSION = v1.27.3 | ||
| KUBECTL_BINARY = /usr/local/bin/kubectl |
There was a problem hiding this comment.
Can we move this section to be declared up with other dependencies here:
Additionally - we already have a KUBECTL Makefile var:
We should either continue to use that... or remove that.. but I think its too confusing having all these defined:
KUBECTLKUBECTL_VERSIONKUBECTL_BINARY
| WORKSPACEKIND_FILES = data_setup/workspacekinds/jupyterlab_v1beta1_workspacekind.yaml \ | ||
| data_setup/workspacekinds/codeserver_v1beta1_workspacekind.yaml \ | ||
| data_setup/workspacekinds/rstudio_v1beta1_workspacekind.yaml |
There was a problem hiding this comment.
Might be nice to use a file glob pattern here instead of explicitly listing the files - so that in the future if/when we want to add more "test data" (or remove data) - it would just automatically work ...
| install-yq: | ||
| $(call install-if-missing,yq,\ | ||
| sudo wget -q "https://github.com/mikefarah/yq/releases/download/$(YQ_VERSION)/yq_linux_amd64" -O $(YQ_BINARY) && sudo chmod +x $(YQ_BINARY)) |
There was a problem hiding this comment.
we probably want this to be smarter to detect the OS and form the URL appropriately so other (at least MacOS/arm) platforms are supported
| install-kubectl: | ||
| $(call install-if-missing,kubectl,\ | ||
| sudo curl -LO "https://dl.k8s.io/release/$(KUBECTL_VERSION)/bin/linux/amd64/kubectl" && sudo chmod +x kubectl && sudo mv kubectl $(KUBECTL_BINARY)) |
There was a problem hiding this comment.
we probably want this to be smarter to detect the OS and form the URL appropriately so other (at least MacOS/arms) platforms are supported
| endef | ||
|
|
||
| .PHONY: install-yq install-kubectl create-workspacekinds create-workspaces setup-all clean |
There was a problem hiding this comment.
For consistency - I'd follow the convention present in this file to declare a .PHONY directly preceding each Makefile target...
| volumeMounts: | ||
|
|
||
| ## the path to mount the home PVC (NOT MUTABLE) | ||
| home: "/home/jovyan" |
There was a problem hiding this comment.
while admittedly it DOES NOT MATTER - /home/jovyan is a JupyterLab thing
If there is a more appropriate RStudio home directory value.. we should use it... but offhand I'm not clear what that value would be... 🤔
| volumeMounts: | ||
|
|
||
| ## the path to mount the home PVC (NOT MUTABLE) | ||
| home: "/home/jovyan" |
There was a problem hiding this comment.
while admittedly it DOES NOT MATTER - /home/jovyan is a JupyterLab thing
If there is a more appropriate codeserver home directory value.. we should use it... but offhand I'm not clear what that value would be... 🤔
| @for id in $(WORKSPACE_IDS); do \ | ||
| echo "Creating workspace workspace-$$id..."; \ | ||
| eval "case $$id in \ | ||
| 1|2|3) workspace_kind_file=data_setup/workspacekinds/jupyterlab_v1beta1_workspacekind.yaml;; \ | ||
| 4|5|6) workspace_kind_file=data_setup/workspacekinds/codeserver_v1beta1_workspacekind.yaml;; \ | ||
| *) workspace_kind_file=data_setup/workspacekinds/rstudio_v1beta1_workspacekind.yaml;; \ | ||
| esac"; \ | ||
| workspace_kind_name=$$(yq '.metadata.name' "$$workspace_kind_file"); \ | ||
| image_config_id=$$(yq '.spec.podTemplate.options.imageConfig.spawner.default' "$$workspace_kind_file" | sed 's|"||g'); \ | ||
| pod_config_id=$$(yq '.spec.podTemplate.options.podConfig.spawner.default' "$$workspace_kind_file"); \ | ||
| sed "s/{workspace_id}/$$id/g; s/{WorkspaceKind}/$$workspace_kind_name/g; s/{imageConfig_id}/$$image_config_id/g; s/{podConfig_id}/$$pod_config_id/g" $(WORKSPACE_TEMPLATE_FILE) | \ | ||
| sed '/^spec:/a \ paused: true' | \ | ||
| kubectl apply -f -; \ | ||
| done |
There was a problem hiding this comment.
FOR DISCUSSION (not necessarily suggesting a change)
I feel like leveraging kustomize here might be a more natural fit and still gives us the "templating" behavior...
While your solution certainly yields less overall lines of code - kustomize might be "more familiar" to developers
|
/label hold |
|
@ederign: The label(s) In 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/test-infra repository. |
This PR solve Issue #342