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

ci: Test opt-usrlocal-overlays end-to-end in Prow CI #4810

Merged
merged 5 commits into from
Feb 27, 2024

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Jan 30, 2024

Since both Prow and CoreOS CI do a compose and run kola tests, we
can change one of them to cover some different options without losing
coverage on the default path.

Follow-up to #4728.

@jlebon
Copy link
Member Author

jlebon commented Jan 30, 2024

OK weird, for some reason rpm-ostree-2024.2-2.fc39.x86_64.rpm doesn't have /usr/lib64/rpm-ostree/rpm-ostree-0-integration-opt-usrlocal-compat.conf. Will investigate.

@HuijingHei
Copy link
Member

HuijingHei commented Jan 31, 2024

Seems /usr/lib64/rpm-ostree/rpm-ostree-0-integration-opt-usrlocal-compat.conf is added by 3dc18c, which is not included in any release yet.

But during rsync -rlv /cosa/component-install/ overrides/rootfs/, can find usr/lib64/rpm-ostree/rpm-ostree-0-integration-opt-usrlocal-compat.conf.

@jlebon
Copy link
Member Author

jlebon commented Jan 31, 2024

Seems /usr/lib64/rpm-ostree/rpm-ostree-0-integration-opt-usrlocal-compat.conf is added by 3dc18c, which is not included in any release yet.

But during rsync -rlv /cosa/component-install/ overrides/rootfs/, can find usr/lib64/rpm-ostree/rpm-ostree-0-integration-opt-usrlocal-compat.conf.

Yes good catch, thanks. I misremembered and thought I had added rpm-ostree-0-integration-opt-usrlocal-compat.conf as part of #4763.

@jlebon
Copy link
Member Author

jlebon commented Feb 6, 2024

I just moved over Prow to build the RPM too in #4819. Once that merges, I'll rebase this.

@jlebon jlebon force-pushed the pr/opt-usrlocal-testing branch from a4dbe7f to 04f2798 Compare February 7, 2024 12:23
@jlebon
Copy link
Member Author

jlebon commented Feb 7, 2024

/retest-required

@jlebon
Copy link
Member Author

jlebon commented Feb 7, 2024

Prow infra not having a good time apparently.

@jlebon
Copy link
Member Author

jlebon commented Feb 7, 2024

/retest

@jlebon
Copy link
Member Author

jlebon commented Feb 7, 2024

OK right, Prow CI needs coreos/fedora-coreos-config#2838 to work.

@jlebon
Copy link
Member Author

jlebon commented Feb 8, 2024

/retest

@jlebon jlebon force-pushed the pr/opt-usrlocal-testing branch from 04f2798 to 0b5b860 Compare February 9, 2024 20:35
@jlebon
Copy link
Member Author

jlebon commented Feb 12, 2024

Ahh OK, the state overlay stuff is breaking with apply-live because the transient /usr mount shadows the overlay mounts. I think the simplest is probably to have ostree_sysroot_deployment_unlock() to know to recreate the overlay mounts afterwards? The state overlays will show up twice in the mount graph but meh, you can only unlock once, so twice is the worst case. (Hmm, if/once mount-beneath becomes available, possibly we could have the /usr overlay mount use that instead.)

@cgwalters
Copy link
Member

Yeah, there's similar clash between usroverlay and systemd-sysext I think. A yet further wrinkle here is that in the composefs case there is no ostree-generated /usr bind mount - while usroverlay works what we probably really want is an ostree enable-transient verb that just flips the / composefs to gain a transient writable backing overlay (I think that's possible, if not we can just make a new composefs-overlayfs mount and swap).

jlebon added a commit to jlebon/rpm-ostree that referenced this pull request Feb 21, 2024
@jlebon
Copy link
Member Author

jlebon commented Feb 22, 2024

/retest

jlebon added a commit to jlebon/rpm-ostree that referenced this pull request Feb 23, 2024
jlebon added a commit to jlebon/rpm-ostree that referenced this pull request Feb 23, 2024
@HuijingHei
Copy link
Member

  • Get error:
qemu0 kola-runext-misc.sh[2242]: Feb 22 21:33:42 localhost systemd-tmpfiles[1153]: /sysroot/usr/lib/tmpfiles.d/rpm-ostree-autovar.conf:76: Duplicate line for path "/sysroot/var/opt", ignoring.
qemu0 kola-runext-misc.sh[2242]: Feb 22 21:33:54 localhost systemd-tmpfiles[1496]: /usr/lib/tmpfiles.d/rpm-ostree-autovar.conf:76: Duplicate line for path "/var/opt", ignoring.
error: Should not get logs (Duplicate line)

As we have another different definition for /var/opt which is from package filesystem, this is harmless, I think.

[root@cosa-devsh tmpfiles.d]# grep /var/opt * -rn
rpm-ostree-0-integration-opt-usrlocal-compat.conf:5:L /var/opt - - - - ../usr/lib/opt
rpm-ostree-autovar.conf:76:d /var/opt 0755 root root - -   (--> comes from /usr/lib/rpm-ostree/tmpfiles.d/filesystem.conf)
  • Another failed log is localhost systemd-tmpfiles[1642]: Failed to create directory or subvolume "/usr/local/man": Read-only file system, but other directories like /usr/local/bin are all created.

@jlebon jlebon force-pushed the pr/opt-usrlocal-testing branch from d24462b to 62ffba4 Compare February 27, 2024 03:11
jlebon added a commit to jlebon/rpm-ostree that referenced this pull request Feb 27, 2024
@jlebon jlebon force-pushed the pr/opt-usrlocal-testing branch from 62ffba4 to 69d39d9 Compare February 27, 2024 03:15
Since both Prow and CoreOS CI do a compose and run kola tests, we
can change one of them to cover some different options without losing
coverage on the default path.

Follow-up to coreos#4728.
This helped me narrow down an issue I was hitting.
Rather than injecting our "integration" intmpfiles.d dropins in
postprocessing, make it part of the tree during the install phase. This
allows the new code that generates `rpm-ostree-autovar.conf` to take
it into account when trying to get rid of duplicates from the dropins
derived per-package.

Adjust the idempotency marker in `postprocess_final()` to use `usr/lib/
passwd` since we can't use the tmpfiles.d dropin for that anymore there.
@jlebon jlebon force-pushed the pr/opt-usrlocal-testing branch from 1bfc2ed to eee3bb1 Compare February 27, 2024 17:33
@jlebon
Copy link
Member Author

jlebon commented Feb 27, 2024

OK, I think I got it all now! Tests which use apply-live are just skipped for now. Commit "compose: Inject our static tmpfiles.d dropins earlier" is the only non-trivial one; that one probably deserves more attention.

@jlebon
Copy link
Member Author

jlebon commented Feb 27, 2024

/retest

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Looks generally sane to me. I still have some concerns about the state overlay approach, but that's not a reason to block it.

@cgwalters cgwalters enabled auto-merge (rebase) February 27, 2024 20:41
@jlebon
Copy link
Member Author

jlebon commented Feb 27, 2024

I still have some concerns about the state overlay approach, but that's not a reason to block it.

Here's how I see it: the transient root approach and state overlays approach both solve the /opt issue, but within different models of how the OS should work (the container model and the immutable model). The immutable model is still quite desirable (and I would argue should be the default) and solving the /opt issue there is still valuable.

@cgwalters
Copy link
Member

This PR is probably not the best place to debate but...I wouldn't call stateoverlays "immutable" - they're very much about mutable persistent state.

@jlebon
Copy link
Member Author

jlebon commented Feb 27, 2024

This PR is probably not the best place to debate but...I wouldn't call stateoverlays "immutable" - they're very much about mutable persistent state.

I'm not saying state overlays are immutable. Only that it solves the /opt issue within an otherwise immutable OS. It adds the minimum amount of mutability where needed to achieve that.

@cgwalters cgwalters merged commit e1e78cf into coreos:main Feb 27, 2024
17 checks passed
cgwalters pushed a commit that referenced this pull request Feb 27, 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.

3 participants