Skip to content
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

anaconda: preview #522

Merged
merged 2 commits into from
Mar 19, 2024
Merged

anaconda: preview #522

merged 2 commits into from
Mar 19, 2024

Conversation

supakeen
Copy link
Member

@supakeen supakeen commented Mar 17, 2024

Don't hardcode the Final value being passed to the buildstamp stage as reported in #515. We mark Anaconda based images as Preview which is inverted Final.

In other build systems this information comes from pungi which passes it on to koji which then writes it correctly into the image. I'd like for us to support the same but that would involve passing it down the API so let's start here with a small duplication of efforts.

I could be convinced to call the property Final instead so we don't deal with the inversion but it felt more natural to mark images as preview images than saying they're the "final" artifact.

The `Preview` property on Anaconda images and pipelines replaces the
previously hardcoded `Final` property being passed to the Buildstamp
stage.

Note that this is, again, a duplication of efforts so let's find out how
to pass the value of `Preview` on from the API.
@supakeen
Copy link
Member Author

Do we also need to set this for the CoreOS installer?

@AdamWill
Copy link
Contributor

AdamWill commented Mar 17, 2024

This needs to be set for any anaconda installer image. It controls whether anaconda shows the pre-release warnings, which we always want for any pre-release image, regardless of the payload. For the separate coreos installer...I dunno if that implements any pre-release warnings at present.

I am not sure the logic here works (though I don't know offhand exactly how we determine what the Branched number is). The problem is when we come to build the RCs for Final. We do not want to put the pre-release warnings on those images, because these are the composes we are aiming to actually release - ideally we build just one RC, and ship it. But at the time we build the RCs, the release is still Branched, it has not yet become a stable release.

This is why pungi's logic for this uses the compose "label" and considers composes with a label starting "RC-" to be "final". That is the label format used (only) for final release candidate composes.

@supakeen
Copy link
Member Author

supakeen commented Mar 17, 2024

Right, the CoreOS installer is not an Anaconda installer but does have a buildstamp file. I don't know if it does anything regarding previews.

As for the other clarification: that's a good point and a reason why this value should really be filled in from the requesting side. We have two options for that:

  1. A customization that goes all the way through the API (or blueprint).
  2. A separate field in the API to set this.

I'll CC people for that part: @thozza, @achilleas-k, and @ondrejbudai: for our Fedora builds we'd like to set a certain value in the buildstamp file that indicates if the image is a preview release. This information is known to pungi, in turn to koji, but after that we lose the information. How should we approach this?

And I'll also poke @pcdubs and @nullr0ute as the IoT and Minimal images are also affected by this logic.

@AdamWill
Copy link
Contributor

AdamWill commented Mar 17, 2024

Small correction: right now the info is not passed through to the koji-osbuild layer by pungi, but doing so would be trivial (see diff in #515 ).

@AdamWill
Copy link
Contributor

Also I don't think this is actually Fedora specific, I believe it also affects RHEL. We want pre-release RHEL images to clearly indicate their status just as much as (or more than) fedora ones, I would think.

@AdamWill
Copy link
Contributor

AdamWill commented Mar 17, 2024

Note it may still be practical to do something like this as a temporary workaround. I think osbuild does know the value I'm calling "label" (that's what productmd calls it) at some level, because it winds up in the filename. In e.g. Fedora-Workstation-Live-osb-40_Beta-1.7.x86_64.iso - the filename of the Workstation live image built by osbuild in the latest Beta candidate compose - the Beta-1.7 part is the "label". I don't know offhand where osbuild gets that string from, but clearly it has it somewhere.

edit: siiiigh. Beta-1.7 is indeed the compose's "label" in productmd terms (see the metadata), but I don't think that's how it gets in the filename...rather, I think in the filename we have the compose's "release" and "version" in pungi terms (which are also used as "release" and "version" for all koji tasks created for the compose), because those are 1.7 and 40_Beta respectively. So we've got something like %name-%version-%release, and the fact that this winds up creating a string that includes the productmd "label" is...not exactly a coincidence, but something like it.

In conclusion, names are awful and I hate all this nonsense. Anyway, if that pungi/koji "version" is making it down to osbuild in the right context, we could do something like "if the version has 'Beta' in it, set isfinal false". However, that doesn't help for nightlies, because on those the version is just "40"...

@AdamWill
Copy link
Contributor

AdamWill commented Mar 18, 2024

It might actually be logically easier (for this "have osbuild figure out whether the image is final, as a workaround" path) if we default to not final (I.e. pre-release) and figure out a condition to decide the images are final.

@achilleas-k
Copy link
Member

@AdamWill exposing it in the API and letting the build configuration (from pungi/koji) set it should solve the issue, right? Sorry, I'm just not sure if I'm understanding all the comments completely.

@AdamWill
Copy link
Contributor

AdamWill commented Mar 18, 2024

yes, if we can do that, it will definitely solve the issue. I was just struggling to figure out how to implement that.

Some of the discussion is around having osbuild "figure out" whether the image should be considered final - essentially duplicating what pungi does - as a possible workaround/short-term fix for this, if fixing it 'properly' is too hard to do quickly. That's what this PR is trying to do (only it won't really work, as written).

@supakeen
Copy link
Member Author

supakeen commented Mar 18, 2024

@AdamWill It depends, this fix is quick it'll take me a bit to work this through koji-osbuild and osbuild-composer.

My suggestion is; merge this PR for short term, leave open the issue to keep track of this for the service side of it. I'm also open to closing this PR.

@AdamWill
Copy link
Contributor

yeah...merging this right now would dtrt in practice at least until final. even for final...if we only have non-blocking images built by osbuild that actually run anaconda (is it just the experimental osbuild workstation live, or am I forgetting something?), it's not critical that this would make those still show the pre-release warning, I guess...

I was going to try and whip up a variant that would actually give the correct results, I might still try and do that, but it's not urgent.

@AdamWill
Copy link
Contributor

oh, we build IoT images with osbuild, I guess, and IoT has an installer image. so we would somehow want to make sure that for F40 final that image does not show pre-release warnings.

@achilleas-k
Copy link
Member

Alright, let's merge this and start pushing it up the stack to reach the service while we do the proper fix in parallel. For me, adding it as a customization and putting the control in the builders hands is better than trying to be smart about it.

@achilleas-k achilleas-k added this pull request to the merge queue Mar 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 18, 2024
@supakeen supakeen added this pull request to the merge queue Mar 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 18, 2024
@achilleas-k achilleas-k added this pull request to the merge queue Mar 19, 2024
@achilleas-k
Copy link
Member

I moved #526 ahead of this PR in the merge-queue which should make it pass.

Merged via the queue into osbuild:main with commit 6df70ab Mar 19, 2024
16 checks passed
@AdamWill
Copy link
Contributor

Has this been deployed yet? It looks like the osbuild ws live in the latest F40 beta candidate still doesn't have the pre-release warnings.

@supakeen
Copy link
Member Author

Has this been deployed yet? It looks like the osbuild ws live in the latest F40 beta candidate still doesn't have the pre-release warnings.

No, this is currently following our normal release schedule (which means it takes 1-2 weeks for it to end up on production). Does it need to go faster? I thought this was (for now) a relatively low-priority change :)

@AdamWill
Copy link
Contributor

it's not disastrous since the image it affects is not release blocking, it just would've been nice to have it fixed for the Beta. no big deal, I should've let you know.

@AdamWill
Copy link
Contributor

OK, so this has now gone wrong exactly the way I thought it would: we have F40 Final-targeted images showing the warnings when they shouldn't.

The IoT case is awkward because IoT doesn't have separate 'candidate' composes. It runs composes every day automatically, which in productmd terms are 'candidate' composes not 'nightly' composes (a difference from the main Fedora nightly composes), and we just pick one to release. probably all we can do here is implement some kind of lever for the IoT team to turn 'isFinal' on or off when they want to.

The Workstation case should be possible to handle better, though, because we should be able to distinguish when we're building an image as part of an RC compose and set it as final.

@AdamWill
Copy link
Contributor

AdamWill commented Apr 10, 2024

To write this down somewhere so I don't forget it, here's how this works for installer images not built by osbuild. Pungi has this heuristic which decides that any compose which has a label and whose label starts "RC-", "Update-", or "SecurityFix-" is 'supported'. When generating lorax commands, the two Pungi phases for building installer images using lorax - https://pagure.io/pungi/blob/master/f/pungi/phases/ostree_installer.py and https://pagure.io/pungi/blob/master/f/pungi/phases/buildinstall.py - use that compose.supported property when setting something that ultimately determines whether lorax is run with --isfinal (they each have two paths, for doing this via Koji or not, but they each use compose.supported on both paths).

This gets things right for mainline Fedora composes - nightlies and Beta candidates (and Alpha candidates, when we did those) get the warnings, Final candidates (which have "RC-" labels) do not. (I assume it also gets things right for RHEL composes, of course). For Fedora IoT composes it just meant they never, ever showed the warnings, because all IoT composes look more or less like "Final candidates" - they are all candidate composes for the RC milestone. Of course this wasn't really correct but we never bothered trying to fix it.

@thozza
Copy link
Member

thozza commented Apr 11, 2024

This makes me think that the most reasonable solution would be to make this customizable using BP customizations, which can be passed by Pungi when triggering the image build. IOW do not have the pre-release defined for specific Fedora versions in our tooling...

@AdamWill
Copy link
Contributor

yeah, that might well overall be the best, and matching how the old tools work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants