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

Add Makefile cluster sync target #8

Merged
merged 3 commits into from
Mar 5, 2024

Conversation

ormergi
Copy link
Contributor

@ormergi ormergi commented Mar 4, 2024

Cluster sync build the controller and deploy it to the local cluster
enabling faster iterations.

Introduce 'kind-push' target, it enables pushing the controller image
to cluster nodes container runtime local registry using KinD.

cluster-sync flow:

  • Remove the installed CRDs, Deployment and all related object (namesapce, sa, rbac, etc..)
  • Generate manifest (CRD, RBAC, etc..).
  • Generates code (DeepCopy, etc..).
  • go fmt, go vet
  • Deploy the controller CRDs.
  • Build controller the image.
  • Push the image to cluster nodes container runtime local registry (using KinD).
  • Generate manifests (Namespace, SA, Deployment, etc.., using Kustomize)
  • Deploy controller and all related objects.

The default container image tag, represented by $IMG, is changed as follows to
make kind load work as expected:
'localhost/overlay-network-controller:devel'

@ormergi ormergi force-pushed the makefile-cluster-sync branch 2 times, most recently from 3e7b578 to 0eef43b Compare March 4, 2024 19:01
Since config/crd/kustomizeconfig.yaml not exist it should be removed from
config/crd/kustomization.yaml manifest.

Without this change 'make manifests' fails.

Signed-off-by: Or Mergi <[email protected]>
@ormergi ormergi force-pushed the makefile-cluster-sync branch 2 times, most recently from 790322b to 0bb3b61 Compare March 4, 2024 20:17
Makefile Outdated
@@ -46,8 +46,13 @@ ifeq ($(USE_IMAGE_DIGESTS), true)
BUNDLE_GEN_FLAGS += --use-image-digests
endif

IMAGE_REGISTRY ?= quay.io
IMAGE_ORG ?= omergi
Copy link
Owner

Choose a reason for hiding this comment

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

IMAGE_ORG ?= slefserviceoverlay

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 removed the image org for now and set the registry to localhost to make kind load work as expected (leaving the image with no org cause the node to fetch image from docker.io).

Let set the image org name and a remote regeistry once we setup local/remote registry.

@@ -261,3 +262,14 @@ cluster-up:
.PHONY: cluster-down
cluster-down:
./automation/cluster.sh --down

KIND_BIN ?= kind
CLUSTER_NAME ?= ovn
Copy link
Owner

Choose a reason for hiding this comment

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

Where do we specify it is the name of the cluster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cluster-up target uses ovn-org/ovn-kubernetes scripts to create cluster for tests, default name is "ovn".
I will post a follow up PR to make it visible in the Makefile.

Makefile Outdated
docker save ${IMG} -o ${IMAGE_TAR}
$(KIND_BIN) load image-archive --name=${CLUSTER_NAME} ${IMAGE_TAR}

.PHONY:
Copy link
Owner

Choose a reason for hiding this comment

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

.PHONY: cluster-sync

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Makefile Outdated
$(KIND_BIN) load image-archive --name=${CLUSTER_NAME} ${IMAGE_TAR}

.PHONY:
cluster-sync: undeploy uninstall test install docker-build kind-push deploy
Copy link
Owner

Choose a reason for hiding this comment

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

Why unit tests have to run on cluster-sync?

Copy link
Owner

Choose a reason for hiding this comment

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

Please make the build work with any oci bin.
OCI_BIN ?= $(shell if podman ps >/dev/null 2>&1; then echo podman; elif docker ps >/dev/null 2>&1; then echo docker; fi)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why unit tests have to run on cluster-sync?

My bad, no need to run unit tests as part of cluster-sync.
Changed to use generate fmt and vet instead on test.

We could use build target, but since docker-build already build the go binary we avoid building it twice by calling docker-build only.

Also, I had to change 'undelpoy' target to have `kustomize' target as a prerequisite to avoid failures when Kustomize is not installed yet.

Please make the build work with any oci bin. OCI_BIN ?= $(shell if podman ps >/dev/null 2>&1; then echo podman; elif docker ps >/dev/null 2>&1; then echo docker; fi)

I have posted another PR that introduce OCI_BIN #7

resources:
- manager.yaml
images:
- name: controller
newName: quay.io/omergi/overlay-network-controller
Copy link
Owner

Choose a reason for hiding this comment

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

Please change according to the new proposed image name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

ormergi added 2 commits March 5, 2024 17:13
Cluster sync build the controller and deploy it to the local cluster
enabling faster iterations.

Introduce 'kind-push' target, it enables pushing the controller image
to cluster nodes container runtime local registry using KinD.

cluster-sync flow:
- Remove the installed CRDs, Deployment and all related object (namesapce, sa, rbac, etc..)
- Generate manifest (CRD, RBAC, etc..).
- Generates code (DeepCopy, etc..).
- go fmt, go vet
- Build controller the image.
- Push the image to cluster nodes container runtime local registry (using kind).
- Deploy the controller CRDs.
- Generate manifests (Namespace, SA, Deployment, etc.., using Kustomize)
- Deploy controller and all related objects.

The default container image tag, represented by IMG, is changed as follows to
make kind load work as expected:
  'localhost/overlay-network-controller:devel'

Add 'kustomize' target as a prerequisite of 'undelpoy' target to avoid failures
when Kustomize is not installed.

Signed-off-by: Or Mergi <[email protected]>
This commit change introduced by 'make deploy'.
It replaces the default image name ("controller") with the image name spesified
in Makefile by $IMG.

Signed-off-by: Or Mergi <[email protected]>
@ormergi ormergi force-pushed the makefile-cluster-sync branch from 0bb3b61 to 88afbd6 Compare March 5, 2024 15:16
@AlonaKaplan
Copy link
Owner

The default container image tag, represented by $IMG, is changed as follows to make kind load work as expected: 'quay.io/omergi/overlay-network-controller:devel'

Please update the image.

@AlonaKaplan AlonaKaplan merged commit fc38133 into AlonaKaplan:main Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants