swupd-client icon indicating copy to clipboard operation
swupd-client copied to clipboard

update inactive partition

Open pohly opened this issue 8 years ago • 10 comments

We'd like to do A/B partition updates with swupd. "swupd verify --install/--fix" almost work, but not quite.

The expectation is:

  • Existing files are verified (attributes + content).
  • This full check should be on by default, with an option to disable it when the existing file content is trusted and the hash for it is known (i.e. there's a manifest for it) - not crucial.
  • Delta packs should be used when possible.
  • Switching between unrelated update streams has to work (for exampe, when comparing versions the code must be aware that version 100 on disk might not be the same as the target version 100 when the update stream is different).
  • This is needs to be a "first class citizen" in the features supported by swupd:
    • top-level command would be better than hiding it behind "verify" (not crucial)
    • automated testing for the feature under various circumstances
  • The swupd state directory either has to be on the target partition, or there must be an option to put the staging directory there.
  • The return code of the command must indicate success or failure.

"swupd verify --install" does not actually replace modified files. To reproduce with 3.12.0:

# rm -rf /tmp/swupd-state/ /tmp/swupd-root/ && mkdir /tmp/swupd-root
# swupd verify --install --picky -c https://download.clearlinux.org/update/ -v https://download.clearlinux.org/update/ -F 19 -m 17700 --time -S /tmp/swupd-state -p /tmp/swupd-root
...
# swupd verify --install --picky -c https://download.clearlinux.org/update/ -v https://download.clearlinux.org/update/ -F 19 -m 17730 --time -S /tmp/swupd-state -p /tmp/swupd-root
...
Adding any missing files

Inspected 2093 files
  0 files were missing
Calling post-update helper scripts.
...
# swupd verify --fix --picky -c https://download.clearlinux.org/update/ -v https://download.clearlinux.org/update/ -F 19 -m 17730 --time -S /tmp/swupd-state -p /tmp/swupd-root
...
Adding any missing files

Fixing modified files
Hash mismatch for file: /tmp/swupd-root/usr/lib/os-release
	fixed
Hash mismatch for file: /tmp/swupd-root/usr/lib/systemd/systemd-bootchart
	fixed
Hash mismatch for file: /tmp/swupd-root/usr/share/clear/version
	fixed
Hash mismatch for file: /tmp/swupd-root/usr/share/clear/versionstamp
	fixed
--picky removing extra files under /tmp/swupd-root/usr
Inspected 2067 files
  0 files were missing
  4 files did not match
    4 of 4 files were fixed
    0 of 4 files were not fixed
  0 files found which should be deleted

Only the "verify --fix" really changed the files.

Running the "verify --install -m 17700" followed by "verify --fix -m 17730" under strace and inspecting /tmp/swupd-state shows that it downloaded individual files instead of the pack-os-core-from-17700.tar.

I don't think it is necessary to support a "copy" mode of operation for moving files from staging to the actual tree. Just ensure that one can put the staging directory into the partition even when the state directory is elsewhere.

pohly avatar Sep 13 '17 10:09 pohly

Much of the issues raised above can be addressed by using the update command.

swupd update -c https://download.clearlinux.org/update/ \
-v https://download.clearlinux.org/update/ -F 19 -m 17700 --time \
-S /tmp/swupd-state -p /tmp/swupd-root`

The idea we have discussed is rsyncing over partition A to partition B and running the update on partition B. This means swupd doesn't have to download everything (with --install) for a blank partition.

To address most of your specific expectations:

Existing files are verified (attributes + content).

The update path calls deal_with_hash_mismatches which performs this check. The verify --fix path also calls this, while verify --install does not, as its intended use is only to add all OS files to a blank partition/directory.

This full check should be on by default, with an option to disable it when the existing file content is trusted and the hash for it is known (i.e. there's a manifest for it) - not crucial.

Do you mean you want to verify the updated partition every time you update? Then simply run swupd verify --fix after running the update command.

Delta packs should be used when possible.

The verify codepaths do not use delta packs. verify --fix runs iteratively by downloading/staging/renaming one file at a time in order to get the system in as unbroken a state as possible in the event of a broken system.

update of course uses delta packs.

Switching between unrelated update streams has to work (for exampe, when comparing versions the code must be aware that version 100 on disk might not be the same as the target version 100 when the update stream is different).

switching between unrelated update streams would require a verify --fix so that all the files on the system are checked, not just the ones in the current update.

  • This is needs to be a "first class citizen" in the features supported by swupd:
    • top-level command would be better than hiding it behind "verify" (not crucial)

Referring to --install, we agree. An issue will be opened to track this. It should also probably be called something else so users don't run swupd install <bundle name> and accidentally start installing the OS.

The swupd state directory either has to be on the target partition, or there must be an option to put the staging directory there.

The code should actually allow the hardlinking to fall back to copying across boundaries, if this isn't working it should be looked at.

matthewrsj avatar Sep 13 '17 19:09 matthewrsj

On Wed, 2017-09-13 at 12:01 -0700, Matthew Johnson wrote:

Much of the issues raised above can be addressed by using the update command.

swupd update -c https://download.clearlinux.org/update/
-v https://download.clearlinux.org/update/ -F 19 -m 17700 --time
-S /tmp/swupd-state -p /tmp/swupd-root`

The "update" command does not accept -m and always updates to the latest version. Sorry, I forgot to mention here that this needs to work in a push model where someone decides to install version XXX on device AAA, and device AAA then gets a notification to do that. The device then must not update to version YYY even if that has been released in the meantime. But I agree, once support for -m is added, "swupd update" should work.

To address most of your specific expectations:

Existing files are verified (attributes + content).

The update path calls deal_with_hash_mismatches which performs this check. 

Do you mean that "swupd update" always verifies all files? According to strace, it didn't. I was worried that by doing "swupd update + swupd verify" we'd end up hashing files, but that doesn't seem to be the case.

The verify --fix path also calls this, while verify --install does not, as its intended use is only to add all OS files to a blank partition/directory.

So the proposal is:* rsync* swupd update (no verification according to my test)* swupd verify --fix (explicit verification, if desired) Should work.

This is needs to be a "first class citizen" in the features supported by swupd:

You did not quote one sub-bullet of this point: "automated testing for the feature under various circumstances". The motivation for having this mode of operation supported directly by swupd was to push also testing upstream. If we build the solution around existing swupd operations, then a failure in a combination of those parts would only be detected after a swupd release. I'm fine doing it as you suggested, I'm just  pointing out the drawback.

The swupd state directory either has to be on the target partition, or there must be an option to put the staging directory there.

The code should actually allow the hardlinking to fall back to copying across boundaries, if this isn't working it should be looked at.

No, copying as fallback isn't desired. What seems to be missing is a "- -stagedir" parameter so that one can do:"swupd  update/verify -- statedir /tmp/swupd-state --stagedir /fast/build/swupd-root/.staging -- path  /fast/build/swupd-root"

pohly avatar Sep 14 '17 09:09 pohly

The "update" command does not accept -m and always updates to the latest version.

No, it does not accept -m currently, but this would be a relatively trivial feature to add and I see no problem adding it. Of course, the version passed to -m would have to be greater (or equal to then run verify --fix?) than the current version.

Do you mean that "swupd update" always verifies all files? According to strace, it didn't.

No, swupd update by itself does not verify all files on the file system, which is why I recommended running update followed by verify --fix

You did not quote one sub-bullet of this point: "automated testing for the feature under various circumstances".

Right, I'm not quite sure what you mean. We have functional and integration tests in place that are automatically run every time a commit is pushed to master. Do you mean we should add testing there, or something else? If you just mean adding testing around this I'm all for it.

No, copying as fallback isn't desired. What seems to be missing is a "- -stagedir" parameter so that one can do:"swupd update/verify -- statedir /tmp/swupd-state --stagedir /fast/build/swupd-root/.staging -- path /fast/build/swupd-root"

Currently staging is done in the state dir. You are proposing the staging be done in a configurable location?

matthewrsj avatar Sep 14 '17 16:09 matthewrsj

On Thu, 2017-09-14 at 09:01 -0700, Matthew Johnson wrote:

No, it does not accept -m currently, but this would be a relatively trivial feature to add and I see no problem adding it. Of course, the version passed to -m would have to be greater (or equal to then run verify --fix?) than the current version.

Agreed.

You did not quote one sub-bullet of this point: "automated testing for the feature under various circumstances".

Right, I'm not quite sure what you mean. We have functional and integration tests in place that are automatically run every time a commit is pushed to master. Do you mean we should add testing there, or something else? If you just mean adding testing around this I'm all for it.

It would be useful to include testing for the complete proposed workflow, including all the necessary sanity checks. At which point we talk about writing a script which implements the workflow, and then we are back at whether a dedicated swupd command might not be the better solution.

Currently staging is done in the state dir. You are proposing the staging be done in a configurable location?

Yes.

pohly avatar Sep 14 '17 18:09 pohly

Depending on whether swupd-client is invoked for the currently active filesystem or some inactive one, post-update actions vary. What's currently hard-coded into swupd-client (/usr/bin/systemctl daemon-reexec/reload, /usr/bin/systemctl restart update-triggers.target) is not applicable when updating the inactive partition. Same for run_preupdate_scripts (runs /usr/bin/clr_pre_update.sh).

Given that the current proposal is to use a combination of various swupd operations to achieve the final filesystem update, the helper script needs its own post-update hook mechanism instead of relying on systemd.

What about adding a "--no-scripts" option to "swupd update" and (if necessary) "swupd verify" which suppresses running these additional external helpers?

pohly avatar Sep 20 '17 11:09 pohly

I think that's viable but keep in mind that the inactive partition wouldn't get the swupd tmpfiles created for it which could lead to breakage due to unexpected missing parts in the inactive partition.

bryteise avatar Sep 20 '17 14:09 bryteise

On Wed, 2017-09-20 at 14:36 +0000, William Douglas wrote:

I think that's viable but keep in mind that the inactive partition wouldn't get the swupd tmpfiles created for it which could lead to breakage due to unexpected missing parts in the inactive partition.

Can you elaborate more on that? What exactly would be missing? How can this be handled properly for the use case described in this issue?

pohly avatar Sep 20 '17 14:09 pohly

So I just did some testing, and I think something changed or I have forgotten how the system works but I don't see the tmpfiles being created by update, verify --fix or verify --install so it won't make anything worse than current but it seems like that's a bug. Regardless, if you don't have the tmpfiles run at system startup you'll find you are missing things like /etc/resolv.conf. Just making sure that service runs at startup avoids this issue though (or if you ship things in /etc or /var you'll need to do some sort of overlay in order to be stateless).

bryteise avatar Sep 20 '17 15:09 bryteise

On Wed, 2017-09-20 at 08:00 -0700, William Douglas wrote:

So I just did some testing, and I think something changed or I have forgotten how the system works but I don't see the tmpfiles being created by update, verify --fix or verify --install so it won't make anything worse than current but it seems like that's a bug.

Depends on what life system update is supposed to achieve in Clear Linux. Arjan recently explained that it is the job of the OS, not swupd, to maintain suitable post-update actions. So if this is considered a bug, then it needs to be filed against Clear Linux, not swupd.

Clear Linux is not restarting services either, so I guess not running systemd-tmpfiles after an update is just another example of why currently a reboot is necessary to actual get the system up-to-date.

Regardless, if you don't have the tmpfiles run at system startup you'll find you are missing things like /etc/resolv.conf. Just making sure that service runs at startup avoids this issue though (or if you ship things in /etc or /var you'll need to do some sort of overlay in order to be stateless).

For the A/B setup describe here I envision that it strictly separates between code from partition A and B, i.e. except for some actions that are crucial for rebooting (like boot loader changes) I'd expect to set up post-update actions so that they run after rebooting into the updated partition.

pohly avatar Sep 21 '17 06:09 pohly

A full script implementing this idea is now here: https://github.com/pohly/meta-swupd/blob/master/recipes-core/swupd-client/files/swupd-update-partition.sh

It's currently not doing the rsync part, but that can be added.

pohly avatar Sep 29 '17 15:09 pohly