-
Notifications
You must be signed in to change notification settings - Fork 95
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
Move version to standalone file #378
Conversation
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.
Just a couple of recommendations. Looks great! Did we want to do this with bazel instead of make? Just a thought
Makefile
Outdated
|
||
.PHONY: release/versionbump | ||
release/versionbump: | ||
$(MAKE) CHANNEL=stable IS_DEFAULT_CHANNEL=1 release/update-pkg-manifest && \ |
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.
@alinadonisa correct me if I am wrong but I think we don’t release to stable right away.
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.
@alinadonisa correct me if I am wrong but I think we don’t release to stable right away.
@rail, @chris we will release on beta channel first and after the feedback we go for the stable channel
RH_DEPLOY_PATH="deploy/certified-metadata-bundle" | ||
RH_DEPLOY_FULL_PATH="$(RH_DEPLOY_PATH)/cockroach-operator/" | ||
RH_COCKROACH_DATABASE_IMAGE=registry.connect.redhat.com/cockroachdb/cockroach:v20.2.5 | ||
RH_OPERATOR_IMAGE?=registry.connect.redhat.com/cockroachdb/cockroachdb-operator:v1.6.12-rc.1 | ||
RH_COCKROACH_DATABASE_IMAGE=registry.connect.redhat.com/cockroachdb/cockroach:$(COCKROACH_DATABASE_VERSION) |
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.
We are going to have to push all of the versions in the operator.yaml - @keith-mcclellan comments?
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.
So if we're pushing a version of the operator container, we decided we're always going to push a metadata bundle even if nothing has changed and vice versa. Otherwise it'll get impossible to track the inter-dependency.
@rail we should keep the generated files that are published on OpenShift on git. If we could automate this behaviour it would be great. For now it is enough to use the generated files from git. Please take in consideration that if the CI fails we will need to increment the RH_BUNDLE_VERSION, regenerate the metadata and push again.
|
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.
Please take in consideration that if the CI fails we will need to increment the RH_BUNDLE_VERSION, regenerate the metadata and push again.
I believe with the proposed approach because we use the same version for the operator and the bundle, this should be less an issue.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @alinadonisa, @chris, and @chrislovecnm)
Makefile, line 181 at r1 (raw file):
Previously, alinadonisa (Alina) wrote…
@alinadonisa correct me if I am wrong but I think we don’t release to stable right away.
@rail, @chris we will release on beta channel first and after the feedback we go for the stable channel
I'll set the default values in Makefile
to use beta and add some instructions in README.md
.
@rail Chris made a bunch of changes to the makefile to resolve a bunch of build issues and move us to bazel 4, can you update this? Happy to give you my feedback at that point. |
Before this change in order to bump the operator version one would need to edit `Makefile` and `manifests/operator.yaml` to set the operator version. Also the OpenShift metadata bundle version has to be set and someone has to run a command to generate a new directory with metadata files. This patch moves the version into a separate file to make it more automation friendly. At the same time it starts using the same version (except the "v" prefix) for OpenShift metadata bundle. Added a make target that automates a regular release preparation step.
Rebased. |
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
I would appreciate your feedback on this change.
One question that I have is wether we should keep the generated metadata commited in the repo or we generate it runtime as a part of the release process and discard the changes.
Before this change in order to bump the operator version one would need
to edit
Makefile
andmanifests/operator.yaml
to set the operatorversion. Also the OpenShift metadata bundle version has to be set and
someone has to run a command to generate a new directory with metadata
files.
This patch moves the version into a separate file to make it more
automation friendly. At the same time it starts using the same version
(except the "v" prefix) for OpenShift metadata bundle.
Added a make target that automates a regular release preparation step.
This change is