Disable grub-boot-success in RHEL 9 edge commits to fix greenboot interference #51

say-paul · 2023-07-25T10:53:18Z

grub_boot_success is stepping over greenboot as it is marking boot as successful in case of any ssh connection that persists over two min. The fix is to update the unit stage to add dropin for user service and then adding the stage for edge-container and commits with the condition that grub-boot-success.timer not to trigger if greenboot exists.

This pull request includes:

adequate testing for the new functionality or fixed issue
adequate documentation informing people about the change such as
- submit a PR for the guides repository if this PR changed any behavior described there: https://www.osbuild.org/guides/

supakeen · 2023-07-25T11:13:00Z

This change correctly disables the service(s) though I think this should be done through systemd presets; I'd like acks from @nullr0ute and @runcom. I'd assume this change to be temporary (?) and I'd like the bug report to be linked in the comments you've added to the disabledServices and disabledIotServices.

I'm also reasonably sure Fedora IoT already disables this in their presets as it's not running on my IoT instances.

say-paul · 2023-07-25T12:45:29Z

It will stay until we evaluate and decide if we want to drop the entire grub-tools.

say-paul · 2023-07-25T14:29:55Z

Fixes fedora-iot/greenboot#108

runcom · 2023-07-26T10:41:14Z

This change correctly disables the service(s) though I think this should be done through systemd presets; I'd like acks from @nullr0ute and @runcom. I'd assume this change to be temporary (?) and I'd like the bug report to be linked in the comments you've added to the disabledServices and disabledIotServices.

I'm also reasonably sure Fedora IoT already disables this in their presets as it's not running on my IoT instances.

@supakeen sure thing - for fedora-iot it definitely makes sense to disable this via presets, although I can't see it disabled in https://src.fedoraproject.org/rpms/fedora-release/blob/rawhide/f/80-iot.preset - does it make sense to understand why it's not running on your iot systems them too? then we can just disable it in the presets dang, I just figured it's here https://src.fedoraproject.org/rpms/fedora-release/blob/rawhide/f/80-iot-user.preset - so we probably don't need this change at all for IoT - @say-paul is there a reason you changed it for fedora too?
For RHEL we don't have presets so for now yes, this change makes sense

pkg/distro/rhel9/edge.go

say-paul · 2023-07-26T12:05:50Z

I just figured it's here https://src.fedoraproject.org/rpms/fedora-release/blob/rawhide/f/80-iot-user.preset - so we probably don't need this change at all for IoT - @say-paul is there a reason you changed it for fedora too?

It is ignorance from my side as I didn't know where to look for the fedora presets, now I know, will revert the changes accordingly.

achilleas-k

LGTM. Thanks!

achilleas-k · 2023-07-26T15:49:50Z

Builds are failing with

Failed to disable unit, unit grub-boot-success.timer does not exist.

on CS9, RHEL 9.2, and RHEL 9.3. It's working on RHEL 9.0 and 9.1.

Note that rhel-9 is currently an alias for 9.1 (we should fix that).

say-paul · 2023-07-27T05:51:17Z

I guess we can leverage : org.osbuild.systemd.preset to disable as presets similar to fedora.

say-paul · 2023-07-27T06:16:09Z

tried to disable it via preset: using org.osbuild.systemd.preset in the new commit , but I am not seeing that its executed as part of the pipeline in osbuild-composer, what am I missing?

supakeen · 2023-07-27T06:43:56Z

Stuffing things in ImageConfig won't make them appear on the pipeline object; you can either add the code that puts it in the OSCustomizations (osCustomizations in images.go reads from ImageConfig if set and uses that). Or you can directly set it on the pipeline; for example how InstallWeakDeps is set in images.go:337.

As an aside; I always get fuzzy when thinking about these things in ostree but I think you'd want to set presets in the commit. Not during deployment.

pkg/manifest/ostree_deployment.go

runcom · 2023-07-27T07:53:09Z

I guess we can leverage : org.osbuild.systemd.preset to disable as presets similar to fedora.

achilleas mentioned the unit fails to be located at all when disabling, why is that tho? how presets can change that?
look at this commit for presets also achilleas-k/osbuild-composer@c2f7ddd

achilleas-k · 2023-07-27T09:00:27Z

Converted to draft to avoid merging accidentally on green.

achilleas-k · 2023-07-27T09:10:56Z

@say-paul Current approach looks good, you just need to wire up the EnabledPresets and DisabledPresets from ImageConfig down to the NewOSTreeDeployment pipeline.

Here:

images/pkg/image/ostree_raw.go

Lines 80 to 81 in 17eb47b

    
           osPipeline.EnabledServices = img.Workload.GetServices() 
        
           osPipeline.DisabledServices = img.Workload.GetDisabledServices()

But since it's a new option on ImageConfig that can be used anywhere, you should also look for other pipelines to wire it up for other image types, like in OSCustomizations.

runcom · 2023-07-27T09:47:24Z

@say-paul Current approach looks good, you just need to wire up the EnabledPresets and DisabledPresets from ImageConfig down to the NewOSTreeDeployment pipeline.

although, we want it in the commit right?

say-paul · 2023-07-27T09:58:36Z

@runcom we need in both commit and deployment.

runcom · 2023-07-27T10:03:54Z

@runcom we need in both commit and deployment.

why would that be?

say-paul · 2023-07-27T10:51:45Z

@runcom don't we want it anyway if the commit doesn't have it setup? Though I am thinking presets will be written twice if we have add the stage during both commit and deployment..

achilleas-k · 2023-08-17T09:29:55Z

Found my mistake. The generator scripts also use osbuild to generate the manifest ID by inspecting the manifests, so they need the pinned osbuild as well and I missed it.
Should be good now.

pkg/manifest/os.go

We only test on Fedora 38. Remove all the other distro specifications. We can bring them back when we need them again.

osbuild/osbuild@61da612

New script for setting up high priority repository for installing an osbuild RPM built from a specific commit for testing against versions before they're released.

thozza

Changes look good, except for putting unconditional configuration of the image in distro/rhel9/images.go. I would say that even the conditional configuration above should not be really here. It should be conditionally added to the image's ImageConfig.

This is not the place for specific image configuration, this function is only the "glue" to pass configuration values to other parts of the code.

pkg/distro/rhel9/images.go

pkg/distro/rhel9/edge.go

The stage is modified to add dropin file for user service this is achieved by adding a unit-type which can be set to system or global.Seperate struct is created for the unit section parameters accepted by systemd. Relevant testing added. Signed-off-by: Sayan Paul <paul.sayan@gmail.com>

This will disable the grub-boot-success timer if greenboot exists to ensure greenboot functions correctly and the timer does not modify the boot_success flag when a ssh session persists for more than 2 min. This customization is added to containerImage and edgeCommitImage. Signed-off-by: Sayan Paul <paul.sayan@gmail.com>

say-paul · 2023-08-21T07:21:38Z

Changes look good, except for putting unconditional configuration of the image in distro/rhel9/images.go. I would say that even the conditional configuration above should not be really here. It should be conditionally added to the image's ImageConfig.

This is not the place for specific image configuration, this function is only the "glue" to pass configuration values to other parts of the code.

@thozza Done

Done

thozza

LGTM

say-paul force-pushed the rhel_disable_grub_success branch 2 times, most recently from 89ea2f9 to 2e36aef Compare July 25, 2023 11:04

say-paul mentioned this pull request Jul 25, 2023

grub-boot-success.timer interfering with greenboot fedora-iot/greenboot#108

Open

say-paul force-pushed the rhel_disable_grub_success branch from 2e36aef to 2f17a2b Compare July 25, 2023 12:36

runcom reviewed Jul 26, 2023

View reviewed changes

pkg/distro/rhel9/edge.go Outdated Show resolved Hide resolved

say-paul force-pushed the rhel_disable_grub_success branch from 2f17a2b to 23580b4 Compare July 26, 2023 12:22

achilleas-k previously approved these changes Jul 26, 2023

View reviewed changes

achilleas-k enabled auto-merge July 26, 2023 14:15

runcom previously approved these changes Jul 26, 2023

View reviewed changes

say-paul dismissed stale reviews from runcom and achilleas-k via 17eb47b July 27, 2023 06:10

say-paul commented Jul 27, 2023

View reviewed changes

pkg/manifest/ostree_deployment.go Outdated Show resolved Hide resolved

achilleas-k marked this pull request as draft July 27, 2023 09:00

auto-merge was automatically disabled July 27, 2023 09:00
Pull request was converted to draft

achilleas-k force-pushed the rhel_disable_grub_success branch 2 times, most recently from e383fd0 to c430a74 Compare August 17, 2023 09:29

achilleas-k approved these changes Aug 17, 2023

View reviewed changes

achilleas-k enabled auto-merge August 17, 2023 09:32

achilleas-k requested changes Aug 17, 2023

View reviewed changes

pkg/manifest/os.go Outdated Show resolved Hide resolved

achilleas-k force-pushed the rhel_disable_grub_success branch from c430a74 to 719716d Compare August 17, 2023 10:21

achilleas-k mentioned this pull request Aug 17, 2023

Generate a notice when images are rebuilt in testing #123

Open

achilleas-k changed the title ~~fix: disable grub boot success~~ Disable grub-boot-success for RHEL 9 edge commits to fix greenboot interference Aug 17, 2023

achilleas-k changed the title ~~Disable grub-boot-success for RHEL 9 edge commits to fix greenboot interference~~ Disable grub-boot-success in RHEL 9 edge commits to fix greenboot interference Aug 17, 2023

achilleas-k mentioned this pull request Aug 17, 2023

Set up osbuild pre-release repos for testing #119

Closed

achilleas-k added 3 commits August 17, 2023 13:14

Schutzfile: trim to fedora-38

f60e002

We only test on Fedora 38. Remove all the other distro specifications. We can bring them back when we need them again.

Schutzfile: update osbuild commit

401a37e

osbuild/osbuild@61da612

schutzbot: set up osbuild repo for commit

0d09ae9

New script for setting up high priority repository for installing an osbuild RPM built from a specific commit for testing against versions before they're released.

achilleas-k force-pushed the rhel_disable_grub_success branch from 719716d to 2f315b2 Compare August 17, 2023 11:18

thozza previously requested changes Aug 17, 2023

View reviewed changes

pkg/distro/rhel9/images.go Outdated Show resolved Hide resolved

pkg/distro/rhel9/images.go Outdated Show resolved Hide resolved

say-paul commented Aug 17, 2023

View reviewed changes

pkg/distro/rhel9/edge.go Show resolved Hide resolved

say-paul added 2 commits August 17, 2023 15:09

say-paul force-pushed the rhel_disable_grub_success branch from 779322c to ad4db28 Compare August 17, 2023 19:10

say-paul requested review from achilleas-k, runcom and thozza August 17, 2023 19:13

achilleas-k approved these changes Aug 17, 2023

View reviewed changes

achilleas-k added this pull request to the merge queue Aug 21, 2023

thozza approved these changes Aug 21, 2023

View reviewed changes

Merged via the queue into osbuild:main with commit ea273dd Aug 21, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable grub-boot-success in RHEL 9 edge commits to fix greenboot interference #51

Disable grub-boot-success in RHEL 9 edge commits to fix greenboot interference #51

say-paul commented Jul 25, 2023 •

edited

Loading

supakeen commented Jul 25, 2023 •

edited

Loading

say-paul commented Jul 25, 2023 •

edited

Loading

say-paul commented Jul 25, 2023

runcom commented Jul 26, 2023 •

edited

Loading

say-paul commented Jul 26, 2023

achilleas-k left a comment

achilleas-k commented Jul 26, 2023 •

edited

Loading

say-paul commented Jul 27, 2023

say-paul commented Jul 27, 2023

supakeen commented Jul 27, 2023 •

edited

Loading

runcom commented Jul 27, 2023 •

edited

Loading

achilleas-k commented Jul 27, 2023

achilleas-k commented Jul 27, 2023 •

edited

Loading

runcom commented Jul 27, 2023

say-paul commented Jul 27, 2023 •

edited

Loading

runcom commented Jul 27, 2023

say-paul commented Jul 27, 2023

achilleas-k commented Aug 17, 2023

thozza left a comment •

edited

Loading

say-paul commented Aug 21, 2023

thozza left a comment

Disable grub-boot-success in RHEL 9 edge commits to fix greenboot interference #51

Disable grub-boot-success in RHEL 9 edge commits to fix greenboot interference #51

Conversation

say-paul commented Jul 25, 2023 • edited Loading

supakeen commented Jul 25, 2023 • edited Loading

say-paul commented Jul 25, 2023 • edited Loading

say-paul commented Jul 25, 2023

runcom commented Jul 26, 2023 • edited Loading

say-paul commented Jul 26, 2023

achilleas-k left a comment

Choose a reason for hiding this comment

achilleas-k commented Jul 26, 2023 • edited Loading

say-paul commented Jul 27, 2023

say-paul commented Jul 27, 2023

supakeen commented Jul 27, 2023 • edited Loading

runcom commented Jul 27, 2023 • edited Loading

achilleas-k commented Jul 27, 2023

achilleas-k commented Jul 27, 2023 • edited Loading

runcom commented Jul 27, 2023

say-paul commented Jul 27, 2023 • edited Loading

runcom commented Jul 27, 2023

say-paul commented Jul 27, 2023

achilleas-k commented Aug 17, 2023

thozza left a comment • edited Loading

Choose a reason for hiding this comment

say-paul commented Aug 21, 2023

thozza left a comment

Choose a reason for hiding this comment

say-paul commented Jul 25, 2023 •

edited

Loading

supakeen commented Jul 25, 2023 •

edited

Loading

say-paul commented Jul 25, 2023 •

edited

Loading

runcom commented Jul 26, 2023 •

edited

Loading

achilleas-k commented Jul 26, 2023 •

edited

Loading

supakeen commented Jul 27, 2023 •

edited

Loading

runcom commented Jul 27, 2023 •

edited

Loading

achilleas-k commented Jul 27, 2023 •

edited

Loading

say-paul commented Jul 27, 2023 •

edited

Loading

thozza left a comment •

edited

Loading