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

SCT container should not include kubectl, eksctl, help by default - to cut its size by half #8279

Closed
mykaul opened this issue Aug 7, 2024 · 13 comments
Assignees
Labels
New Hydra Version PR# introduces new Hydra version

Comments

@mykaul
Copy link
Contributor

mykaul commented Aug 7, 2024

See

RUN curl -fsSLo /usr/local/bin/kubectl https://dl.k8s.io/release/v$KUBECTL_VERSION/bin/linux/amd64/kubectl && \
- it downloads 3 large binaries that are hardly being used (only in operator related tests), that somewhat bloat our container.
We are downloading this container gozillion of times (650K in the last 7 days I believe). Those 3 binaries take approx half of the size of the whole container.

Instead, the operator related tests should deploy them when needed, or have their own container based off the main one.

The container is also public and those binaries have known security vulnerabilities (we could and should update them, but it's a separate topic)

@mykaul mykaul added the New Hydra Version PR# introduces new Hydra version label Aug 7, 2024
@fruch
Copy link
Contributor

fruch commented Aug 7, 2024

if we'll split to other containers that we build, they would be public as well.

it's would need change in a lot of of k8s implemetions, and would be big testing effort to keep operator case working after such a change.

@soyacz
Copy link
Contributor

soyacz commented Aug 8, 2024

@fruch while you're working on new AMI's for sct-runners, maybe would be worth to docker pull ... hydra image, so later tests would only download changed layers.
We could from time to time rebuild runner AMI with updated layers (we rarely change k8s stuff/base packages etc. - mostly python dependencies which is the last layer)

@mykaul
Copy link
Contributor Author

mykaul commented Aug 8, 2024

if we'll split to other containers that we build, they would be public as well.

it's would need change in a lot of of k8s implemetions, and would be big testing effort to keep operator case working after such a change.

If we base the operator hydra on top of the regular one ('hydra-k8s') I doubt it's a big deal. And we can refresh them together, etc.

@fruch
Copy link
Contributor

fruch commented Aug 8, 2024

@fruch while you're working on new AMI's for sct-runners, maybe would be worth to docker pull ... hydra image, so later tests would only download changed layers.
We could from time to time rebuild runner AMI with updated layers (we rarely change k8s stuff/base packages etc. - mostly python dependencies which is the last layer)

We might consider it, it isn't gonna change anything drastically, since we are changing those every couple of k8s releases.

@fruch
Copy link
Contributor

fruch commented Aug 15, 2024

@vponomaryov

do you have estimation on moving all this CLI usages to something like HelmContainerMixin ?
so we can yank them out of the hydra image ?

@vponomaryov
Copy link
Contributor

vponomaryov commented Aug 15, 2024

We are downloading this container gozillion of times (650K in the last 7 days I believe). Those 3 binaries take approx half of the size of the whole container.

Removal of these binaries doesn't release approx half of the size of the whole container..
Archived variant: 478.85Mb -> 411.35Mb, it is 14.1% diff.
Unarchived variant: 2.03Gb -> 1.78Gb, it is 12.32% diff.

@vponomaryov

do you have estimation on moving all this CLI usages to something like HelmContainerMixin ? so we can yank them out of the hydra image ?

It will require creation of a new docker image. Or maybe even two - one for EKS based on the eksctl and one for GKE, based on it's k8s image. We don't have mixed scenarios, so, if we care about minimizing network usage then 2 new images sounds as more correct approach.
It will require retesting various special cases, because switching to dockerized approach will force us to debug some interfaces changes.
Also, exiting versions of binaries we use are kind of old. New ones may introduce some API changes.

All of it in summary sounds for about 3 days if no blockers appear.

@fruch
Copy link
Contributor

fruch commented Aug 15, 2024

We are downloading this container gozillion of times (650K in the last 7 days I believe). Those 3 binaries take approx half of the size of the whole container.

Removal of these binaries doesn't release approx half of the size of the whole container.. Archived variant: 478.85Mb -> 411.35Mb, it is 14.1% diff. Unarchived variant: 2.03Gb -> 1.78Gb, it is 12.32% diff.

@vponomaryov
do you have estimation on moving all this CLI usages to something like HelmContainerMixin ? so we can yank them out of the hydra image ?

It will require creation of a new docker image. Or maybe even two - one for EKS based on the eksctl and one for GKE, based on it's k8s image. We don't have mixed scenarios, so, if we care about minimizing network usage then 2 new images sounds as more correct approach. It will require retesting various special cases, because switching to dockerized approach will force us to debug some interfaces changes. Also, exiting versions of binaries we use are kind of old. New ones may introduce some API changes.

isn't there existing official images of those ? (or semi official) that we can count on ?
(no that that it's complicated to create our owns, probably safer with less surprises)

All of it in summary sounds for about 3 days if no blockers appear.

@mykaul
Copy link
Contributor Author

mykaul commented Aug 15, 2024

Archived variant: 478.85Mb -> 411.35Mb, it is 14.1% diff.
Unarchived variant: 2.03Gb -> 1.78Gb, it is 12.32% diff.

I got completely different numbers. Unsure how. For 12 or 14%, it's not worth the effort. I don't know what is archived and what isn't - I downloaded each binary and placed them all in a single directory - unarchived (as they are in the container!) and compared their overall size with the overall container size - perhaps I missed something?

@vponomaryov
Copy link
Contributor

... I don't know what is archived and what isn't ...

Archived it is what we really pull from image registry.
Unarchived it is what we have locally after pullling and ready to be used.

I downloaded each binary and placed them all in a single directory - unarchived (as they are in the container!) and compared their overall size with the overall container size - perhaps I missed something?

Not having clear steps, hard to say what is different.
I can say that in my calculation I removed exactly following stuff:

  • kubectl binary
  • eksctl binary
  • helm binary
  • google-cloud-sdk-gke-gcloud-auth-plugin package

Then just rebuilt the docker image without it and provided numbers here.
Image I got: https://hub.docker.com/layers/vponomarovatscylladb/hydra/v1.75-remove-k8s-stuff-v2/images/sha256-6494d8da65029dd269b7c43deba1248087f7b923a904559b0d26f201690224d0?context=repo
Current image we use: https://hub.docker.com/layers/scylladb/hydra/v1.74-tcconfig-0.29.1/images/sha256-876dcc23bb9397c361c183afe8afa18d8fae1094227dc674afddd411d6cf423f?context=explore

@mykaul
Copy link
Contributor Author

mykaul commented Aug 15, 2024

If that's the case, let's not bother. I guess the binaries are compressed well and therefore do not take too much size overall.
Maybe the numbers here don't matter eventually (pre-compression?):
image

@mykaul
Copy link
Contributor Author

mykaul commented Aug 15, 2024

This apt-get update is new though?
image

@vponomaryov
Copy link
Contributor

This apt-get update is new though? image

Built 2+ hours ago right before comment with calculations.

If that's the case, let's not bother. I guess the binaries are compressed well and therefore do not take too much size overall. Maybe the numbers here don't matter eventually (pre-compression?): image

Yes, these values are pre-compression.

So, these 245Mb is part of the 2.03Gb of not compressed image.

@mykaul
Copy link
Contributor Author

mykaul commented Aug 15, 2024

If that's the case, let's not bother. Closing.

@mykaul mykaul closed this as not planned Won't fix, can't repro, duplicate, stale Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Hydra Version PR# introduces new Hydra version
Projects
None yet
Development

No branches or pull requests

4 participants