-
Notifications
You must be signed in to change notification settings - Fork 554
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
push images to quay.io with canary tag #302
Conversation
Lgtm. Thanks |
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 am fine with the changes, just a couple of questions/suggestions.
Further, shouldn't we change the deploy script in the 0.3 branch for the canary to take effect, as any PR merged to that branch would use the deploy script from its branch and hence the change needs to be reflected there as well?
@@ -17,10 +17,10 @@ | |||
CONTAINER_CMD?=docker | |||
|
|||
RBD_IMAGE_NAME=$(if $(ENV_RBD_IMAGE_NAME),$(ENV_RBD_IMAGE_NAME),quay.io/cephcsi/rbdplugin) | |||
RBD_IMAGE_VERSION=$(if $(ENV_RBD_IMAGE_VERSION),$(ENV_RBD_IMAGE_VERSION),v1.0.0) | |||
RBD_IMAGE_VERSION=$(if $(ENV_RBD_IMAGE_VERSION),$(ENV_RBD_IMAGE_VERSION),v1.0.0-canary) |
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.
(nit) should the defaults be something else other than canary? like "test" or "dev" or so.
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.
deploy.sh
Outdated
export CEPHFS_IMAGE_VERSION='v0.3.0-canary' | ||
elif [ "${TRAVIS_BRANCH}" == 'master' ]; then | ||
export RBD_IMAGE_VERSION='v1.0.0-canary' | ||
export CEPHFS_IMAGE_VERSION='v1.0.0-canary' |
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.
(Question) What happens to Helm charts, do these get canary versions as well automatically when they are pushed?
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.
helm chats will be pushed only if there is a change in 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.
Tried joining the dots, but was unable to, so here are my questions,
-
Assume we do 'canary' builds for the plugins, and one of the PRs changed the version in the helm charts, how will this be published here, for use with the appropriate plugin version?
-
When we make a release, i.e move a version out of 'canary', we would need to push the helm charts for that release as well, even if there are no changes, right?
-
As a user, how do I know which helm chart brings in what release of the plugins into my cluster?
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.
Tried joining the dots, but was unable to, so here are my questions,
- Assume we do 'canary' builds for the plugins, and one of the PRs changed the version in the helm charts, how will this be published here, for use with the appropriate plugin version?
this is not there yet to figure out which helm version is support for ce[h-csi version, need to figure it out.
- When we make a release, i.e move a version out of 'canary', we would need to push the
helm charts for that release as well, even if there are no changes, right?
if we are including the helm chats in the release, yes we have to
- As a user, how do I know which helm chart brings in what release of the plugins into my cluster?
need to add a document section for this one, one thing what we can do is we can have the same release version number for helm chat and csi plugin
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.
It's a bit confusing to apply a -canary
name on a tagged version, I think it's confusing become people won't probably realize that v0.3 is not the ceph-csi version but the csi spec version.
I believe we should rethink that a little bit.
What's the strategy to let's say push a new version of our ceph-csi driver for the csi v0.3 spec. Are we going to increment? v0.3.x?
added
|
It's uggly but typically a tag for ceph-csi might look like
With that we can easily increment like |
It is useful to have a CSI version and then the ceph-csi plugin version, so what is suggested above looks good.
What about per PR builds per version, this is where IMO the canary for that version helps for whoever wants to test/use the bleeding edge. IOW, like what is used in kubernetes-csi. We can again adopt (or use our own rules) as to when a canary is released, and we up the "z" version for the next release. |
@ShyamsundarR @leseb IMO we can follow |
With the possibility that in the future we maybe compatible to multiple CSI spec versions with the same plugin binaries, I now think we should NOT specialize our plugin version using the CSI spec that it supports. Instead we can call out a supported version matrix on the README.md and retain the compatibility information there. This seems like a more prudent approach at the moment. Considering this, versioning and other process artifacts can look as follows, RELEASE-VER: vx.y.z
Branch: csi-v0.3
NOTE: All future branches adopt the same strategy as above. Branches are created per major (or above) release number changes. Branch: master
This would possibly lead to the next set of questions on when releases are made, and life-cycle of releases etc. we possibly need to tackle that next. |
whats pending on this PR? making |
That is one if we agree to what is proposed, I would like to understand if the proposal is fine and what we need to tune for the same? |
updated deploy.sh script to push images to quay.io with canary tag. Once we merge a PR against master and csi-v0.3 branches we need to build and push the latest image with canary tag. by doing this we can avoid the accidental update of the images which are already deployed with stable tags (i.e v1.0.0 or v0.3.0). Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
updated PR |
Lgtm. Thanks |
Syncing latest changes from upstream devel for ceph-csi
updated deploy.sh script to push images to quay.io with canary tag.
Once we merge a PR against master and csi-v0.3 branches we need to build and push the latest
image with canary tag. by doing this we can avoid the accidental update of the images which
are already deployed with stable tags (i.e v1.0.0 or v0.3.0).
Signed-off-by: Madhu Rajanna madhupr007@gmail.com