rewrite depsolve external #196

supakeen · 2024-09-12T08:47:41Z

This PR splits out the depsolve external along the same idiom that the partition externals follow. Draft because it needs a manifest as well.

src/otk_external_osbuild/command/depsolve_dnf4.py

supakeen · 2024-09-13T09:37:01Z

Needs:

Tests to be able to call the Python externals as well
Updated comments in the externals.
Maybe schemas, or followup?

mvo5

Thank you! It's super nice to see this coming together. A few suggestions/ideas inline for your consideration.

src/otk_external_osbuild/command/depsolve_dnf4.py

src/otk_external_osbuild/command/depsolve_dnf4_gen_curl_source.py

mvo5 · 2024-09-13T13:55:17Z

Needs:

* [ ]  Tests to be able to call the Python externals as well

* [ ]  Updated comments in the externals.

* [ ]  Maybe schemas, or followup?

Fwiw, I think the schema is fine in a followup as e.g. images or osbuild do not use dnfjson schemas either.

example/centos/centos-9-x86_64-ami.yaml

supakeen · 2024-09-16T09:48:44Z

Use .const.internal keys on the data.
Rename to fit current idiom.
Make external available to tests.
Provide .const.kernel (or .kernel?) containing kernel version and name.

This commit adds a new `normalize_rpm_refs()` helper that will normalize the rpm file inputs for comparison, i.e. it will sort the hashes and remove duplicates. This is important because the "images" library writes duplicated input rpm packages from the inputs as duplicated hashes into the references. But in our test we only need to worry about that exactly the right rpm files got included, the order is not important.

Remove unused subcommands of the `osbuild_external`. Signed-off-by: Simon de Vlieger <[email protected]>

Externals no longer receive their name within the tree structure that they get from `otk`. Signed-off-by: Simon de Vlieger <[email protected]>

This splits out the depsolve external along the lines of the previously written partition externals. Using the external is now a two-step process where you run one external in a define and then use its output to generate stages and sources. Signed-off-by: Simon de Vlieger <[email protected]>

Use depsolver for the CentOS 9 QCOW and AMI omnifests Signed-off-by: Simon de Vlieger <[email protected]>

In `images` we mock depsolves so we can run them very fast under test. This introduces the same functionality to the depsolve external. Signed-off-by: Simon de Vlieger <[email protected]>

Use a `const.internal` subkey for the dnf4 depsolving external. This follows the ideas set forth in the partition tables. `.const` means that the format can be considered un-changing and `.internal` means that this shouldn't be used directly but only through other externals in the same group. Signed-off-by: Simon de Vlieger <[email protected]>

Renames the depsolve externals to fit into the partition externals naming scheme. Signed-off-by: Simon de Vlieger <[email protected]>

Bit of a hacky way to copy the (installed) Python externals into the search path for the manifest tests. Also sets testing environment to True. Signed-off-by: Simon de Vlieger <[email protected]>

Signed-off-by: Simon de Vlieger <[email protected]>

Properly output the RPM stage. Signed-off-by: Simon de Vlieger <[email protected]>

Drop the second GPG key as they are not in the reference manifests. Signed-off-by: Simon de Vlieger <[email protected]>

Signed-off-by: Simon de Vlieger <[email protected]>

…ists

Provide a map keyed by package name of the depsolve result that can be used by omnifests to look up package information. Signed-off-by: Simon de Vlieger <[email protected]>

supakeen · 2024-09-16T14:16:53Z

I've opted for a very similar approach to rpmmd.GetVerStrFromPackageSpecList; we get an additional non-internal map keyed by package names that contains the parts that can be used to form the necessary information.

Perhaps we only want the (N)EVRA in there?

With the rewrite of the depsolver these are not extremely poc anymore. Signed-off-by: Simon de Vlieger <[email protected]>

Signed-off-by: Simon de Vlieger <[email protected]>

mvo5 · 2024-09-16T14:58:18Z

example/centos/centos-9-x86_64-qcow2.yaml

+            - tar
+            - tuned
+            - tcpdump
+          exclude:


Finding a way to test this would be really good, this will probably require changes in the gen-manifests mock dnfjson mode in images though so probably followup material but IMHO quite important to ensure we really have the same images.

mvo5

Thanks, this moving along nicely, some comemnts inline.I really want to look a bit more at the gen-manifests reference image generator to get more meaningful mock data, right now we do not test anything like excludes or we don't mock versions or arch which is important for the testing of the grub2 kernel bits.

mvo5 · 2024-09-16T15:23:02Z

src/otk_external_osbuild/command/gen_depsolve_dnf4.py

+    # other externals.
+    data["tree"]["const"]["internal"]["packages"] = packages
+
+    # We also store all resolved packages and some meta information about


This adds a very broad API to this otk external, I personally would start more targeted, we need the kernel name and version and in images the kernel is handled specially only. So maybe we could do the same here? Something like just expose a const.kernel.{name,version}? We will need to adjust fragments/grub2.yam to match the kernel and the string there includes 0-0.noarch right now which also indicates that we need to tweak our tests to make this better testable.

Here is an alternative idea 5f378bb

We can set mock data to always be 0, 0, and noarch instead of the empty strings they are now.

I chose the broad API to not limit ourselves here also taking some inspiration from osbuild-mpp where it is currently possible to get the version of any package.

I've done the former bit and use that information in the example; it's a tiny bit tedious and maybe we want to assign separately in the define. Not that important to me right now.

The discussion about if this API is too wide needs to be resolved as well :)

Thanks, that is interesting! How is this used by folks that use osbuild-mpp - are there more use-cases then the kernel and if so, which ones? Would love to learn more here.

The only call to the equivalent function in images that's not for looking up the kernel is to check if dracut-config-rescue exists in the package set, but it doesn't care about version.
https://github.com/osbuild/images/blob/67a2b2f65a2e0969039a3de17bf2c266390d01ab/pkg/manifest/os.go#L651

I can imagine a case where this might be useful. We've never had to do this but perhaps a stage's options might change depending on the version of a given package (in the build root or in the OS).

I lean towards simplicity for these externals, but I also like the generalisation of being able to "query" for any package's version by name. It feels useful, so my instinct right now is to generalise it so it's easier to grow in the future.

src/otk_external_osbuild/command/gen_depsolve_dnf4.py

Signed-off-by: Simon de Vlieger <[email protected]>

supakeen · 2024-09-16T16:26:04Z

Addressed the mock thing in a separate commit (can be squashed if we want) but keeps the review a bit cleaner for now. Last outstanding thing is the package versions, do we want all of them to be available of just the provided kernel package name?

Use the depsolved kernel version and release for grub. Note that this assumes there is a `packages.os` which might not be true for all situations. Perhaps this should be a separate define in the examples. Signed-off-by: Simon de Vlieger <[email protected]>

supakeen · 2024-09-17T09:31:46Z

This PR has been fully split out into #205, #204, #203.

supakeen force-pushed the rewrite-depsolve-external branch 5 times, most recently from 5beb39b to 5af9a2b Compare September 12, 2024 09:02

mvo5 reviewed Sep 12, 2024

View reviewed changes

src/otk_external_osbuild/command/depsolve_dnf4.py Outdated Show resolved Hide resolved

mvo5 mentioned this pull request Sep 12, 2024

cmd: rename otk externals osbuild/images#922

Merged

supakeen force-pushed the rewrite-depsolve-external branch 5 times, most recently from 75dd20c to c6655c9 Compare September 13, 2024 07:53

mvo5 reviewed Sep 13, 2024

View reviewed changes

example/centos/centos-9-x86_64-ami.yaml Show resolved Hide resolved

mvo5 reviewed Sep 13, 2024

View reviewed changes

example/centos/centos-9-x86_64-ami.yaml Outdated Show resolved Hide resolved

supakeen force-pushed the rewrite-depsolve-external branch 2 times, most recently from 55cb4e0 to 71891bd Compare September 16, 2024 09:47

supakeen force-pushed the rewrite-depsolve-external branch 2 times, most recently from 9364c37 to 0223a56 Compare September 16, 2024 10:13

mvo5 mentioned this pull request Sep 16, 2024

test: add a new normalize_rpm_refs helper #200

Merged

supakeen added 6 commits September 16, 2024 15:29

external: remove unused subcommands

c12c626

Remove unused subcommands of the `osbuild_external`. Signed-off-by: Simon de Vlieger <[email protected]>

external: don't expect name

b7707e6

Externals no longer receive their name within the tree structure that they get from `otk`. Signed-off-by: Simon de Vlieger <[email protected]>

example: centos9-qcow/ami with depsolve

d2e61cf

Use depsolver for the CentOS 9 QCOW and AMI omnifests Signed-off-by: Simon de Vlieger <[email protected]>

external: mock depsolve

1c1ac86

In `images` we mock depsolves so we can run them very fast under test. This introduces the same functionality to the depsolve external. Signed-off-by: Simon de Vlieger <[email protected]>

supakeen and others added 8 commits September 16, 2024 15:29

external: rename depsolve externals

364d4f3

Renames the depsolve externals to fit into the partition externals naming scheme. Signed-off-by: Simon de Vlieger <[email protected]>

test: copy python externals into externals

64ce109

Bit of a hacky way to copy the (installed) Python externals into the search path for the manifest tests. Also sets testing environment to True. Signed-off-by: Simon de Vlieger <[email protected]>

example: sort packages

e660454

Signed-off-by: Simon de Vlieger <[email protected]>

external: fix depsolve output

53eb4b2

Properly output the RPM stage. Signed-off-by: Simon de Vlieger <[email protected]>

example: drop gpg key(s)

cd69d3e

Drop the second GPG key as they are not in the reference manifests. Signed-off-by: Simon de Vlieger <[email protected]>

example: remove unused file

9978335

Signed-off-by: Simon de Vlieger <[email protected]>

centos-9-x86_64-qcow2.: use packages.os in os pipeline

569159f

centos-9-x86_64-ami: fixup packages for ami based on images package l…

4428e7e

…ists

mvo5 force-pushed the rewrite-depsolve-external branch from 4c83c00 to 4428e7e Compare September 16, 2024 13:40

external: provide package information

3810989

Provide a map keyed by package name of the depsolve result that can be used by omnifests to look up package information. Signed-off-by: Simon de Vlieger <[email protected]>

supakeen added 2 commits September 16, 2024 16:18

chore: remove warning comments

6b734b2

With the rewrite of the depsolver these are not extremely poc anymore. Signed-off-by: Simon de Vlieger <[email protected]>

external: exception contains stderr and stdout

14967a7

Signed-off-by: Simon de Vlieger <[email protected]>

supakeen marked this pull request as ready for review September 16, 2024 14:20

supakeen mentioned this pull request Sep 16, 2024

Figure out a better way to test externals #201

Open

mvo5 reviewed Sep 16, 2024

View reviewed changes

mvo5 mentioned this pull request Sep 16, 2024

gen-manifests: create predictable version/release numbers in mockDepsolve osbuild/images#937

Merged

external: remove a line of code

c141f07

Signed-off-by: Simon de Vlieger <[email protected]>

mvo5 mentioned this pull request Sep 16, 2024

gen_depsolve_dnf4: include version/release/arch for kernel #202

Closed

supakeen force-pushed the rewrite-depsolve-external branch from daffa31 to a7b8a91 Compare September 16, 2024 16:37

mvo5 mentioned this pull request Sep 17, 2024

example: centos9-qcow/ami with depsolve #204

Merged

supakeen mentioned this pull request Sep 17, 2024

external: kernel version from depsolve (COMPOSER-2354) #205

Closed

supakeen closed this Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rewrite depsolve external #196

rewrite depsolve external #196

supakeen commented Sep 12, 2024

supakeen commented Sep 13, 2024 •

edited

Loading

mvo5 left a comment

mvo5 commented Sep 13, 2024

supakeen commented Sep 16, 2024 •

edited

Loading

supakeen commented Sep 16, 2024

mvo5 Sep 16, 2024

mvo5 left a comment

mvo5 Sep 16, 2024

supakeen Sep 16, 2024

supakeen Sep 16, 2024

mvo5 Sep 16, 2024

achilleas-k Sep 17, 2024 •

edited

Loading

supakeen commented Sep 16, 2024

supakeen commented Sep 17, 2024 •

edited

Loading

rewrite depsolve external #196

rewrite depsolve external #196

Conversation

supakeen commented Sep 12, 2024

supakeen commented Sep 13, 2024 • edited Loading

mvo5 left a comment

Choose a reason for hiding this comment

mvo5 commented Sep 13, 2024

supakeen commented Sep 16, 2024 • edited Loading

supakeen commented Sep 16, 2024

mvo5 Sep 16, 2024

Choose a reason for hiding this comment

mvo5 left a comment

Choose a reason for hiding this comment

mvo5 Sep 16, 2024

Choose a reason for hiding this comment

supakeen Sep 16, 2024

Choose a reason for hiding this comment

supakeen Sep 16, 2024

Choose a reason for hiding this comment

mvo5 Sep 16, 2024

Choose a reason for hiding this comment

achilleas-k Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

supakeen commented Sep 16, 2024

supakeen commented Sep 17, 2024 • edited Loading

supakeen commented Sep 13, 2024 •

edited

Loading

supakeen commented Sep 16, 2024 •

edited

Loading

achilleas-k Sep 17, 2024 •

edited

Loading

supakeen commented Sep 17, 2024 •

edited

Loading