From 9e2ceca06fc5f226ab6141df65daa2cd0fe1843f Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 18 Sep 2019 11:21:42 -0400 Subject: [PATCH] app/deploy: Gate 77 exit behind --unchanged-exit-77 This has a bit of history, but essentially in 1c01141e, we made both `upgrade` and `deploy` automatically exit 77 if there were no changes. Then in c3f1e7c8, we only changed `upgrade` so that it became gated behind `--upgrade-unchanged-exit-77`. I think we should carry this forward into `deploy` as well. The way I look at this is: the default UX shouldn't require users to care about special exit codes. That's something scripts care about. In its vanilla form, either a command should error out or succeed. This patch tries to add some consistency by introducing a new `--unchanged-exit-77` in both `deploy` and `upgrade` (where it just replaces the previous switch). The naming here matches what `install` has too. So... this does break backwards compatibility for any scripts which relied on that behaviour. Though the only app I know today which wants deploy semantics and doesn't use the D-Bus API is Zincati, which actually hit this issue. There's also RHCOS, though the `pivot` there uses `rebase`, not `deploy`. So overall, I think this is worth breaking now while we're still in a transitionary period in the downstreams? Closes: #1906 Approved by: cgwalters --- man/rpm-ostree.xml | 22 ++++++++++++++-------- src/app/rpmostree-builtin-deploy.c | 8 ++++++-- src/app/rpmostree-builtin-upgrade.c | 11 +++++++---- tests/vmcheck/test-download-only.sh | 3 ++- tests/vmcheck/test-misc-2.sh | 2 +- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/man/rpm-ostree.xml b/man/rpm-ostree.xml index d20064a2..10c9088b 100644 --- a/man/rpm-ostree.xml +++ b/man/rpm-ostree.xml @@ -179,11 +179,13 @@ Boston, MA 02111-1307, USA. - In addition to exit status 0 for success and 1 for error, - this command also uses exit status 77 to indicate that the - system is already on the specified commit. This tristate - return model is intended to support idempotency-oriented - systems automation tools like Ansible. + + --unchanged-exit-77 + + to exit status 77 to indicate that the system is already + on the specified commit. This tristate return model is + intended to support idempotency-oriented systems + automation tools like Ansible. @@ -424,9 +426,13 @@ Boston, MA 02111-1307, USA. - In addition to exit status 0 for success and 1 for error, - this command also uses exit status 77 to indicate that no - upgrade is available. + + --unchanged-exit-77 + + to exit status 77 to indicate that the system is already + up to date. This tristate return model is intended to + support idempotency-oriented systems automation tools like + Ansible. diff --git a/src/app/rpmostree-builtin-deploy.c b/src/app/rpmostree-builtin-deploy.c index b83e79c7..3f756c2d 100644 --- a/src/app/rpmostree-builtin-deploy.c +++ b/src/app/rpmostree-builtin-deploy.c @@ -34,6 +34,7 @@ static gboolean opt_cache_only; static gboolean opt_download_only; static gboolean opt_lock_finalization; static gboolean opt_disallow_downgrade; +static gboolean opt_unchanged_exit_77; static GOptionEntry option_entries[] = { { "os", 0, 0, G_OPTION_ARG_STRING, &opt_osname, "Operate on provided OSNAME", "OSNAME" }, @@ -47,6 +48,7 @@ static GOptionEntry option_entries[] = { { "download-only", 0, 0, G_OPTION_ARG_NONE, &opt_download_only, "Just download latest ostree and RPM data, don't deploy", NULL }, { "lock-finalization", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &opt_lock_finalization, "Prevent automatic deployment finalization on shutdown", NULL }, { "disallow-downgrade", 0, 0, G_OPTION_ARG_NONE, &opt_disallow_downgrade, "Forbid deployment of chronologically older trees", NULL }, + { "unchanged-exit-77", 0, 0, G_OPTION_ARG_NONE, &opt_unchanged_exit_77, "If no new deployment made, exit 77", NULL }, { NULL } }; @@ -179,7 +181,8 @@ rpmostree_builtin_deploy (int argc, if (g_variant_n_children (result) == 0) { - invocation->exit_code = RPM_OSTREE_EXIT_UNCHANGED; + if (opt_unchanged_exit_77) + invocation->exit_code = RPM_OSTREE_EXIT_UNCHANGED; return TRUE; } @@ -189,7 +192,8 @@ rpmostree_builtin_deploy (int argc, { if (!rpmostree_has_new_default_deployment (os_proxy, previous_deployment)) { - invocation->exit_code = RPM_OSTREE_EXIT_UNCHANGED; + if (opt_unchanged_exit_77) + invocation->exit_code = RPM_OSTREE_EXIT_UNCHANGED; return TRUE; } diff --git a/src/app/rpmostree-builtin-upgrade.c b/src/app/rpmostree-builtin-upgrade.c index 22cfea89..62de5ba2 100644 --- a/src/app/rpmostree-builtin-upgrade.c +++ b/src/app/rpmostree-builtin-upgrade.c @@ -37,6 +37,7 @@ static gboolean opt_allow_downgrade; static gboolean opt_preview; static gboolean opt_check; static gboolean opt_upgrade_unchanged_exit_77; +static gboolean opt_unchanged_exit_77; static gboolean opt_cache_only; static gboolean opt_download_only; static char *opt_automatic; @@ -48,11 +49,13 @@ static GOptionEntry option_entries[] = { { "reboot", 'r', 0, G_OPTION_ARG_NONE, &opt_reboot, "Initiate a reboot after operation is complete", NULL }, { "allow-downgrade", 0, 0, G_OPTION_ARG_NONE, &opt_allow_downgrade, "Permit deployment of chronologically older trees", NULL }, { "check-diff", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &opt_preview, "Check for upgrades and print package diff only", NULL }, - { "preview", 0, 0, G_OPTION_ARG_NONE, &opt_preview, "Just preview package differences", NULL }, - { "check", 0, 0, G_OPTION_ARG_NONE, &opt_check, "Just check if an upgrade is available", NULL }, + { "preview", 0, 0, G_OPTION_ARG_NONE, &opt_preview, "Just preview package differences (implies --unchanged-exit-77)", NULL }, + { "check", 0, 0, G_OPTION_ARG_NONE, &opt_check, "Just check if an upgrade is available (implies --unchanged-exit-77)", NULL }, { "cache-only", 'C', 0, G_OPTION_ARG_NONE, &opt_cache_only, "Do not download latest ostree and RPM data", NULL }, { "download-only", 0, 0, G_OPTION_ARG_NONE, &opt_download_only, "Just download latest ostree and RPM data, don't deploy", NULL }, - { "upgrade-unchanged-exit-77", 0, 0, G_OPTION_ARG_NONE, &opt_upgrade_unchanged_exit_77, "If no upgrade is available, exit 77", NULL }, + /* legacy alias for --unchanged-exit-77 */ + { "upgrade-unchanged-exit-77", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &opt_upgrade_unchanged_exit_77, "If no upgrade is available, exit 77", NULL }, + { "unchanged-exit-77", 0, 0, G_OPTION_ARG_NONE, &opt_unchanged_exit_77, "If no new deployment made, exit 77", NULL }, { "trigger-automatic-update-policy", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &opt_automatic, "For automated use only; triggered by automatic timer", NULL }, { "lock-finalization", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &opt_lock_finalization, "Prevent automatic deployment finalization on shutdown", NULL }, { NULL } @@ -229,7 +232,7 @@ rpmostree_builtin_upgrade (int argc, { if (!rpmostree_has_new_default_deployment (os_proxy, previous_deployment)) { - if (opt_upgrade_unchanged_exit_77) + if (opt_upgrade_unchanged_exit_77 || opt_unchanged_exit_77) invocation->exit_code = RPM_OSTREE_EXIT_UNCHANGED; return TRUE; } diff --git a/tests/vmcheck/test-download-only.sh b/tests/vmcheck/test-download-only.sh index 954a92eb..c7dfdf1d 100755 --- a/tests/vmcheck/test-download-only.sh +++ b/tests/vmcheck/test-download-only.sh @@ -97,7 +97,8 @@ echo "ok offline rebase & install" rc=0 vm_rpmostree upgrade --upgrade-unchanged-exit-77 || rc=$? assert_streq "$rc" "77" -vm_rpmostree upgrade -C --upgrade-unchanged-exit-77 || rc=$? +# also try out new alias for this +vm_rpmostree upgrade -C --unchanged-exit-77 || rc=$? assert_streq "$rc" "77" echo "ok check for change with --cache-only" diff --git a/tests/vmcheck/test-misc-2.sh b/tests/vmcheck/test-misc-2.sh index 97824c93..194310eb 100755 --- a/tests/vmcheck/test-misc-2.sh +++ b/tests/vmcheck/test-misc-2.sh @@ -203,7 +203,7 @@ vm_assert_status_jq '.deployments[0]["pending-base-checksum"]' # hard reset to booted csum (simulates what deploy does to remote refspecs) vm_cmd ostree reset vmcheck $(vm_get_booted_csum) rc=0 -vm_rpmostree deploy $(vm_get_booted_csum) > out.txt || rc=$? +vm_rpmostree deploy $(vm_get_booted_csum) --unchanged-exit-77 > out.txt || rc=$? if [ $rc != 77 ]; then assert_not_reached "trying to re-deploy same commit didn't exit 77" fi