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

docker: add hubble CLI binary to the base cilium image #11784

Merged
merged 1 commit into from Jun 2, 2020

Conversation

rolinh
Copy link
Member

@rolinh rolinh commented May 29, 2020

This increases the size of the cilium image by ~12MB (from 408MB to
420MB) but on the other end provides a tailor-made tool to troubleshoot
network issues.
It is expected that once Hubble Relay is GA, having the hubble CLI
binary as part of the base image will no longer be necessary.

This increases the size of the cilium image by ~12MB (from 408MB to
420MB) but on the other end provides a tailor-made tool to troubleshoot
network issues.
It is expected that once Hubble Relay is GA, having the hubble CLI
binary as part of the base image will no longer be necessary.

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>
@rolinh rolinh added kind/feature This introduces new functionality. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/hubble Impacts hubble server or relay labels May 29, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 May 29, 2020
@rolinh
Copy link
Member Author

rolinh commented May 29, 2020

Note: this is a draft as it is expected to fail until Hubble v0.6.0 is tagged and the image available on quay.io.

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Even if we're adding 12MB, the v1.8.0 images are probably smaller than the v1.7.x images (AFAICT ~40% smaller) and we're gaining a bunch of functionality here. My concern would be more about locking a particular version of the CLI into this tree but honestly I think the hubble CLI is likely mature enough at this point that it won't be a problem.

(Btw I compared the image sizes you mentioned against https://hub.docker.com/repository/registry-1.docker.io/cilium/cilium/tags?page=1 and I'm guessing you're quoting uncompressed sizes, so the actual distribution size with the added binary is likely even smaller.)

@rolinh
Copy link
Member Author

rolinh commented May 29, 2020

I'm guessing you're quoting uncompressed sizes

This is correct; I should have made that clear.

@glibsm
Copy link
Member

glibsm commented May 29, 2020

image

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 36.887% when pulling 9f414c4 on pr/rolinh/hubble-cli-binary into 5e5c626 on master.

@glibsm
Copy link
Member

glibsm commented May 29, 2020

Do we want to point to the latest 0.6.X branch with patch fixes, or pin to 0.6.0 specifically? I'm thinking latest 0.6 is probably the best course of action so we can commit bug fixes and such without the need for a patch cilium release

@michi-covalent
Copy link
Contributor

Do we want to point to the latest 0.6.X branch with patch fixes, or pin to 0.6.0 specifically? I'm thinking latest 0.6 is probably the best course of action so we can commit bug fixes and such without the need for a patch cilium release

personally i prefer to pin the image to a specific tag but i can go either way. @rolinh ?

@michi-covalent
Copy link
Contributor

@michi-covalent michi-covalent marked this pull request as ready for review May 29, 2020 21:51
@michi-covalent michi-covalent requested a review from a team as a code owner May 29, 2020 21:51
@joestringer
Copy link
Member

I'm thinking latest 0.6 is probably the best course of action so we can commit bug fixes and such without the need for a patch cilium release

The flip side is if there's no explicit change in the codebase then any regression leaks in then simply compiling a new Cilium image will render in the broken version. That's hard to track because the build is dependent on the time that you generate the image (including any subsequent fix).

@rolinh
Copy link
Member Author

rolinh commented May 30, 2020

Do we want to point to the latest 0.6.X branch with patch fixes, or pin to 0.6.0 specifically? I'm thinking latest 0.6 is probably the best course of action so we can commit bug fixes and such without the need for a patch cilium release

I'd prefer to pin a specific version as well. Yes, this means an extra step when we tag a new hubble CLI bugfix release but on the other end, the hubble CLI update commit will also be mentioned in cilium's release notes. Note that in both cases, a new cilium image has to be tagged for the newer hubble CLI to be picked up anyway.

@michi-covalent
Copy link
Contributor

test-me-please

@michi-covalent
Copy link
Contributor

retest-4.19

@michi-covalent
Copy link
Contributor

retest-4.9

@gandro gandro added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 2, 2020
@tgraf tgraf merged commit 751b507 into master Jun 2, 2020
1.8.0 automation moved this from In progress to Merged Jun 2, 2020
@tgraf tgraf deleted the pr/rolinh/hubble-cli-binary branch June 2, 2020 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature This introduces new functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/hubble Impacts hubble server or relay
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

9 participants