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

batch of fixes and enhancements - early january'2025 #257

Merged
merged 12 commits into from
Jan 13, 2025

Conversation

rpardini
Copy link
Contributor

@rpardini rpardini commented Jan 12, 2025

further enhancements:

  • can now build (and develop) Hook for any arch under any arch, including for amd64 under Darwin arm64
  • full support for building on macOS+brew, including on arm64
  • added shellfmt tool and bash code format enforcing on CI (in addition to shellcheck)
  • avoid pulling skopeo image over and over again

build: implement shellfmt (and lint which does both shellcheck/fmt)

  • for consistent bash formatting
  • include an .editorconfig for IDE's

gha: switch to lint (which does both shellcheck and shellfmt)

linuxkit: bump 1.5.2 -> 1.5.3

build: implement build-host dependency handling for macOS+brew

  • if on macOS+brew:
    • detects missing deps and installs them with brew
    • exports PATH with brew-based GNU versions first
      • coreutils, gnu-sed and gnu-tar included

build: Dockerfile: fix FROM xx AS casing

  • to squash recent BuildKit warnings

build: pass --verbose 2 to linuxkit if DEBUG=yes

  • can help with some edge cases

build: refactor skopeo pull and list-tags functions

  • this only affects Armbian kernel flavours
  • avoids pulling if found in local cache

build: use skopeo v1.17.0 instead of latest

  • since we now use the local tag

build: armbian: kernel: refactor Dockerfile with helper

  • building the Armbian kernels would produce different hashes depending on the arch of the host
  • moving the affected code into the Dockerfile would lead to escaping pain; instead implement a docker.sh helper
  • in practice, all code in the Dockerfile is hashed, but the arch decision is now therein and hash won't change
  • also, allows for reuse, which is bound to come later

build: docker: detect & export DOCKER_HOST from current docker context

build: kernel: force target arch on cross-built kernel docker image manifest

  • our kernel builds are done in arch-independent Dockerfiles
  • but those get the build-host's architecture, despite the contents being correct
  • when locally developing on a kernel that is != host-arch
    • those get the host-arch in the image
    • but LinuxKit refuses to use it due to arch mismatch
  • (when pushed to a registry, the arch info is discarded, and LK is ok with that)
  • thus
    • introduce ensure_docker_image_architecture(imagetag, arch)
      • which just hacks at manifests via a docker save/docker load
    • call it from both default and armbian kernel builds

build: docker: avoid Docker Inc's "What's next" hints

  • enough spam already, thanks

Signed-off-by: Ricardo Pardini [email protected]

@rpardini rpardini force-pushed the fixes-batch-early-jan-2025 branch from 00ab717 to 3853872 Compare January 12, 2025 00:12
- for consistent bash formatting
- include an .editorconfig for IDE's

Signed-off-by: Ricardo Pardini <[email protected]>
Signed-off-by: Ricardo Pardini <[email protected]>
- if on macOS+brew:
  - detects missing deps and installs them with brew
  - exports PATH with brew-based GNU versions first
    - coreutils, gnu-sed and gnu-tar included

Signed-off-by: Ricardo Pardini <[email protected]>
- to squash recent BuildKit warnings

Signed-off-by: Ricardo Pardini <[email protected]>
- can help with some edge cases

Signed-off-by: Ricardo Pardini <[email protected]>
- this only affects Armbian kernel flavours
- avoids pulling if found in local cache

Signed-off-by: Ricardo Pardini <[email protected]>
- since we now use the local tag

Signed-off-by: Ricardo Pardini <[email protected]>
- building the Armbian kernels would produce different hashes depending on the arch of the host
- moving the affected code into the Dockerfile would lead to escaping pain; instead implement a docker.sh helper
- in practice, all code in the Dockerfile is hashed, but the arch decision is now therein and hash won't change
- also, allows for reuse, which is bound to come later

Signed-off-by: Ricardo Pardini <[email protected]>
…text`

- Some Docker-in-VM solutions (like Docker Desktop, colima, etc) set a non-default docker context pointing to the correct socket
- Seems LinuxKit fumbles detecting this and ends up silently failing all local-Docker-daemon cache hits
  - that is fine for CI, where all images are (beforehand) pushed to the registry (and thus LK ends up pulling from remote), but not during local development
  - reported to upstream LinuxKit: linuxkit/linuxkit#4092

Signed-off-by: Ricardo Pardini <[email protected]>
…anifest

- our kernel builds are done in arch-independent Dockerfiles
- but those get the build-host's architecture, despite the contents being correct
- when locally developing on a kernel that is != host-arch
  - those get the host-arch in the image
  - but LinuxKit refuses to use it due to arch mismatch
- (when pushed to a registry, the arch info is discarded, and LK is ok with that)
- thus
  - introduce `ensure_docker_image_architecture(imagetag, arch)`
    - which just hacks at manifests via a docker save/docker load
  - call it from both default and armbian kernel builds

Signed-off-by: Ricardo Pardini <[email protected]>
- enough spam already, thanks

Signed-off-by: Ricardo Pardini <[email protected]>
@rpardini rpardini force-pushed the fixes-batch-early-jan-2025 branch from 3853872 to 4356a41 Compare January 12, 2025 00:13
@jacobweinstock jacobweinstock added the ready-to-merge Signal to Mergify to merge the PR. label Jan 13, 2025
@mergify mergify bot merged commit ab0689c into tinkerbell:main Jan 13, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Signal to Mergify to merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants