🌱 Add make targets for setting up Tilt #7097

Jont828 · 2022-08-19T21:58:39Z

What this PR does / why we need it: Add make targets to create a kind cluster and set up Tilt with make tilt-up and delete kind cluster with kind-reset

k8s-ci-robot · 2022-08-19T21:58:41Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

k8s-ci-robot · 2022-08-19T21:58:47Z

@Jont828: This issue is currently awaiting triage.

If CAPI contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

ykakarap · 2022-08-20T01:44:28Z

Makefile

+
+.PHONY: tilt-up
+tilt-up: kind-cluster ## Start tilt and build kind cluster if needed.
+	EXP_CLUSTER_RESOURCE_SET=true EXP_MACHINE_POOL=true tilt up


Is there a reason we are setting these env values here. A user can set the feature gates in tilt-settings.yaml, right?

I would also prefer having only one place where env vars / feature flags are enabled (tilt-settings.yaml) vs. overwriting them here hard-coded

I think I just copied that over from my own set up. Just to clarify are we good to remove those env vars and just have it run tilt-up?

I'm fine with that

killianmuldoon

I think the kind-reset target could be useful - but we should make the cluster name explicit in the kind-cluster target to ensure they're always targetting the same cluster.

The tilt up target doesn't seem that useful on top of the flow make kind-cluster && tilt up and, as @ykakarap said, we shouldn't be including env variables there.

Makefile

ykakarap · 2022-08-24T01:50:50Z

Makefile

@@ -669,6 +669,14 @@ test-capd-junit: $(SETUP_ENVTEST) $(GOTESTSUM) ## Run unit and integration tests
 kind-cluster: ## Create a new kind cluster designed for development with Tilt
 	hack/kind-install-for-capd.sh

+.PHONY: kind-reset
+kind-reset: ## Destroys the "capi-test" kind cluster.
+	kind delete cluster --name=capi-test || true


I dont think we can always assume that the kind cluster's name will be capi-test given that make kind-cluster can create clusters with other names. We can either enforce the cluster name as @killianmuldoon suggested or have the kind-reset target also support the CAPI_KIND_CLUSTER_NAME like the kind-cluster target does.

I dont have strong opinions on either options.

I would prefer to use the same logic here as we have in the script. Which means use the env var CAPI_KIND_CLUSTER_NAME if it is set and fallback to capi-test if the env var is not set

Makefile

Jont828 · 2022-08-24T22:14:27Z

@sbueringer I've added make targets for removing .tiltbuild files and local charts per your comment in #7018, what do you think about this approach?

Makefile

ykakarap · 2022-08-25T01:42:05Z

Makefile


 .PHONY: clean-bin
 clean-bin: ## Remove all generated binaries
 	rm -rf $(BIN_DIR)
 	rm -rf $(TOOLS_BIN_DIR)

+.PHONY: clean-tilt
+clean-tilt: ## Remove all files in .tiltbuild
+	rm -rf ./tiltbuild


I am good with adding this target but just FYI tilt-prepare takes care of updating the files in .tiltbuild if it observes any changes. Dropping this comment here incase this target was just added as part of the "pull the correct chart" problem.

Yeah I added it since @sbueringer mentioned a while back we didn't have any way to clean up unused files in .tiltbuild. For example when we change the output filename it won't remove the old file.

This doesn't remove the .tiltbuild stuff in e.g. docker, controlplane, bootstrap, right? I guess we should remove ./**/.tiltbuild ?

This also won't clean artifacts in provider repos, so it could be slightly confusing, but those are just binaries I think.

You are right, the .tiltbuild folders under providers only contain the manager binaries for now. I think we should be fine with just cleaning up .tiltbuild in the CAPI root.

Let's delete all of them. Only deleting one is a bit inconsequential. I'm fine with just listing the 4 rm -rf calls here. After all they are pretty stable in core CAPI

Makefile

fabriziopandini · 2022-08-25T09:45:10Z

nice improvement, lgtm pending open comments
here (or in a follow up PR) we should go trough developers' docs and promote usage of this new targets

Jont828 · 2022-08-25T16:13:56Z

@fabriziopandini Sure thing. Do we want to put it on the Tilt dev page?

Makefile

Jont828 · 2022-08-26T22:20:44Z

I added some docs, PTAL when you have some time!

Makefile

fabriziopandini

one small nit, otherwise lgtm

fabriziopandini · 2022-08-29T09:49:40Z

Makefile

@@ -669,6 +672,14 @@ test-capd-junit: $(SETUP_ENVTEST) $(GOTESTSUM) ## Run unit and integration tests
 kind-cluster: ## Create a new kind cluster designed for development with Tilt
 	hack/kind-install-for-capd.sh

+.PHONY: kind-reset
+kind-reset: ## Destroys the kind cluster with the name $CAPI_KIND_CLUSTER_NAME.


Suggested change

kind-reset: ## Destroys the kind cluster with the name $CAPI_KIND_CLUSTER_NAME.

clean-kind: ## Cleanup the kind cluster with the name $CAPI_KIND_CLUSTER_NAME.

Also, this target should be moved down in the file so it appears under the clean section (before clean tilt)

fabriziopandini · 2022-08-29T09:52:20Z

docs/book/src/developer/tilt.md

+After stopping Tilt, you can clean up your kind cluster and development environment by running
+
+```bash
+make kind-reset
+```


q: should this be part of make clean (like all the clean targets)?

I can see why they make sense as 2 different targets. make clean is to delete all auto-generated files that are now part of the project folder strucutre. make kind-reset doesn't really fall under the same category.

In that case do we still want to rename make kind-reset to make clean-tilt? I had it next to make kind-cluster in the file as it's the inverse operation.

In that case do we still want to rename make kind-reset to make clean-tilt?

Did you mean rename kind-reset to clean-kind? If so, then yeah, I think that sounds good. (As suggested here: #7097 (comment))

I had it next to make kind-cluster in the file as it's the inverse operation.

Having make kind-reset next to make kind-cluster makes sense, since as you said they it is the inverse operation.

docs/book/src/developer/tilt.md

Jont828 · 2022-09-01T00:18:39Z

Alright I pushed some changes and squashed, I think we're ready for a last pass of reviews!

ykakarap

/lgtm

ykakarap

Changes look good.

killianmuldoon

/lgtm

Makefile

ykakarap

/lgtm

fabriziopandini · 2022-09-02T09:04:47Z

/lgtm
/approve

k8s-ci-robot · 2022-09-02T09:05:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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:

~~OWNERS~~ [fabriziopandini]

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 added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 19, 2022

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 19, 2022

k8s-ci-robot requested review from CecileRobertMichon and stmcginnis August 19, 2022 21:58

Jont828 force-pushed the make-tilt branch from 2040335 to a794c1e Compare August 19, 2022 22:03

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 19, 2022

Jont828 marked this pull request as ready for review August 19, 2022 23:21

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 19, 2022

k8s-ci-robot requested review from JoelSpeed and vincepri August 19, 2022 23:21

ykakarap reviewed Aug 20, 2022

View reviewed changes

killianmuldoon reviewed Aug 22, 2022

View reviewed changes

sbueringer reviewed Aug 22, 2022

View reviewed changes

Makefile Show resolved Hide resolved

ykakarap reviewed Aug 24, 2022

View reviewed changes

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 24, 2022

Jont828 force-pushed the make-tilt branch from 595f1d3 to 780c741 Compare August 24, 2022 20:28

Jont828 commented Aug 24, 2022

View reviewed changes

Makefile Show resolved Hide resolved

Jont828 force-pushed the make-tilt branch from f73d105 to 5113731 Compare August 24, 2022 22:24

ykakarap reviewed Aug 25, 2022

View reviewed changes

Jont828 force-pushed the make-tilt branch from 5113731 to 84f9bae Compare August 25, 2022 16:11

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 25, 2022

Jont828 mentioned this pull request Aug 25, 2022

🌱 Update Cluster API Visualizer chart to v1.0.0 #7018

Merged

ykakarap reviewed Aug 26, 2022

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Makefile Show resolved Hide resolved

Jont828 force-pushed the make-tilt branch 2 times, most recently from b3b4d81 to d76b044 Compare August 26, 2022 21:48

killianmuldoon reviewed Aug 29, 2022

View reviewed changes

Makefile Outdated Show resolved Hide resolved

fabriziopandini reviewed Aug 29, 2022

View reviewed changes

sbueringer reviewed Aug 29, 2022

View reviewed changes

docs/book/src/developer/tilt.md Outdated Show resolved Hide resolved

Jont828 force-pushed the make-tilt branch from fb4116d to aef8718 Compare September 1, 2022 00:18

ykakarap approved these changes Sep 1, 2022

View reviewed changes

k8s-ci-robot assigned ykakarap Sep 1, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 1, 2022

ykakarap reviewed Sep 1, 2022

View reviewed changes

killianmuldoon approved these changes Sep 1, 2022

View reviewed changes

k8s-ci-robot assigned killianmuldoon Sep 1, 2022

fabriziopandini reviewed Sep 1, 2022

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Add make targets for Tilt, kind, and clean up

c40decf

Jont828 force-pushed the make-tilt branch from aef8718 to c40decf Compare September 1, 2022 16:25

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 1, 2022

ykakarap approved these changes Sep 2, 2022

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 2, 2022

k8s-ci-robot assigned fabriziopandini Sep 2, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 2, 2022

k8s-ci-robot merged commit 0043155 into kubernetes-sigs:main Sep 2, 2022

k8s-ci-robot added this to the v1.3 milestone Sep 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 Add make targets for setting up Tilt #7097

🌱 Add make targets for setting up Tilt #7097

Jont828 commented Aug 19, 2022

k8s-ci-robot commented Aug 19, 2022

k8s-ci-robot commented Aug 19, 2022

ykakarap Aug 20, 2022

sbueringer Aug 22, 2022

Jont828 Aug 24, 2022

sbueringer Aug 25, 2022

killianmuldoon left a comment

ykakarap Aug 24, 2022

sbueringer Aug 24, 2022

Jont828 commented Aug 24, 2022

ykakarap Aug 25, 2022

Jont828 Aug 25, 2022

killianmuldoon Aug 25, 2022

ykakarap Aug 26, 2022

sbueringer Aug 26, 2022

fabriziopandini commented Aug 25, 2022

Jont828 commented Aug 25, 2022

Jont828 commented Aug 26, 2022

fabriziopandini left a comment

fabriziopandini Aug 29, 2022

fabriziopandini Aug 29, 2022

ykakarap Aug 29, 2022

Jont828 Aug 30, 2022

ykakarap Aug 31, 2022 •

edited

Loading

Jont828 commented Sep 1, 2022

ykakarap left a comment

ykakarap left a comment

killianmuldoon left a comment

ykakarap left a comment

fabriziopandini commented Sep 2, 2022

k8s-ci-robot commented Sep 2, 2022

	kind-reset: ## Destroys the kind cluster with the name $CAPI_KIND_CLUSTER_NAME.
	clean-kind: ## Cleanup the kind cluster with the name $CAPI_KIND_CLUSTER_NAME.

🌱 Add make targets for setting up Tilt #7097

🌱 Add make targets for setting up Tilt #7097

Conversation

Jont828 commented Aug 19, 2022

k8s-ci-robot commented Aug 19, 2022

k8s-ci-robot commented Aug 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

killianmuldoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jont828 commented Aug 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabriziopandini commented Aug 25, 2022

Jont828 commented Aug 25, 2022

Jont828 commented Aug 26, 2022

fabriziopandini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ykakarap Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

Jont828 commented Sep 1, 2022

ykakarap left a comment

Choose a reason for hiding this comment

ykakarap left a comment

Choose a reason for hiding this comment

killianmuldoon left a comment

Choose a reason for hiding this comment

ykakarap left a comment

Choose a reason for hiding this comment

fabriziopandini commented Sep 2, 2022

k8s-ci-robot commented Sep 2, 2022

ykakarap Aug 31, 2022 •

edited

Loading