app: smarter deployment change detection
Commands like `upgrade` and `deploy` need to know if a new deployment was actually laid down so that it may print a pkg diff if so. This is implemented by listening for changes to the DefaultDeployment D-Bus property. D-Bus emits a signal when the deployment variant changes value. However, in #595, with the introduction of `pending-*` related keys, the deployment variant no longer represents data solely tied to that specific deployment. In this case, because `deploy` operations currently set the ref to the resolved checksum, it can happen that deploying the same base commit when the current refspec *isn't* pointing to that base commit will result in the `pending-*` keys dropping out and a default deployment change notification going out. In this patch, we strengthen how we determine whether a new deployment was laid down by actually looking at the deployment id, rather than just assuming that a change to the property implies a new deployment. Closes: #981 Closes: #984 Approved by: cgwalters
This commit is contained in:
parent
077d7c1a9e
commit
4ad3ea58c6
@ -52,7 +52,6 @@ rpmostree_builtin_deploy (int argc,
|
||||
g_autoptr(GOptionContext) context = NULL;
|
||||
glnx_unref_object RPMOSTreeOS *os_proxy = NULL;
|
||||
glnx_unref_object RPMOSTreeSysroot *sysroot_proxy = NULL;
|
||||
g_autoptr(GVariant) new_default_deployment = NULL;
|
||||
g_autofree char *transaction_address = NULL;
|
||||
const char * const packages[] = { NULL };
|
||||
const char *revision;
|
||||
@ -93,6 +92,8 @@ rpmostree_builtin_deploy (int argc,
|
||||
cancellable, &os_proxy, error))
|
||||
return EXIT_FAILURE;
|
||||
|
||||
g_autoptr(GVariant) previous_deployment = rpmostree_os_dup_default_deployment (os_proxy);
|
||||
|
||||
if (opt_preview)
|
||||
{
|
||||
if (!rpmostree_os_call_download_deploy_rpm_diff_sync (os_proxy,
|
||||
@ -105,8 +106,6 @@ rpmostree_builtin_deploy (int argc,
|
||||
}
|
||||
else
|
||||
{
|
||||
rpmostree_monitor_default_deployment_change (os_proxy, &new_default_deployment);
|
||||
|
||||
g_autoptr(GVariant) options =
|
||||
rpmostree_get_options_variant (opt_reboot,
|
||||
TRUE, /* allow-downgrade */
|
||||
@ -174,7 +173,7 @@ rpmostree_builtin_deploy (int argc,
|
||||
}
|
||||
else if (!opt_reboot)
|
||||
{
|
||||
if (new_default_deployment == NULL)
|
||||
if (!rpmostree_has_new_default_deployment (os_proxy, previous_deployment))
|
||||
return RPM_OSTREE_EXIT_UNCHANGED;
|
||||
|
||||
/* do diff without dbus: https://github.com/projectatomic/rpm-ostree/pull/116 */
|
||||
|
@ -60,7 +60,6 @@ rpmostree_builtin_upgrade (int argc,
|
||||
g_autoptr(GOptionContext) context = g_option_context_new ("");
|
||||
glnx_unref_object RPMOSTreeOS *os_proxy = NULL;
|
||||
glnx_unref_object RPMOSTreeSysroot *sysroot_proxy = NULL;
|
||||
g_autoptr(GVariant) new_default_deployment = NULL;
|
||||
g_autofree char *transaction_address = NULL;
|
||||
_cleanup_peer_ GPid peer_pid = 0;
|
||||
const char *const *install_pkgs = NULL;
|
||||
@ -107,6 +106,8 @@ rpmostree_builtin_upgrade (int argc,
|
||||
cancellable, &os_proxy, error))
|
||||
return EXIT_FAILURE;
|
||||
|
||||
g_autoptr(GVariant) previous_deployment = rpmostree_os_dup_default_deployment (os_proxy);
|
||||
|
||||
if (opt_preview || opt_check)
|
||||
{
|
||||
if (!rpmostree_os_call_download_update_rpm_diff_sync (os_proxy,
|
||||
@ -117,8 +118,6 @@ rpmostree_builtin_upgrade (int argc,
|
||||
}
|
||||
else
|
||||
{
|
||||
rpmostree_monitor_default_deployment_change (os_proxy, &new_default_deployment);
|
||||
|
||||
g_autoptr(GVariant) options =
|
||||
rpmostree_get_options_variant (opt_reboot,
|
||||
opt_allow_downgrade,
|
||||
@ -184,7 +183,7 @@ rpmostree_builtin_upgrade (int argc,
|
||||
}
|
||||
else if (!opt_reboot)
|
||||
{
|
||||
if (new_default_deployment == NULL)
|
||||
if (!rpmostree_has_new_default_deployment (os_proxy, previous_deployment))
|
||||
{
|
||||
if (opt_upgrade_unchanged_exit_77)
|
||||
return RPM_OSTREE_EXIT_UNCHANGED;
|
||||
|
@ -42,21 +42,28 @@ rpmostree_usage_error (GOptionContext *context,
|
||||
(void) glnx_throw (error, "usage error: %s", message);
|
||||
}
|
||||
|
||||
static void
|
||||
default_deployment_change_cb (GObject *object,
|
||||
GParamSpec *pspec,
|
||||
GVariant **value)
|
||||
static const char*
|
||||
get_id_from_deployment_variant (GVariant *deployment)
|
||||
{
|
||||
g_object_get (object, pspec->name, value, NULL);
|
||||
g_autoptr(GVariantDict) dict = g_variant_dict_new (deployment);
|
||||
const char *id;
|
||||
g_assert (g_variant_dict_lookup (dict, "id", "&s", &id));
|
||||
return id;
|
||||
}
|
||||
|
||||
void
|
||||
rpmostree_monitor_default_deployment_change (RPMOSTreeOS *os_proxy,
|
||||
GVariant **deployment)
|
||||
gboolean
|
||||
rpmostree_has_new_default_deployment (RPMOSTreeOS *os_proxy,
|
||||
GVariant *previous_deployment)
|
||||
{
|
||||
/* This will set the GVariant if the default deployment changes. */
|
||||
g_signal_connect (os_proxy, "notify::default-deployment",
|
||||
G_CALLBACK (default_deployment_change_cb), deployment);
|
||||
g_autoptr(GVariant) new_deployment = rpmostree_os_dup_default_deployment (os_proxy);
|
||||
|
||||
/* trivial case */
|
||||
if (g_variant_equal (previous_deployment, new_deployment))
|
||||
return FALSE;
|
||||
|
||||
const char *previous_id = get_id_from_deployment_variant (previous_deployment);
|
||||
const char *new_id = get_id_from_deployment_variant (new_deployment);
|
||||
return !g_str_equal (previous_id, new_id);
|
||||
}
|
||||
|
||||
/* Print the diff between the booted and pending deployments */
|
||||
|
@ -31,6 +31,10 @@ rpmostree_usage_error (GOptionContext *context,
|
||||
const char *message,
|
||||
GError **error);
|
||||
|
||||
gboolean
|
||||
rpmostree_has_new_default_deployment (RPMOSTreeOS *os_proxy,
|
||||
GVariant *previous_deployment);
|
||||
|
||||
gboolean
|
||||
rpmostree_print_treepkg_diff (OstreeSysroot *sysroot,
|
||||
GCancellable *cancellable,
|
||||
@ -41,8 +45,4 @@ rpmostree_print_treepkg_diff_from_sysroot_path (const gchar *sysroot_path,
|
||||
GCancellable *cancellable,
|
||||
GError **error);
|
||||
|
||||
void
|
||||
rpmostree_monitor_default_deployment_change (RPMOSTreeOS *os_proxy,
|
||||
GVariant **deployment);
|
||||
|
||||
G_END_DECLS
|
||||
|
@ -118,3 +118,21 @@ fi
|
||||
assert_not_file_has_content err.txt "Writing rpmdb"
|
||||
assert_file_has_content err.txt "File exists"
|
||||
echo "ok detecting file name conflicts before writing rpmdb"
|
||||
|
||||
# check that the way we detect deployment changes is not dependent on pending-*
|
||||
# https://github.com/projectatomic/rpm-ostree/issues/981
|
||||
vm_rpmostree cleanup -rp
|
||||
csum=$(vm_cmd ostree commit -b vmcheck --tree=ref=vmcheck)
|
||||
# restart to make daemon see the pending checksum
|
||||
vm_cmd systemctl restart rpm-ostreed
|
||||
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=$?
|
||||
if [ $rc != 77 ]; then
|
||||
assert_not_reached "trying to re-deploy same commit didn't exit 77"
|
||||
fi
|
||||
assert_file_has_content out.txt 'No change.'
|
||||
vm_assert_status_jq '.deployments[0]["pending-base-checksum"]|not'
|
||||
echo "ok changes to deployment variant don't affect deploy"
|
||||
|
Loading…
Reference in New Issue
Block a user