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 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

Conversation

metiletan
Copy link

…-ui stage

@metiletan metiletan changed the title Add 2nd tag with major and minor versions to the image in at kamu-web… Add 2nd tag with major and minor versions to the image at kamu-web-ua Nov 22, 2023
@romcheg romcheg requested a review from sergiimk November 22, 2023 17:41
#########################################################################################

.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.

@@ -5,13 +5,17 @@ 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.

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.

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.

3 participants