Add support for modules #2760

jlebon · 2021-04-16T22:22:10Z

Add support for composing with modules and for package layering modules
on the client-side.

This doesn't support modules in rpm-ostree compose extensions yet,
which is relevant for RHCOS. We can look at adding that in a follow-up.

openshift-ci-robot · 2021-04-16T22:22:12Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

src/libpriv/rpmostree-core.cxx

jlebon · 2021-04-16T22:23:55Z

Testing this with:

diff --git a/manifest.yaml b/manifest.yaml
index aaf361d..7b23804 100644
--- a/manifest.yaml
+++ b/manifest.yaml
@@ -14,6 +14,8 @@ repos:
   # still pinned
   - fedora
   - fedora-updates
+  - fedora-modular
+  - fedora-updates-modular

 # All Fedora CoreOS streams share the same pool for locked files.
 # This will be in fedora-coreos.yaml in the future so it can be more easily be
diff --git a/manifests/fedora-coreos-base.yaml b/manifests/fedora-coreos-base.yaml
index 2dc6e9a..6065a1e 100644
--- a/manifests/fedora-coreos-base.yaml
+++ b/manifests/fedora-coreos-base.yaml
@@ -192,6 +192,9 @@ packages:
   # https://github.com/coreos/fedora-coreos-tracker/issues/509
   - zram-generator

+modules:
+  - cri-o:1.20
+
 # This thing is crying out to be pulled into systemd, but that hasn't happened
 # yet.  Also we may want to add to rpm-ostree something like arch negation;
 # basically right now it doesn't exist on s390x.

$ cosa fetch
...
cri-o-1.20.0-1.module_f33+10488+8050703d.x86_64 (fedora-updates-modular)
cri-tools-1.19.0-1.module_f33+10488+8050703d.x86_64 (fedora-updates-modular)
...

jlebon · 2021-04-23T18:55:14Z

Now with client-side support!

# rpm-ostree module install cri-o:1.20
...
Added:
  cri-o-1.20.0-1.module_f33+10488+8050703d.x86_64
  cri-tools-1.19.0-1.module_f33+10488+8050703d.x86_64

# rpm-ostree status
State: idle
Deployments:
  ostree://fedora:fedora/x86_64/coreos/stable
                   Version: 33.20210328.3.0 (2021-04-12T15:21:05Z)
                BaseCommit: 0fc4a4c205dbcdfd6ba68912bfbf2c90911e4a2341b3dda0a254ab6541224b83
              GPGSignature: Valid signature by 963A2BEB02009608FE67EA4249FD77499570FF31
                      Diff: 2 added
            LayeredModules: 'cri-o:1.20'

* ostree://fedora:fedora/x86_64/coreos/stable
                   Version: 33.20210328.3.0 (2021-04-12T15:21:05Z)
                    Commit: 0fc4a4c205dbcdfd6ba68912bfbf2c90911e4a2341b3dda0a254ab6541224b83
              GPGSignature: Valid signature by 963A2BEB02009608FE67EA4249FD77499570FF31
                  Unlocked: development

jlebon · 2021-04-23T18:55:44Z

Requires: rpm-software-management/libdnf#1206
Requires: rpm-software-management/libdnf#1207

This is also still missing tests. (And oh yeah, lots of prep patches to split out!)

jlebon · 2021-04-23T18:56:32Z

One really important thing to understand in this IMO is that modules are entirely a repomd concept. Once the RPMs are on disk, there's nothing in the rpmdb for example to tell that you have module NSVCA/P installed.

One consequence of this for example is that whenever we have to deal with modules, we always have to setup the sack to have libdnf parse the repomd.

cgwalters · 2021-04-26T18:42:19Z

One really important thing to understand in this IMO is that modules are entirely a repomd concept. Once the RPMs are on disk, there's nothing in the rpmdb for example to tell that you have module NSVCA/P installed.

I always thought it was weird that yum/dnf/zypper etc. have distinct out of band databases from the rpm one. Perhaps now with the switch to sqlite that could be fixed?

OTOH we also have a "database" of sorts in the origin file, which is #2326

cgwalters

Nice work on this! The level of machinery involved to plumb through new things like this is unfortunate; I was hoping to address that as part of work on #2326 but we should definitely land this first.

docs/treefile.md

rust/src/modularity.rs

src/app/rpmostree-libbuiltin.cxx

jlebon · 2021-04-27T16:56:07Z

OK, got something that works pretty well now, so will start to split things out.

The way I'm judging how successful this PR is for the compose side is by trying to express current RHCOS compose semantics via these new knobs. And with the latest here, we can now do that. (Edit: I realized I should add some context here: basically once rpm-ostree knows about modules, then current RHCOS composes break because it currently relies on rpm-ostree being non-modularity aware. One hack around this would be to spam module_hotfixes=true in all the repos, but ideally we should express what we actually want in module terms too anyway.) Here's what I'm testing with:

diff --git a/rhaos.repo b/rhaos.repo
index c6781a2..4f03ed9 100644
--- a/rhaos.repo
+++ b/rhaos.repo
@@ -7,6 +7,3 @@
 enabled=1
 gpgcheck=0
 baseurl=http://.../RHAOS/plashets/4.8-el8/building/$basearch/os
-# Exclude protobuf until:
-# https://issues.redhat.com/browse/ART-2355
-exclude=nss-altfiles kernel protobuf
diff --git a/rhel8.repo b/rhel8.repo
index f6aff14..25c5331 100644
--- a/rhel8.repo
+++ b/rhel8.repo
@@ -12,9 +12,6 @@ gpgcheck=0
 baseurl=http://.../appstream/os/
 enabled=1
 gpgcheck=0
-# Exclude toolbox to make sure we use coreos/toolbox from rhaos repo
-# kata-containers wants non-modular libpmem
-exclude=conmon toolbox libpmem*module*

diff --git a/manifest.yaml b/manifest.yaml
index ad4cac0..54faada 100644
--- a/manifest.yaml
+++ b/manifest.yaml
@@ -31,6 +31,23 @@ repos:
   - rhel-8-fast-datapath-aarch64
   - rhel-8-server-ose

+repo-packages:
+  # we always want the kernel from BaseOS
+  - repo: rhel-8-baseos
+    packages:
+    - kernel
+  # there's an equal version nss-altfiles from RHAOS we don't want
+  - repo: rhel-8-appstream
+    packages:
+    - nss-altfiles
+  - repo: rhel-8-server-ose
+    packages:
+    # we don't want the modular version of this
+    - toolbox
+    # we want this one from RHAOS to make sure it's in sync with cri-o
+    # https://github.com/openshift/installer/pull/3062#issuecomment-582614009
+    - conmon
+
 # We include hours/minutes to avoid version number reuse
 automatic-version-prefix: "48.84.<date:%Y%m%d%H%M>"
 # This ensures we're semver-compatible which OpenShift wants
@@ -254,6 +271,11 @@ packages:
  # https://github.com/openshift/machine-config-operator/pull/2421
  - conntrack-tools

+modules:
+  enable:
+    - container-tools:rhel8
+    - virt:rhel
+
 packages-x86_64:
   # Temporary add of open-vm-tools. Should be removed when containerized
   - open-vm-tools

I think some of these things we can clean up further, but at least we know we can express what we want.

jlebon · 2021-05-07T19:23:11Z

Rebased this now! Also now includes only enabling modules, e.g.:

[root@qemu0 ~]# rpm-ostree module enable cri-o:1.20
Staging deployment... done
[root@qemu0 ~]# rpm-ostree install cri-tools
Checking out tree febf21a... done
...
Added:
  cri-tools-1.19.0-1.module_f34+10489+4277ba4d.x86_64
Run "systemctl reboot" to start a reboot
[root@qemu0 ~]# rpm-ostree status
State: idle
Deployments:
  ostree://fedora:fedora/x86_64/coreos/testing-devel
                   Version: 34.20210507.dev.0 (2021-05-07T01:16:20Z)
                BaseCommit: febf21a7a23306d472705261390223210a40bdc47c7654e98a23708abed1bcb2
              GPGSignature: (unsigned)
                      Diff: 1 added
           LayeredPackages: cri-tools
            EnabledModules: cri-o:1.20

* ostree://fedora:fedora/x86_64/coreos/testing-devel
                   Version: 34.20210507.dev.0 (2021-05-07T01:16:20Z)
                    Commit: febf21a7a23306d472705261390223210a40bdc47c7654e98a23708abed1bcb2
              GPGSignature: (unsigned)
                  Unlocked: development

This is now blocked on rpm-software-management/libdnf#1207. And also still need to add tests!

cgwalters

I only gave this a superficial skim so far but I think it's looking good!

rust/src/modularity.rs

jlebon · 2021-06-21T14:28:28Z

OK, now with more tests! This also requires rpm-software-management/libdnf#1278. There are a few more tests I'd like to add, and I also need to fix polkit interactions, but overall this is ready for more in-depth review!

jlebon · 2021-07-23T18:43:04Z

Ahh right OK. This is unblocked on the libdnf side, but I need to adapt things for the origin/treespec rework.

jlebon · 2021-07-23T20:28:06Z

Rebased!

cgwalters

This is awesome! Really well done!

rust/src/modularity.rs

rust/src/treefile.rs

src/daemon/rpmostree-sysroot-upgrader.cxx

src/daemon/rpmostreed-transaction-types.cxx

cgwalters · 2021-07-24T13:24:42Z

src/libpriv/rpmostree-core.cxx

            }
        }
    }

+  gboolean we_got_modules = FALSE;


(Random aside, we can probably use more C++ for stuff like this, i.e. auto we_got_modules = false;)

Yeah I guess so. Though really the end goal is Rust anyway, so meh... slightly leaning towards sticking with gboolean instead to be consistent.

src/libpriv/rpmostree-util.cxx

cgwalters · 2021-07-24T13:31:27Z

src/daemon/rpmostreed-transaction-types.cxx

@@ -601,6 +601,10 @@ typedef struct {
  char  **install_pkgs; /* strv but strings owned by modifiers */
  GUnixFDList *install_local_pkgs;
  char  **uninstall_pkgs; /* strv but strings owned by modifiers */
+  char  **enable_modules; /* strv but strings owned by modifiers */


Unrelated but I think this PR demonstrates really well we still have far too much "mechanical stuff" to plumb through to add new stuff like this. So many conversions from GVariant to C data types (char**) back into GKeyFile then back into char** etc...with Rust and C++ in there too...

Hmm. You know what's tempting is to change UpdateDeployment to go from GVariant a{sv} -> Treefile internally. That would be a huge step towards #2326 in letting the API take a new treefile directly.

The thing is that UpdateDeployment is "delta-based", whereas Treefile is about layering. E.g. we need to be able to say "stop layering this package".

Yep, good point. We could at least represent additions via treefile. But maybe it'd be messier to have a half-declarative and half-imperative API?

Random thought: We could have two treefiles: One for additions, and one for removals. The "removal" treefile would have

packages: - foo - bar

as usual, but the code would interpret it as things to remove?

The `:` character is shell-safe and needs no escaping. Prep for modularity, which uses colons a lot and so not escaping it will make `rpm-ostree status` cleaner.

Prep for using it in modularity code.

This stopped being used after coreos#2972.

We already query enabled repos higher up in this function.

That way, we don't e.g. have stale RPMs or tests in there.

Support for modules is something that has come up many times in the past. Most recently in FCOS: coreos/fedora-coreos-tracker#767 This commit adds support for both composing with modules on the server side and package layering modules (or just enabling them) on the client side. This doesn't support modules in `rpm-ostree compose extensions` yet, which is relevant for RHCOS. We can look at adding that in a follow-up.

jlebon · 2021-07-26T20:49:14Z

Updated!

jlebon · 2021-07-26T21:02:18Z

Also discovered rpm-software-management/libdnf#1299 while respinning and testing this, but we don't strictly need it before merging.

nothingneko · 2022-01-09T22:29:38Z

Hi, how can we include Modules (Like Clang) in our treefiles, we're building based on AlmaLinux and need a few

openshift-ci-robot added the do-not-merge/work-in-progress label Apr 16, 2021

jlebon commented Apr 16, 2021

View reviewed changes

src/libpriv/rpmostree-core.cxx Outdated Show resolved Hide resolved

jlebon force-pushed the pr/modularity branch 2 times, most recently from 78fbdc6 to 3e7c820 Compare April 23, 2021 18:53

jlebon changed the title ~~WIP: Add support for composing with modules~~ WIP: Add support for modules Apr 23, 2021

jlebon mentioned this pull request Apr 23, 2021

Kubernetes v1.24+ container runtime on Fedora CoreOS coreos/fedora-coreos-tracker#767

Open

jlebon force-pushed the pr/modularity branch from 3e7c820 to aa9d7cd Compare April 23, 2021 21:00

jlebon mentioned this pull request Apr 23, 2021

fix libdnf critical #2477

Open

cgwalters reviewed Apr 26, 2021

View reviewed changes

docs/treefile.md Show resolved Hide resolved

rust/src/modularity.rs Show resolved Hide resolved

src/app/rpmostree-libbuiltin.cxx Outdated Show resolved Hide resolved

jlebon force-pushed the pr/modularity branch from aa9d7cd to 9e79903 Compare April 27, 2021 16:44

openshift-ci bot added the needs-rebase label Apr 27, 2021

This was referenced Apr 27, 2021

Prep patches for modularity and pinned repo packages #2785

Merged

DnfContext: make dnf_context_module_install like dnf module install rpm-software-management/libdnf#1207

Merged

jlebon force-pushed the pr/modularity branch from 9e79903 to de95235 Compare May 7, 2021 19:19

openshift-ci bot removed the needs-rebase label May 7, 2021

jlebon force-pushed the pr/modularity branch 2 times, most recently from ce20364 to f3abc33 Compare May 10, 2021 19:03

cgwalters reviewed May 18, 2021

View reviewed changes

rust/src/modularity.rs Outdated Show resolved Hide resolved

openshift-ci bot added the needs-rebase label May 19, 2021

jlebon force-pushed the pr/modularity branch 2 times, most recently from 75f7141 to 397227e Compare May 19, 2021 14:44

openshift-ci bot removed the needs-rebase label May 19, 2021

openshift-ci bot removed the do-not-merge/work-in-progress label Jun 21, 2021

jlebon mentioned this pull request Jun 22, 2021

Release 2021.6? #2919

Closed

openshift-ci bot added the needs-rebase label Jun 26, 2021

jlebon force-pushed the pr/modularity branch from 33917a4 to e324b40 Compare July 23, 2021 20:27

openshift-ci bot removed the needs-rebase label Jul 23, 2021

cgwalters previously approved these changes Jul 24, 2021

View reviewed changes

cgwalters added the reviewed label Jul 26, 2021

jlebon added 6 commits July 26, 2021 16:34

rust/utils: Mark : as shell-safe

defa8b8

The `:` character is shell-safe and needs no escaping. Prep for modularity, which uses colons a lot and so not escaping it will make `rpm-ostree status` cleaner.

rust/utils: Add safe wrapper for print_treepkg_diff_from_sysroot_path

99a6b39

Prep for using it in modularity code.

daemon/upgrader: Drop unused variable

98a6d26

This stopped being used after coreos#2972.

core: Drop duplicate querying for enabled repos

7238bd9

We already query enabled repos higher up in this function.

tests/kolainst: Nuke tests/kola before regenerating

711b528

That way, we don't e.g. have stale RPMs or tests in there.

jlebon dismissed cgwalters’s stale review via 8de71b8 July 26, 2021 20:48

jlebon force-pushed the pr/modularity branch from e324b40 to 8de71b8 Compare July 26, 2021 20:48

jlebon removed the reviewed label Jul 26, 2021

cgwalters approved these changes Jul 27, 2021

View reviewed changes

cgwalters merged commit df64da4 into coreos:main Jul 27, 2021

This was referenced Jul 30, 2021

manifest: adapt for rpm-ostree modularity support openshift/os#603

Merged

Add rpm-ostree support for modules #1435

Closed

manifests/fedora-coreos-base: stop disabling modular repos coreos/fedora-coreos-config#1149

Closed

hardoverflow mentioned this pull request Nov 16, 2021

Failed to install cri-o packages on Fedora CoreOS kubernetes-sigs/kubespray#8197

Closed

jlebon deleted the pr/modularity branch April 23, 2023 23:32

miabbott mentioned this pull request Aug 26, 2024

Package can't be found by rpm-ostree compose tree but can be found by dnf #5054

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for modules #2760

Add support for modules #2760

jlebon commented Apr 16, 2021 •

edited

Loading

openshift-ci-robot commented Apr 16, 2021

jlebon commented Apr 16, 2021

jlebon commented Apr 23, 2021

jlebon commented Apr 23, 2021 •

edited

Loading

jlebon commented Apr 23, 2021

cgwalters commented Apr 26, 2021

cgwalters left a comment

jlebon commented Apr 27, 2021 •

edited

Loading

jlebon commented May 7, 2021

cgwalters left a comment

jlebon commented Jun 21, 2021 •

edited

Loading

jlebon commented Jul 23, 2021

jlebon commented Jul 23, 2021

cgwalters left a comment

cgwalters Jul 24, 2021

jlebon Jul 26, 2021

cgwalters Jul 24, 2021

jlebon Jul 26, 2021

cgwalters Jul 27, 2021

cgwalters Jul 28, 2021

jlebon commented Jul 26, 2021

jlebon commented Jul 26, 2021

nothingneko commented Jan 9, 2022

Add support for modules #2760

Add support for modules #2760

Conversation

jlebon commented Apr 16, 2021 • edited Loading

openshift-ci-robot commented Apr 16, 2021

jlebon commented Apr 16, 2021

jlebon commented Apr 23, 2021

jlebon commented Apr 23, 2021 • edited Loading

jlebon commented Apr 23, 2021

cgwalters commented Apr 26, 2021

cgwalters left a comment

Choose a reason for hiding this comment

jlebon commented Apr 27, 2021 • edited Loading

jlebon commented May 7, 2021

cgwalters left a comment

Choose a reason for hiding this comment

jlebon commented Jun 21, 2021 • edited Loading

jlebon commented Jul 23, 2021

jlebon commented Jul 23, 2021

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters Jul 24, 2021

Choose a reason for hiding this comment

jlebon Jul 26, 2021

Choose a reason for hiding this comment

cgwalters Jul 24, 2021

Choose a reason for hiding this comment

jlebon Jul 26, 2021

Choose a reason for hiding this comment

cgwalters Jul 27, 2021

Choose a reason for hiding this comment

cgwalters Jul 28, 2021

Choose a reason for hiding this comment

jlebon commented Jul 26, 2021

jlebon commented Jul 26, 2021

nothingneko commented Jan 9, 2022

jlebon commented Apr 16, 2021 •

edited

Loading

jlebon commented Apr 23, 2021 •

edited

Loading

jlebon commented Apr 27, 2021 •

edited

Loading

jlebon commented Jun 21, 2021 •

edited

Loading