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.

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

new(docker): streamline docker images #3273

Merged
merged 11 commits into from
Nov 5, 2024
Merged

new(docker): streamline docker images #3273

merged 11 commits into from
Nov 5, 2024

Conversation

FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Jul 4, 2024 •
edited
Loading

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area build

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #3165

Special notes for your reviewer:

TODO:

  • falcoctl must not be shipped with Falco -> for now workarounded by using rm -rf (only in the Falco image)
  • falco packages shall not have any dep (split packages in falco-kmod, falco-ebpf, and just falco (modern)), otherwise falco-debian image will ship gcc/dkms... too as deps of the Falco package
  • update docs

Does this PR introduce a user-facing change?:

new(docker): streamline docker images

@poiana
Copy link
Contributor

poiana commented Jul 4, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana requested review from Andreagit97 and Kaizhe July 4, 2024 07:22
@FedeDP
Copy link
Contributor Author

FedeDP commented Jul 4, 2024

For

falco packages shall not have any dep (split packages in falco-kmod, falco-ebpf, and just falco (modern)), otherwise falco-debian image will ship gcc/dkms... too as deps of the Falco package

We might want to install the Falco package without installing any deps, perhaps. I am not sure whether it is going to work though, but i guess it should.

leogr
leogr reviewed Jul 24, 2024
View reviewed changes
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

Great job 👏

Left a few comments, just nitpicking 👼

docker/falco/Dockerfile Show resolved Hide resolved
docker/falco/Dockerfile Show resolved Hide resolved
docker/falco/Dockerfile Outdated Show resolved Hide resolved
docker/driver-loader/docker-entrypoint.sh Outdated Show resolved Hide resolved
@leogr
Copy link
Member

leogr commented Aug 28, 2024

Tentatively for
/milestone 0.39.0

@poiana poiana added this to the 0.39.0 milestone Aug 28, 2024
@FedeDP FedeDP force-pushed the new/docker_images branch from 862edfb to e6d1540 Compare August 29, 2024 08:35
@poiana poiana added size/XXL and removed size/XL labels Aug 29, 2024
@FedeDP
Copy link
Contributor Author

FedeDP commented Sep 5, 2024

/milestone 0.40.0

@poiana poiana modified the milestones: 0.39.0, 0.40.0 Sep 5, 2024
@FedeDP FedeDP force-pushed the new/docker_images branch from e6d1540 to 84ad86e Compare October 17, 2024 06:49
leogr
leogr reviewed Oct 29, 2024
View reviewed changes
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

I did a second pass, and I think we are close :)

See comments below.

.github/workflows/reusable_build_docker.yaml Outdated Show resolved Hide resolved
docker/driver-loader-buster/Dockerfile Outdated Show resolved Hide resolved
docker/driver-loader/Dockerfile Outdated Show resolved Hide resolved
docker/falco/Dockerfile Show resolved Hide resolved
@poiana poiana added size/XL and removed size/XXL labels Oct 30, 2024
@leogr
Copy link
Member

leogr commented Oct 30, 2024

/test test-dev-packages-arm64 / test-packages

@poiana
Copy link
Contributor

poiana commented Oct 30, 2024

@leogr: No presubmit jobs available for falcosecurity/falco@master

In response to this:

/test test-dev-packages-arm64 / test-packages

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 kubernetes-sigs/prow repository.

@FedeDP FedeDP changed the title wip: new(docker): streamline docker images new(docker): streamline docker images Oct 30, 2024
leogr
leogr reviewed Oct 30, 2024
View reviewed changes
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

Hey @FedeDP

I've realized that running Falco within the driver loader differs from the original proposal's spirit. Also, using CMD would not have allowed arguments to be passed to the driver loader.

So, I've reverted those changes and reviewed command line usage examples.

I've also updated the documentation to reflect this.

Please take a look at the the suggested changes below.

docker/driver-loader/docker-entrypoint.sh Outdated Show resolved Hide resolved
docker/driver-loader/Dockerfile Outdated Show resolved Hide resolved
docker/driver-loader-buster/docker-entrypoint.sh Outdated Show resolved Hide resolved
docker/driver-loader-buster/Dockerfile Outdated Show resolved Hide resolved
docker/driver-loader-buster/Dockerfile Outdated Show resolved Hide resolved
docker/driver-loader/Dockerfile Outdated Show resolved Hide resolved
docker/README.md Outdated Show resolved Hide resolved
docker/README.md Outdated Show resolved Hide resolved
docker/README.md Outdated Show resolved Hide resolved
leogr
leogr previously approved these changes Oct 31, 2024
View reviewed changes
@poiana
Copy link
Contributor

poiana commented Nov 5, 2024

@ekoops: changing LGTM is restricted to collaborators

In response to this:

Just left a couple of comments.

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 kubernetes-sigs/prow repository.

leogr
leogr reviewed Nov 5, 2024
View reviewed changes
docker/README.md Outdated Show resolved Hide resolved
@leogr leogr force-pushed the new/docker_images branch from 6795242 to 3d690f6 Compare November 5, 2024 14:27
leogr and others added 2 commits November 5, 2024 15:39
@leogr
update(docker): no CMD for falco-driver-loader images
25b800b 
Co-authored-by: Federico Di Pierro <[email protected]>

Signed-off-by: Leonardo Grasso <[email protected]>
@leogr @ekoops
docs(docker): update images description
245aa2e 
Co-authored-by: Leonardo Di Giovanna <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
@leogr leogr force-pushed the new/docker_images branch from 3d690f6 to 245aa2e Compare November 5, 2024 14:40
@poiana poiana added size/XXL and removed size/XL labels Nov 5, 2024
leogr
leogr previously approved these changes Nov 5, 2024
View reviewed changes
@leogr leogr requested a review from ekoops November 5, 2024 14:41
sgaist
sgaist previously approved these changes Nov 5, 2024
View reviewed changes
ekoops
ekoops suggested changes Nov 5, 2024
View reviewed changes
docker/falco-debian/Dockerfile Outdated Show resolved Hide resolved
@poiana
Copy link
Contributor

poiana commented Nov 5, 2024

@ekoops: changing LGTM is restricted to collaborators

In 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 kubernetes-sigs/prow repository.

@leogr @ekoops
fix(docker/falco-debian): usage label
583f50e 
Co-authored-by: Leonardo Di Giovanna <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
@leogr leogr dismissed stale reviews from sgaist and themself via 583f50e November 5, 2024 16:03
@leogr leogr requested review from sgaist, leogr, ekoops and LucaGuerra and removed request for ekoops November 5, 2024 16:14
@poiana poiana merged commit 3fa8bc9 into master Nov 5, 2024
34 checks passed
@poiana poiana deleted the new/docker_images branch November 5, 2024 16:38
This was referenced Nov 6, 2024
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leogr leogr leogr approved these changes

@sgaist sgaist sgaist approved these changes

@Andreagit97 Andreagit97 Awaiting requested review from Andreagit97

@Kaizhe Kaizhe Awaiting requested review from Kaizhe

@LucaGuerra LucaGuerra Awaiting requested review from LucaGuerra

@ekoops ekoops Awaiting requested review from ekoops

Assignees

@sgaist sgaist

@leogr leogr

Labels
approved area/build dco-signoff: yes kind/feature lgtm release-note size/XXL
Projects
Falco Roadmap
Status: Done
Milestone
0.40.0
Development

Successfully merging this pull request may close these issues.

[Proposal] Streamlining docker images
5 participants
@FedeDP @poiana @leogr @sgaist @ekoops