Skip to content

test: reviewdog/action-shellcheck on netboot-extension#112

Open
iav wants to merge 16 commits into
mainfrom
enh/netboot-extension
Open

test: reviewdog/action-shellcheck on netboot-extension#112
iav wants to merge 16 commits into
mainfrom
enh/netboot-extension

Conversation

@iav
Copy link
Copy Markdown
Owner

@iav iav commented May 11, 2026

Fork-internal PR to exercise the reviewdog workflow on a real feature branch. Not for merge.

Mirrors upstream PR armbian#9656 (enh/netboot-extension) from this fork's tree. After opening, dispatch the workflow from `audit/shellcheck-comparison`:

```
gh workflow run maintenance-lint-scripts-reviewdog.yml -R iav/armbian -f pr_number= --ref audit/shellcheck-comparison
```

Expected: reviewdog posts inline review-comments on shellcheck findings within the PR diff (only changed lines).

iav added 3 commits May 8, 2026 22:44
Make the network-boot u-boot template work for both arm64 (Image/booti)
and armv7 (zImage/bootz). Load kernel/initrd/dtb from the local boot
partition; mount root over NFS. Take console settings from DTB
`/chosen/stdout-path` instead of hardcoded baud — boards like helios64
(1500000) and others (115200) work without per-board overrides.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Igor Velkov <[email protected]>
Allow the rootfs stage to produce a compressed archive, an exported
directory tree, or both. Compression is configurable. When
`ROOTFS_COMPRESSION=none` is set without `ROOTFS_EXPORT_DIR`, fail
fast — there would be no rootfs artifact otherwise.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Igor Velkov <[email protected]>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b94cc3be0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread extensions/netboot/netboot.sh Outdated
Comment thread extensions/netboot/netboot-deploy.sh
@iav iav force-pushed the enh/netboot-extension branch from 8d3feb0 to 6dbb539 Compare May 11, 2026 16:20
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6dbb539fcd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread extensions/netboot/netboot-deploy.sh Outdated
Comment thread extensions/netboot/netboot-deploy.sh Outdated
@iav
Copy link
Copy Markdown
Owner Author

iav commented May 11, 2026

@chatgpt-codex-connector please re-review — two P2 items from the previous batch have been addressed:

  1. nfs-root defaults (netboot-deploy.sh:128/now 138): added a host_pre_docker_launch hook that injects ROOTFS_TYPE=nfs-root into ARMBIAN_CLI_FINAL_RELAUNCH_ARGS so the in-container do_main_configuration evaluates its case block with the implied type; the existing extension_prepare_config hook now also mirrors FIXED_IMAGE_SIZE=256 for the non-relaunch path.
  2. EXTRA_IMAGE_SUFFIXES (netboot.sh:122): switched from writing to the scalar EXTRA_IMAGE_SUFFIX to appending into EXTRA_IMAGE_SUFFIXES[], so do_main_configuration's rebuild+readonly does not silently drop the per-host suffix.

Head now at c31aeaeeb91e. The other two P2s (TFTP-order and amd64 kernel-name) are not yet addressed.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c31aeaeeb9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread extensions/netboot/netboot-deploy.sh Outdated
Comment thread extensions/netboot/netboot-deploy.sh Outdated
@iav iav force-pushed the enh/netboot-extension branch from c31aeae to 4b3c1d6 Compare May 11, 2026 17:17
@iav
Copy link
Copy Markdown
Owner Author

iav commented May 11, 2026

@chatgpt-codex-connector review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90da7bc81c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread extensions/netboot/netboot-deploy.sh Outdated
Comment thread extensions/netboot/netboot-deploy.sh Outdated
@iav
Copy link
Copy Markdown
Owner Author

iav commented May 11, 2026

@chatgpt-codex-connector review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d5d2c56e2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread extensions/netboot/netboot-deploy.sh
@iav
Copy link
Copy Markdown
Owner Author

iav commented May 11, 2026

@chatgpt-codex-connector review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 01cba07b4a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread extensions/netboot/netboot-deploy.sh
iav and others added 9 commits May 11, 2026 23:41
Add opt-in netboot extension for diskless U-Boot PXE/NFS boot. Generates
a TFTP tree (kernel + DTB + uInitrd + per-board pxelinux.cfg with
extlinux APPEND for NFS root) alongside or instead of a flashable image.
Supports per-host MAC-tagged configs, builder-as-NFS-server via
ROOTFS_EXPORT_DIR, ROOTSERVER discovery from DHCP siaddr in initramfs,
and a `netboot_artifacts_ready` post-hook for deploy automation.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Igor Velkov <[email protected]>
New rootfs type for full network boot: the only thing on the device's
local storage is U-Boot itself. Kernel, DTB, optional uInitrd and PXE
config come from TFTP; rootfs is mounted over NFS.

A new case branch in do_main_configuration auto-enables the netboot
extension, symmetric with existing fs-f2fs-support / fs-btrfs wiring.
The legacy ROOTFS_TYPE=nfs (hybrid: kernel on local storage, only /
over NFS) is untouched — both paths coexist.

- nfs-root case branch in ROOTFS_TYPE dispatch calls
  enable_extension "netboot"
- prepare_partitions skips root partition creation and SD-size sanity
  check
- check_filesystem_compatibility_on_host skipped for nfs-root
- create_image_from_sdcard_rootfs early-returns for nfs-root after the
  pre_umount hook has grabbed TFTP artifacts from /boot: SDCARD.raw is
  dropped, the .img pipeline (mv to DESTIMG, write-to-SD, fingerprint,
  compress) is skipped. For nfs-root the only deliverables are the
  rootfs archive / ROOTFS_EXPORT_DIR tree and the TFTP staging dir —
  producing a boot-partition .img would be misleading (nothing on the
  device reads it).

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Signed-off-by: Igor Velkov <[email protected]>
Documents the netboot extension: artifact server setup (tftpd-hpa +
nfs-kernel-server), TFTP tree layout, DHCP options 66/67 on the
network DHCP server, userpatches.conf knobs, the
netboot_artifacts_ready hook, a full end-to-end helios64 walkthrough,
and a troubleshooting section.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Signed-off-by: Igor Velkov <[email protected]>
In the rootfs cache and image pipelines, switch tar/rsync calls to:

  rsync -aHWh -XAXS --numeric-ids ...
  tar cp --xattrs --xattrs-include='security.*' --acls --selinux --sparse ...

Without this, xattrs (notably `security.capability` for setcap'd binaries
like /usr/bin/ping) and POSIX ACLs were stripped during the rootfs →
tarball → image (or → NFS export) round-trip.

The tar xattr include pattern defaults to `security.*` (file capabilities
and SELinux contexts) — broader patterns also pick up source-fs internals
(`bcachefs_*`, `btrfs.*`, `zfs.*` from the build host) that have no
meaning on the target and produce extract-time errors. Tunable via:

  - `ROOTFS_TAR_XATTR_INCLUDE` (env, replaces default include pattern)
  - `ROOTFS_TAR_EXTRA_FLAGS` (bash array, appended to tar args)
  - `ROOTFS_RSYNC_XATTR_FLAGS` (env, replaces rsync xattr/ACL flags)
  - `pre_create_rootfs_archive` extension hook (set the above lazily,
    e.g. depending on ARCH/RELEASE)

Drive-by: scope the `post_create_rootfs_archive` hook dispatch to
`ROOTFS_TYPE=nfs|nfs-root` (the only types that produce an archive)
and add `zst` to the `expected:` list in the unknown-compression
error message.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Igor Velkov <[email protected]>
Reference implementation of the `netboot_artifacts_ready` post-hook:
rsyncs the TFTP tree and unpacks the rootfs archive into an NFS export
on a remote server over SSH.

  - Setting `NETBOOT_DEPLOY_SSH=user@host` implies `ROOTFS_TYPE=nfs-root`
    so the operator does not have to repeat it on the command line.
  - An early probe hook (`extension_prepare_config__060_…`) tries
    `touch && rm` on the target TFTP root before the long build, so a
    misconfigured SSH key / sudo / known_hosts fails fast with a clear
    diagnostic instead of after `./compile.sh` has run for 40 minutes.
  - Host identity is explicit: `known_hosts` file by default
    (`NETBOOT_DEPLOY_SSH_KNOWN_HOSTS`), optional TOFU
    (`NETBOOT_DEPLOY_SSH_TOFU=yes`) for first-time deployments.
  - SSH key passed via `NETBOOT_DEPLOY_SSH_KEY`; `sudo` on the
    target is opt-in via `NETBOOT_DEPLOY_SUDO=yes`.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Igor Velkov <[email protected]>
Add troubleshooting entries for the most common netboot failure modes:
'NFS over TCP not available from <gateway>' (DHCP boot-server / siaddr
unset or wrong) and the corresponding dnsmasq/OpenWRT 'dhcp-boot' fix.
Show how to verify with /proc/net/pnp on a booted client.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Igor Velkov <[email protected]>
Add a 10-minute mount-stall safety net via two initramfs hooks:

  - init-premount/zz-netboot-watchdog forks a background shell that
    sleeps 600 s and then triggers an immediate reboot via
    `echo b > /proc/sysrq-trigger` if the NFS root mount has not
    completed by then.
  - nfs-bottom/zz-netboot-watchdog-cancel kills that background
    shell after a successful mount.

Active only for ROOTFS_TYPE=nfs-root. Without the watchdog, a
misconfigured server or transient network failure would hang the target
in initramfs forever; with it, the board self-recovers and tries again.

Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Igor Velkov <[email protected]>
…uid_owner

netboot extension exports an unpacked rootfs tree under output/netboot-export/<suffix>/
and reset_uid_owner runs over it. Debian/Ubuntu rootfs trees routinely contain
dangling symlinks (e.g. /etc/systemd/system/multi-user.target.wants/ entries
referring to services from packages that aren't installed). GNU chown without -h
follows the symlink and fails on a missing target with 'cannot dereference', so
post-docker cleanup returns exit 2 even though the build itself succeeded.

chown -h sets the owner of the symlink inode itself rather than its target; it
is a no-op for regular files and directories. This is the semantically correct
choice when walking a filesystem tree, regardless of the netboot use case.

Surfaced by armbian#9656 (netboot extension); applies to any extension
that performs reset_uid_owner over a real rootfs tree on host.

Signed-off-by: Igor Velkov <[email protected]>
Whitespace-only fixes picked up by shfmt — comment column alignment in
configuration/main-config.sh, tab indent on a stray line, and a stripped
trailing blank line in rootfs/rootfs-create.sh. No behavioural change.

Signed-off-by: Igor Velkov <[email protected]>
@iav iav force-pushed the enh/netboot-extension branch from 01cba07 to f9e363c Compare May 11, 2026 20:48
@iav
Copy link
Copy Markdown
Owner Author

iav commented May 11, 2026

@chatgpt-codex-connector review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f9e363c41d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread extensions/netboot/netboot-deploy.sh Outdated
Comment thread lib/functions/image/rootfs-to-image.sh
iav added 3 commits May 12, 2026 03:40
…fore probe (codex P2)

The deploy probe at `extension_prepare_config__060_netboot_deploy_probe_target`
was passing ${NETBOOT_DEPLOY_SSH_KEY} directly to ssh via `-i`, but
that path is the host-bind-mounted file owned by the host user. OpenSSH
refuses identity files whose owner is neither root nor the current
user, so probe fails with 'Bad owner' even on CI/batch credentials
that would work fine at deploy time — the deploy hook already copies
the key to a root-owned scratch path (lines 354-369). Mirror the
deploy's scratch-key handling in the probe so the same configuration
exercises the same auth chain at both phases.

Cleanup runs right after the probe completes (no subshell EXIT trap
here — the function lives in the caller's scope, a trap would leak).

Reported by chatgpt-codex on PR armbian#9656.

Assisted-by: Claude:claude-opus-4.7
`declare exclude_home="--exclude=\"/home/*\""` evaluates to a single
string `--exclude="/home/*"` — the inner quotes are part of the
value, not shell syntax. Bash does not re-parse quotes stored in
variables, so when expanded unquoted on the rsync command line rsync
receives `--exclude="/home/*"` as the pattern (with literal quote
characters). /home then matches nothing, gets copied through, and
`--delete-excluded` cannot purge stale home directories.

Switch to an array of plain arguments: `exclude_home=(--exclude=/home/*)`.
The catch-all bash quoting around the value goes away, the single
glob char is fine inside a single-element array literal because the
array is bound before any pathname expansion would happen, and
`"${exclude_home[@]}"` on the rsync command line expands to zero
arguments when ${INCLUDE_HOME_DIR}=yes (array empty) or one argument
`--exclude=/home/*` otherwise. Both call sites (lines 61 and 203)
updated.

Pre-existing since c45a63e (2023-11-21), surfaced by chatgpt-codex
review on PR armbian#9656 while netboot work touched the surrounding
function.

Assisted-by: Claude:claude-opus-4.7
…d_logged (coderabbit)

CodeRabbit noted that `run_host_command_logged` rebuilds its argv via
`bash -c "$*"`, so an array passed as "${exclude_home[@]}" loses
its quoting at the bash -c re-parse step and the pattern's '*' becomes
a glob candidate again.

Switch to a single-quoted string carried inside the variable's value:
`exclude_home="'--exclude=/home/*'"`. Used $exclude_home unquoted
at the call sites:
- Empty case (INCLUDE_HOME_DIR=yes): variable is empty → expansion is
  zero args → rsync sees nothing.
- Non-empty case: expansion is one word `'--exclude=/home/*'` whose
  embedded single-quotes survive bash -c re-parse and hide the '*'
  from globbing, so rsync gets the literal pattern.

Cleaner than ${array[*]@q} which would emit a literal `''` (two-char
quoted-empty) on the empty-array path and pass an empty arg to rsync.

Reported by coderabbit on PR armbian#9656 review of 7aa19b8.

Assisted-by: Claude:claude-opus-4.7
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9446841788

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +716 to +718
run_host_command_logged_raw rsync "${rsync_base[@]}" \
"${kernel_src}" \
"${NETBOOT_DEPLOY_SSH}:${NETBOOT_DEPLOY_TFTP_ROOT}/${NETBOOT_TFTP_PREFIX}/${kernel_name}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Publish kernel only after modules update succeeds

In standalone compile.sh kernel deploys, this pushes the new TFTP kernel before the later /lib/modules/${kver}/ rsync and remote-root check. If that NFS-side step fails for this run (for example non-root SSH with NETBOOT_DEPLOY_SUDO=no, disk full, interrupted rsync, or sudo misconfiguration), the server is left serving the fresh kernel while the NFS root still has the old modules, which is the kernel/modules mismatch this hook is meant to avoid. Stage the TFTP update until after the modules rsync succeeds, or swap it atomically only after the NFS side is complete.

Useful? React with 👍 / 👎.

Comment on lines +488 to +491
run_host_command_logged_raw ssh "${ssh_opts[@]}" "${NETBOOT_DEPLOY_SSH}" \
"${sudo_prefix}mkdir -p ${q_nfs_path} \
&& ${sudo_prefix}tar -xp --numeric-owner --xattrs --xattrs-include='*' --acls --selinux -f ${q_remote_archive} -C ${q_nfs_path}; \
ret=\$?; rm -f ${q_remote_archive}; exit \${ret}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid updating the live rootfs before TFTP can finish

For full-image SSH deploys with NETBOOT_ROOTFS_ARCHIVE, this extracts the new rootfs directly into the live NFS path before any of the staged TFTP rsync/swap or pxelinux.cfg update has succeeded. If a later TFTP step fails in this run (for example permissions under NETBOOT_DEPLOY_TFTP_ROOT, a full filesystem, or an interrupted rsync), clients keep booting the old kernel/initrd from TFTP against the newly extracted rootfs/modules, creating the inverse kernel/modules mismatch; stage the rootfs or add rollback so the live NFS tree is only switched after the TFTP publish is guaranteed.

Useful? React with 👍 / 👎.

Comment on lines +472 to +473
run_host_command_logged_raw ssh "${ssh_opts[@]}" "${NETBOOT_DEPLOY_SSH}" \
"test -d ${q_nfs_path} && ${sudo_prefix}find ${q_nfs_path} -mindepth 1 -delete ; true"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Abort when the requested NFS pre-wipe fails

When NETBOOT_DEPLOY_NFS_DELETE=yes, the unconditional trailing ; true masks every failure from the remote find ... -delete, not just the harmless "path does not exist" case. If the delete fails because of permissions, a read-only/stale NFS handle, immutable files, or sudo issues, the deploy continues to untar over the old tree and reports success while removed packages/configs/modules remain in the NFS root; only ignore the missing-directory case and let delete errors stop the deploy.

Useful? React with 👍 / 👎.

Comment on lines +754 to +755
run_host_command_logged_raw ssh "${ssh_opts[@]}" "${NETBOOT_DEPLOY_SSH}" \
"test -d ${q_pxelinux_dir} && ${sudo_prefix}find ${q_pxelinux_dir} -type f -exec sed -i '\\#'${q_initrd_pattern}'#d' {} + ; true"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Fail when INITRD cleanup cannot update PXE configs

In the kernel-only deploy path, this SSH command always exits successfully because of the trailing ; true, so it also hides real failures from find/sed when pxelinux.cfg exists but cannot be edited (for example wrong permissions, NETBOOT_DEPLOY_SUDO not covering the directory visibility check, a read-only TFTP root, or a sed error). In that scenario the deploy has already removed ${NETBOOT_TFTP_PREFIX}/uInitrd, but the PXE stanza still contains INITRD .../uInitrd, so the next U-Boot PXE boot aborts on the missing initrd while the build reports success; only suppress the absent-directory case and propagate update failures.

Useful? React with 👍 / 👎.

Comment on lines +195 to +198
_export_resolved="$(realpath -m "${ROOTFS_EXPORT_DIR}")"
_sdcard_resolved="$(realpath -m "${SDCARD}")"
if [[ -z "${_export_resolved}" || "${_export_resolved}" == "/" || "${_export_resolved}" == "${_sdcard_resolved}" ]]; then
exit_with_error "Refusing rsync --delete to unsafe ROOTFS_EXPORT_DIR" "${ROOTFS_EXPORT_DIR}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Block unsafe plain-nfs export destinations

For ROOTFS_TYPE=nfs builds that set ROOTFS_EXPORT_DIR without the netboot extension, this guard only rejects / and ${SDCARD} before the later rsync --delete --delete-excluded writes the rootfs contents into the destination. A typo such as ROOTFS_EXPORT_DIR=/etc or /boot passes this check and then deletes/replaces host files under that directory, so the same blacklist used by netboot (or an equivalent safe-prefix check) needs to apply here too.

Useful? React with 👍 / 👎.

…r /dev/nfs

update-initramfs runs four times during an nfs-root build:

  1. linux-image kernel-install (in chroot, before customize_image)
  2. post_customize_image__netboot_request_root_path (this extension)
  3. post_customize_image__netboot_initramfs_watchdog   (this extension)
  4. any later kernel-package upgrade on the running netboot host

Each call emits the cosmetic warning

  W: Couldn't identify type of root file system '/dev/nfs' for fsck hook

because initramfs-tools can't classify /dev/nfs and the fsck hook is
unconditionally probed. fsck is meaningless on an NFS-mounted root, so
ship /etc/initramfs-tools/conf.d/netboot-no-fsck with FSCKFIX=no.

Installed via a new post_customize_image hook ordered before the existing
request_root_path / watchdog hooks (definition order in extension files
maps to hook invocation order), so calls 2-4 above pick it up. Call 1
runs before customize_image hooks and still emits the warning once;
suppressing it would require an earlier hook point that doesn't fit this
extension.

Assisted-by: Claude:claude-opus-4.7
@iav iav force-pushed the enh/netboot-extension branch from 2cf98e3 to 36ac04d Compare May 13, 2026 15:02
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36ac04d40b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


if [[ $ROOTFS_TYPE == nfs || $ROOTFS_TYPE == nfs-root ]]; then
# ROOTFS_COMPRESSION: zstd/zst (default, .tar.zst) | gzip (.tar.gz) | none (skip archive)
declare rootfs_compression="${ROOTFS_COMPRESSION:-zstd}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Probe zstd before making it the deploy default

With ENABLE_EXTENSIONS=netboot-deploy and no explicit ROOTFS_COMPRESSION, this now produces a .tar.zst archive that the deploy hook uploads and extracts on the remote NFS server. I checked tar --help: zstd extraction is a filter through the external zstd program, but the README's minimal deploy target only requires GNU tar/nfs-kernel-server and the probe never verifies zstd remotely. On a normal minimal NFS server without zstd installed, the build succeeds and then the deploy fails late during rootfs untar; either default the deploy path to gzip or probe/document/install the remote zstd dependency before spending the build.

Useful? React with 👍 / 👎.

test -n "${distro_bootpart}" || setenv distro_bootpart 1
setenv boot_dev "${devtype} ${devnum}:${distro_bootpart}"

# Probe mainline marker: mainline rootfs has /boot/.next, legacy sunxi (pre-DT)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid requiring setexpr in the NFS boot script

This normalization runs unconditionally in the U-Boot script, but many Armbian U-Boot defconfigs in the repo leave CONFIG_CMD_SETEXPR disabled. In that environment, boards whose fdtfile still ends in .dts hit the following load ... dtb/${dtb_name} with the unmodified .dts name and reset because only the compiled .dtb exists. Keep this conversion in the generated script/build side, or guard it with a fallback that does not depend on the optional setexpr command.

Useful? React with 👍 / 👎.

# fails on unknown/changed host keys instead of silently trusting. Identity
# is supplied via either NETBOOT_DEPLOY_SSH_KNOWN_HOSTS (bind-mounted in
# the host hook above) or NETBOOT_DEPLOY_SSH_TOFU=yes (ephemeral, below).
declare NETBOOT_DEPLOY_SSH_OPTS="${NETBOOT_DEPLOY_SSH_OPTS:--o BatchMode=yes}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Honor explicit known_hosts outside Docker

When PREFER_DOCKER=no or the build is already running on the host, host_pre_docker_launch__netboot_deploy_mount_known_hosts never bind-mounts NETBOOT_DEPLOY_SSH_KNOWN_HOSTS to /root/.ssh/known_hosts, but the SSH options here still only add the TOFU override and otherwise rely on the default known-hosts location. In that supported non-relaunch path, an explicit known-hosts file outside ~/.ssh/known_hosts is ignored, so the probe and deploy fail host-key verification despite the user supplying the trust file; add UserKnownHostsFile=${NETBOOT_DEPLOY_SSH_KNOWN_HOSTS} when not using the bind mount.

Useful? React with 👍 / 👎.

# Normalize the variable to the resolved path so the in-container deploy
# hook checks the same file we bind-mounted (a symlinked input would
# otherwise be reported as missing inside the container).
declare -g NETBOOT_DEPLOY_SSH_KEY="${host_key}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Propagate the resolved SSH key path into Docker

When NETBOOT_DEPLOY_SSH_KEY is a symlink or a relative path in a Docker build, this normalization happens after produce_relaunch_parameters has already populated the arguments passed to the container, so the container still receives the original path while only the resolved host_key is bind-mounted. The later probe/deploy then hits NETBOOT_DEPLOY_SSH_KEY set but not visible in container even though the key was valid on the host; update the relaunch args/env to the resolved path or mount the original path as well.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant