Namespace support. #134
Namespace support. #134
Conversation
- Add support for setting the namespace during the manifest creations - Add make targets for creation of kind cluster for local development
Codecov Report
@@ Coverage Diff @@
## main #134 +/- ##
=======================================
Coverage 63.11% 63.11%
=======================================
Files 1 1
Lines 732 732
=======================================
Hits 462 462
Misses 220 220
Partials 50 50
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Makefile
Outdated
|
|
||
| ## local configurations | ||
|
|
||
| deploy-develmode: manifests kustomize ## Deploy controller in debug mode to the K8s cluster specified in ~/.kube/config. |
There was a problem hiding this comment.
The target name says "devel" mode while the descriptions says "debug". But, most importantly:
- What's the difference to the existing
deploytarget? - Why does it override the deployment manifests file at
$(DEPLOYMENT_FILE), but then won't use it?
There was a problem hiding this comment.
At the I am going to say I had a valid reason for doing it but I can not find it now. I will remove and use the deploy target.
| @@ -182,16 +186,40 @@ docker-push: ## Push docker image with the manager. | |||
| ##@ Deployment | |||
|
|
|||
| install: manifests kustomize install-authorino ## Install CRDs into the K8s cluster specified in ~/.kube/config. | |||
| @if [ $(NAMESPACE) != '' ];then \ | |||
| echo "Setting Custom Namespace: $(NAMESPACE)"; \ | |||
| cd $(PROJECT_DIR)/config/install && $(KUSTOMIZE) edit set namespace $(NAMESPACE); \ | |||
There was a problem hiding this comment.
Do you have the diff of what this target yields with a custom namespace vs the default one? Likely, this is only because of the role bindings created for the operator, so it sets the right ServiceAccount, correct?
There was a problem hiding this comment.
I'll create a diff
There was a problem hiding this comment.
The diff is in the gist. I comment out then clean up code when creating the diff.
https://gist.github.com/Boomatang/becfda6a1659f7e864ac039a439889bd
Makefile
Outdated
| .PHONY: local-cleanup | ||
| local-cleanup: | ||
| $(MAKE) kind-delete-cluster | ||
|
|
||
| .PHONY: local-env-setup | ||
| local-env-setup: | ||
| $(MAKE) kind-delete-cluster | ||
| $(MAKE) kind-create-cluster | ||
| $(KUSTOMIZE) build config/crd | kubectl apply -f - | ||
|
|
||
| .PHONY: local-setup | ||
| local-setup: export OPERATOR_IMAGE := authorino-operator:dev | ||
| local-setup: | ||
| $(MAKE) local-env-setup | ||
| $(MAKE) docker-build | ||
| echo "Deploying Authorino control plane" | ||
| $(KIND) load docker-image ${OPERATOR_IMAGE} --name ${KIND_CLUSTER_NAME} | ||
| $(MAKE) install | ||
| $(MAKE) deploy | ||
|
|
||
| .PHONY: local-redeploy | ||
| local-redeploy: export OPERATOR_IMAGE := authorino-operator:dev | ||
| local-redeploy: | ||
| $(MAKE) docker-build | ||
| echo "Deploying Authorino control plane" | ||
| $(KIND) load docker-image ${OPERATOR_IMAGE} --name ${KIND_CLUSTER_NAME} | ||
|
|
||
| @if [ $(NAMESPACE) != '' ];then \ | ||
| kubectl rollout restart deployment -n $(NAMESPACE) authorino-operator; \ | ||
| echo "Wait for all deployments to be up"; \ | ||
| kubectl -n $(NAMESPACE) wait --timeout=300s --for=condition=Available deployments --all; \ | ||
| else \ | ||
| kubectl rollout restart deployment -n $(DEFAULT_REPO) authorino-operator; \ | ||
| echo "Wait for all deployments to be up"; \ | ||
| kubectl -n $(DEFAULT_REPO) wait --timeout=300s --for=condition=Available deployments --all; \ | ||
| fi | ||
|
|
||
| ## kind configuration | ||
|
|
||
| KIND_CLUSTER_NAME ?= authorino-local | ||
|
|
||
| .PHONY: kind-create-cluster | ||
| kind-create-cluster: kind ## Create the "authorino-local" kind cluster. | ||
| $(KIND) create cluster --name $(KIND_CLUSTER_NAME) --config utils/kind-cluster.yaml | ||
|
|
||
| .PHONY: kind-delete-cluster | ||
| kind-delete-cluster: kind ## Delete the "authorino-local" kind cluster. | ||
| $(KIND) delete cluster --name $(KIND_CLUSTER_NAME) |
There was a problem hiding this comment.
I'm guessing these local cluster-related targets are all to support testing the operator deployed to a custom namespace. Was that the initial motivation?
To test local changes in the operator, I often find myself running the following:
kind create cluster
make docker-build OPERATOR_IMAGE=authorino-operator:local
kind load docker-image authorino-operator:local
make manifests && sed -e 's/quay.io\/kuadrant\/authorino-operator:latest/authorino-operator:local/' config/deploy/manifests.yaml | kubectl apply -f -Some other times:
kind create cluster
kubectl create namespace authorino-operator
make manifests install runThere was a problem hiding this comment.
My initial reason for adding the local cluster related targets was for similarity in the dev process. Both authorino and kuadrant-operator uses the same naming. The steps you stated are fine but they are not documented so when starting in the project it is unclear how to start with a dev cluster.
|
In general, I think the changes here are welcome for the developer workflow, i.e., the one where you clone the repo, make local changes, and want to try them out by rebuilding and deploying to an almost always local cluster, whilst switching the namespace to something other than the default For the non-OLM user workflow nonetheless, i.e., the one by which the operator is installed by simply applying the pre-build manifests (without cloning the repo, without custom build of the binary or container image), it's a step forward, but likely won't suffice. For that, we may need to incorporate some of the changes here into the install script to be introduced with #137. I'm thinking a possible subsequent step after merging both PRs would be making the install script to rely as well on kustomize, to rebuild the manifests "on-the-fly" including an optional custom namespace. MGC team did sth similar with https://github.com/Kuadrant/multicluster-gateway-controller/blob/main/hack/quickstart-setup.sh. |
Closes #41