`ostree admin kargs edit-in-place` assertion reached when deployments are pending

[travier]

See https://github.com/ostreedev/ostree/issues/2664:

Jul 25 13:57:17 fedora ostree-sysroot-readonly[2417]: + ostree admin kargs edit-in-place --append-if-missing=rw
Jul 25 13:57:17 fedora audit[2427]: ANOM_ABEND auid=4294967295 uid=0 gid=0 ses=4294967295 subj=system_u:system_r:unconfined_service_t:s0 pid=2427 comm="ostree" exe="/usr/bin/ostree" sig=6 res=1
Jul 25 13:57:17 fedora ostree-sysroot-readonly[2427]: **
Jul 25 13:57:17 fedora ostree-sysroot-readonly[2427]: OSTree:ERROR:src/libostree/ostree-sysroot-deploy.c:3618:ostree_sysroot_deployment_set_kargs_in_place: assertion failed: (!ostree_deployment_is_staged (deployment))
Jul 25 13:57:17 fedora ostree-sysroot-readonly[2427]: Bail out! OSTree:ERROR:src/libostree/ostree-sysroot-deploy.c:3618:ostree_sysroot_deployment_set_kargs_in_place: assertion failed: (!ostree_deployment_is_staged (deployment))
Jul 25 13:57:17 fedora systemd[1]: Created slice system-systemd\x2dcoredump.slice - Slice /system/systemd-coredump.
Jul 25 13:57:17 fedora audit: BPF prog-id=131 op=LOAD
Jul 25 13:57:17 fedora audit: BPF prog-id=132 op=LOAD
Jul 25 13:57:17 fedora audit: BPF prog-id=133 op=LOAD
Jul 25 13:57:17 fedora audit[1]: SERVICE_START pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 msg='unit=systemd-coredump@0-2428-0 comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=>
Jul 25 13:57:17 fedora systemd[1]: Started [email protected] - Process Core Dump (PID 2428/UID 0).
Jul 25 13:57:17 fedora [2430]: [164B blob data]
Jul 25 13:57:17 fedora systemd-coredump[2429]: [🡕] Process 2427 (ostree) of user 0 dumped core.
                                               
                                               Module linux-vdso.so.1 with build-id 539b6f5c64df6de29594035a5c6820a79cbe1e44
                                               Module libcrypt.so.2 with build-id 9924d3a82ca8fa6b94bc574a6520bcf84ccf831f
                                               Stack trace of thread 2427:
                                               #0  0x00007fccfb34dc4c __pthread_kill_implementation (libc.so.6 + 0x8ec4c)
                                               #1  0x00007fccfb2fd9c6 raise (libc.so.6 + 0x3e9c6)
                                               #2  0x00007fccfb2e77f4 abort (libc.so.6 + 0x287f4)
                                               #3  0x00007fccfb5ced18 g_assertion_message.cold (libglib-2.0.so.0 + 0x1ed18)
                                               #4  0x00007fccfb62ebae g_assertion_message_expr (libglib-2.0.so.0 + 0x7ebae)
                                               #5  0x00007fccfb9a856d ostree_sysroot_deployment_set_kargs_in_place (libostree-1.so.1 + 0x8256d)
                                               #6  0x0000556a2e4d7e08 ot_admin_kargs_builtin_edit_in_place (ostree + 0x32e08)
                                               #7  0x0000556a2e4d4f47 ot_admin_builtin_kargs (ostree + 0x2ff47)
                                               #8  0x0000556a2e4cd540 ostree_builtin_admin (ostree + 0x28540)
                                               #9  0x0000556a2e4c0ad3 main (ostree + 0x1bad3)
                                               #10 0x00007fccfb2e8550 __libc_start_call_main (libc.so.6 + 0x29550)
                                               #11 0x00007fccfb2e8609 __libc_start_main@@GLIBC_2.34 (libc.so.6 + 0x29609)
                                               #12 0x0000556a2e4c0d75 _start (ostree + 0x1bd75)
                                               ELF object binary architecture: AMD x86-64

[travier]

This is lower priority as this should not happen in the currently planned use case but it would be great to not have ostree crash and exit cleanly instead.

[HuijingHei]

Thanks @travier for your report! Maybe should skip pending deployments? (https://github.com/ostreedev/ostree/blob/main/src/libostree/ostree-sysroot-deploy.c#L3618)

[travier]

I'd say yes, skip pending deployments

[travier]

A warning should be enough

[cgwalters]

I had to actually go look at the code for this, but it's worth noting that staged deployments today capture override kargs. So in theory, we should also apply edits to those I think.

For example, let's try e.g.:

$ rpm-ostree kargs --append=foo=bar
$ ostree admin kargs edit-in-place --append-if-missing=rw

The desired behavior is to have both i.e. foo=bar rw right?

[cgwalters]

Implementing this will require deserializing the staged deployment (see _ostree_sysroot_reload_staged) modifying its kargs member, then reserializing it (see the serialization bits in ostree_sysroot_stage_tree_with_options).

[HuijingHei]

Thanks @cgwalters and @travier for the suggestion, will do this when I finish my handy task

[travier]

Indeed, make sense to apply to the pending deployment too.