OCPBUGS-83863: Dockerfile: remove rhel8, use rhel9 as base, add rhel10 support#228
OCPBUGS-83863: Dockerfile: remove rhel8, use rhel9 as base, add rhel10 support#228sdodson wants to merge 4 commits into
Conversation
RHEL 8 is end-of-life. Remove the rhel8 build stage, switch the windows builder to the rhel-9 image, and use rhel9-built binaries as the default in /usr/src/plugins/bin/. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
The rhel9/bin/ directory contains the same binaries as bin/. Use hardlinks to avoid duplicating them in the image layer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Add a rhel10/bin/ directory populated with hardlinks to the rhel9 binaries. For now the rhel9 binaries are assumed to be compatible with rhel10. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
openshift-ci-robot
added
the
jira/valid-reference
|
@sdodson: This pull request references Jira Issue OCPBUGS-83863, which is valid. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-bug
WalkthroughThe Dockerfile removes the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sdodson 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 24-26: The two separate RUN layers creating hardlinks should be
combined into a single RUN so the hardlinks share the same inode; update the
Dockerfile so the ln operations that link /usr/src/plugins/bin/* into
/usr/src/plugins/rhel9/bin/ and /usr/src/plugins/rhel10/bin/ are executed in one
RUN instruction (i.e., run both ln commands together in the same shell
invocation) to avoid duplicate file data across layers and preserve inode
identity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5cdb716f-6ea1-43d1-9996-e288ff5a7acc
📒 Files selected for processing (1)
Dockerfile
| RUN ln /usr/src/plugins/bin/* /usr/src/plugins/rhel9/bin/ | ||
| # For now assume rhel9 binaries are compatible with rhel10 | ||
| RUN ln /usr/src/plugins/bin/* /usr/src/plugins/rhel10/bin/ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
engine="podman"
command -v podman >/dev/null 2>&1 || engine="docker"
$engine build -t cni-hardlink-check -f Dockerfile .
$engine run --rm cni-hardlink-check sh -ec '
for f in /usr/src/plugins/bin/*; do
b=$(basename "$f")
i_bin=$(stat -c "%d:%i" "$f")
i_r9=$(stat -c "%d:%i" "/usr/src/plugins/rhel9/bin/$b")
i_r10=$(stat -c "%d:%i" "/usr/src/plugins/rhel10/bin/$b")
if [ "$i_bin" != "$i_r9" ] || [ "$i_bin" != "$i_r10" ]; then
echo "hardlink mismatch: $b -> bin=$i_bin rhel9=$i_r9 rhel10=$i_r10"
exit 1
fi
done
echo "all hardlinks match"
'Repository: openshift/containernetworking-plugins
Length of output: 128
🏁 Script executed:
# First, let's see the full Dockerfile context around the flagged lines
fd -t f -name "Dockerfile" | head -5Repository: openshift/containernetworking-plugins
Length of output: 312
🌐 Web query:
Docker overlayfs hardlink inode preservation across layers RUN
💡 Result:
No, Docker's overlayfs (specifically overlay2, the current default) does not preserve the same inode numbers for hardlinks created in a RUN instruction across different image layers. Hardlinks created within the same RUN layer (single layer) share the same inode and are preserved correctly during layer commit, as shown in examples where ls -li reports the same inode and link count >1 intra-layer. However, when creating hardlinks in subsequent RUN instructions to files from previous layers, the inode is not preserved across layers: the new hardlink gets a new inode in the current layer's diff directory, resulting in separate storage (duplication of file data in the layer tarball), increased layer size, and different inode numbers when inspected in the final image. This is due to how layers are committed to tar archives—tar only preserves hardlinks if both links are in the same layer's changes; cross-layer references cannot be maintained as inodes are layer-local. Official Docker docs confirm layers contain "hard links to data shared with lower layers" at the storage driver level for mounting, but this does not extend to preserving identical inodes for user-created hardlinks across committed layers. Recent issues (2024) confirm this behavior persists with BuildKit.
Citations:
- 1: Docker removes hardlinks to files from other layers on commit moby/moby#5632
- 2: Regression: When Docker layer contains hardlinks to files from previous layers, the resulting image increases in size much more than it should moby/moby#48140
- 3: https://docs.docker.com/engine/storage/drivers/overlayfs-driver
- 4: https://docs.docker.com/engine/storage/drivers/
🌐 Web query:
overlayfs copy-on-write hardlink behavior separate layers
💡 Result:
In OverlayFS, hard links do not span across separate layers (upper and lower). The merged view presents files from either the upper or lower layer, but since upper and lower are distinct filesystem trees (often on the same underlying fs but treated separately), hard links cannot connect inodes across layers. When a write operation triggers copy-up of a file from the lower layer to the upper layer, OverlayFS creates a new independent file in the upper layer. If the original lower file had multiple hard links (nlink > 1), this copy-up "breaks" those links by default: the upper copy is a separate inode, so changes to it do not propagate to other lower hard link names, and vice versa. The kernel documentation explicitly states: "If this feature [inode index] is disabled and a file with multiple hard links is copied up, then this will 'break' the link. Changes will not be propagated to other names referring to the same inode." Creating a new hard link in the overlay (to a lower file) also triggers copy-up of the target file to the upper layer before linking. The "index=on" mount option (inodes index feature) preserves hard link semantics during copy-up by creating an index entry (a hard link in a hidden index directory to the upper inode, keyed by the lower file handle). This ensures consistent lookup of the same upper inode for all lower hard links, maintaining the union nlink count. Without index=on, hard links are broken on copy-up. Sources confirm hard links do not work across layers natively, and copy-up creates separate inodes unless indexing is used.
Citations:
- 1: https://docs.kernel.org/filesystems/overlayfs.html
- 2: https://www.kernel.org/doc/Documentation/filesystems/overlayfs.txt
- 3: https://influentcoder.com/posts/overlayfs/
- 4: https://kernel.org/doc/html/v5.15/filesystems/overlayfs.html
- 5: https://android.googlesource.com/kernel/common/+/f87032aec41e0b00ae5fa9103eb8e7b2d1f8416b/Documentation/filesystems/overlayfs.rst
- 6: https://docs.kernel.org/6.1/filesystems/overlayfs.html
- 7: https://github.com/torvalds/linux/blob/master/fs/overlayfs/inode.c
🏁 Script executed:
cat -n Dockerfile | head -40Repository: openshift/containernetworking-plugins
Length of output: 1619
Combine both hardlink operations in a single RUN layer to preserve inode identity and avoid storage duplication.
Lines 24 and 26 create hardlinks in separate Docker layers. Due to how overlayfs manages layer composition, each subsequent RUN instruction creates a new inode for hardlinked files, resulting in duplicate file data stored in each layer's tarball. Consolidating both ln commands into one RUN layer ensures all hardlinks reference the same inode across bin, rhel9/bin, and rhel10/bin.
Proposed fix
-RUN ln /usr/src/plugins/bin/* /usr/src/plugins/rhel9/bin/
-# For now assume rhel9 binaries are compatible with rhel10
-RUN ln /usr/src/plugins/bin/* /usr/src/plugins/rhel10/bin/
+# For now assume rhel9 binaries are compatible with rhel10
+RUN ln /usr/src/plugins/bin/* /usr/src/plugins/rhel9/bin/ && \
+ ln /usr/src/plugins/bin/* /usr/src/plugins/rhel10/bin/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN ln /usr/src/plugins/bin/* /usr/src/plugins/rhel9/bin/ | |
| # For now assume rhel9 binaries are compatible with rhel10 | |
| RUN ln /usr/src/plugins/bin/* /usr/src/plugins/rhel10/bin/ | |
| # For now assume rhel9 binaries are compatible with rhel10 | |
| RUN ln /usr/src/plugins/bin/* /usr/src/plugins/rhel9/bin/ && \ | |
| ln /usr/src/plugins/bin/* /usr/src/plugins/rhel10/bin/ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` around lines 24 - 26, The two separate RUN layers creating
hardlinks should be combined into a single RUN so the hardlinks share the same
inode; update the Dockerfile so the ln operations that link
/usr/src/plugins/bin/* into /usr/src/plugins/rhel9/bin/ and
/usr/src/plugins/rhel10/bin/ are executed in one RUN instruction (i.e., run both
ln commands together in the same shell invocation) to avoid duplicate file data
across layers and preserve inode identity.
Pass -ldflags "-s -w" to go build to reduce binary size by stripping the symbol table and DWARF debug information. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
|
@sdodson: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary
/usr/src/plugins/bin/and populaterhel9/bin/via hardlinks instead of a duplicate COPYrhel10/bin/directory populated with hardlinks to the rhel9 binaries, assuming compatibility for now-ldflags "-s -w"to reduce image sizeTest plan
/usr/src/plugins/bin/contains the expected CNI plugin binaries/usr/src/plugins/rhel9/bin/and/usr/src/plugins/rhel10/bin/contain hardlinks to the same binaries/usr/src/plugins/windows/bin/contains the Windows binariesrhel8/directory exists in the final image🤖 Generated with Claude Code