PR#575: phases/ostree: Set rpm-ostree version from compose $date.$serial - pungi

pungi

#575 phases/ostree: Set rpm-ostree version from compose $date.$serial

Closed 7 years ago by lsedlar. Opened 7 years ago by walters.

walters/pungi set-rpmostree-version into master

phases/ostree: Set rpm-ostree version from compose $date.$serial

Colin Walters • 7 years ago

4d2c7f5

pungi/phases/ostree.py

file modified

+6 -1

		`@@ -127,7 +127,12 @@`
		`]`

		`version = config.get('version', None)`
		`- if version:`
		`+ if version is None and compose.compose_label is not None:`
		`+ # Currently for FAH, everything is RC, let's strip it, so we have`
		`+ # $major.$date.$serial`
lsedlar commented 7 years ago Where does the date get in?
lsedlar commented 7 years ago Ah, I checked the compose config and apparently the label is `RC-20170321.0`.
		`+ nonrclabel = compose.compose_label.strip('RC-')`
		`+ version = '{}.{}'.format(compose.conf['release_version'], nonrclabel)`
		`+ if version is not None:`
		`cmd.append('--version=%s' % version)`

		`if extra_config_file:`

walters commented 7 years ago

See: https://pagure.io/atomic-wg/issue/229

This is just the start of a longer thread around better integrating
the metadata between pungi and (rpm-)ostree.

Signed-off-by: Colin Walters walters@verbum.org

NOTE: Not tested locally.

dustymabe commented on line 8 of pungi/phases/ostree.py 7 years ago

should be config.get('version', None) I assume.

rebased

7 years ago

walters commented 7 years ago

Yep, fixed.

ausil commented 7 years ago

As you can see in the Jenkins job output there is invalid syntax in this request

walters commented 7 years ago

I fixed that earlier. It looks like compose_label isn't mocked? Hopefully that's what should be provided. I changed the code to not error out if it's None.

rebased

7 years ago

lsedlar commented on line 8 of pungi/phases/ostree.py 7 years ago

Where does the date get in?

lsedlar commented on line 8 of pungi/phases/ostree.py 7 years ago

Ah, I checked the compose config and apparently the label is RC-20170321.0.

lsedlar commented 7 years ago

The code change is very specific to Fedora Atomic Host composes. While right now that is the main (only even?) consumer of this functionality, I'm not sure it's a good idea to couple code with one particular use case.

Why are the cloud composes always done with RC label? Why is the type production and not nightly?

There is a somewhat similar use case for setting version and release for live media. For nightly composes without label it gets version from pungi config and release from date, type and respin (26-20170323.n.0). For labeled composes it's the major version with milestone (from label) and release is the rest of label (25_Beta-1.0). The values are generated here.

Could we reuse that code? With the strange RC-20170323.0 compose the version would be 25 and release 20170323.0, which you could just join with a dot manually. If you decided to remove the label and do it as a regular nightly compose, the value would become 25.20170323.n.0 or 25.20170323.0 for production compose without label.

For other phases that use autogenerated version, the logic only activates if the config explicitly defines version as None. That way it's opt-in and possible to not set version at all should someone want it.

walters commented 7 years ago

I don't see it as specific to FAH - it would also apply to FAWS I believe, although there we only have an installer ISO and ostree repo - no cloud images.

The baseline goal here is that the ostree commit version should be the same across architectures. Although actually this won't really achieve that in corner cases where a package only changes on one architecture. To do that we'd have to disable rpm-ostree's caching, or actually, we should invalidate the cache.

I don't know enough about pungi and the configuration to understand what you're suggesting. I'm not really in a position myself to easily test changing both the pungi config and pungi itself, I just made this patch by looking at the code.

Offhand, it sounds like what you're suggesting would be better, but someone with access to e.g. the staging environment is going to need to drive testing changes here.

lsedlar commented 7 years ago

I put my suggestion into #579.

lsedlar commented 7 years ago

Fixed in #592. Closing this one.