adding a comment so that I get notifications from this PR

I'm a bit confused - we only drop into this section if 'version in config- but don't we want to do it ratherif 'version' not in config`?

There are three possible outcomes: either the version should not be used at all, or it should use some fixed value, or it should be automatically generated. The patch assumes that if version is not specified in the config at all, it should not be used. If it has some non-empty value, it will be used as is, and the automatic generation only happens when the value in config is empty (either None or empty string).

I see, I'd missed that we were intentionally differentiating None vs ''. Basically for Fedora we should set version = ''.

OK, that's fine.

Could we possibly be more explicit?

• version not specified => "should not be used"
• version non-empty => "value used as is"
• version == 'GENERATE' => "generate a value to be used"

We could possibly even make this more generic and come up with different generators that are used based on the certain constant values used as the version.

@dustymabe That's a good suggestion. I agree that triggering the auto generator on None is really not intuitive.

So how about this:

• Pungi config without any version → rpm-ostree called without version

• Pungi config with a string as a version → rpm-ostree called with that particular version

ostree = [
    ("^Atomic$", {
        "aarch64": {
            …,
            "version": "my-latest-version",
        }
    })
]

These two options already work, and the proposed addition would be:

• Pungi config with version set to a specific constant → a value is generated by some particular generator (this is replacement for the explicit None)

ostree = [
    ("^Atomic$", {
        "aarch64": {
            …,
            "version": AUTOGENERATE_LABEL_OR_DATE_TYPE_RESPIN,
        }
    })
]

It probably needs a better name. It would also be nice to reuse the same approach with constants where other versions or releases are generated (at least for live media and image build).

I updated the patch with the proposal above. It's not currently used for other version consumers than ostree.

Offhand, it seems reasonable to me. Let's try it in stage?

Let's see what happens: https://koji.stg.fedoraproject.org/compose/rawhide/Fedora-Rawhide-20170330.n.1/

Both started tasks seem have the version filled in correctly.
https://koji.stg.fedoraproject.org/koji/taskinfo?taskID=90036811
https://koji.stg.fedoraproject.org/koji/taskinfo?taskID=90036812
Let's try also with a label Beta-1.2: https://koji.stg.fedoraproject.org/compose/rawhide/Fedora-26-20170330.0/

Looking at the output, did that write to:

https://kojipkgs.fedoraproject.org/compose/atomic/rawhide/

?

Feels like it's missing a /stage/ in there?

Actually, what is the difference between /compose/atomic/rawhide and just /atomic/rawhide? They appear to be the same. So this task wrote to the "production" rawhide repo?

I don't actually see a new commit there though.

The tasks to create the commit are not yet finished, they are still waiting for a builder to take them. They should write here: https://koji.stg.fedoraproject.org/compose/ostree/rawhide/

The naming of things in stage is really messed up: I started with a rawhide config, but currently it's using f26 tag to take packages from (which lags behind production a bit anyway).

And it didn't work: in the runroot we also run a script provided by pungi package. However in stage the buildroot contained old and buggy version 4.1.12, which lead to failure in the tasks. I have updated the buildroot and restarted the composes. There is issue #395 about removing this problematic part.

https://koji.stg.fedoraproject.org/compose/rawhide/Fedora-Rawhide-20170331.n.0/
https://koji.stg.fedoraproject.org/compose/rawhide/Fedora-26-20170331.0/

Now bwrap is failing: https://koji.stg.fedoraproject.org/koji/taskinfo?taskID=90037016
However the --add-metadata-string=version=Rawhide.20170331.n.0 argument is passed to rpm-ostree correctly.

so that should be working with the 2017.3-2 version of rpm-ostree, although can we get the latest version 2017.3-3 of rpm-ostree into this compose?

@walters is there some reason why your patch would not be properly detecting systemd-nspawn and thus still trying to drop net privs?

Hm, I don't think we want the Rawhide. prefix there, right?

As far as debugging the nspawn/bwrap issue - let's verify that stage has the same versions of mock/rpm-ostree etc which are known to work?

Hm, I don't think we want the Rawhide. prefix there, right?

That is the "version" in the case of rawhide, right? It would be 26 for fedora 26 i think.

As far as debugging the nspawn/bwrap issue - let's verify that stage has the same versions of mock/rpm-ostree etc which are known to work?

From the output mock 1.3.4 is being used which is the version that had all the fixes we needed. Also it has rpm-ostree 2017.3-2 installed into the mock chroot, so that should be a version that works as well since it should have the patch for detecting systemd-nspawn and not dropping privs. Either way please let's try with 2017.3-3 and see what happens.

bump

I have imported ostree-2017.3-2.fc26 (which is the latest in production koji) into stage and restarted the task:
https://koji.stg.fedoraproject.org/koji/taskinfo?taskID=90037856

need rpm-ostree-2017.3-3. I think this gets installed inside the mock chroot.

can you update ticket with link to the last compose you ran? did it succeed?

The compose is here: https://koji.stg.fedoraproject.org/compose/rawhide/Fedora-26-20170404.0/
The task failed again due to a misconfigured repo: https://koji.stg.fedoraproject.org/koji/taskinfo?taskID=90037921
I'm not sure yet if that is a new bug in pungi (most likely not related to this PR) or if I have a mistake in the config.
Particularly I'm not sure where the bad repo it's complaining about is coming from: https://koji.stg.fedoraproject.org/compose/rawhide/Fedora-26-20170404.0/work/ostree-1/

The compose is here: https://koji.stg.fedoraproject.org/compose/rawhide/Fedora-26-20170404.0/
The task failed again due to a misconfigured repo: https://koji.stg.fedoraproject.org/koji/taskinfo?taskID=90037921
I'm not sure yet if that is a new bug in pungi (most likely not related to this PR) or if I have a mistake in the config.
Particularly I'm not sure where the bad repo it's complaining about is coming from: https://koji.stg.fedoraproject.org/compose/rawhide/Fedora-26-20170404.0/work/ostree-1/

The failure is caused by mis-match versions of Pungi on compose server and brew, the version on brew doesn't have this commit: https://pagure.io/pungi/pull-request/581 .

@qwan Thank you, that explains a lot. I completely forgot about that one. I have reverted the commit on compose box (it's running from a git checkout) and restarted the compose: https://koji.stg.fedoraproject.org/compose/rawhide/Fedora-26-20170405.0/

Victory: https://koji.stg.fedoraproject.org/koji/taskinfo?taskID=90037972
The final repo is in https://koji.stg.fedoraproject.org/compose/atomic/rawhide/

yay.. So the version in that ostree is Version: 26_Beta.1.2. Can we get one without a label that matches the 20170405 "generated" version scheme?

That version was generated for a production compose with label Beta-1.2. If there was no label (which for production compose requires --no-label command line option), the generated value would be 26.20170405.1 (it's a second compose today). I verified this by starting the compose and checking in debugger.

I restarted the koji task with that version: https://koji.stg.fedoraproject.org/koji/taskinfo?taskID=90037977
The output will end up here: https://koji.stg.fedoraproject.org/compose/atomic/26/

looks good to me:

[vagrant@vanilla-f25atomic ~]$ rpm-ostree status
State: idle
Deployments:
  fedora-atomic:fedora/26/x86_64/atomic-host
             Version: 26.20170405.1 (2017-04-05 14:35:20)
              Commit: f39cdfd8d5e776b64353930f3288bc3e90b71c2edaab46b7b3c27010bfea2aea
              OSName: fedora-atomic

@walters - any objection to merging this?

LGTM 👍

@lsedlar - do you know when this might be able to land in prod for f26?

@dustymabe In theory there's nothing blocking this. I'm not very happy with this particular implementation (it feels like over-complicated to me). I had an idea for an alternative (#589), but that does not feel much better either.

@lsedlar can you summarize the difference between #589 and this PR?

The config file is using a parser and format defined in kobo.

Both implementation rely on details of the fact that the config can use variables (as in assign value of one option to another one). Once the config is parsed, the code works with a big Python dict with all the data and all references are resolved.

In this PR we inject the OSTREE_VERSION… as a name for a special singleton object. Whenever the version is needed, and the config contains this special object (as opposed to a string), we would expand it. The downside is that the config is no longer a simple structure (so for example I needed to extend a JSON encoder that creates a dump of the configuration).

The other approach defines the name to point to the actual version value, so that code using the option can just expect a string there. The downside is that we need to parse the config twice (first with the predefined constants empty so that we can generate the values for them, and then with correct values).

@lsedlar and I had a conversation about this. Rather than making things complicated we decided to pursue an approach that uses a simple string value of "OSTREE_VERSION_FROM_LABEL_DATE_TYPE_RESPIN" in the config and have the code do the generation of the version based on that string. This should simplify the code and also allow for other generated version strings to be used in the future if anyone comes up with a need for other versioning.

The third approach is in #592. I dislike that approach the least. I also prefixed the "magic" strings with a ! character to make it even more obvious something is going on.

Fixed in #592. Closing this one.