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

Add example of using livemedia-creator inside GitHub Actions #1374

Closed
wants to merge 1 commit into from
Closed

Add example of using livemedia-creator inside GitHub Actions #1374

wants to merge 1 commit into from

Conversation

Cameronsplaze
Copy link
Contributor

No description provided.

@Cameronsplaze
Copy link
Contributor Author

The action finally builds! I didn't realize the @anaconda-tools was required in packages, that took a bit to figure out.

The one catch is when I try to install the iso to virt-manager, it skips the install screen and jumps straight to asking for a user/pass. Is that expected? Is there a way to still force a install?

@Cameronsplaze
Copy link
Contributor Author

I think I found it, I was missing the 'firstboot' flag here. It's going through the build process now, and I'll try to install it tomorrow then mark the PR as ready. This is the first ks I've made, so any suggestions are welcomed!

@Cameronsplaze
Copy link
Contributor Author

I can't get the firstboot flag to work. Instead of taking you to the setup screen, it takes you to the user login screen. Any idea what I might be missing? I tried a few ideas locally, and nothing worked. I'm not sure if this is related, but the "Test this Media" option at startup would fail too, "checkisomd5@*.service failed to start".

@bcl
Copy link
Contributor

bcl commented Feb 5, 2024

I'm not sure what's going on with firstboot and the checksum. I almost never use firstboot so I'm not sure of the current state of it in Anaconda. A few suggestions for this PR:

  • Don't put the action into the repo workflow, put it in ./docs/github-action.yml (I don't really want it running on every commit here)
  • Add overwrite: true to the upload artifact action so that it doesn't accumulate a bunch of old iso builds.
  • Document the action in comments at the top of the action
  • Start with a known good kickstart, either the examples in ./docs/ or the official fedora spin kickstarts after flattening them. For the example it should be possible to just use the one from ./docs/fedora-livemedia.ks instead of making a new one.

@Cameronsplaze
Copy link
Contributor Author

Don't put the action into the repo workflow, put it in ./docs/github-action.yml (I don't really want it running on every commit here)

The only trigger I have at the top is workflow_dispatch, so it can only run if you manually trigger it from this console GUI. And personally if I'm looking for action examples in repos, the first place I check is the .github/workflows dir, or seeing the list of actions in the Actions tab above. Would you still like it moved?

Add overwrite: true to the upload artifact action

Good idea, done!

Document the action in comments at the top of the action

I added something real basic, but wasn't sure what else to add. Is there any extra info/clarifying you'd like added?

Start with a known good kickstart ... ./docs/fedora-livemedia.ks

Cool that one works! I deleted the example I added. Originally I tried the fedora-minimized.ks file, but it doesn't have dracut-live installed (along with something else I forget), so I just assumed the examples were for lorax itself. I completely missed that one, thanks!

@Cameronsplaze Cameronsplaze marked this pull request as ready for review February 15, 2024 17:39
@Cameronsplaze
Copy link
Contributor Author

I realized it was possible for the host release to drift away from the release you're building since it was fedora:latest. I switched it to an input variable so everything can reference the same number and stay in sync.

The build failed since this is the first time I ran it after deleting the old ks file, there was still a path to it in one spot. I made that path a input variable too to help with changes in the future.

Copy link
Contributor

@bcl bcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, can you squash all the commits into one clean one with a description of the action and the variables. And add the release to the output filename. Then I think it will be good. Thanks!

.github/workflows/example-livemedia-creator.yml Outdated Show resolved Hide resolved
@Cameronsplaze
Copy link
Contributor Author

Can you squash all the commits into one clean one with a description of the action and the variables

Is this something I can do? Everything I can find says the option appears when you click the merge button. If it doesn't, there's some settings you can adjust under the branch protection rules to make it appear and/or force it to happen every time.

...one with a description of the action and the variables

If my theories right, here's something you can modify/paste into the box that appears on merging

Added example of how to use livemedia-creator in github actions, and still build in fedora.

Inputs:
- fedora_release: Both the release to build, and build on. Since we use '--no-virt', they must be the same.
- kickstart_path: The path to the kickstart file to build from.

@bcl
Copy link
Contributor

bcl commented Feb 16, 2024

No, you should squash things locally then force-push to your branch. Something like this should work:

git rebase master
git rebase --interactive 55f8077e6649f7e4df6bf55c5306b87faa6d0149

You will be presented with a 'menu' of commits to apply and an action to take. Change all but the first to 'squash', save the file, and then edit the commit message -- it will have the text from all the squashed commits included, just change it to what you posted above.

Then you should be able to push to your branch, but using --force because it is changing the history instead of adding to it.
Github will then pick it up and change this PR.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7934328653

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 16 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 43.521%

Files with Coverage Reduction New Missed Lines %
src/pylorax/dnfbase.py 16 57.51%
Totals Coverage Status
Change from base Build 7750295129: -0.02%
Covered Lines: 1614
Relevant Lines: 3500

💛 - Coveralls

blivet-gui-runtime doesn't depend on PolicyKit-authentication-agent
only blivet-gui which is not on the installation images depends on
polkit.

Remove some unneccessary storage packages from runtime-install

Anaconda already depends on following packages:
- filesystem tools: btrfs-progs, ntfs-3g, ntfsprogs, jfsutils,
                    f2fs-tools, xfsprogs, dosfstools, e2fsprogs
- libblockdev-lvm-dbus
- udisks2-iscsi

hostname is no longer needed by dracut for iSCSI, see
dracutdevs/dracut@ebe1821

Automatic commit of package [lorax] release [40.0-1].

Created by command:

/usr/bin/tito tag --keep-version

Adjust runtime-postinstall.tmpl for systemd config files move

Systemd config files were moved from /etc to /usr/lib/systemd,
so this snippet fails. Instead of editing the config file, just create
a drop-in snippet with the desired configuration.

Add python3-libdnf5 to the list of test packages

This allows 'make test-in-podman' to work.

libdnf5: Switch lorax to use libdnf5

At some point libdnf5 will be replacing libdnf. It is written in C++
with a SWIG interface, so accessing the API is quite different than
before.

This changes the template parser to use libdnf5 to install packages and
remove files. Tests pass, and building the boot.iso is working.

This simplifies the download callback a bit, we no longer have any idea what the total
download size is, so just print the package count and any errors.

The transaction callback logs install and script events.

Fix writing out debug info for package files and sizes

spec: Switch to using python3-libdnf5

Updates for latest libdnf5 changes

test-packages: Make sure python3-libdnf5 is installed

ltmpl: Add transaction error handling

dnfbase: Fix url substitution support

This fixes support for using $releasever (set by lorax --version) and
$basearch (set from the host machine type or from lorax --buildarch) in
the repo urls.

test: Add pigz to test-packages

ltmpl: Filter out other arches, clean up naming

Because of the way the template filter is implemented it was picking up
i686 packages along with the x86_64 ones. This adds an arch filter that
only includes the basearch packages and 'noarch'.

Also clean up the naming in install_pkg

Automatic commit of package [lorax] release [40.1-1].

Created by command:

/usr/bin/tito tag

test-in-podman: Fix problem running in github actions

Something is different when running rawhide (Fedora 40) using podman on
GitHub. rsync has started returning errors about setting permissions.
Work around this by passing `--no-perms` to rsync, it still works fine
locally and clears up whatever the issue is with the action.

ltmpl: Remove duplicate package objects from dnf5 results

In dnf5 if you have multiple repositories with overlapping content, and
the repo priorities are the same (eg. the default of 99) it will return
multiple results with the same nevra. The transaction will fail when
trying to install the duplicate rpms.

This de-duplicates the results from `_pkgver`, assuming that packages
with the same nevra and repo priority are interchangeable.

Note that this only works at an individual `installpkg` level, not across
the whole transaction. So it is possible for separate `installpkg`
commands that select the same package to cause a crash -- currently this
is not an issue, but is something to fix in the future.

Includes a duplicate package in the ltmpl test to make sure this stays
fixed.

Fixes #1356

Automatic commit of package [lorax] release [40.2-1].

Created by command:

/usr/bin/tito tag

ltmpl: Check for errors after running the transaction

Thanks to the discussion in
rpm-software-management/dnf5#1074 lorax will
now correctly log installation size errors like:

The transaction process has ended abruptly:
installing package llvm-libs-17.0.6-2.fc40.x86_64 needs 107MB more space on the / filesystem
installing package libXv-1.0.12-1.fc40.x86_64 needs 107MB more space on the / filesystem
installing package libXcomposite-0.4.6-1.fc40.x86_64 needs 107MB more space on the / filesystem

runtime-install: Work around problem with conflicting packages

See:
rpm-software-management/dnf5#1111

dnf5 is returning anaconda-install-img-deps for 2 arches even though the
noarch version number is lower than the x86_64 package.

Work around this by selecting version 40.15 or later.

Automatic commit of package [lorax] release [40.3-1].

Created by command:

/usr/bin/tito tag

init commit: Adding example action that uses livemedia-creator

Adding dracut-live to the ks, so livemedia-creator can run

Is the error because it can't remove dracut-live maybe?

Switched references from fedora 39, to fedora minimized

revert back, couldn't get it to work with livemedia-creator

init commit: Adding example ks to run with github actions

Pointing the action to the new ks file instead

Changing boot location back to none, seeing if that's where the error is coming from

Testing if it's because of a package missing from the minimal environment

ltmpl: Pass packages to add_rpm_install as strings

According to the discussion in:
rpm-software-management/dnf5#1090 (comment)

dnf5 handles NEVRA strings and package objects differently. So instead
of passing in objects pass in the full NEVRA string. If a single install
command has duplicates this won't matter, I already remove the dupes.
But it should prevent problems where separate install commands end up
selecting duplicate packages.

runtime-cleanup: anaconda's new interface needs stdbuf

Depends indirectly on stdbuf because cockpit-storage is using it.

runtime-install: wget2-wget has replaced wget

wget has been retired:
https://src.fedoraproject.org/rpms/wget/c/ce69c17
in favor of wget2-wget:
https://fedoraproject.org/wiki/Changes/Wget2asWget

Signed-off-by: Adam Williamson <[email protected]>

runtime-install: drop retired pcmciautils

pcmciautils was just retired in Rawhide:
https://src.fedoraproject.org/rpms/pcmciautils/c/24639b0

Signed-off-by: Adam Williamson <[email protected]>

aarch64: Escape volid before using it

s390: Escape volid before using it

ltmpl: Handle installing provides with resolve_pkg_spec

Previously the installpkg command used filter_match to select packages,
which works great most of the time. It fails when the package is only
available as a provide. This happened recently with wget moving to
wget2-wget, causing builds to fail because it couldn't find wget.

This changes installpkg to use the resolve_pkg_spec function, which as
it happens also support globs and version comparison operators. DNF
doesn't like the `==` operator, but I have retained support for it by
translating it to a single `=`. I have dropped support for `=>` and `=<`
since they are never used and are and odd way to specify `>=` and `>=`
which of course still work.

runtime-install: drop kdump-anaconda-addon

I'm proposing making this part of anaconda-install-env-deps
instead in rhinstaller/anaconda#5205 .
It's more correct.

Signed-off-by: Adam Williamson <[email protected]>

Adding anaconda-tools to the install list, maybe that's why moddep.lst doesn't exist

Upgraded upload-artifact action because of deprecation notice on older version

Added 'firstboot' flag to get user to go through the install screen

Switched log from virt-install.log to program.log, since we're running in --no-virt mode now

adding extra debug lines to try to get firstboot working

docs mentioned initial-setup-gui, trying that out too

mkksiso: Add support for adding an anaconda updates.img

Anaconda supports an updates.img file that can be used to assist in
development, the files in it are added to the boot.iso's rootfs before
Anaconda is started.

Use `mkksiso --updates /path/to/updates.img` to add the image to the
iso. It can be used by itself or in combination with other options.

It would be great way to avoid requirement for HTTP server for local
development.

Automatic commit of package [lorax] release [40.4-1].

Created by command:

/usr/bin/tito tag

Cleaned up list, grouping similar items. Added example for trying to get firstboot working

Added basic comment at top, added override flag for uplaoding artifact, and testing out different KS file

fedora-livemedia.ks worked, no need for a extra ks file

Adding fedora_release to be an input, so that host always matches the OS it's trying to build

Missed that the ks file was used in two places, so made it a variable so that can't happen again. Also adds flexability to try livemedia-creator against different ks in the repo

forgot to specify type: string, though I think this is the default anyways

Added input variable to filename of iso

Adding dracut-live to the ks, so livemedia-creator can run

Is the error because it can't remove dracut-live maybe?

revert back, couldn't get it to work with livemedia-creator

init commit: Adding example ks to run with github actions

Changing boot location back to none, seeing if that's where the error is coming from

Testing if it's because of a package missing from the minimal environment

ltmpl: Pass packages to add_rpm_install as strings

According to the discussion in:
rpm-software-management/dnf5#1090 (comment)

dnf5 handles NEVRA strings and package objects differently. So instead
of passing in objects pass in the full NEVRA string. If a single install
command has duplicates this won't matter, I already remove the dupes.
But it should prevent problems where separate install commands end up
selecting duplicate packages.

runtime-cleanup: anaconda's new interface needs stdbuf

Depends indirectly on stdbuf because cockpit-storage is using it.

runtime-install: wget2-wget has replaced wget

wget has been retired:
https://src.fedoraproject.org/rpms/wget/c/ce69c17
in favor of wget2-wget:
https://fedoraproject.org/wiki/Changes/Wget2asWget

Signed-off-by: Adam Williamson <[email protected]>

runtime-install: drop retired pcmciautils

pcmciautils was just retired in Rawhide:
https://src.fedoraproject.org/rpms/pcmciautils/c/24639b0

Signed-off-by: Adam Williamson <[email protected]>

aarch64: Escape volid before using it

s390: Escape volid before using it

ltmpl: Handle installing provides with resolve_pkg_spec

Previously the installpkg command used filter_match to select packages,
which works great most of the time. It fails when the package is only
available as a provide. This happened recently with wget moving to
wget2-wget, causing builds to fail because it couldn't find wget.

This changes installpkg to use the resolve_pkg_spec function, which as
it happens also support globs and version comparison operators. DNF
doesn't like the `==` operator, but I have retained support for it by
translating it to a single `=`. I have dropped support for `=>` and `=<`
since they are never used and are and odd way to specify `>=` and `>=`
which of course still work.

runtime-install: drop kdump-anaconda-addon

I'm proposing making this part of anaconda-install-env-deps
instead in rhinstaller/anaconda#5205 .
It's more correct.

Signed-off-by: Adam Williamson <[email protected]>

Adding anaconda-tools to the install list, maybe that's why moddep.lst doesn't exist

Added 'firstboot' flag to get user to go through the install screen

adding extra debug lines to try to get firstboot working

docs mentioned initial-setup-gui, trying that out too

mkksiso: Add support for adding an anaconda updates.img

Anaconda supports an updates.img file that can be used to assist in
development, the files in it are added to the boot.iso's rootfs before
Anaconda is started.

Use `mkksiso --updates /path/to/updates.img` to add the image to the
iso. It can be used by itself or in combination with other options.

It would be great way to avoid requirement for HTTP server for local
development.

Automatic commit of package [lorax] release [40.4-1].

Created by command:

/usr/bin/tito tag

Cleaned up list, grouping similar items. Added example for trying to get firstboot working

fedora-livemedia.ks worked, no need for a extra ks file

forgot to specify type: string, though I think this is the default anyways

Added input variable to filename of iso
@Cameronsplaze
Copy link
Contributor Author

I tried a few times but couldn't figure out why the squash wasn't working. I thought I was using the wrong commit, so I tried HEAD~<numer_of_commits> too, no luck. I updated my branch with your master a few times, so maybe those in the middle are what were added? Out of curiosity too, is there a down side to squishing on merge through GitHub's button?

Also if it works with you, I created a new branch real fast and just "created" the file in that. It's this PR: #1382. If it works, that's probably faster than trying to untangle that last commit above.

Thanks!

@bcl
Copy link
Contributor

bcl commented Feb 19, 2024

Yeah, the default for github is to make merge commits, which I can't stand :) So just making a clean branch is fine.

@bcl bcl closed this Feb 19, 2024
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