Skip to content

feat: setup tilt for easier local controller development #220

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

Open
wants to merge 12 commits into
base: notebooks-v2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion workspaces/controller/.dockerignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
# More info: https://docs.docker.com/engine/reference/builder/#dockerignore-file
# Ignore build and test binaries.
bin/
71 changes: 71 additions & 0 deletions workspaces/controller/DEVELOPMENT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# Development Guide
Copy link
Member

Choose a reason for hiding this comment

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

@Al-Pragliola can we please add something about running the the tests locally before raising a PR:

  • make lint
  • make test (for unit tests without a local kube/kind)
  • make test-e2e (run intergration tests on a local kube)

As people wont know they need to do this otherwise

Copy link
Author

Choose a reason for hiding this comment

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

wrote a small section in 7879d29


## Table of Contents

- [Development Guide](#development-guide)
- [Table of Contents](#table-of-contents)
- [Introduction](#introduction)
- [Prerequisites](#prerequisites)
- [Getting Started](#getting-started)
- [In an existing cluster](#in-an-existing-cluster)
- [Using kind](#using-kind)
- [Teardown](#teardown)
- [Troubleshooting](#troubleshooting)
- ["Build Failed: failed to dial gRPC: unable to upgrade to h2c, received 404"](#build-failed-failed-to-dial-grpc-unable-to-upgrade-to-h2c-received-404)

## Introduction

This guide will help you set up a development environment for the Kubeflow Notebooks project.

## Prerequisites

- Go >= 1.22
- Kubectl >= 1.22
- A Kubernetes cluster (e.g. [kind](https://kind.sigs.k8s.io/#installation-and-usage))
- Cert-manager installed in the cluster, see [cert-manager installation guide](https://cert-manager.io/docs/installation/#default-static-install)

## Getting Started

This project uses [Tilt](https://tilt.dev/) to manage the development environment. Tilt will watch for changes in the project and automatically rebuild the Docker image and redeploy the application in the **current Kubernetes context**.

This comment was marked as resolved.


### Cluster Selection

Make sure you have a Kubernetes cluster running and `kubectl` is configured to use it.
* `kubectl config current-context` will report which context Tilt will interact with

💡 For development purposes, you may find using `kind` to be beneficial. You can create your own local cluster with the following command:
- `kind create cluster`
- This command will also change the `current-context` of `kubectl` to the `kind` cluster that is created

### Running Tilt

1. Run the following command to start Tilt:

```bash
make -C devenv tilt-up
```

ℹ️ Please make sure you are in the `workspaces/controller` directory when executing the command.

2. Hit `space` to open the Tilt dashboard in your browser and here you can see the logs and status of every resource deployed.


## Teardown

To stop Tilt and remove all resources created by it, run:

```bash
make -C devenv tilt-down
```

ℹ️ Please make sure you are in the `workspaces/controller` directory when executing the command.

## Troubleshooting

### "Build Failed: failed to dial gRPC: unable to upgrade to h2c, received 404"

If you see the following error message when tilt builds the image, try setting `DOCKER_BUILDKIT=0`:

```bash
DOCKER_BUILDKIT=0 make -C devenv tilt-up
```
2 changes: 2 additions & 0 deletions workspaces/controller/devenv/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bin/*
tilt_config.json
58 changes: 58 additions & 0 deletions workspaces/controller/devenv/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
detected_OS := $(shell uname -s)
real_OS := $(detected_OS)
arch := $(shell uname -m)
goarch := $(shell go env GOARCH)
ifeq ($(detected_OS),Darwin)
detected_OS := mac
real_OS := darwin
endif
ifeq ($(detected_OS),Linux)
detected_OS := linux
real_OS := linux
endif

## Cleanup targets

.PHONY: cleanup-crds
cleanup-crds:
@echo "Cleaning up CRDs..."
@kubectl delete -f ../config/crd/bases/ || true

## Requirements

.PHONY: check-cert-manager
check-cert-manager: cmctl
@echo "Verifying cert-manager installation..."
@$(LOCALBIN)/cmctl check api > /dev/null 2>&1 || (printf "cert-manager is either not installed or not ready. Please install cert-manager or wait until it becomes ready.\nInstallation instructions can be found here: https://cert-manager.io/docs/installation/\n\n" && exit 1)
@echo "cert-manager is installed and ready."

## Location to install dependencies to
LOCALBIN ?= $(shell pwd)/bin
$(LOCALBIN):
mkdir -p $(LOCALBIN)

## Tool Binaries
TILT ?= $(LOCALBIN)/tilt
CMCTL ?= $(LOCALBIN)/cmctl

## Tool Versions
TILT_VERSION := 0.33.22
CMCTL_VERSION := 2.1.1

.PHONY: cmctl
.PHONY: $(CMCTL)
cmctl: $(CMCTL)
$(CMCTL): $(LOCALBIN)
test -s $(LOCALBIN)/cmctl || curl -fsSL https://github.com/cert-manager/cmctl/releases/download/v$(CMCTL_VERSION)/cmctl_$(real_OS)_$(goarch).tar.gz | tar -xz -C $(LOCALBIN) cmctl

.PHONY: tilt
.PHONY: $(TILT)
tilt: $(TILT)
$(TILT): $(LOCALBIN)
test -s $(LOCALBIN)/tilt || curl -fsSL https://github.com/tilt-dev/tilt/releases/download/v$(TILT_VERSION)/tilt.$(TILT_VERSION).$(detected_OS).$(arch).tar.gz | tar -xz -C $(LOCALBIN) tilt

tilt-up: tilt check-cert-manager

This comment was marked as resolved.

Copy link
Author

Choose a reason for hiding this comment

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

thanks for checking this PR, yes the Makefile had a bug in line 25, cmctl was meant to be in line 26, I fixed it in the latest commit eda5804

$(LOCALBIN)/tilt up

tilt-down: tilt cleanup-crds
$(LOCALBIN)/tilt down
122 changes: 122 additions & 0 deletions workspaces/controller/devenv/Tiltfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
load("ext://restart_process", "docker_build_with_restart")

load_dynamic("./configs/tiltfiles/Tiltfile.setup")

config.define_string_list("services")

parsed_config = config.parse()

for service in parsed_config.get("services", []):
load_dynamic("./configs/tiltfiles/Tiltfile.%s" % (service))

manifests = kustomize("../config/default")

objects = decode_yaml_stream(manifests)

for o in objects:
if o["kind"] == "Deployment" and o.get("metadata").get("name") in ["workspace-controller-controller-manager"]:
o["spec"]["template"]["spec"]["securityContext"] = {"runAsNonRoot": False, "readOnlyRootFilesystem": False}
o["spec"]["template"]["spec"]["containers"][0]["imagePullPolicy"] = "Always"

overridden_manifests = encode_yaml_stream(objects)
Comment on lines +14 to +21

Choose a reason for hiding this comment

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

Curious why we are programmatically modifying manifests here... instead of using kustomize overlays?

Copy link
Author

Choose a reason for hiding this comment

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

I didn't want to create an overlay for local development, but I am open to do it if the community agrees

Choose a reason for hiding this comment

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

@thesuperzapper what are your thoughts on a kustomize local development overlay vs. this programmatic approach?


k8s_yaml(overridden_manifests, allow_duplicates=True)

local_resource(
"cert-manager-req-check",
serve_cmd="sleep infinity",
labels="requirements",
readiness_probe=probe(
exec=exec_action(
command=["/bin/sh", "-c", "./bin/cmctl check api"]
), initial_delay_secs=5, timeout_secs=60
)
)

Choose a reason for hiding this comment

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

@Al-Pragliola - I'm confused by this config block (and it has a portability issue)

(1) sleep infinity fails for me - locally I changed this to: serve_cmd="while true; do sleep 3600; done",

(2) however, I'm not sure of the purpose of this block to begin with... if I understand correctly... local_resource does not support a readiness_probe attribute... so this entire section seems to ONLY then issue a perpetual sleep command ... The cert-manager check is also being done as part of the Makefile commands - so can we remove this block from Tilefilt?

Please advise.

Copy link
Author

@Al-Pragliola Al-Pragliola Apr 28, 2025

Choose a reason for hiding this comment

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

@andyatmiami

(1) ok I'll change it to make it compatible with macos users
(2) local_resource has the readiness_probe attribute https://docs.tilt.dev/api.html#api.local_resource , so this resource was needed to trigger the deployment of the controller when cert-manager was ready. I would like to not remove the check from Tilt because if someone does not use the makefile target and just runs tilt up, it would skip the check and fail to deploy the controller

Copy link
Author

Choose a reason for hiding this comment

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

(1) addressed in 2c096e2


k8s_resource(
new_name="certs",
objects=[
"workspace-controller-serving-cert:certificate",
"workspace-controller-selfsigned-issuer:issuer"
],
labels="controller",
resource_deps=[
"controller-namespace",
"cert-manager-req-check"
]
)

k8s_resource(
new_name="reqs",
objects=[
"workspace-controller-controller-manager:serviceaccount",
"workspace-controller-leader-election-role:role",
"workspace-controller-manager-role:clusterrole",
"workspace-controller-workspace-editor-role:clusterrole",
"workspace-controller-workspace-viewer-role:clusterrole",
"workspace-controller-workspacekind-editor-role:clusterrole",
"workspace-controller-workspacekind-viewer-role:clusterrole",
"workspace-controller-leader-election-rolebinding:rolebinding",
"workspace-controller-manager-rolebinding:clusterrolebinding",
"workspace-controller-validating-webhook-configuration:validatingwebhookconfiguration"
],
labels="controller",
resource_deps=[
"controller-namespace"
]
)

k8s_resource(
new_name="crds",
objects=[
"workspacekinds.kubeflow.org:customresourcedefinition",
"workspaces.kubeflow.org:customresourcedefinition"
],
labels="controller",
resource_deps=[
"controller-namespace",
]
)

k8s_resource(
new_name="controller-namespace",
objects=["workspace-controller-system:Namespace:default"],
labels="requirements"
)

k8s_resource(
workload="workspace-controller-controller-manager",
new_name="controller",
labels="controller",
resource_deps=[
"controller-namespace",
"cert-manager-req-check",
"certs"
]
)

local_resource(
"manager-bin",
"CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o bin/manager cmd/main.go",
dir = "../",
deps = [
"../cmd",
"../internal",
"../go.mod",
"../go.sum",
],
labels="controller",
)

docker_build_with_restart(
"ghcr.io/kubeflow/notebooks/workspace-controller",
context = "../",
dockerfile = "../tilt.dockerfile",
entrypoint = ["/manager"],
only=[
"bin/",
],
live_update = [
sync("../bin/manager", "/manager"),
],
)
4 changes: 4 additions & 0 deletions workspaces/controller/devenv/configs/tiltfiles/Tiltfile.setup
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Disable analytics
analytics_settings(False)

update_settings(k8s_upsert_timeout_secs = 120)
7 changes: 7 additions & 0 deletions workspaces/controller/tilt.dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
FROM alpine:3.12

WORKDIR /

COPY bin/manager /manager

ENTRYPOINT ["/manager"]
Loading