Refactor `Makefile` and GH workflows by afritzler · Pull Request #7 · ironcore-dev/network-operator

afritzler · 2025-07-11T08:27:28Z

Refactor Makefile to a kubebuilder style format
Adapt GH workflows
Add dependabot configuration

Fixes #6

afritzler · 2025-07-11T08:31:47Z

Ignore the api-ref docs gen for now as we have no api package yet.

- Refactor `Makefile` to a kubebuilder style format - Adapt GH workflows - Add dependabot configuration

felix-kaestner · 2025-07-11T10:06:09Z

.github/ISSUE_TEMPLATE/bug_report.md

+**Screenshots**
+If applicable, add screenshots to help explain your problem.
+


I'm having a hard time to understand how screenshots would actually be a good way to explain a problem with a k8s operator. I guess this is just the template copied from somewhere?

This just some dummy template which needs to be adjusted :)

felix-kaestner · 2025-07-11T10:09:49Z

.github/dependabot.yml

While I don't have a strong opinion here, Renovate is actually favored in all https://github.com/SAP-cloud-infrastructure / https://github.com/sapcc so let's decide on this in a bigger round.

Me neither. We haven't enabled/used renovate in the ironcore-dev org yet. I guess in the POC state of the project this part is not really a show stopper and can be fixed later.

felix-kaestner · 2025-07-11T10:11:46Z

.github/release-drafter.yml

Any reason to not go with https://goreleaser.com?

Just to provide some background, why imo GoReleaser would be preffered:

Greater adoption throughout the go/k8s ecosystem, incl. usage by kubebuilder (see here), who’s Makefile style we are adapting here.

GoReleaser also appears to have a more active development:

GoReleaser:

Last Commit: Yesterday

Open Issues: 17

Release Drafter:

Last Commit: 6 Months ago

Open Issues: 142

On a side note, the GoReleaser Project is currently actively sponsored by SAP –https://github.com/sponsors/caarlos0#sponsors.

I would be fine to switch to go releaser to generate the release notes. Let me revert that back.

felix-kaestner · 2025-07-11T10:16:00Z

.github/workflows/test-chart.yml

+      - name: Verify kind installation
+        run: kind version
+
+      - name: Create kind cluster


What speaks against using the helm/kind-action@v1 action as used previously?

felix-kaestner · 2025-07-11T10:17:19Z

.github/workflows/test-e2e.yml

+        run: kind version
+
+      - name: Create kind cluster
+        run: kind create cluster


Same as above

felix-kaestner · 2025-07-11T10:37:14Z

Makefile

+
+.PHONY: docker-build-controller-manager
+docker-build-controller-manager: ## Build controller-manager.
+	docker build --target manager -t ${CONTROLLER_IMG} .


We are missing the BININFO* Build Arguments here. Same applies to other tasks that build the docker image.

felix-kaestner · 2025-07-11T10:43:36Z

Dockerfile

+# Copy the go source
+COPY cmd/manager/main.go cmd/manager/main.go
+#COPY api/ api/
+COPY internal/ internal/
+
+# Build
+# the GOARCH has not a default value to allow the binary be built according to the host where the command
+# was called. For example, if we call make docker-build in a local env which has the Apple Silicon M1 SO
+# the docker BUILDPLATFORM arg will be linux/arm64 when for Apple x86 it will be linux/amd64. Therefore,
+# by leaving it empty we can ensure that the container and binary shipped on it will have the same platform.
+FROM builder AS manager-builder
+RUN --mount=type=cache,target=/root/.cache/go-build \
+    --mount=type=cache,target=/go/pkg \


Why copy the source files instead of bind-mounting them as did previously. Ref: https://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guide/

felix-kaestner · 2025-07-11T10:45:20Z

Dockerfile

+FROM builder AS manager-builder
+RUN --mount=type=cache,target=/root/.cache/go-build \
+    --mount=type=cache,target=/go/pkg \
+    CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -a -o manager cmd/manager/main.go


We are missing -ldflags '-s -w -X github.com/sapcc/go-api-declarations/bininfo.binName=network-operator -X github.com/sapcc/go-api-declarations/bininfo.version=$(BININFO_VERSION) -X github.com/sapcc/go-api-declarations/bininfo.commit=$(BININFO_COMMIT_HASH) -X github.com/sapcc/go-api-declarations/bininfo.buildDate=$(BININFO_BUILD_DATE) $(GO_LDFLAGS)' here.

Also I'm in favor of striping debug symbols from the binary.

felix-kaestner · 2025-07-11T11:00:32Z

dist/chart/.helmignore

We already have the helm chart under charts/network-operator which seems to be inline with the standard convention used by many other projects.

I'm aware that kubebuilder decided for dist/chart but I don't think that's a good choice. There also seems to be a related issue in kubernetes-sigs/kubebuilder#4320, which hopefully gets resolved soon (there are already open PRs for it) to make this configurable.

And to cite the official Helm Documentation:

The directory name is the name of the chart (without versioning information).

The idea is to have a kustomize first approach here and let the kubebuilder helm plugin do the rest. @SzymonSAP built in the support for the chart generation in e.g. the metal-operator.

My comment is only about the directory, where the plugin generates the chart into.
Using kubebuilder helm plugin has also been done previously to generate the helm chart in the charts/ directory. I'm just saying that this is already there and I would prefer to have it in that location. :)

felix-kaestner · 2025-07-11T11:09:59Z

Makefile

+docker-buildx: ## Build and push docker image for the manager for cross-platform support
+	# copy existing Dockerfile and insert --platform=${BUILDPLATFORM} into Dockerfile.cross, and preserve the original Dockerfile
+	sed -e '1 s/\(^FROM\)/FROM --platform=\$$\{BUILDPLATFORM\}/; t' -e ' 1,// s//FROM --platform=\$$\{BUILDPLATFORM\}/' Dockerfile > Dockerfile.cross
+	- $(CONTAINER_TOOL) buildx create --name network-operator-builder


Having CONTAINER_TOOL suggests that anything other than docker would actually be possible, while neither podman nor nerdctl have a buildx create/use command. So for me, this doesn't really make sense. I'm fine if we commit on docker, but then I would leave out the $CONTAINER_TOOL variable.

On another note, the sed call to insert --platform=${BUILDPLATFORM} is not needed, since the Dockerfile already contains that.

I assume that's what kubebuilder generates by default?

afritzler · 2025-08-20T11:42:04Z

Closing in favor of #33

afritzler requested review from a team as code owners July 11, 2025 08:27

afritzler force-pushed the chore/project-setup branch from cf79a59 to ef96513 Compare July 11, 2025 08:28

afritzler added the chore label Jul 11, 2025

afritzler force-pushed the chore/project-setup branch from ef96513 to f679bf5 Compare July 11, 2025 08:31

afritzler force-pushed the chore/project-setup branch 3 times, most recently from a6f57f3 to 00b1ea3 Compare July 11, 2025 09:13

afritzler added the ok-to-image label Jul 11, 2025

Refactor Makefile and GH workflows

8a61a74

- Refactor `Makefile` to a kubebuilder style format - Adapt GH workflows - Add dependabot configuration

afritzler force-pushed the chore/project-setup branch from 00b1ea3 to 8a61a74 Compare July 11, 2025 09:17

felix-kaestner requested changes Jul 11, 2025

View reviewed changes

hardikdr added this to Roadmap Jul 12, 2025

hardikdr added the area/metal-automation Automation processes within the Metal project. label Jul 15, 2025

felix-kaestner mentioned this pull request Jul 16, 2025

rfc: add Interface resource + controller and initial provider setup #2

Merged

afritzler closed this Aug 20, 2025

github-project-automation bot moved this to Done in Roadmap Aug 20, 2025

afritzler deleted the chore/project-setup branch August 20, 2025 11:42

felix-kaestner mentioned this pull request Aug 20, 2025

Refactor Makefile, GH workflows and other project housekeeping tasks #33

Closed

		Screenshots
		If applicable, add screenshots to help explain your problem.

Conversation

afritzler commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afritzler commented Jul 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felix-kaestner Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felix-kaestner Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felix-kaestner Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felix-kaestner Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

afritzler commented Aug 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

afritzler commented Jul 11, 2025 •

edited

Loading

felix-kaestner Jul 11, 2025 •

edited

Loading

felix-kaestner Jul 11, 2025 •

edited

Loading

felix-kaestner Jul 16, 2025 •

edited

Loading

felix-kaestner Jul 11, 2025 •

edited

Loading