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.

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

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

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

@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.

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
@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
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
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
@leogr leogr requested a review from sgaist November 5, 2024 14:08
Copy link
Contributor

@ekoops ekoops left a comment

Choose a reason for hiding this comment

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

Just left a couple of comments.

docker/README.md Outdated Show resolved Hide resolved
docker/driver-loader-buster/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:

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.

docker/README.md Outdated Show resolved Hide resolved
leogr and others added 2 commits November 5, 2024 15:39
Co-authored-by: Federico Di Pierro <nierro92@gmail.com>

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
Co-authored-by: Leonardo Di Giovanna <leonardodigiovanna1@gmail.com>
Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
leogr
leogr previously approved these changes Nov 5, 2024
@leogr leogr requested a review from ekoops November 5, 2024 14:41
sgaist
sgaist previously approved these changes Nov 5, 2024
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.

Co-authored-by: Leonardo Di Giovanna <leonardodigiovanna1@gmail.com>
Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Proposal] Streamlining docker images
5 participants