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.

Sign up for GitHub

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

Jump to bottom

Add support for arbitrary kickstart file injection into ISOs (HMS-3879) #4135

Merged
merged 9 commits into from
May 28, 2024
Merged

Add support for arbitrary kickstart file injection into ISOs (HMS-3879) #4135

merged 9 commits into from
May 28, 2024

Conversation

achilleas-k
Copy link
Member

@achilleas-k achilleas-k commented May 7, 2024 •
edited
Loading

@achilleas-k achilleas-k force-pushed the images/update/user-kickstarts branch 2 times, most recently from d6a0d49 to e068cac Compare May 21, 2024 12:02
@achilleas-k
Copy link
Member Author

Rebased on #4157

@achilleas-k achilleas-k requested review from ondrejbudai and removed request for ondrejbudai May 21, 2024 16:04
@achilleas-k achilleas-k marked this pull request as ready for review May 22, 2024 11:02
@achilleas-k achilleas-k force-pushed the images/update/user-kickstarts branch from e068cac to 078cbac Compare May 22, 2024 11:03
@ochosi ochosi changed the title Add support for arbitrary kickstart file injection into ISOs Add support for arbitrary kickstart file injection into ISOs (HMS-3879) May 23, 2024
@achilleas-k achilleas-k force-pushed the images/update/user-kickstarts branch 2 times, most recently from 7ab0e49 to c870e00 Compare May 23, 2024 12:22
@achilleas-k achilleas-k requested review from thozza and ondrejbudai May 23, 2024 12:23
thozza
thozza previously approved these changes May 23, 2024
View reviewed changes
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

LGTM

@achilleas-k
Copy link
Member Author 

  - package osbuild-119-1.20190819git4dbf8e2.fc38.noarch requires (osbuild-selinux if selinux-policy-targeted), but none of the providers can be installed

🤔
https://gitlab.com/redhat/services/products/image-builder/ci/osbuild-composer/-/jobs/6922670338#L710

@achilleas-k
Copy link
Member Author

Seems the issue is actually

  - nothing provides selinux-policy >= 38.33-1.fc38 needed by osbuild-selinux-119-1.20190819git4dbf8e2.fc38.noarch
  - nothing provides selinux-policy-base >= 38.33-1.fc38 needed by osbuild-selinux-119-1.20190819git4dbf8e2.fc38.noarch

@thozza
Copy link
Member

thozza commented May 23, 2024

Seems the issue is actually

  - nothing provides selinux-policy >= 38.33-1.fc38 needed by osbuild-selinux-119-1.20190819git4dbf8e2.fc38.noarch
  - nothing provides selinux-policy-base >= 38.33-1.fc38 needed by osbuild-selinux-119-1.20190819git4dbf8e2.fc38.noarch

The osbuild repo (and CI building RPMs) probably uses newer rpmrepo snapshots with newer selinux-policy package.

@achilleas-k
Copy link
Member Author

ah of course, didn't think of that. Thanks!

@achilleas-k
Copy link
Member Author

Pulled in #4147

@achilleas-k achilleas-k force-pushed the images/update/user-kickstarts branch from c870e00 to d9f510d Compare May 23, 2024 15:26
thozza
thozza previously approved these changes May 23, 2024
View reviewed changes
ondrejbudai
ondrejbudai previously approved these changes May 23, 2024
View reviewed changes
Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

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

😍

@achilleas-k achilleas-k dismissed stale reviews from ondrejbudai and thozza via 6433b8f May 23, 2024 17:08
@achilleas-k
Copy link
Member Author

Also found the issue that I noticed here: #4147 (comment)

@achilleas-k achilleas-k mentioned this pull request May 23, 2024
Closed
@achilleas-k achilleas-k force-pushed the images/update/user-kickstarts branch from 6433b8f to 7551bb8 Compare May 23, 2024 19:27
@thozza
Copy link
Member

thozza commented May 27, 2024

@achilleas-k osbuild/images#705 breaks the "weldr api distro alias" test case, because it relies on the existence of /etc/dnf/vars/releasever in the 8.10 SAP image. You may try to change the release it uses to 8.8 or some other EUS release which still creates the file.

@achilleas-k
Copy link
Member Author

@achilleas-k osbuild/images#705 breaks the "weldr api distro alias" test case, because it relies on the existence of /etc/dnf/vars/releasever in the 8.10 SAP image. You may try to change the release it uses to 8.8 or some other EUS release which still creates the file.

Would it make sense to change the alias test to read /etc/os-release? That way we could build any image type, not rely on SAP, and read the VERSION_ID.

@thozza
Copy link
Member

thozza commented May 27, 2024 •
edited
Loading

@achilleas-k osbuild/images#705 breaks the "weldr api distro alias" test case, because it relies on the existence of /etc/dnf/vars/releasever in the 8.10 SAP image. You may try to change the release it uses to 8.8 or some other EUS release which still creates the file.

Would it make sense to change the alias test to read /etc/os-release? That way we could build any image type, not rely on SAP, and read the VERSION_ID.

IMO no, because that file comes from the RPM and it will not really tell you which distro version composer used when generating the manifest. The DNF vars file in SAP images is really the only thing that gets baked into the image and reflects the distro version used by composer.

IOW, if the rhel-8 alias pointed to rhel-8.8, but you used 8.10 repositories, the VERSION_ID would be 8.10 and composer would actually use 8.8 distro version.

@achilleas-k
Copy link
Member Author

Right, we want to make sure the internal distro object and code path is correct regardless of repos. Gotcha.

@achilleas-k
Copy link
Member Author

Added a config to the test that overrides the RHEL-8 alias.

Small question for follow-up, but I wonder if it's necessary to be building the images here. I realise end-to-end testing is good, but maybe for this particular feature it's enough to just generate a manifest and check that we have a dnf config stage with the releasever set appropriately and save ourselves some testing time.

@thozza
Copy link
Member

thozza commented May 27, 2024

Small question for follow-up, but I wonder if it's necessary to be building the images here. I realise end-to-end testing is good, but maybe for this particular feature it's enough to just generate a manifest and check that we have a dnf config stage with the releasever set appropriately and save ourselves some testing time.

Distro aliases is a feature of composer, so you can't really test it in the images repo just by generating manifest. I'm not sure how would we reasonably generate just the manifest when testing a behavior in composer 🤔 If we could do that, then yeah, it would be OK to just inspect the manifest 😉

@achilleas-k
Copy link
Member Author

Right. I wasn't thinking of moving it to images, but maybe we could do something like cancel the build and inspect the manifest.

Anyway, a discussion for another time.

thozza
thozza previously approved these changes May 27, 2024
View reviewed changes
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

👍

@achilleas-k achilleas-k force-pushed the images/update/user-kickstarts branch from f763dc0 to 91d91ca Compare May 28, 2024 10:10
@achilleas-k
Copy link
Member Author

Pushed again. Had to fix the RHEL 8.8 runner name for the alias test (ga instead of nightly).

thozza
thozza approved these changes May 28, 2024
View reviewed changes
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

🤞

schutzbot and others added 9 commits May 28, 2024 22:40
@schutzbot @achilleas-k
schutzfile: Update snapshots to 20240514
aeee6d0
@dependabot @achilleas-k
build(deps): bump the go-deps group across 1 directory with 11 updates
023f008 
---
updated-dependencies:
- dependency-name: cloud.google.com/go/compute
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
- dependency-name: cloud.google.com/go/storage
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
- dependency-name: github.com/hashicorp/go-retryablehttp
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
- dependency-name: github.com/openshift-online/ocm-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
- dependency-name: github.com/osbuild/images
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
- dependency-name: github.com/vmware/govmomi
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
- dependency-name: golang.org/x/oauth2
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
- dependency-name: golang.org/x/sys
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
- dependency-name: google.golang.org/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
...

Signed-off-by: dependabot[bot] <[email protected]>
@achilleas-k
Update osbuild/images v0.63.0
bda175b 
Pulling in (among others):
- osbuild/images#700
    - Blueprint User customization change required.
- osbuild/images#705
@achilleas-k
blueprint: add new kickstart.contents option
af52175
@achilleas-k
Schutzfile: bump pinned osbuild version to v119
8365503
@achilleas-k
tools/gen-ssh: upgrade openssh before generating key
32d14aa 
openssl gets installed as a dependency of the osbuild-composer-tests but
it might not update openssh at the same time, which can cause a version
mismatch when running ssh-keygen:

  OpenSSL version mismatch. Built against 30000000, you have 30200010

This is currently happening in CentOS Stream 9 so let's make sure
openssh is up to date before running ssh-keygen.
@achilleas-k
test/dot-notation-aliases: override rhel-8 alias
89eab2f 
The test relies on the existence of the /etc/dnf/vars/releasever file
and the value it specifies.  In SAP images, this file specifies the
locked release version for the system and it is written by the image
definition itself.  This makes it a good candidate for ensuring we are
using the correct distro version when building the image.

However, RHEL 8.10 SAP images don't lock the distro version because 8.10
is the last minor version in the RHEL 8 cycle.

Let's override the alias for RHEL 8 to point to 8.8, which is a version
that should contain a releasever.  The exact value we use isn't
important.  What we are testing here is the mechanism for resolving
aliases.
@achilleas-k
test/dot-notation-aliases: typo fixes and cleanups
4368941 
- Change the alias example at the top of the file to use RHEL 9 -> 9.4
  instead of 8 -> 8.10 which is no longer true for the test.
- Fix some comment typos.
- Clean trailing whitespaces.
@achilleas-k
gitlab: run the RHEL 8 alias test on RHEL 8.8 (GA)
f409390
@achilleas-k achilleas-k force-pushed the images/update/user-kickstarts branch from 91d91ca to f409390 Compare May 28, 2024 20:40
@achilleas-k achilleas-k enabled auto-merge (rebase) May 28, 2024 20:40
@achilleas-k achilleas-k merged commit 580903f into osbuild:main May 28, 2024
47 of 63 checks passed
@achilleas-k achilleas-k deleted the images/update/user-kickstarts branch May 28, 2024 22:27
achilleas-k added a commit to osbuild/osbuild.github.io that referenced this pull request Jul 2, 2024
@achilleas-k
blueprint-reference: describe new kickstart customization
1be9aea 
Describe the new installer.kickstart customization that allows users to
inject their own custom kickstart content into an ISO build.
See osbuild/osbuild-composer#4135
@achilleas-k achilleas-k mentioned this pull request Jul 2, 2024
Merged
ondrejbudai pushed a commit to osbuild/osbuild.github.io that referenced this pull request Jul 2, 2024
@achilleas-k @ondrejbudai
blueprint-reference: describe new kickstart customization
eef82ba 
Describe the new installer.kickstart customization that allows users to
inject their own custom kickstart content into an ISO build.
See osbuild/osbuild-composer#4135
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thozza thozza thozza approved these changes

@ondrejbudai ondrejbudai ondrejbudai left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@achilleas-k @thozza @ondrejbudai @schutzbot