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.

Sign up for GitHub

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 2nd tag with major and minor versions to the image at kamu-web-ua #243

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add 2nd tag with major and minor versions to the image at kamu-web-ua #243

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3306c4
Add 2nd tag with major and minor versions to the image in at kamu-web…
metiletan Nov 22, 2023
8f64bc0
Push image with major.minor tag
metiletan Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions images/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,24 @@ IMAGE_REPO = ghcr.io/kamu-data
KAMU_WEB_UI_VERSION = $(shell cat ../package.json | jq -r '.version')
KAMU_WEB_UI_IMAGE = $(IMAGE_REPO)/kamu-web-ui

KAMU_WEB_UI_MAJOR_VERSION = $(shell echo $(KAMU_WEB_UI_VERSION) | cut -d. -f 1)
KAMU_WEB_UI_MINOR_VERSION = $(shell echo $(KAMU_WEB_UI_VERSION) | cut -d. -f 2)
Copy link
Member

Choose a reason for hiding this comment

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

What is the overall intent here? Are you suggesting to only specify major.minor app version in charts, i.e. allowing the same chart to pull a newer patched application version?

Would this necessitate pull: Always in all deployments?

// Please include more context in PR description in future.

Copy link

Choose a reason for hiding this comment

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

Note taken on the more description.

TL;DR

The intent is to allow version skew between charts and applications. Setting Always pull policy will be required, but it's not a problem. Details bellow.

 The intention is to make it possible to have a version skew between charts and applications and to avoid bumping chart versions every time there are new releases of applications and vice versa.

The charts will only reference the major.minor tag, therefore every x.y.* version of the chart will be compatible with x.y.* version of the app. If users want to Always have the latest patch version of applications it would indeed require them to set the image pull policy to Always. This imposes the necessity to keep compatibility between pods running different patch versions. The latter, however, is the basic requirement of smooth updates and rollbacks in k8s.

It's important to note that for any production environment, it is recommended, to set the image by its digest, not by the tag. Kamu charts allow that. In this case nothing but users control the strictly selected version of the image. In this scenario, the pull policy may be specified as IfNotPresent since digests always change.

Copy link
Member

Choose a reason for hiding this comment

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

It's important to note that for any production environment, it is recommended, to set the image by its digest

This is exactly what I was wondering - whether this change was implying that we should deploy to prod while allowing version skew.

I agree this can be useful for quick setup and less important environments, but in prod I would like to know exactly which image is used by every single pod.


#########################################################################################

.PHONY: kamu-web-ui
kamu-web-ui:
docker build \
--build-arg KAMU_WEB_UI_VERSION=$(KAMU_WEB_UI_VERSION) \
-t $(KAMU_WEB_UI_IMAGE):$(KAMU_WEB_UI_VERSION) \
-t $(KAMU_WEB_UI_IMAGE):$(KAMU_WEB_UI_MAJOR_VERSION).$(KAMU_WEB_UI_MINOR_VERSION) \
Copy link

Choose a reason for hiding this comment

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

It's also necessary to push this new tag.

kamu-web-ui/


.PHONY: kamu-web-ui-push
kamu-web-ui-push:
docker push $(KAMU_WEB_UI_IMAGE):$(KAMU_WEB_UI_VERSION)
docker push $(KAMU_WEB_UI_IMAGE):$(KAMU_WEB_UI_MAJOR_VERSION).$(KAMU_WEB_UI_MINOR_VERSION)
docker tag $(KAMU_WEB_UI_IMAGE):$(KAMU_WEB_UI_VERSION) $(KAMU_WEB_UI_IMAGE):latest
Copy link

Choose a reason for hiding this comment

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

@sergiimk Is there any particular reason this latest tag is added here and not during the build stage?

Copy link
Member

Choose a reason for hiding this comment

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

No reason. Ideally I wanted to avoid producing a bunch of tags locally and only tag during push, but unfortunately docker does not allow something like docker push <image> -t <name1> -t <name2> -t <name3>.

Feel free to move to build phase for consistency.

docker push $(KAMU_WEB_UI_IMAGE):latest

Expand Down
Loading