cilium-cli icon indicating copy to clipboard operation
cilium-cli copied to clipboard

Add optimized cilium-cli image

Open marcofranssen opened this issue 1 year ago β€’ 3 comments

This image will allow users to run cilium commands within a Job on the cluster.

The image is optimized for size and attack surface. Furthermore the Dockerfile utilizes buildx features to have fast builds and allows to build the image for multiple architectures.

[!Note] I can file another PR for the first commit if desired. I choose not to affect the current CI image, but maybe it is a good idea to replace that one in the future with the new one. That image size is massive multi-GB, vs 185MB).

Resolves #2780

marcofranssen avatar Sep 02 '24 08:09 marcofranssen

Commits 42d8aa8484759be350f351b556237066e904d4e8, ed8ea68d92b8f7daf10779856b30d891422e6966 do not match "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@michi-covalent @tklauser with the merge of cilium-cli into cilium/cilium it's becoming confusing to where contributors can submit their changes for cilium-cli.

Yes, now I'm confused πŸ˜•

Should I create such PR on the cilium repo?

Maybe this repo has to be archived?

Let me know how to proceed.

marcofranssen avatar Sep 04 '24 09:09 marcofranssen

with the merge of cilium-cli into cilium/cilium it's becoming confusing to where contributors can submit their changes for cilium-cli.

sorry for the delay, we build release artifacts from this repo. assuming we want to build this docker image for each release tag, this is the right repo for this pull request.

@marcofranssen let's limit this pull request to Dockerfile changes only. once it gets approved & merged, i'll make necessary changes to github workflows.

michi-covalent avatar Sep 05 '24 20:09 michi-covalent

Commit 018b43f7320dcca8d4eccede111316ecf8c921b4 does not match "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@michi-covalent maybe merge this one first https://github.com/cilium/cilium-cli/pull/2842

This PR builds on top of that and reduces the scope of this PR.

marcofranssen avatar Nov 01 '24 10:11 marcofranssen

Builds on top of #2842 to reduce the scope of this PR. Can we please move forward with both. These PRs are out there now for several weeks without progress on a review of these relative simple PRs. I'm waiting to be able to use the improvements.

marcofranssen avatar Nov 11 '24 09:11 marcofranssen

@marcofranssen this PR picked up a merge conflict and needs a rebase.

tklauser avatar Nov 12 '24 08:11 tklauser

@marcofranssen this PR picked up a merge conflict and needs a rebase.

Done

marcofranssen avatar Nov 12 '24 11:11 marcofranssen

Rebased the branch again to resolve conflicts

marcofranssen avatar Nov 20 '24 19:11 marcofranssen

Did another rebase to get branch fully in sync with upstream

marcofranssen avatar Nov 25 '24 12:11 marcofranssen

Rebased PR again to resolve the merge conflict.

marcofranssen avatar Nov 27 '24 16:11 marcofranssen

Commit b9d0135205135e9ce23e7ee3edaf7ebcb3bf3da2 does not match "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

Commit 553f2d9fe5939a32dc19c1e95eb132b67715a0e6 does not match "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

Commit 553f2d9fe5939a32dc19c1e95eb132b67715a0e6 does not match "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

Rebased once more and signed off the one commit I forgot about.

marcofranssen avatar Dec 07 '24 12:12 marcofranssen

reminder for @tklauser and myself: set up release environment before the next release πŸš€

michi-covalent avatar Dec 08 '24 00:12 michi-covalent

ping @cilium/ci-structure for review πŸš€πŸ™

edit: never mind, i requested to update this PR to only modify Dockerfile πŸ‘ https://github.com/cilium/cilium-cli/pull/2782#discussion_r1874586803

michi-covalent avatar Dec 08 '24 00:12 michi-covalent

What should I do to get this merged? I adjusted the workflow to use a separate GH environment for the release image? Went trough the open conversations but not sure how to progress now.

I feel all required changes are in.

marcofranssen avatar Dec 16 '24 19:12 marcofranssen

What should I do to get this merged? I adjusted the workflow to use a separate GH environment for the release image? Went trough the open conversations but not sure how to progress now.

There are still two open questions from @aanm which AFAICT haven't been addressed or answered: https://github.com/cilium/cilium-cli/pull/2782#pullrequestreview-2488571607 and https://github.com/cilium/cilium-cli/pull/2782#discussion_r1875847962

tklauser avatar Dec 17 '24 10:12 tklauser

Also, can't we replace cilium/cilium-cli Dockerfile with the on we have in cilium/cilium? It seems this remains unanswered.

In a previous PR I contributed this was also already the question. It seems that is something to be figured out within the cilium repo owners.

In my previous PR it was decided to continue using this repo until a decision is made. Would be great if we can unblock this PR and move forward and figure out the other part in a follow-up.

marcofranssen avatar Dec 17 '24 12:12 marcofranssen

What should I do to get this merged? I adjusted the workflow to use a separate GH environment for the release image? Went trough the open conversations but not sure how to progress now.

There are still two open questions from @aanm which AFAICT haven't been addressed or answered: #2782 (review) and #2782 (comment)

I feel one was already answered, but gave it another shot with different wording.

The other one seems to be unrelated to this PR. It was already a discussion on a previous PR and there it was decided to not hold back community contribution while the cilium team figures it out internally on how and when to make that change.

marcofranssen avatar Dec 17 '24 12:12 marcofranssen

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Feb 09 '25 02:02 github-actions[bot]

Rebased once again to resolve conflicts. I feel all questions have been answered. What else is required?

marcofranssen avatar Feb 10 '25 09:02 marcofranssen

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Mar 13 '25 02:03 github-actions[bot]

@tklauser @aanm Rebased it again… What is required to get this merged?

marcofranssen avatar Mar 13 '25 16:03 marcofranssen

sorry this hasn't been progressing, let's take a step back. recently there was this pull request https://github.com/cilium/cilium/pull/37970 that got rid of all the cloud provider binaries. i'll open a pull request some time this week to make a similar change in cilium-cli repo so that we can use the same Dockerfile for CI and release.

michi-covalent avatar Mar 18 '25 22:03 michi-covalent

OK, please ping me here once that is in, then I will rebase the PR and update it.

marcofranssen avatar Mar 19 '25 22:03 marcofranssen

ok i merged https://github.com/cilium/cilium-cli/pull/2980. i also updated https://github.com/cilium/cilium-cli/blob/main/.github/workflows/images.yaml to handle the release environment so it should push a docker image to quay.io/cilium/cilium-cli when the next release gets tagged.

michi-covalent avatar Mar 26 '25 21:03 michi-covalent

Ok, majority of my changes I discarded, renamed the PR and keep it just to optimizing couple image layers.

marcofranssen avatar Mar 27 '25 09:03 marcofranssen