ostree-sysroot: make simple_write_deployment smarter

This is a follow-up to https://github.com/ostreedev/ostree/pull/1097.
We make simple_write_deployment smart enough so that it can be used for
rpm-ostree's purposes. This is mostly an upstreaming of logic that
already existed there.

Notably we correctly append NOT_DEFAULT deployments *after* the booted
deployment and we now support RETAIN_PENDING and RETAIN_ROLLBACK flags
to have more granularity on deployment pruning.

Expose these new flags on the CLI using new options (as well as expose
the previously existing NOT_DEFAULT flag as --not-as-default).

I couldn't add tests for --retain-pending because the merge deployment
is always the topmost one. Though I did check that it worked in a VM.

Closes: #1110
Approved by: cgwalters

diff --git a/bash/ostree b/bash/ostree
index f4305f69dc38b442e377f438d67a3797a8ea4c4d..fadb054d90dbd15a5c6ced9954cebf90f8de5d12 100644 (file)
--- a/bash/ostree
+++ b/bash/ostree
@@ -253,6 +253,9 @@ _ostree_admin_deploy() {
     local boolean_options="
         $main_boolean_options
         --retain
+        --retain-pending
+        --retain-rollback
+        --not-as-default
         --karg-proc-cmdline
     "
 

diff --git a/man/ostree-admin-deploy.xml b/man/ostree-admin-deploy.xml
index 347a4ba919c1443e58946ef190f0786c57328d64..ec705b934384273dfc913a964d95c0ea7688d929 100644 (file)
--- a/man/ostree-admin-deploy.xml
+++ b/man/ostree-admin-deploy.xml
@@ -89,6 +89,30 @@ Boston, MA 02111-1307, USA.
                 </para></listitem>
             </varlistentry>
 
+            <varlistentry>
+                <term><option>--retain-pending</option></term>
+
+                <listitem><para>
+                    Do not delete pending deployments.
+                </para></listitem>
+            </varlistentry>
+
+            <varlistentry>
+                <term><option>--retain-rollback</option></term>
+
+                <listitem><para>
+                    Do not delete rollback deployments.
+                </para></listitem>
+            </varlistentry>
+
+            <varlistentry>
+                <term><option>--not-as-default</option></term>
+
+                <listitem><para>
+                    Append rather than prepend new deployment.
+                </para></listitem>
+            </varlistentry>
+
             <varlistentry>
                 <term><option>--karg-proc-cmdline</option></term>
 

diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c
index 65cfb7c34f8026048cd9812199a01179c88b09fc..8e6c475c9864a152c479efa24ab3432758464d28 100644 (file)
--- a/src/libostree/ostree-sysroot.c
+++ b/src/libostree/ostree-sysroot.c
@@ -1487,6 +1487,12 @@ ostree_sysroot_init_osname (OstreeSysroot       *self,
  * If %OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN is
  * specified, then all current deployments will be kept.
  *
+ * If %OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN_PENDING is
+ * specified, then pending deployments will be kept.
+ *
+ * If %OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN_ROLLBACK is
+ * specified, then rollback deployments will be kept.
+ *
  * If %OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_NOT_DEFAULT is
  * specified, then instead of prepending, the new deployment will be
  * added right after the booted or merge deployment, instead of first.
@@ -1507,10 +1513,14 @@ ostree_sysroot_simple_write_deployment (OstreeSysroot      *sysroot,
 {
   const gboolean postclean =
     (flags & OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_NO_CLEAN) == 0;
-  const gboolean retain =
-    (flags & OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN) > 0;
   const gboolean make_default =
     !((flags & OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_NOT_DEFAULT) > 0);
+  const gboolean retain_pending =
+    (flags & OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN_PENDING) > 0;
+  const gboolean retain_rollback =
+    (flags & OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN_ROLLBACK) > 0;
+  gboolean retain =
+    (flags & OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN) > 0;
 
   g_autoptr(GPtrArray) deployments = ostree_sysroot_get_deployments (sysroot);
   OstreeDeployment *booted_deployment = ostree_sysroot_get_booted_deployment (sysroot);
@@ -1526,34 +1536,54 @@ ostree_sysroot_simple_write_deployment (OstreeSysroot      *sysroot,
       added_new = TRUE;
     }
 
+  /* without a booted and a merge deployment, retain_pending/rollback become meaningless;
+   * let's just retain all deployments in that case */
+  if (!booted_deployment && !merge_deployment && (retain_pending || retain_rollback))
+    retain = TRUE;
+
+  /* tracks when we come across the booted deployment */
+  gboolean before_booted = TRUE;
+  gboolean before_merge = TRUE;
   for (guint i = 0; i < deployments->len; i++)
     {
       OstreeDeployment *deployment = deployments->pdata[i];
-      const gboolean is_merge_or_booted =
-        ostree_deployment_equal (deployment, booted_deployment) ||
-        ostree_deployment_equal (deployment, merge_deployment);
-
-      /* Keep deployments with different osnames, as well as the
-       * booted and merge deployments
+      const gboolean osname_matches =
+        (osname == NULL || g_str_equal (ostree_deployment_get_osname (deployment), osname));
+      const gboolean is_booted = ostree_deployment_equal (deployment, booted_deployment);
+      const gboolean is_merge = ostree_deployment_equal (deployment, merge_deployment);
+
+      if (is_booted)
+        before_booted = FALSE;
+      if (is_merge)
+        before_merge = FALSE;
+
+      /* use the booted deployment as the "crossover" point between pending and rollback
+       * deployments, fall back on merge deployment */
+      const gboolean passed_crossover = booted_deployment ? !before_booted : !before_merge;
+
+      /* Retain deployment if:
+       *   - we're explicitly asked to, or
+       *   - the deployment is for another osname, or
+       *   - we're keeping pending deployments and this is a pending deployment, or
+       *   - this is the merge or boot deployment, or
+       *   - we're keeping rollback deployments and this is a rollback deployment
        */
-      if (retain ||
-          (osname != NULL && strcmp (ostree_deployment_get_osname (deployment), osname) != 0) ||
-          is_merge_or_booted)
-        {
-          g_ptr_array_add (new_deployments, g_object_ref (deployment));
-        }
-
-      if ((!added_new) && is_merge_or_booted)
+      if (retain
+          || !osname_matches
+          || (retain_pending && !passed_crossover)
+          || (is_booted || is_merge)
+          || (retain_rollback && passed_crossover))
+        g_ptr_array_add (new_deployments, g_object_ref (deployment));
+
+      /* add right after booted/merge deployment */
+      if (!added_new && passed_crossover)
         {
           g_ptr_array_add (new_deployments, g_object_ref (new_deployment));
           added_new = TRUE;
         }
     }
 
-  /* In this non-default case , an improvement in the future would be
-   * to put the new deployment right after the current default in the
-   * order.
-   */
+  /* add it last if no crossover defined (or it's the first deployment in the sysroot) */
   if (!added_new)
     {
       g_ptr_array_add (new_deployments, g_object_ref (new_deployment));

diff --git a/src/libostree/ostree-sysroot.h b/src/libostree/ostree-sysroot.h
index 3d2446f97b3e348bfe62961f69559534486718b6..f2573d6bd75172ca1c910ad5db2e763ea2b02d2f 100644 (file)
--- a/src/libostree/ostree-sysroot.h
+++ b/src/libostree/ostree-sysroot.h
@@ -207,6 +207,8 @@ typedef enum {
   OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN = (1 << 0),
   OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_NOT_DEFAULT = (1 << 1),
   OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_NO_CLEAN = (1 << 2),
+  OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN_PENDING = (1 << 3),
+  OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN_ROLLBACK = (1 << 4),
 } OstreeSysrootSimpleWriteDeploymentFlags;
 
 _OSTREE_PUBLIC

diff --git a/src/ostree/ot-admin-builtin-deploy.c b/src/ostree/ot-admin-builtin-deploy.c
index d67de791ede9892061ccb1ad8091d9ee827ad7eb..fa588f876a5060a02aee1c308ed0dd7ab649321d 100644 (file)
--- a/src/ostree/ot-admin-builtin-deploy.c
+++ b/src/ostree/ot-admin-builtin-deploy.c
@@ -33,6 +33,9 @@
 #include <glib/gi18n.h>
 
 static gboolean opt_retain;
+static gboolean opt_retain_pending;
+static gboolean opt_retain_rollback;
+static gboolean opt_not_as_default;
 static char **opt_kernel_argv;
 static char **opt_kernel_argv_append;
 static gboolean opt_kernel_proc_cmdline;
@@ -47,7 +50,10 @@ static char *opt_origin_path;
 static GOptionEntry options[] = {
   { "os", 0, 0, G_OPTION_ARG_STRING, &opt_osname, "Use a different operating system root than the current one", "OSNAME" },
   { "origin-file", 0, 0, G_OPTION_ARG_FILENAME, &opt_origin_path, "Specify origin file", "FILENAME" },
-  { "retain", 0, 0, G_OPTION_ARG_NONE, &opt_retain, "Do not delete previous deployment", NULL },
+  { "retain", 0, 0, G_OPTION_ARG_NONE, &opt_retain, "Do not delete previous deployments", NULL },
+  { "retain-pending", 0, 0, G_OPTION_ARG_NONE, &opt_retain_pending, "Do not delete pending deployments", NULL },
+  { "retain-rollback", 0, 0, G_OPTION_ARG_NONE, &opt_retain_rollback, "Do not delete rollback deployments", NULL },
+  { "not-as-default", 0, 0, G_OPTION_ARG_NONE, &opt_not_as_default, "Append rather than prepend new deployment", NULL },
   { "karg-proc-cmdline", 0, 0, G_OPTION_ARG_NONE, &opt_kernel_proc_cmdline, "Import current /proc/cmdline", NULL },
   { "karg", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_kernel_argv, "Set kernel argument, like root=/dev/sda1; this overrides any earlier argument with the same name", "NAME=VALUE" },
   { "karg-append", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_kernel_argv_append, "Append kernel argument; useful with e.g. console= that can be used multiple times", "NAME=VALUE" },
@@ -145,22 +151,28 @@ ot_admin_builtin_deploy (int argc, char **argv, GCancellable *cancellable, GErro
       _ostree_kernel_args_append_argv (kargs, opt_kernel_argv_append);
     }
 
-  {
-    g_auto(GStrv) kargs_strv = _ostree_kernel_args_to_strv (kargs);
-
   g_autoptr(OstreeDeployment) new_deployment = NULL;
-    if (!ostree_sysroot_deploy_tree (sysroot,
-                                     opt_osname, revision, origin,
-                                     merge_deployment, kargs_strv,
-                                     &new_deployment,
-                                     cancellable, error))
-      return FALSE;
-  }
+  g_auto(GStrv) kargs_strv = _ostree_kernel_args_to_strv (kargs);
+  if (!ostree_sysroot_deploy_tree (sysroot, opt_osname, revision, origin, merge_deployment,
+                                   kargs_strv, &new_deployment, cancellable, error))
+    return FALSE;
+
+  OstreeSysrootSimpleWriteDeploymentFlags flags = 0;
+  if (opt_retain)
+    flags |= OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN;
+  else
+    {
+      if (opt_retain_pending)
+        flags |= OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN_PENDING;
+      if (opt_retain_rollback)
+        flags |= OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN_ROLLBACK;
+    }
+
+  if (opt_not_as_default)
+    flags |= OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_NOT_DEFAULT;
 
-  if (!ostree_sysroot_simple_write_deployment (sysroot, opt_osname,
-                                               new_deployment, merge_deployment,
-                                               opt_retain ? OSTREE_SYSROOT_SIMPLE_WRITE_DEPLOYMENT_FLAGS_RETAIN : 0,
-                                               cancellable, error))
+  if (!ostree_sysroot_simple_write_deployment (sysroot, opt_osname, new_deployment,
+                                               merge_deployment, flags, cancellable, error))
     return FALSE;
 
   return TRUE;

diff --git a/tests/admin-test.sh b/tests/admin-test.sh
index 55de72356e6dc887eddebdb7e5be9c8122417892..d5566959796872843adbec4d69998b3d17682aa8 100644 (file)
--- a/tests/admin-test.sh
+++ b/tests/admin-test.sh
@@ -19,6 +19,8 @@
 
 set -euo pipefail
 
+echo "1..$((21 + ${extra_admin_tests:-0}))"
+
 function validate_bootloader() {
     cd ${test_tmpdir};
     bootloader=""
@@ -130,6 +132,8 @@ rm -r  sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/testdirectory
 rm sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/aconfigfile
 ln -s /ENOENT sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/a-new-broken-symlink
 ${CMD_PREFIX} ostree admin deploy --retain --os=testos testos:testos/buildmaster/x86_64-runtime
+assert_not_has_dir sysroot/boot/loader.0
+assert_has_dir sysroot/boot/loader.1
 linktarget=$(readlink sysroot/ostree/deploy/testos/deploy/${rev}.4/etc/a-new-broken-symlink)
 test "${linktarget}" = /ENOENT
 assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/os-release 'NAME=TestOS'
@@ -138,9 +142,36 @@ assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.4/etc/a-new-c
 assert_not_has_file sysroot/ostree/deploy/testos/deploy/${rev}.4/etc/aconfigfile
 ${CMD_PREFIX} ostree admin status
 validate_bootloader
-
 echo "ok deploy with modified /etc"
 
+# we now have 5 deployments, let's bring that back down to 1
+for i in $(seq 4); do
+  ${CMD_PREFIX} ostree admin undeploy 0
+done
+assert_has_file sysroot/boot/loader/entries/ostree-testos-0.conf
+assert_not_has_file sysroot/boot/loader/entries/ostree-testos-1.conf
+assert_not_has_file sysroot/boot/loader/entries/ostree-otheros-1.conf
+${CMD_PREFIX} ostree admin deploy --not-as-default --os=otheros testos:testos/buildmaster/x86_64-runtime
+assert_has_dir sysroot/boot/loader.0
+assert_not_has_dir sysroot/boot/loader.1
+assert_has_file sysroot/boot/loader/entries/ostree-testos-0.conf
+assert_has_file sysroot/boot/loader/entries/ostree-otheros-1.conf
+${CMD_PREFIX} ostree admin status
+validate_bootloader
+
+echo "ok deploy --not-as-default"
+
+${CMD_PREFIX} ostree admin deploy --retain-rollback --os=otheros testos:testos/buildmaster/x86_64-runtime
+assert_not_has_dir sysroot/boot/loader.0
+assert_has_dir sysroot/boot/loader.1
+assert_has_file sysroot/boot/loader/entries/ostree-otheros-0.conf
+assert_has_file sysroot/boot/loader/entries/ostree-testos-1.conf
+assert_has_file sysroot/boot/loader/entries/ostree-otheros-2.conf
+${CMD_PREFIX} ostree admin status
+validate_bootloader
+
+echo "ok deploy --retain-rollback"
+
 os_repository_new_commit
 ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmaster/x86_64-runtime
 newrev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos:testos/buildmaster/x86_64-runtime)
@@ -153,7 +184,7 @@ assert_file_has_content sysroot/ostree/deploy/testos/deploy/${newrev}.0/etc/os-r
 assert_file_has_content sysroot/ostree/deploy/testos/deploy/${newrev}.0/etc/a-new-default-config-file "a new default config file"
 assert_file_has_content sysroot/ostree/deploy/testos/deploy/${newrev}.0/etc/new-default-dir/moo "a new default dir and file"
 # And persist /etc changes from before
-assert_not_has_file sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/aconfigfile
+assert_not_has_file sysroot/ostree/deploy/testos/deploy/${rev}.4/etc/aconfigfile
 ${CMD_PREFIX} ostree admin status
 validate_bootloader
 

diff --git a/tests/test-admin-deploy-grub2.sh b/tests/test-admin-deploy-grub2.sh
index 6f785df521ce15dc742d8151b43ff25d78e552f4..60c7d05f4bcc2201b9d36d5acebd24477cdd5bd1 100755 (executable)
--- a/tests/test-admin-deploy-grub2.sh
+++ b/tests/test-admin-deploy-grub2.sh
@@ -19,11 +19,11 @@
 
 set -euo pipefail
 
-echo "1..19"
-
 . $(dirname $0)/libtest.sh
 
 # Exports OSTREE_SYSROOT so --sysroot not needed.
 setup_os_repository "archive-z2" "grub2 ostree-grub-generator"
 
+extra_admin_tests=0
+
 . $(dirname $0)/admin-test.sh

diff --git a/tests/test-admin-deploy-syslinux.sh b/tests/test-admin-deploy-syslinux.sh
index 3516c4786cefb3b1b5033ca532d16a7dae221b3b..08f26b5e28c8c41dbda49e67cec99d34b3f0f75d 100755 (executable)
--- a/tests/test-admin-deploy-syslinux.sh
+++ b/tests/test-admin-deploy-syslinux.sh
@@ -19,13 +19,13 @@
 
 set -euo pipefail
 
-echo "1..22"
-
 . $(dirname $0)/libtest.sh
 
 # Exports OSTREE_SYSROOT so --sysroot not needed.
 setup_os_repository "archive-z2" "syslinux"
 
+extra_admin_tests=3
+
 . $(dirname $0)/admin-test.sh
 
 # Test the legacy dirs

diff --git a/tests/test-admin-deploy-uboot.sh b/tests/test-admin-deploy-uboot.sh
index c7a6c751cb558bdab42e21fa54cae48a6f6b0f15..55059b345defaa9b9a8126de2e106ae0af545ce8 100755 (executable)
--- a/tests/test-admin-deploy-uboot.sh
+++ b/tests/test-admin-deploy-uboot.sh
@@ -20,13 +20,13 @@
 
 set -euo pipefail
 
-echo "1..20"
-
 . $(dirname $0)/libtest.sh
 
 # Exports OSTREE_SYSROOT so --sysroot not needed.
 setup_os_repository "archive-z2" "uboot"
 
+extra_admin_tests=1
+
 . $(dirname $0)/admin-test.sh
 
 cd ${test_tmpdir}