search

cloudfoundry-incubator / kubecf

Cloud Foundry on Kubernetes
Apache License 2.0
115 stars 62 forks source link

Unreleased semver should contain last released tag and be sortable by timestamp #1415

Open viccuad opened 4 years ago

viccuad commented 4 years ago

Is your feature request related to a problem? Please describe.

Having all unreleased versions be like 0.2.0-g<hash>-<timestamp> (example kubecf-v0.2.0-1996.g1f49deea.tgz) is messy:

See the versions on: https://s3.console.aws.amazon.com/s3/buckets/kubecf/?region=us-west-2&tab=overview released from https://concourse.suse.dev/teams/main/pipelines/kubecf/jobs/publish-master

Describe the solution you'd like

One that:

Bonus points if parts of the version string such as git hash, timestamp, PR number are discernible because we are not using always hyphens to separate them, but an underscore or the like. This simplifies regexps.

Example: <latest-released-tag>-unreleased-<timestamp>_g<hash>_<whatever you want> (example 2.6.0-unreleased_202010011538_g1f49deea) Timestamp should be ISO date style without hyphens: 202010011538 is first of october of 2020, at 15:38 PM. This is alphanumerically sortable.

Describe alternatives you've considered

Additional context

https://semver.org/ http://www.fifi.org/doc/debian-policy/policy.html/ch-versions.html

jandubois commented 4 years ago

There has been a lot of discussion about the version format in https://github.com/cloudfoundry-incubator/kubecf-tools/pull/2. It is a pretty long and confusing discussion, but it touches most/all of the points you raise here. I think we have to start from the end of that thread and not start over from scratch.

The main problem right now is that the release branch is not being merged back to master whenever we tag a release (and this only applies for making new "latest" releases, the 2.2.3 release would not be merged because at that time 2.3.0 was already out). The resulting versions are sortable by semver rules, as long as they come from the same branch. There is no inherent sorting relationship between commits from different branch; you will have to fall back on build time, which is semantically meaningless.

If you read the previous thread, you'll see that many issues are supposed to be resolved by our workflow and cannot be dealt with purely by the versioning code.

As for including build timestamps, those would be "build elements" that would be appended to the end of a semver following a + sign. You'll see that the discussion about the versioning module explicitly excludes any "build elements", as they should not be derived from the git repo, but have to be appended by the build system itself.

prabalsharma commented 4 years ago

Marked it as Critical bug, as this wrong tag causes pre release pipelines to not pick up latest master builds.

s3 concourse resource can lock the latest build only if its version tag is same or larger than that the previous build's version tag. Right now, the latest is 0.2.0 which is smaller than the previous one i.e 2.5.0

jandubois commented 4 years ago

Marked it as Critical bug, as this wrong tag causes pre release pipelines to not pick up latest master builds.

The main version can be fixed by merging the latest release into the master branch. This is a workflow issue, not a code issue.

But how is the pipeline picking the latest build when there are multiple builds using the same version? Versions will need to be sorted using proper semver comparison; it is not a simple string or numeric comparison.

viovanov commented 4 years ago

@prabalsharma 0.2.0 is not "latest", it's a master development build "latest" is 2.5.0

viovanov commented 4 years ago

Is your feature request related to a problem? Please describe.

Having all unreleased versions be like 0.2.0-g<hash>-<timestamp> (example kubecf-v0.2.0-1996.g1f49deea.tgz) is messy:

  • we don't know which builds correspond to what version.

I don't understand, there's a commit hash associated with a build, if that commit hash is not tagged.

  • we can't order them alphanumerically so they reflect the timestamp order.

As @jandubois mentioned, you have to use semver sorting.

  • we are relying on the timestamp creation of the file when it gets added into s3 in CI; troublesome because if we republish an old tgz, it looks as if it were new.

That's not an issue of versioning; whatever uses that behavior should implement semver instead.

See the versions on: https://s3.console.aws.amazon.com/s3/buckets/kubecf/?region=us-west-2&tab=overview released from https://concourse.suse.dev/teams/main/pipelines/kubecf/jobs/publish-master

Describe the solution you'd like

One that:

  • bumps the previous released version
  • marks the version as unreleased, and makes it be bigger than the previous released version: 2.5.0 < 2.6.0-unreleased < 2.6.0.

This is already happening. kubecf-bundle-v0.2.0-1996.g1f49deea.tgz

See the semver definion: https://semver.org/#spec-item-11, 11.3:

When major, minor, and patch are equal, a pre-release version has lower precedence than a normal version: Example: 1.0.0-alpha < 1.0.0.

  • and is sortable by date

Yes, any master build is < than any released build.

Bonus points if parts of the version string such as git hash, timestamp, PR number are discernible because we are not using always hyphens to separate them, but an underscore or the like. This simplifies regexps.

I don't know what you mean, we have a spec here, everything looks ok and discernable to me. We should try to use semver libraries where possible, not our own regular expressions.

Example: <latest-released-tag>-unreleased-<timestamp>_g<hash>_<whatever you want> (example 2.6.0-unreleased_202010011538_g1f49deea) Timestamp should be ISO date style without hyphens: 202010011538 is first of october of 2020, at 15:38 PM. This is alphanumerically sortable.

You can use a date as an additional pre-release identifier, as the spec suggests. I don't find it particularly useful for master builds though, as you can't force push to master.

Describe alternatives you've considered

Additional context

https://semver.org/ http://www.fifi.org/doc/debian-policy/policy.html/ch-versions.html

jandubois commented 4 years ago

You can use a date as an additional pre-release identifier, as the spec suggests.

Nit-picking: It would be a build identifier, not a pre-release identifier.

It has been mentioned here: https://github.com/cloudfoundry-incubator/kubecf-tools/pull/2#issuecomment-658439124

Except the sample version is wrong v0.0.0-nightly.20200715 should be v0.0.0-nightly+20200715 because the timestamp is a build identifier (it contains information about the build, not about the source code).

jandubois commented 4 years ago

/me sighs

I just realized that build identifiers are ignored for the purpose of precedence in semver comparison. 😿

So I guess you have to use the build timestamp as a pre-release identifier, even though that would be conceptually wrong.

/me adds version strings to the list of hateful things

time zones
daylight savings time
character encodings
distributed stateful systems
[... long list elided ...]
version strings
prabalsharma commented 3 years ago

format kubecf-bundle-v0.0.0-20201102205022.1989.g2d94fe65.tgz does not work for concourse. At leat with old builds being in the same bucket. Maybe if we move new builds to a master-builds/ folder it might work. Right now for s3 regex kubecf-bundle-v0.0.0-(.*).tgz, concourse s3 resource picks build 2020-06-17 23:10:07 90689 kubecf-bundle-v0.0.0-fef5111.tgz instead of latest master build with format kubecf-bundle-v0.0.0-20201102205022.1989.g2d94fe65.tgz

jandubois commented 3 years ago

It was my understanding that @viccuad moved all the old builds out of the bucket when we switched to the new naming system.

viccuad commented 3 years ago

I didn't do that yet, since when we changed the naming master was failing for unrelated reasons (stemcell image missing), and that would have left us with no builds in the bucket.

The change to consume the new format should be trivial, we need to just change the regexp. Opened https://github.com/SUSE/cap/pull/72

I think this issue can be closed, it got duplicated by https://github.com/cloudfoundry-incubator/kubecf/issues/1523, which is already closed thanks to https://github.com/cloudfoundry-incubator/kubecf-tools/pull/12.