8000 build: Update `iptables-wrapper` in runtime image by HadrienPatte · Pull Request #39996 · cilium/cilium · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

build: Update iptables-wrapper in runtime image #39996

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

build: Update iptables-wrapper in runtime image #39996

wants to merge 2 commits into from

Conversation

HadrienPatte
Copy link
Member

The current iptables-wrapper version is 3 years old. iptables-wrapper has since been rewritten from a bash script to a small go binary.

This PR updates iptables-wrapper to the latest version (diff) and adapts the cilium-runtime image Dockerfile to account for the fact that it's now a go binary that needs to be built.

Test:

docker run -it runtime /bin/bash
root@85f5aa71cb42:/# stat $(which iptables)
  File: /usr/sbin/iptables -> /usr/sbin/iptables-wrapper
  Size: 26              Blocks: 0          IO Block: 4096   symbolic link
root@85f5aa71cb42:/# iptables
iptables v1.8.8 (nf_tables): no command specified
Try `iptables -h' or 'iptables --help' for more information.

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!
build: Update iptables-wrapper in runtime image

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 11, 2025
HadrienPatte
HadrienPatte commented Jun 11, 2025
View reviewed changes
images/runtime/build-iptables-wrapper.sh Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

Largely inspired from build-gops.sh for consistency

images/runtime/Dockerfile
Comment on lines -34 to -37
# Update ubuntu packages to the most recent versions
RUN apt-get update && \
apt-get upgrade -y && \
apt-get install -y jq
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved/merged with images/runtime/install-runtime-deps.sh

images/runtime/build-gops.sh
Comment on lines -28 to -30

x86_64-linux-gnu-strip /out/linux/amd64/bin/gops
aarch64-linux-gnu-strip /out/linux/arm64/bin/gops
Copy link
Member Author

Choose a reason for hiding this comment

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

Manually striping the binaries is not necessary as they are already built with -ldflags "-s -w":

  • -s Omit the symbol table and debug information.
  • -w Omit the DWARF symbol table.

ref

@HadrienPatte HadrienPatte marked this pull request as ready for review June 11, 2025 14:12
@HadrienPatte HadrienPatte requested a review from a team as a code owner June 11, 2025 14:12
@HadrienPatte HadrienPatte requested a review from rolinh June 11, 2025 14:12
@joestringer joestringer added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Jun 13, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 13, 2025
@joestringer
Copy link
Member
joestringer commented Jun 13, 2025 •
edited
Loading

I think that with this change making changes to the base images, we'll need to more actively shepherd this in. Typically these images are maintained by folks with write privileges into the repo ("Reviewer" on the community ladder) and we will push a PR branch to this repo in order to update the image. The "Base Image Release Build" workflow can then push an updated commit into the tree to ensure that the base image change propagates into all the relevant dockerfiles.

I've approved the workflow to do that update but given that the PR is opened from outside the repository, I expect it will fail due to lack of permissions to push to your branch. However you may be able to just update the same files as what that action is doing and re-push, then the base image lint workflow will test whether the change was right (and hopefully just pass).

@HadrienPatte HadrienPatte requested review from a team as code owners June 16, 2025 09:12
@HadrienPatte HadrienPatte requested review from aanm and christarazi June 16, 2025 09:12
@HadrienPatte HadrienPatte temporarily deployed to release-base-images June 16, 2025 09:12 — with GitHub Actions Inactive
@HadrienPatte HadrienPatte mentioned this pull request Jun 16, 2025
Merged
@aanm aanm added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 16, 2025
@HadrienPatte
build: Update iptables-wrapper in runtime image
0e1b18c 
Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
@HadrienPatte
images: update cilium-runtime
319a460 
Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
@HadrienPatte HadrienPatte force-pushed the update-iptables-wrapper branch from 5ef49c1 to 319a460 Compare June 17, 2025 12:23
@joestringer
Copy link
Member

Now that you're a reviewer, I would suggest opening a fresh PR by pushing a branch like pr/hadrienpatte/update-iptables-wrapper into cilium/cilium and opening from there. Then you should be able to approve the deployment to update the PR with the relevant docker image changes.

@HadrienPatte HadrienPatte mentioned this pull request Jun 17, 2025
Open
8 tasks
@HadrienPatte
Copy link
Member Author

Thanks, I opened #40099 to replace this PR

@HadrienPatte HadrienPatte deleted the update-iptables-wrapper branch June 17, 2025 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rolinh rolinh rolinh approved these changes

@christarazi christarazi christarazi approved these changes

@aanm aanm aanm approved these changes

Assignees
No one assigned
Labels
dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@HadrienPatte @joestringer @rolinh @christarazi @aanm
0