Keep last version of deployment

[r4f4]

Putting this up for initial review while I try to work on adding tests.
This PR is implementing the solution proposed in coreos/rpm-ostree#1905.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: r4f4
To complete the pull request process, please assign cgwalters
You can assign the PR to them by writing /assign @cgwalters in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rh-atomic-bot]

Can one of the admins verify this patch?
I understand the following commands:

• bot, add author to whitelist
• bot, test pull request
• bot, test pull request once

[jlebon]

bot, add author to whitelist

[jlebon]

We can also handle opt_not_as_default and the other opt_retain_* options too now, right? Shouldn't be too hard, though you don't have to do this in this PR!

[r4f4]

Ok, I'll try to do it in a separate PR.

[cgwalters]

Hmm. Not opposed but this feels a bit like it's crossing into logic that may be better purely in rpm-ostree? On the other hand maybe any future apt/ebuild-ostree tools would want this too.

[r4f4]

Rebased and squashed commits.

[r4f4]

Hmm. Not opposed but this feels a bit like it's crossing into logic that may be better purely in rpm-ostree? On the other hand maybe any future apt/ebuild-ostree tools would want this too.

@jlebon any comments?

[jlebon]

Sorry for not getting back to this!

Hmm. Not opposed but this feels a bit like it's crossing into logic that may be better purely in rpm-ostree? On the other hand maybe any future apt/ebuild-ostree tools would want this too.

Yeah, I guess this walks a fine line. I can definitely see it belonging in rpm-ostree first before getting upstreamed to ostree once it becomes useful. Though for that, rpm-ostree would have to duplicate a lot of the logic from ostree_sysroot_simple_write_deployment. Also, the version metadata key concept does live here, so it doesn't seem too outlandish either to have this feature here?

[rh-atomic-bot]

☔ The latest upstream changes (presumably #1981) made this pull request unmergeable. Please resolve the merge conflicts.

[jlebon]

I'm using this rpm-ostree patch to test this:

diff --git a/src/daemon/rpmostree-sysroot-upgrader.c b/src/daemon/rpmostree-sysroot-upgrader.c
index e3f5acef..d16ea710 100644
--- a/src/daemon/rpmostree-sysroot-upgrader.c
+++ b/src/daemon/rpmostree-sysroot-upgrader.c
@@ -1380,11 +1380,12 @@ rpmostree_sysroot_upgrader_deploy (RpmOstreeSysrootUpgrader *self,

       g_auto(RpmOstreeProgress) task = { 0, };
       rpmostree_output_task_begin (&task, "Staging deployment");
-      if (!ostree_sysroot_stage_tree (self->sysroot, self->osname,
+      if (!ostree_sysroot_stage_tree_with_flags (self->sysroot, self->osname,
                                       target_revision, origin,
                                       self->cfg_merge_deployment,
                                       self->kargs_strv,
                                       &new_deployment,
+                                      OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN_PREVIOUS_VERSION,
                                       cancellable, error))
         return FALSE;
     }

So here's how you would test this using cosa (assuming your cosa workdir is at /srv/fcos):

ostree-dir$ make install DESTDIR=/src/fcos/overrides/rootfs
rpm-ostree-dir$ make install DESTDIR=/src/fcos/overrides/rootfs
/srv/fcos$ cosa build
/srv/fcos$ cosa run

Inside the VM, you can do something like this:

$ ostree commit -b tmp --tree=ref=<checksum-of-booted-deployment> --add-metadata-string version=my-version-1
$ rpm-ostree rebase :tmp
<reboot>
$ ostree commit -b tmp --tree=ref=tmp --add-metadata-string version=my-version-2
$ rpm-ostree upgrade
<reboot>
$ rpm-ostree install ltrace

Once you're up and running, you can use make vmsync in rpm-ostree to hack on top of this without having to rebuild FCOS each time (there's a cosa run --ssh-config which spits out an SSH config file you can give to make vmsync).

[jlebon]

@r4f4 Could you rebase and squash all the fixups? Will give it a final review and test!

[r4f4]

Rebased and squashed!

[rh-atomic-bot]

☔ The latest upstream changes (presumably #2025) made this pull request unmergeable. Please resolve the merge conflicts.

[openshift-ci-robot]

@r4f4: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[r4f4]

Are you guys still interested on this or should I close the PR?

[jlebon]

Yes, I'm still interested in this! I need to set aside some time to test it out.

[r4f4]

Ok. I'll try to rebase and fix the conflicts.

[jlebon]

OK cool, this works well for me in both the staging and non-staging paths (after adding the missing bit in ot-admin-builtin-deploy.c).

Here's how you can pretty easily test this locally in a VM:

[root@qemu0 ~]# rpm-ostree status
State: idle
Deployments:
* ostree://fedora:fedora/x86_64/coreos/next
                   Version: 34.20210503.1.1 (2021-05-12T22:57:51Z)
                    Commit: c229af6df100607f73216ecda14dfc3a71e4c7094f4a7cdd260dc97a5c650a3e
              GPGSignature: Valid signature by 8C5BA6990BDB26E19F2A1A801161AE6945719A39
                  Unlocked: development

[root@qemu0 ~]# ostree commit -b previous-version --add-metadata-string=version=34.20210503.1.0 --tree=ref=c229af6df100607f73216ecda14dfc
3a71e4c7094f4a7cdd260dc97a5c650a3e
b6437a1dae3345aa73908bee2fc4606f56e81a338cb4a9082c51042572294534

[root@qemu0 ~]# ostree admin deploy previous-version --not-as-default
Copying /etc changes: 8 modified, 0 removed, 30 added
Bootloader updated; bootconfig swap: yes; bootversion: boot.0.1, deployment count change: 1

[root@qemu0 ~]# ostree commit -b same-version --add-metadata-string=version=34.20210503.1.1 --tree=ref=c229af6df100607f73216ecda14dfc3a71
e4c7094f4a7cdd260dc97a5c650a3e
3478fbc53e070b3d57c86893417329385cd7bc746670d8d2933b93e51b595b02

[root@qemu0 ~]# ostree admin deploy same-version --retain-previous-version
Copying /etc changes: 8 modified, 0 removed, 30 added
Bootloader updated; bootconfig swap: yes; bootversion: boot.1.1, deployment count change: 1
Freed objects: 190 bytes

[root@qemu0 ~]# rpm-ostree status
State: idle
Deployments:
  ostree://same-version
                   Version: 34.20210503.1.1 (2021-05-14T20:11:40Z)
                    Commit: 3478fbc53e070b3d57c86893417329385cd7bc746670d8d2933b93e51b595b02

* ostree://fedora:fedora/x86_64/coreos/next
                   Version: 34.20210503.1.1 (2021-05-12T22:57:51Z)
                    Commit: c229af6df100607f73216ecda14dfc3a71e4c7094f4a7cdd260dc97a5c650a3e
              GPGSignature: Valid signature by 8C5BA6990BDB26E19F2A1A801161AE6945719A39
                  Unlocked: development

  ostree://previous-version
                   Version: 34.20210503.1.0 (2021-05-14T20:11:00Z)
                    Commit: b6437a1dae3345aa73908bee2fc4606f56e81a338cb4a9082c51042572294534

Then you can also test the staging path by nuking the pending deployment (rpm-ostree cleanup -p) and re-deploying, but this time with --stage followed by ostree admin finalize-staged.

In practice at the rpm-ostree level I think we should just omit the flag in the rpm-ostree rebase case, which is like what we're doing here since we're changing branches, but this works to test the change here. (Or maybe even only enable this in the NO_PULL_BASE case to start.)

[r4f4]

@jlebon I think I've addressed all the points and the test is now passing [2021-05-20T17:04:53.378Z] --- PASS: ext.ostree.destructive.staged-deploy.sh (151.61s)

[openshift-ci]

@r4f4: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.