-
Notifications
You must be signed in to change notification settings - Fork 1
Add 2nd tag with major and minor versions to the image at kamu-web-ua #243
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
Open
metiletan
wants to merge
2
commits into
master
Choose a base branch
from
version
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
|
||
| ######################################################################################### | ||
|
|
||
| .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) \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's also necessary to push this new tag. |
||
| kamu-web-ui/ | ||
|
|
||
|
|
||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.minorapp version in charts, i.e. allowing the same chart to pull a newer patched application version?Would this necessitate
pull: Alwaysin all deployments?// Please include more context in PR description in future.
There was a problem hiding this comment.
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.
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.TL;DR
The intent is to allow version skew between charts and applications. Setting
Alwayspull policy will be required, but it's not a problem. Details bellow.The charts will only reference the
major.minortag, 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 toAlways. 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
IfNotPresentsince digests always change.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.