As detailed in
gitlab.gnome.org/GNOME/glib/-/issues/600#note_877282, volatile
isn't actually needed in these contexts because the atomic operations
already give us strong enough guarantees. In GCC 11, this triggers a
diagnostic due to the volatile qualifier getting dropped anyway.
There is a WIP to do the same in glib:
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719
This obsoletes this downstream patch:
https://src.fedoraproject.org/rpms/ostree/c/b8c5a6fb
The timeout timer should always be one-shot, so let's just always
destroy it in the callback. The main context has its own ref on it, so
it won't be freed behind its back.
This *should* fix a leak that was brought up in
https://bugzilla.redhat.com/show_bug.cgi?id=1891761.
Reported-by: Milan Crha <mcrha@redhat.com>
This is a basic `.packit.yaml` integration file which will allow us have
continuous builds of OSTree in cosa and upstream CI. If things go well,
we'll likely deploy this in other build tools like rpm-ostree.
Prompted by wanting to get #2155 out to unblock
https://github.com/coreos/rpm-ostree/pull/2170.
In FCOS and RHCOS, the need to configure software in the initramfs has
come up multiple times. Sometimes, using kernel arguments suffices.
Other times, it really must be a configuration file. Rebuilding the
initramfs on the client-side however is a costly operation. Not only
does it add complexity to the update workflow, it also erodes a lot of
the value obtained from using the baked "blessed" initramfs from the
tree itself.
One elegant way to address this is to allow specifying multiple
initramfses. This is supported by most bootloaders (notably GRUB) and
results in each initrd being overlayed on top of each other.
This patch allows libostree clients to leverage this so that they can
avoid regenerating the initramfs entirely. libostree itself is agnostic
as to what kind and how much data overlay initrds contain. It's up to
the clients to enforce such boundaries.
To implement this, we add a new ostree_sysroot_stage_overlay_initrd
which takes a file descriptor and returns a checksum. Then users can
pass these checksums when calling the deploy APIs via the new array
option `overlay_initrds`. We copy these files into `/boot` and add them
to the BLS as another `initrd` entry.
And make the `override_kernel_argv` one of those options. This is mostly
a mechanical move here, no functional change otherwise.
Prep for adding a new option.
That test will not make it into the fedora-coreos-config repo until the
libostree fix gets percolated down. PR is:
https://github.com/coreos/fedora-coreos-config/pull/586
But we want to make sure that the fix does work and that we don't
regress on it. So manually fetch it for now.
When we remount `/sysroot` as read-only, we also make `/etc` read-only.
This is usually OK because we then remount `/var` read-write, which also
flips `/etc` back to read-write... unless `/var` is a separate
filesystem and not a bind-mount to the stateroot `/var`.
Fix this by just remounting `/etc` read-write in the read-only sysroot
case.
Eventually, I think we should rework this to set everything up the way
we want from the initramfs (#2115). This would also eliminate the window
during which `/etc` is read-only while `ostree-remount` runs.
We were bind-mounting the initramfs' `/etc` (to itself) instead of the
target deployment `/etc` (to itself). Since we're already `chdir`'ed
into it, we can just drop the leading slash.
Don't mention deprecation in the description for
`ostree_sysroot_deploy_tree` since there are legitimate use cases for it
(e.g. to create the first deployment via `ostree admin deploy`).
Instead, make the comment clearly redirect to the staging API when
booted into the sysroot.
Tighten up how we handle kargs here so it's more clear. When we call
`sysroot_finalize_deployment`, any karg overrides have already been set
on the bootconfig object of the deployment. So re-setting it here is
redundant and confusing.
- Use `REV` instead of `REF` in places where we meant it.
- Fix `commit --parent` actually taking a commit checksum and not a ref.
- Fix `ostree admin switch` using `REF` instead of `REFSPEC`.
The default `_NPROCESSORS_ONLN` heuristic we have isn't cgroups aware.
So it thinks it has e.g. 40 CPUs when running in a k8s pod. This can
then blow through our allocated resource limits.
Declare some modest amount of RAM and CPU resources and override `make`
parallelism.
This matches what rpm-ostree now does in
https://github.com/coreos/rpm-ostree/pull/2155.
If `glnx_make_lock_file` falls back to `flock`, on NFS this uses POSIX
locks (`F_SETLK`). As such, we need to be able to handle `EACCES` as
well as `EAGAIN` (see `fnctl(2)`).
I think this is what coreos-ostree-importer has been hitting, which runs
on RHEL7 in the Fedora infra and does locking over an NFS share where
multiple apps could concurrently pull things into the repo.
And move everything that was in it directly in `ci/`. There's a bunch
more cleanups here that we need to do (and more changes to upstream from
the rpm-ostree copies of this).
We were doing a check to verify that `ostree admin upgrade` wouldn't
accept a downgrade without `--allow-downgrade`. However, there's no
guarantee that the commit it's upgrading from is older than HEAD^ (what
we're upgrading to). Specifically, if the test runs fast enough, the
timestamps could be equal, since the lowest resolution is seconds.
Rework the test so that we first upgrade to HEAD, which we're sure is at
least 1 second apart from HEAD^, and *then* check that downgrade
protection is enforced.
We also can't use `rev-parse testos/buildmaster/x86_64-runtime` as a way
to know what commit the host is sitting on since the ref might've gone
ahead. Instead, just use `ostree admin status | head -n1`. (I played
with using the `ostree/I/J/K` refs, but those depend on what the
boot/subbootversion is and can easily change if we change previous
tests).
For the same reason as https://github.com/coreos/rpm-ostree/pull/2094.
What we care most about is that the new commit we pull is newer than the
one we're currently sitting on, not necessarily that it's newer than the
branch itself, which it might not be if e.g. we're trying to deploy a
commit older than the tip but still newer than the deployment (via
`--override-commit`).
The way `timestamp-check` works might be too restrictive in some
situations. Essentially, we need to support the case where users want to
pull an older commit than the current tip, but while still guaranteeing
that it is newer than some even older commit.
This will be used in Fedora CoreOS. For more information see:
https://github.com/coreos/rpm-ostree/pull/2094https://github.com/coreos/fedora-coreos-tracker/issues/481
I've only noticed this by inspection. But I think it's possible for
`cleanup_txn_dir` to get called with the `staging-...-lock` file since
it matches the prefix.
Make the checking here stronger by verifying that it's a directory. If
it's not a directory (lockfile), then follow the default pruning expiry
logic so that we still cleanup stray lockfiles eventually.
We were using `--no-test-exit-error` for upgrade tests but weren't
actually checking for test failures after.
Instead of running kola directly, just use the `fcosKola` custom step
which automatically takes care of e.g. running tests in parallel and
archiving results.
For repo structure directories like `objects`, `refs`, etc... we should
be more permissive and let the system's `umask` narrow down the
permission bits as wanted.
This came up in a context where we want to be able to have read/write
access on an OSTree repo on NFS from two separate OpenShift apps by
using supplemental groups[1] so we don't require SCCs for running as the
same UID (supplemental groups are part of the default restricted SCC).
[1] https://docs.openshift.com/container-platform/3.11/install_config/persistent_storage/persistent_storage_nfs.html#nfs-supplemental-groups
To allow for FIPS mode, we need to also install the HMAC file from
`/usr/lib/modules` to `/boot` alongside the kernel image where the
`fips` dracut module will find it. For details, see:
https://github.com/coreos/fedora-coreos-tracker/issues/302
Note I didn't include the file in the boot checksum since it's itself a
checksum of the kernel, so we don't really gain much here other than
potentially causing an unnecessary bootcsum bump.
I was hitting `SIGSEGV` when running `cosa build` and narrowed it down
to #1954. What's happening here is that because we're using the default
context, when we unref it in the out path, it may not actually destroy
the `GSource` if it (the context) is still ref'ed elsewhere. So then,
we'd still get events from it if subsequent operations iterated the
context.
This patch is mostly a revert of #1954, except that we still keep a ref
on the `GSource`. That way it is always safe to destroy it afterwards.
(And I've also added a comment to explain this better.)
We're creating the timer source and then passing ownership to the
context, but because we didn't free the pointer, we would still call
`g_source_destroy` in the exit path. We'd do this right after doing
`unref` on the context too, which would have already destroyed and
unref'ed the source.
Drop that and just restrict the scope of that variable down to make
things more obvious.
Just noticed this after reviewing #1953.
Tiny release. Just want to get out the important bugfixes instead of
backporting patches (notably the gpg-agent stuff and
`ostree-finalize-staged.service` ordering).
Closes: #1927
Approved by: cgwalters
In Fedora 31, `systemd-journal-flush.service` uses a new
`--smart-relinquish-var` switch which fixes the
`umount: /var: target is busy` bug by telling journald to stop logging
to `/var` and back to `/run` again during shutdown.
This interacted with `ostree-finalize-staged.service` in a tricky way:
since we weren't strongly ordered against it, when we happened to
finalize after `/var` is relinquished, we never persisted the output
from that service to disk. This then threw off `rpm-ostree status` when
trying to find the completion message to know that finalization went
well.
Just fix this by adding an explicit `After=` on that unit. That way we
shut down *before* `systemd-journal-flush.service` (the `/var`
relinquish bit happens in its `ExecStop=`).
For more info, see:
https://github.com/systemd/systemd/commit/3ff7a50d66e3f851d3d9f132b740a7fb2055aa1dhttps://github.com/systemd/systemd/commit/1e187d2dd52cbb4f0bb30e4d96acf7f72a145b91https://bugzilla.redhat.com/show_bug.cgi?id=1751272Closes: #1926
Approved by: cgwalters
We would stop passing through `--` and args after it to the underlying
command in `ostree_run`. This made it impossible to use `--` to tell the
parser that following args starting with `-` really are positional.
AFAICT, that logic for `--` here came from a time when we parse options
manually in a big loop, in which case breaking out made sense (see
97558276e4).
There's an extra step here, which is that glib by default leaves the
`--` in the list of args, so we need to take care to remove it from the
list after parsing.
Closes: #1898Closes: #1899
Approved by: rfairley
There's a valid use case for enabling the timestamp downgrade check
while still also using override commits.
We'll make use of this in Fedora CoreOS, where the agent specifies the
exact commit to upgrade to, while still enforcing that it be newer.
Closes: #1891
Approved by: cgwalters
Teach `ostree-finalize-staged.service` to check for a file in `/run` to
determine if it should do the finalization. This will be used in
RPM-OSTree, where we want to be able to separate out "preparing updates"
from "making update the default" for more fine-grained control. See:
https://github.com/projectatomic/rpm-ostree/issues/1748Closes: #1841
Approved by: cgwalters
This can happen if a deployment was staged and later cleaned up. Though
just as a helper when debugging issues, let's explicitly mention that
case.
Closes: #1841
Approved by: cgwalters
Rather than wrapping each instance of `sd_journal_*` with
`HAVE_SYSTEMD`, let's just add some convenience macros that are just
no-op if we're not compiling with systemd.
Closes: #1841
Approved by: cgwalters
Otherwise, we'll be subject to whatever `umask` is currently. Normally,
processes should respect `umask` when creating files and directories,
but specifically for `ostree admin unlock` (or `rpm-ostree usroverlay`),
this poses a problem since e.g. a `/usr` with mode 0700 will break any
daemon that doesn't run as root and needs to read files under `/usr`,
such as polkitd.
This patch just does a `chmod()` after the `mkdir()`. An alternative
would be to do `umask(0000)` after forking into the child process
that'll call `mount()`, but that'd require also moving the `mkdir()`
calls into there, making for a more intrusive patch.
Closes: #1843
Approved by: cgwalters
Log a structured journal message when resolving the deployment path.
This will be used by the `rpm-ostree history` command to find past
deployments the system has booted into.
Closes: #1842
Approved by: cgwalters
Really, all `ostree admin finalize-staged` needs is access to `/sysroot`
and `/boot`. So let's activate it right after `local-fs.target` so that
it gets deactivated later in the shutdown process. This should allow us
to conflict with less services still running and possibly writing things
to `/etc`.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1672283Closes: #1840
Approved by: cgwalters
In Silverblue right now, the boot menu title looks like this:
Fedora 29.20190301.0 (Workstation Edition) 29.20190301.0 (ostree)
This is because RPM-OSTree's `mutate-os-release` feature is enabled,
which injects the OSTree version string directly into `VERSION` and
`PRETTY_NAME`. So appending the version string again is a bit redundant.
Let's just do a simple substring check here before adding the version to
the title.
Closes: #1829
Approved by: cgwalters
Even with the previous docstring, I didn't understand at first the
relationship between the `ostree-grub-generator` script and
`ostree-bootloader-grub2.c`. Throw some more docs to clarify things a
bit.
Closes: #1791
Approved by: cgwalters
When falling back to copying objects when importing them into a
bare-user repo, we only actually need to transfer over the
`user.ostreemeta` xattr.
This allows the destination repo to be on a separate filesystem that
might not even support `security.selinux`. (I hit this while importing
over virtio-9p).
Closes: #1771
Approved by: cgwalters
I found this useful while hacking on rpm-ostree but I think it might be
useful enough to upstream. This stat is really helpful for validating
that a pipeline is hitting the devino cache sweet spot.
Closes: #1772
Approved by: cgwalters
It might be "local", but e.g. we may be crossing filesystems. So there
are valid use cases for only wanting to pull the commit metadata with
`pull-local`.
Closes: #1769
Approved by: cgwalters
This is the alias version of #1749. I.e. we want to make sure that one
can't even create an alias which would end up dangling.
See also: https://pagure.io/releng/issue/7891Closes: #1768
Approved by: sinnykumari
Wrap the `Version` key in the YAML-compatible output of
`ostree --version` with quotes so that it's parsed as a string. The
issues with the previous approach in a nutshell:
```
In [5]: yaml.load("asdf: 2018.10")
Out[5]: {'asdf': 2018.1}
```
It's treating the version number as a floating-point. Now, this is
technically a backwards incompatible change, but given that the previous
approach is inherently broken for our needs, I don't see a way around
breaking it now.
Closes: #1761
Approved by: cgwalters
Rather than manually starting the `ostree-finalize-staged.service` unit,
we can leverage systemd's path units for this. It fits quite nicely too,
given that we already have a path we drop iif we have a staged
deployment.
To give some time for the preset to make it to systems, we don't yet
drop the explicit call to `systemctl start`. Though we do make it
conditional based on a DEBUG env var so that we can actually test it in
CI for now. Once we're sure this has propagated, we can drop the
`systemctl start` path and the env var together.
Closes: #1740
Approved by: cgwalters
Previously, we were preparing the root very late in the boot process;
right before we switch root. The issue with that is that most services
in the initrd that run `After=initrd-root-fs.target` expect that
`/sysroot` already points to the rootfs we'll be pivoting to. Running
this late violates that assumption.
This patch fixes this by making `ostree-prepare-root.service` instead
run right after `sysroot.mount` (the physical sysroot mounted by
systemd) but still before `initrd-root-fs.target` (which is the target
signalling that `/sysroot` is now valid and ready).
This should make it easier to integrate OSTree with other initrd
services such as Ignition.
Related: https://github.com/dustymabe/ignition-dracut/issues/20Closes: #1759
Approved by: cgwalters
For the same reasons as #1697. This is especially important in services
that are likely to be used as an `After/Before=` target in other units.
`ostree-prepare-root.service` is one such service.
Closes: #1759
Approved by: cgwalters
Write to the journal when starting to finalize a staged deployment.
Combined with the "Transaction completed" message we already emit, this
makes it easy later on to determine whether the operation was successful
by inspecting the journal. This will be used by `rpm-ostree status`.
Closes: #1750
Approved by: cgwalters
It's a neat way to point folks to the documentation (of course, better
would be to have man pages for each of those services). Also
consistently use Title Case everywhere.
Closes: #1750
Approved by: cgwalters
2018.7 started failing `test-ucontainer.sh`. I don't have the cycles to
look more deeply into what was going on there, but bumping to 2018.8
fixes it at least. (And of course, it's passing in rpm-ostree too.)
Closes: #1728
Approved by: cgwalters
Just include the whole URL that failed if libcurl failed with something
elementary like CURLE_COULDNT_CONNECT or CURLE_COULDNT_RESOLVE_HOST.
Closes: #1731Closes: #1732
Approved by: cgwalters
It started failing with:
```
ERROR: tests/check/test-ucontainer.sh - too few tests run (expected 2, got 0)
tap-driver.sh: internal error getting exit status
tap-driver.sh: fatal: I/O or internal error
make[4]: *** [Makefile:4353: tests/check/test-ucontainer.sh.log] Error 1
make[4]: *** Waiting for unfinished jobs....
```
And the artifacts are not being saved for some reason:
```
+ cleanup
+ mv test-suite.log /var/tmp/checkout
mv: cannot stat 'test-suite.log': No such file or directory
+ true
+ mv vmcheck /var/tmp/checkout
mv: cannot stat 'vmcheck': No such file or directory
+ true
```
Let's just disable this for now so that some other pending patches can
go in while we investigate.
Closes: #1727
Approved by: cgwalters
The latest rpm-ostree release no longer requires `python3-devel` so `dnf
builddep` here is no longer pulling it in, subsequently causing issues
when building an older rpm-ostree release. Let's just bump the release
tag so we don't have to also start keeping track of older dependencies.
Closes: #1708
Approved by: cgwalters
Now that we have `auto-update-summary`, there is no point in having
`commit-update-summary`. The latter also only had an effect through
the `commit` CLI command, whereas the former is embedded directly in
libostree.
There is one corner case that slips through: `commit` would update the
summary file even if orphan commits were created, which we no longer do
here. I can't imagine anyone relying on this, so it seems safe to drop.
Closes: #1689Closes: #1693
Approved by: mwleeds
Mildly bikeshed, though I find the name `auto-update-summary` to be
easier to grok than `change-update-summary`. I think it's because it can
be read as "verb-verb-noun" rather than "noun-verb-noun".
Closes: #1693
Approved by: mwleeds
Using `MAX(0, $x)` here is useless since we're comparing against an
unsigned integer. Just unpack this and only subtract if it's safe to do
so.
Also, explicitly check for `fd >= 0` rather than just `!= -1` to be sure
it's a valid fd. And finally, explicitly check the return value of
`g_input_stream_read_all` as is done everywhere else in the tree and
make it clear that we're purposely ignoring the return value of `_flush`
here, but not in other places.
Discovered by Coverity.
Closes: #1692
Approved by: cgwalters
I initially was going to add a `G_DEFINE_AUTOPTR_CLEANUP_FUNC` for
`FetchStaticDeltaData`, but it honestly didn't seem worth mucking around
ownership everywhere and potentially getting it wrong.
Discovered by Coverity.
Closes: #1692
Approved by: cgwalters
In `write_metadata_object()`, make sure when creating tombstone commits
that we're actually passed an expected checksum to use.
In `write_dir_entry_to_mtree_internal()`, sanity check that `dfd_iter`
is indeed not `NULL` before trying to dereference it.
Discovered by Coverity.
Closes: #1692
Approved by: cgwalters
One gotcha here is that we don't invalidate the RPMs if we're not
sitting on the same commit anymore. Shouldn't be too hard to fix, though
let's at least make a note of it for now.
Closes: #1668
Approved by: cgwalters
I made a logical error in #1617 which resulted in the exact *opposite*
behaviour we want when `/var` is a separate mount.
Split this out and lower the number of negations to make it more obvious
that it's correct.
Closes: #1667Closes: #1668
Approved by: cgwalters
And also print out the output if it still didn't start up in case there
are error messages hidden in there.
This should hopefully help with diagnosing the flakes we've been seeing
in starting it up.
Closes: #1652
Approved by: cgwalters
Jonathan Lebon