SANDBOX-1561 | feature: Makefile targets to build the operator in debug mode#721
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: MikelAlejoBR The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| $(Q)CGO_ENABLED=0 GOARCH=${goarch} GOOS=linux \ | ||
| go build ${V_FLAG} \ | ||
| -gcflags "all=-N -l" \ | ||
| -ldflags "-X ${GO_PACKAGE_PATH}/version.Commit=${GIT_COMMIT_ID} -X ${GO_PACKAGE_PATH}/version.BuildTime=${BUILD_TIME}" \ | ||
| -o $(OUT_DIR)/bin/member-operator-webhook \ | ||
| cmd/webhook/main.go |
There was a problem hiding this comment.
to make it work for the webhook, we would need to update also the template, right?
member-operator/deploy/templates/webhook/member-operator-webhook.yaml
Lines 114 to 115 in 50a1032
MikelAlejoBR
force-pushed
the
SANDBOX-1561-debug-sandbox-resources-member-op
branch
from
cde5155 to
c2dcb57
Compare
WalkthroughAdds configurable multi-stage Docker builds with default Changes
sequenceDiagram
participant Dev as Developer
participant Make as Make
participant Builder as ContainerBuilder
participant GoFetch as debug-initial-build
participant Prod as prod-build
participant Debug as debug-build
participant Final as final-build
Dev->>Make: run podman-image-debug / build-debug
Make->>Builder: docker build (BUILD_TYPE=debug, GOLANG_VERSION, TARGET_ARCH)
Builder->>GoFetch: download Go, extract, build dlv -> /tmp/bin/dlv
Builder->>Prod: build application artifacts in prod-build stage
Prod->>Debug: act as base for debug-build
Debug->>Builder: COPY /tmp/bin/dlv into image
Builder->>Final: FROM ${BUILD_TYPE}-build AS final-build
Final->>Dev: produce final image (debug or prod)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
MikelAlejoBR
force-pushed
the
SANDBOX-1561-debug-sandbox-resources-member-op
branch
from
c2dcb57 to
a719e04
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@build/Dockerfile`:
- Around line 8-9: The Dockerfile LABEL entries use invalid spacing around the
equals sign; update the LABEL statements (specifically the LABEL maintainer and
LABEL author tokens) to use key=value syntax without spaces (e.g.,
maintainer="KubeSaw <devsandbox@redhat.com>") or combine into a single LABEL
instruction, so the Dockerfile parses correctly.
In `@build/Dockerfile.webhook`:
- Around line 8-9: The Dockerfile LABEL entries use spaces around '=' which
breaks Docker's key=value token parsing; update the LABEL directives (the two
lines containing LABEL maintainer and LABEL author) to use key=value format with
no spaces around '=' (e.g., LABEL maintainer="KubeSaw <devsandbox@redhat.com>")
so the Dockerfile parses correctly.
🧹 Nitpick comments (3)
build/Dockerfile (1)
31-46: Fail fast whenGOLANG_VERSIONis missing in debug builds.If
BUILD_TYPE=debugis used withoutGOLANG_VERSION, the download URL becomes invalid and fails with a confusing error. A small guard makes the failure explicit.♻️ Suggested guard
ARG GOLANG_VERSION +RUN test -n "${GOLANG_VERSION}" || (echo "GOLANG_VERSION must be set when BUILD_TYPE=debug" >&2; exit 1)build/Dockerfile.webhook (1)
31-46: Fail fast whenGOLANG_VERSIONis missing in debug builds.If
BUILD_TYPE=debugis used withoutGOLANG_VERSION, the download URL becomes invalid and fails with a confusing error. A small guard makes the failure explicit.♻️ Suggested guard
ARG GOLANG_VERSION +RUN test -n "${GOLANG_VERSION}" || (echo "GOLANG_VERSION must be set when BUILD_TYPE=debug" >&2; exit 1)make/podman.mk (1)
15-31: Avoid pushing debug images under the prod tag.
podman-image-debugandpodman-push-debuguse the same tags as prod, so a debug push can overwrite release images. Consider a-debugsuffix (or a dedicated tag variable) for safety.♻️ Suggested update (suffix debug tag)
podman-image-debug: build-debug - $(Q) podman build --platform "${IMAGE_PLATFORM}" --build-arg BUILD_TYPE='debug' --build-arg GOLANG_VERSION="$$(go version | awk '{print $$3}')" --file build/Dockerfile --tag "${IMAGE}" . - $(Q) podman build --platform "${IMAGE_PLATFORM}" --build-arg BUILD_TYPE='debug' --build-arg GOLANG_VERSION="$$(go version | awk '{print $$3}')" --file build/Dockerfile.webhook --tag "${WEBHOOK_IMAGE}" . + $(Q) podman build --platform "${IMAGE_PLATFORM}" --build-arg BUILD_TYPE='debug' --build-arg GOLANG_VERSION="$$(go version | awk '{print $$3}')" --file build/Dockerfile --tag "${IMAGE}-debug" . + $(Q) podman build --platform "${IMAGE_PLATFORM}" --build-arg BUILD_TYPE='debug' --build-arg GOLANG_VERSION="$$(go version | awk '{print $$3}')" --file build/Dockerfile.webhook --tag "${WEBHOOK_IMAGE}-debug" . podman-push-debug: check-namespace podman-image-debug - $(Q)podman push ${IMAGE} - $(Q)podman push ${WEBHOOK_IMAGE} + $(Q)podman push ${IMAGE}-debug + $(Q)podman push ${WEBHOOK_IMAGE}-debug
MikelAlejoBR
force-pushed
the
SANDBOX-1561-debug-sandbox-resources-member-op
branch
2 times, most recently
from
f4a8dd1 to
26f224d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@build/Dockerfile`:
- Around line 31-46: The Dockerfile currently tries to download Go using ARG
GOLANG_VERSION without checking it; add a guard that fails fast with a clear
error when BUILD_TYPE is "debug" and GOLANG_VERSION is empty. Modify the build
steps around ARG GOLANG_VERSION and the curl/tar lines to perform a pre-check
(using BUILD_TYPE and GOLANG_VERSION) and exit with a descriptive message if
GOLANG_VERSION is not provided, so the failing curl in the existing RUN (the
curl --location ... "/tmp/${GOLANG_VERSION}...") never executes with an empty
version.
In `@build/Dockerfile.webhook`:
- Around line 31-46: Add a fail-fast guard to ensure the build ARG
GOLANG_VERSION is set before attempting the download: check the ARG
GOLANG_VERSION (declared as ARG GOLANG_VERSION) and if empty emit a clear error
and exit early rather than letting the subsequent RUN that performs curl/tar
(the RUN that downloads "/tmp/${GOLANG_VERSION}.linux-amd64.tar.gz") hit a 404;
place this guard immediately before the download RUN so builds using
BUILD_TYPE=debug without GOLANG_VERSION fail with a concise explanatory message.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@make/podman.mk`:
- Around line 15-19: The Makefile target podman-image-debug is missing a .PHONY
declaration; add podman-image-debug to the .PHONY list (or create a new `.PHONY:
podman-image-debug`) so make always runs the podman-image-debug recipe and
downstream targets like podman-push-debug that depend on podman-image-debug
behave correctly; update the existing .PHONY declaration that contains
podman-image, podman-push, and podman-push-debug to include podman-image-debug
or add a separate .PHONY line for podman-image-debug.
♻️ Duplicate comments (2)
build/Dockerfile.webhook (1)
31-46: Fail fast when GOLANG_VERSION is missing.Without a guard, a debug build can hit a confusing 404 on the Go tarball URL. Add an explicit check right after
ARG GOLANG_VERSION.🐛 Proposed guard
ARG GOLANG_VERSION +RUN test -n "$GOLANG_VERSION" || (echo "GOLANG_VERSION is required (e.g., go1.22.1)" >&2; exit 1)#!/bin/bash # Find all GOLANG_VERSION references and where it’s set. rg -n "GOLANG_VERSION" -g 'build/Dockerfile*' -g '*.mk'build/Dockerfile (1)
31-46: Fail fast when GOLANG_VERSION is missing.A missing
GOLANG_VERSIONleads to a cryptic curl failure. Add a guard right after the ARG.🐛 Proposed guard
ARG GOLANG_VERSION +RUN test -n "$GOLANG_VERSION" || (echo "GOLANG_VERSION is required (e.g., go1.22.1)" >&2; exit 1)#!/bin/bash # Find all GOLANG_VERSION references and where it’s set. rg -n "GOLANG_VERSION" -g 'build/Dockerfile*' -g '*.mk'
🧹 Nitpick comments (1)
make/podman.mk (1)
27-31: Consider distinct tags for debug images to avoid overwriting prod.
podman-push-debugcurrently pushes${IMAGE}/${WEBHOOK_IMAGE}(same tag as prod by default). A separate debug tag reduces the risk of accidentally replacing prod artifacts.♻️ Example refactor
IMAGE_TAG ?= ${GIT_COMMIT_ID_SHORT} IMAGE ?= ${TARGET_REGISTRY}/${QUAY_NAMESPACE}/${GO_PACKAGE_REPO_NAME}:${IMAGE_TAG} WEBHOOK_IMAGE ?= ${TARGET_REGISTRY}/${QUAY_NAMESPACE}/${GO_PACKAGE_REPO_NAME}-webhook:${IMAGE_TAG} +DEBUG_IMAGE_TAG ?= ${IMAGE_TAG}-debug +DEBUG_IMAGE ?= ${TARGET_REGISTRY}/${QUAY_NAMESPACE}/${GO_PACKAGE_REPO_NAME}:${DEBUG_IMAGE_TAG} +DEBUG_WEBHOOK_IMAGE ?= ${TARGET_REGISTRY}/${QUAY_NAMESPACE}/${GO_PACKAGE_REPO_NAME}-webhook:${DEBUG_IMAGE_TAG} @@ -podman-image-debug: build-debug - $(Q) podman build --platform "${IMAGE_PLATFORM}" --build-arg BUILD_TYPE='debug' --build-arg GOLANG_VERSION="$$(go version | awk '{print $$3}')" --file build/Dockerfile --tag "${IMAGE}" . - $(Q) podman build --platform "${IMAGE_PLATFORM}" --build-arg BUILD_TYPE='debug' --build-arg GOLANG_VERSION="$$(go version | awk '{print $$3}')" --file build/Dockerfile.webhook --tag "${WEBHOOK_IMAGE}" . +podman-image-debug: build-debug + $(Q) podman build --platform "${IMAGE_PLATFORM}" --build-arg BUILD_TYPE='debug' --build-arg GOLANG_VERSION="$$(go version | awk '{print $$3}')" --file build/Dockerfile --tag "${DEBUG_IMAGE}" . + $(Q) podman build --platform "${IMAGE_PLATFORM}" --build-arg BUILD_TYPE='debug' --build-arg GOLANG_VERSION="$$(go version | awk '{print $$3}')" --file build/Dockerfile.webhook --tag "${DEBUG_WEBHOOK_IMAGE}" . @@ -podman-push-debug: check-namespace podman-image-debug - $(Q)podman push ${IMAGE} - $(Q)podman push ${WEBHOOK_IMAGE} +podman-push-debug: check-namespace podman-image-debug + $(Q)podman push ${DEBUG_IMAGE} + $(Q)podman push ${DEBUG_WEBHOOK_IMAGE}
MikelAlejoBR
force-pushed
the
SANDBOX-1561-debug-sandbox-resources-member-op
branch
5 times, most recently
from
80c8490 to
4132aeb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@build/Dockerfile`:
- Around line 37-43: The RUN instruction that installs tools ends with a
trailing line-continuation after "microdnf clean all" which folds subsequent
Dockerfile directives into the same RUN; edit the RUN block containing "microdnf
install --assumeyes \ curl \ gzip \ tar \ && microdnf clean all \" to remove the
final backslash so the "microdnf clean all" is the end of the RUN command (no
trailing "\") and subsequent Dockerfile directives remain separate.
- Around line 62-64: Modify the entrypoint script so it detects debug mode
(e.g., BUILD_TYPE=debug or a DEBUG env var) and, when enabled, runs the operator
under Delve instead of executing the binary directly; specifically update the
script that currently runs ${OPERATOR} to conditionally run "dlv exec
${OPERATOR} --" (or appropriate dlv flags) when dlv is present/BUILD_TYPE=debug,
otherwise keep the existing normal execution path, and ensure PATH or
/usr/local/bin/dlv availability is considered.
🧹 Nitpick comments (1)
make/podman.mk (1)
8-8: KeepTARGET_ARCHaligned withIMAGE_PLATFORM.If
IMAGE_PLATFORMis overridden (e.g.,linux/arm64) butTARGET_ARCHstays atlinux-amd64, the debug build will download the wrong Go toolchain and fail. Consider deriving the default fromIMAGE_PLATFORMso they stay in sync.♻️ Suggested tweak
- TARGET_ARCH ?= linux-amd64 + # Default to a matching Go archive target; override if you need a custom mapping. + TARGET_ARCH ?= $(subst /,-,$(IMAGE_PLATFORM))
MikelAlejoBR
force-pushed
the
SANDBOX-1561-debug-sandbox-resources-member-op
branch
from
4132aeb to
068a64b
Compare
In order to be able to debug the operator live in the local cluster, we need the binary to be executed with Delve instead, and we need it to be built without any code optimizations or minimization so that the breakpoints work. For that purpose, new Dockerfiles are added which can build the images that way, and the corresponding Make targets make it easy to kick off the whole process with a single command. The Makefile targets also enable the "toolchain-e2e" repository to easily build the "debug" images. SANDBOX-1561
MikelAlejoBR
force-pushed
the
SANDBOX-1561-debug-sandbox-resources-member-op
branch
from
068a64b to
b21b2bd
Compare
|
2 participants
In order to be able to debug the operator live in the local cluster, we need the binary to be executed with Delve instead, and we need it to be built without any code optimizations or minimization so that the breakpoints work.
For that purpose, new Dockerfiles are added which can build the images that way, and the corresponding Make targets make it easy to kick off the whole process with a single command.
The Makefile targets also enable the "toolchain-e2e" repository to easily build the "debug" images.
Related pull requetss
Jira ticket
[SANDBOX-1561]
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.