-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Support pushing images to other container registries #4060
Support pushing images to other container registries #4060
Conversation
* add easy way to add new registries * push images to github container registry * ability to run locally * better tagging of development images Signed-off-by: paulfantom <pawel@krupa.net.pl>
I would like to merge this before 0.49.0 (I am a release shepherd for this version). If/When this is merged I will enable container registry for prometheus-operator. /cc @prometheus-operator/prometheus-operator-reviewers |
@@ -9,9 +9,8 @@ else | |||
endif | |||
|
|||
GO_PKG=github.com/prometheus-operator/prometheus-operator | |||
REPO?=quay.io/prometheus-operator/prometheus-operator | |||
REPO_PROMETHEUS_CONFIG_RELOADER?=quay.io/prometheus-operator/prometheus-config-reloader | |||
REPO_PROMETHEUS_OPERATOR_LINT?=quay.io/prometheus-operator/prometheus-operator-lint |
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.
Lint image is not used anywhere 🤷
For other images, I changed the variable names for clarity. During release values of those variables do not matter as images are retagged by publisher script.
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.
should we remove po-lint
completely?
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.
I do believe there are better tools for the job (like kubeconform
) and we should point to those instead of maintaining our own tool for the same thing. Plus the same feature is actively being implemented in kubectl
.
However, let's do that in a separate PR (here is an issue: #4099). I'll leave this line here though as it is dead code either way.
# exit immediately when a command fails | ||
set -e | ||
# only exit with zero if all commands of the pipeline exit successfully | ||
set -o pipefail | ||
# error on unset variables | ||
set -u |
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.
GITHUB_REF
is not set when using the script locally, hence I deleted this check.
export TAG="${GITHUB_REF##*/}" | ||
|
||
# Push `-dev` images unless commit is tagged | ||
export REPO="${REPO:-"quay.io/prometheus-operator/prometheus-operator-dev"}" | ||
export REPO_PROMETHEUS_CONFIG_RELOADER="${REPO_PROMETHEUS_CONFIG_RELOADER:-"quay.io/prometheus-operator/prometheus-config-reloader-dev"}" | ||
IMAGE_SUFFIX="-dev" |
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.
Does anyone actually use those -dev
images? We don't list them anywhere in any docs so discovery is a bit hard. Also, those are build only from master
branch and branches in the repo, so not really helping during standard development workflow. Maybe we should just remove them? @prometheus-operator/prometheus-operator-reviewers WDYT?
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.
No strong opinion but I can see how it can be helpful when you want to have users testing a bug fix or a change without having them to rebuild locally.
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.
I would agree if those images were built from PRs. As it is right now, the maintainer would need to create a branch to test the bug fix or a change and wait for CI to publish images. Personally, I would be in favor of simplifying image building process than keeping those -dev
images.
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.
Fair enough. If nobody had ever used this possibility, I agree that it's not worth keeping it.
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.
lgtm
Signed-off-by: paulfantom pawel@krupa.net.pl
REGISTRIES
var)Release Note Template (will be copied)