Skip to content
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

Namespace support. #134

Closed
wants to merge 2 commits into from
Closed

Conversation

Boomatang
Copy link
Contributor

  • Add support for setting the namespace during the manifest creations
  • Add make targets for creation of kind cluster for local development

Closes #41

- Add support for setting the namespace during the manifest creations
- Add make targets for creation of kind cluster for local development
@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2023

Codecov Report

Merging #134 (8659e32) into main (b03fcf4) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #134   +/-   ##
=======================================
  Coverage   63.11%   63.11%           
=======================================
  Files           1        1           
  Lines         732      732           
=======================================
  Hits          462      462           
  Misses        220      220           
  Partials       50       50           
Flag Coverage Δ
unit 63.11% <ø> (ø)

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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The target name says "devel" mode while the descriptions says "debug". But, most importantly:

  • What's the difference to the existing deploy target?
  • Why does it override the deployment manifests file at $(DEPLOYMENT_FILE), but then won't use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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); \
Copy link
Collaborator

@guicassolato guicassolato Aug 24, 2023

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll create a diff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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
Comment on lines 369 to 416
.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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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 run

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Makefile Outdated

.PHONY: local-redeploy
local-redeploy: export OPERATOR_IMAGE := authorino-operator:dev
local-redeploy:
Copy link
Collaborator

Choose a reason for hiding this comment

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

For consistency with how Authorino calls it:

Suggested change
local-redeploy:
local-rollout:

@guicassolato
Copy link
Collaborator

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 authorino-operator.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Possibility to deploy the operator in an arbitrary namespace
3 participants