Skip to content

images/builder: add python3 scapy dependency #40838

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
Merged
merged 2 commits into from
Jul 31, 2025
Merged

images/builder: add python3 scapy dependency #40838

merged 2 commits into from
Jul 31, 2025

Conversation

msune
Copy link
Member

@msune msune commented Jul 31, 2025

Add python3-scapy and python3-jinja2 packages to install-deps script. This is requirement coming from the BPF unit test scapy support PR (#40294).

@msune
images/builder: add python3 scapy dependency
9084d3c 
Add python3-scapy and python3-jinja2 packages to install-deps script.
This is requirement coming from the BPF unit test scapy support PR
(cilium#40294).

Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
@msune msune requested a review from a team as a code owner July 31, 2025 06:56
@msune msune requested a review from borkmann July 31, 2025 06:56
@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 Jul 31, 2025
@msune msune had a problem deploying to release-base-images July 31, 2025 06:56 — with GitHub Actions Error
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jul 31, 2025
@msune msune mentioned this pull request Jul 31, 2025
Merged
@msune
Copy link
Member Author

msune commented Jul 31, 2025

Not sure how this needs to be sorted out:

diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
index 72160cf8..c2ef677a 100644
--- a/.devcontainer/devcontainer.json
+++ b/.devcontainer/devcontainer.json
@@ -1,6 +1,6 @@
 {
   "name": "Cilium",
-  "image": "quay.io/cilium/cilium-builder:9982935455338197344c9f6208d4b19e6ed67d83@sha256:d861f7e26da5dcd914fbedb9a0b3d41699bc749e4acefda7c33ab7b7b9b90ce1",
+  "image": "quay.io/cilium/cilium-builder:769ce739568d824078d2cf261de649c54ecb1315",
   "workspaceFolder": "/go/src/github.com/cilium/cilium",
   "workspaceMount": "source=${localWorkspaceFolder},target=/go/src/github.com/cilium/cilium,type=bind",
   "features": {
+ echo 'Builder images out of date, ' 'see https://docs.cilium.io/en/latest/contributing/development/images/#update-cilium-builder-runtime-images.'
+ exit 1
Builder images out of date,  see https://docs.cilium.io/en/latest/contributing/development/images/#update-cilium-builder-runtime-images.
make: *** [Makefile:45: check-builder-image] Error 1
make: Leaving directory '/github/workspace/images'

Any thoughts @HadrienPatte (btw, Daniel is on PTO, is it ok for you to handle this one)?

images: update cilium-{runtime,builder}
928edc5 
Signed-off-by: Cilium Imagebot <noreply@cilium.io>
@auto-committer auto-committer bot requested review from a team as code owners July 31, 2025 07:31
@auto-committer auto-committer bot requested review from kaworu and christarazi July 31, 2025 07:31
@auto-committer auto-committer bot temporarily deployed to release-base-images July 31, 2025 07:32 Inactive
@aanm
Copy link
Member

aanm commented Jul 31, 2025

@msune it has been magically taken care of...

@HadrienPatte
Copy link
Member

HadrienPatte commented Jul 31, 2025 •
edited
Loading

So the way these work is that a CI job rebuilds the images and tries to push to the PR branch the updated image ref, but here, since the branch is on a fork, the CI will not have the permissions to push to it. You can instead look at the logs (here) of that failed job and apply the same diff in a new commit and then the CI should pass, see this doc

Nevermind, looks like it was able to push to your branch 💯

@HadrienPatte HadrienPatte removed the request for review from borkmann July 31, 2025 08:02
michi-covalent
michi-covalent approved these changes Jul 31, 2025
View reviewed changes
Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

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

👍

@msune
Copy link
Member Author

msune commented Jul 31, 2025

I think we have all the necessary reviews now.

@HadrienPatte HadrienPatte removed the request for review from kaworu July 31, 2025 16:45
@HadrienPatte
Copy link
Member

I think all that's missing now is a release note 😉

@HadrienPatte
Copy link
Member

/test

@msune
Copy link
Member Author

msune commented Jul 31, 2025

I think all that's missing now is a release note 😉

I deliberately removed the release note, as per the comment on the template:

<!-- Enter the release note text here if needed or remove this section! -->

Do you think we need one?

@joestringer
Copy link
Member

I don't think a release note is necessary. The blocker for mergeability is setting the class of release note for this change, which seems like misc to me as it is not user facing. @christarazi @HadrienPatte @michi-covalent during review if you see the PR doesn't have a release note category, please consider setting the label as documented in step 7 of the review guide. This helps to avoid the PR falling through the gaps. Thanks!

@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Jul 31, 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 Jul 31, 2025
@joestringer joestringer enabled auto-merge July 31, 2025 17:37
@joestringer joestringer added this pull request to the merge queue Jul 31, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 31, 2025
Merged via the queue into cilium:main with commit 917f96b Jul 31, 2025
78 of 80 checks passed
@msune msune deleted the builder_python3_scapy branch July 31, 2025 17:58
@msune msune restored the builder_python3_scapy branch July 31, 2025 17:58
@msune msune deleted the builder_python3_scapy branch July 31, 2025 18:01
qmonnet added a commit to qmonnet/cilium that referenced this pull request Aug 1, 2025
@qmonnet
images: Remove unused install-builder-deps.sh script
bc3cee6 
It looks like script install-builder-deps.sh has never been used for
generating builder images. Ever since it was introduced in commit
7724ceb ("build: New builder image supporting cross-compilation"),
it's never been called anywhere in the repository. Remove it to avoid
confusion as in cilium#40838.

Link: cilium#40294 (comment)
Signed-off-by: Quentin Monnet <qmo@qmon.net>
@qmonnet
Copy link
Member

qmonnet commented Aug 1, 2025

See also: #40870

@msune
Copy link
Member Author

msune commented Aug 1, 2025

And mind folllow up #40874 now modifying the Dockerfile.

qmonnet added a commit to qmonnet/cilium that referenced this pull request Aug 4, 2025
@qmonnet
images: Remove unused install-builder-deps.sh script
65a5cbe 
It looks like script install-builder-deps.sh has never been used for
generating builder images. Ever since it was introduced in commit
7724ceb ("build: New builder image supporting cross-compilation"),
it's never been called anywhere in the repository. Remove it to avoid
confusion as in cilium#40838.

Link: cilium#40294 (comment)
Signed-off-by: Quentin Monnet <qmo@qmon.net>
github-merge-queue bot pushed a commit that referenced this pull request Aug 4, 2025
@qmonnet
images: Remove unused install-builder-deps.sh script
20730b2 
It looks like script install-builder-deps.sh has never been used for
generating builder images. Ever since it was introduced in commit
7724ceb ("build: New builder image supporting cross-compilation"),
it's never been called anywhere in the repository. Remove it to avoid
confusion as in #40838.

Link: #40294 (comment)
Signed-off-by: Quentin Monnet <qmo@qmon.net>
rabelmervin pushed a commit to rabelmervin/cilium that referenced this pull request Aug 18, 2025
@qmonnet @rabelmervin
images: Remove unused install-builder-deps.sh script
6fa730d 
It looks like script install-builder-deps.sh has never been used for
generating builder images. Ever since it was introduced in commit
7724ceb ("build: New builder image supporting cross-compilation"),
it's never been called anywhere in the repository. Remove it to avoid
confusion as in cilium#40838.

Link: cilium#40294 (comment)
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christarazi christarazi christarazi approved these changes

@HadrienPatte HadrienPatte HadrienPatte approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

Assignees
No one assigned
Labels
kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@msune @aanm @HadrienPatte @joestringer @qmonnet @christarazi @michi-covalent