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

Add alpine-sh and alpine-bash container images #36

Merged
merged 4 commits into from
Jul 2, 2024
Merged

Conversation

tiagolobocastro
Copy link
Contributor

@tiagolobocastro tiagolobocastro commented Jul 1, 2024

ci: update to accommodate new images

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

feat: add new tiny container images

Adds alpine-sh and alpine-bash which are minimalist images
These can be useful for small init containers

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

feat: support multiple images

Support for multiple images (dockerfiles) during the make process
Ensures make test does not leave exited containers
Add make clean to delete buildx builder
Removes the push scripts as those are no longer used

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

@tiagolobocastro
Copy link
Contributor Author

@Abhinandan-Purkait @niladrih comments on the names please :D

@niladrih
Copy link
Member

niladrih commented Jul 2, 2024

IMO, the names shouldn't be tied to their purpose on the kubernetes' 'machine', i.e. as initContainers. It should be tied to the key libs, packages it contains. Here we don't have an entrypoint, or cmd, so we can't tie the name to that.
Maybe..

  • for the bash one, it could be alpine-bash or bash-musl(this will make it clear that it uses busybox, musl and has bash)
  • for the sh one, it's not necessarily just for sh, it's a default alpine, with busybox, etc. Maybe alpine. But with that name, it's confusing if we have just taken an alpine an retagged it with some other version.

Do we need the init-sh container? Should the bash one be enough for our use-cases at the moment? It adds another container to our helm chart and it saves very little space after we optimise it down.

@tiagolobocastro
Copy link
Contributor Author

IMO, the names shouldn't be tied to their purpose on the kubernetes' 'machine', i.e. as initContainers. It should be tied to the key libs, packages it contains. Here we don't have an entrypoint, or cmd, so we can't tie the name to that. Maybe..

hmm I'm not sure, calling it simply by the names of the packages feels somewhat incomplete.
That being said, I'm ok with alpine and alpine-bash as it's pretty clear what it is from the name, wdyt?

Do we need the init-sh container? Should the bash one be enough for our use-cases at the moment? It adds another container to our helm chart and it saves very little space after we optimise it down.

The init-sh container is very simple to add since we adding the bash one, there's no cost of adding it so seemed a no brainer.

@niladrih
Copy link
Member

niladrih commented Jul 2, 2024

I am okay with it.

What about helper-bash? and that would change the old one to helper-linux-utils?

@tiagolobocastro
Copy link
Contributor Author

I am okay with it.

What about helper-bash? and that would change the old one to helper-linux-utils?

I guess so, though again we'd be adding purpose to the name :)
I'd prefer alpine-bash and keep linux-utils as is for now. In the future we can come up with better name for the linux-utils since it has a lot more than just the linux-utils.

@tiagolobocastro tiagolobocastro changed the title Add init-sh and init-bash container images Add alpine-sh and alpine-bash container images Jul 2, 2024
Copy link
Member

@niladrih niladrih left a comment

Choose a reason for hiding this comment

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

This'll require the image repositories to be created. I can take it up for docker.io, need a hand with quay. I'll try to see if my GitHub auth lets me work on ghcr.

Makefile.buildx.mk Outdated Show resolved Hide resolved
Support for multiple images (dockerfiles) during the make process
Ensures make test does not leave exited containers
Add make clean to delete buildx builder
Removes the push scripts as those are no longer used

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Adds alpine-sh and alpine-bash which are minimalist images
These can be useful for small init containers

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro tiagolobocastro merged commit f5d0e1b into main Jul 2, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants