Skip to content

build: migrate from Dapper to single-Dockerfile build system#128

Merged
brandboat merged 8 commits into
harvester:masterfrom
bk201:wip-buildx
Jun 8, 2026
Merged

build: migrate from Dapper to single-Dockerfile build system#128
brandboat merged 8 commits into
harvester:masterfrom
bk201:wip-buildx

Conversation

@bk201

@bk201 bk201 commented May 19, 2026

Copy link
Copy Markdown
Contributor

Problem:

migrate from Dapper to single-Dockerfile build system

Solution:

  • Replace the Dapper-driven Makefile (download + run .dapper binary) with a
    docker build --target workflow.
  • One Dockerfile at the repo root defines a stage per target (build, test,
    validate, validate-ci, generate,
  • generate-manifest) with BuildKit cache mounts for the Go module and build
    caches.
  • Host-side targets (package, package-webhook) continue to run docker buildx
    build directly.
  • Orchestration scripts (ci, default, entry, release) are deleted and replaced
    by Makefile targets that sequence the individual stages.
  • scripts/version gains a git-availability guard and writes scripts/.version_env
    so container builds and git worktree checkouts

Related Issue(s):

harvester/harvester#10604

Test plan:

Additional documentation or context

Copilot AI review requested due to automatic review settings May 19, 2026 08:26
@bk201 bk201 requested review from a team, brandboat and tserong as code owners May 19, 2026 08:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the repository’s build workflow from a Dapper-driven model to a single root-level Dockerfile with per-target build stages, with Makefile targets orchestrating the workflow and enabling BuildKit caching.

Changes:

  • Replaced Dapper-based Make targets with docker build --target … stages (plus local --output extraction where needed).
  • Added a root Dockerfile implementing build/test/validate/generate stages with BuildKit cache mounts.
  • Updated scripts/version to support .git-less builds via a pre-generated scripts/.version_env, and removed Dapper entrypoint/orchestration scripts.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
scripts/version Adds .git-availability guard and writes/loads scripts/.version_env for container builds.
scripts/release Removed legacy Dapper release wrapper.
scripts/entry Removed legacy Dapper entrypoint.
scripts/default Removed legacy Dapper default orchestration.
scripts/ci Removed legacy Dapper CI orchestration.
Makefile New Dockerfile-stage driven build/test/validate/generate/package targets.
Dockerfile.dapper Removed Dapper build image definition.
Dockerfile New multi-stage build defining build/test/validate/generate targets and local outputs.
.gitignore Ignores generated scripts/.version_env.
.dockerignore Expands ignores (including .git) while keeping /bin available for packaging contexts.
Comments suppressed due to low confidence (1)

scripts/version:26

  • After verifying git availability with git -C "${TOP_DIR}" ..., the script runs subsequent git status, git rev-parse, and git tag commands without -C "${TOP_DIR}". If scripts/version is sourced/executed from outside the repo directory, those commands will operate on the caller’s CWD and can fail or produce wrong values even though ${TOP_DIR} is correct. Using git -C "${TOP_DIR}" consistently for all git commands would make the script location-independent.
if [ -n "$(git status --porcelain --untracked-files=no)" ]; then
    DIRTY="-dirty"
fi

COMMIT=$(git rev-parse --short HEAD)
GIT_TAG=$(git tag -l --contains HEAD | head -n 1)


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Makefile
DOCKER_BUILD = docker build \
--progress=$(MK_DOCKER_PROGRESS) \
--build-arg MK_REPO_ID \
--build-arg MK_HOST_ARCH \

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good finding, fix in Export MK_HOST_ARCH

Comment thread Makefile
Comment thread scripts/version
if [[ -f "${_VERSION_ENV}" ]]; then
# shellcheck source=/dev/null
source "${_VERSION_ENV}"
return 0 2>/dev/null || true

@brandboat brandboat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the patch!

Comment thread Makefile Outdated
Comment thread Dockerfile Outdated
@mergify

mergify Bot commented Jun 4, 2026

Copy link
Copy Markdown

This pull request is now in conflict. Could you fix it @bk201? 🙏

bk201 and others added 6 commits June 5, 2026 09:20
- Replace the Dapper-driven Makefile (download + run .dapper binary) with a
  docker build --target workflow.
- One Dockerfile at the repo root defines a stage per target (build, test,
  validate, validate-ci, generate,
- generate-manifest) with BuildKit cache mounts for the Go module and build
  caches.
- Host-side targets (package, package-webhook) continue to run docker buildx
  build directly.
- Orchestration scripts (ci, default, entry, release) are deleted and replaced
  by Makefile targets that sequence the individual stages.
- scripts/version gains a git-availability guard and writes scripts/.version_env
  so container builds and git worktree checkouts

Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
The variable is used in CI workflows to push images to a temp registry.

Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
Co-authored-by: Cooper Tseng <cooper.tseng@suse.com>
Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
Co-authored-by: Cooper Tseng <cooper.tseng@suse.com>
Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
Comment thread Dockerfile
# syntax=docker/dockerfile:1
# check=skip=InvalidDefaultArgInFrom

FROM registry.suse.com/bci/golang:1.26 AS builder

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rebased and adapt changes in 893054d

Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
@brandboat

Copy link
Copy Markdown
Member

CI failed because upgrade_nm.sh run source "${REPO_DIR}/scripts/version". In this patch scripts/version defines

TOP_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)

, which overrides the TOP_DIR defined in upgrade_nm.sh

Could you please help add this patch:
ed48bbc
to fix it? I've verified the CI passes with these changes, you can check the run: https://github.com/harvester/node-manager/actions/runs/27049637627/job/79848984345.

Signed-off-by: Cooper Tseng <cooper.tseng@suse.com>
@bk201

bk201 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

CI failed because upgrade_nm.sh run source "${REPO_DIR}/scripts/version". In this patch scripts/version defines

TOP_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)

, which overrides the TOP_DIR defined in upgrade_nm.sh

Could you please help add this patch: ed48bbc to fix it? I've verified the CI passes with these changes, you can check the run: https://github.com/harvester/node-manager/actions/runs/27049637627/job/79848984345.

Thank you, the commit is included.

@brandboat brandboat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@brandboat brandboat merged commit 859d530 into harvester:master Jun 8, 2026
7 checks passed
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.

3 participants