daemon: Fix cached-update including no-op diffs
The `cached-update` variant would mark a bunch of RPMs as upgraded even if they didn't actually change. The issue turned out to be we were doing the diff all wrong in the staged deployment case. I'm not sure what I was thinking in #1344, but essentially, we were marking all layered RPMs in the staged deployment as updates instead of only marking those layered RPMs which were actually changed EVR. We just simplify the approach here by directly doing a pkglist diff between the booted and staged deployments and consuming that. That's really all there is to it! Reduces the code quite a bit too. Closes: #1446 Closes: #1455 Approved by: cgwalters
This commit is contained in:
Jonathan Lebon
Atomic Bot
parent
a17b4b9be0
commit
7911b14f49
@ -571,17 +571,33 @@ modified_dnfpkg_variant_new (RpmOstreePkgTypes type,
|
||||
dnf_package_get_arch (pkg_new)));
|
||||
}
|
||||
|
||||
static void
|
||||
rpm_diff_add_base_db_diff (RpmDiff *diff,
|
||||
/* element-type RpmOstreePackage */
|
||||
GPtrArray *removed,
|
||||
GPtrArray *added,
|
||||
GPtrArray *modified_old,
|
||||
GPtrArray *modified_new)
|
||||
static gboolean
|
||||
rpm_diff_add_db_diff (RpmDiff *diff,
|
||||
OstreeRepo *repo,
|
||||
RpmOstreePkgTypes type,
|
||||
const char *old_checksum,
|
||||
const char *new_checksum,
|
||||
GPtrArray **out_modified_new,
|
||||
GCancellable *cancellable,
|
||||
GError **error)
|
||||
{
|
||||
g_assert_cmpuint (modified_old->len, ==, modified_new->len);
|
||||
g_autoptr(GPtrArray) removed = NULL;
|
||||
g_autoptr(GPtrArray) added = NULL;
|
||||
g_autoptr(GPtrArray) modified_old = NULL;
|
||||
g_autoptr(GPtrArray) modified_new = NULL;
|
||||
|
||||
RpmOstreePkgTypes type = RPM_OSTREE_PKG_TYPE_BASE;
|
||||
/* Use allow_noent; we'll just skip over the rpm diff if there's no data */
|
||||
RpmOstreeDbDiffExtFlags flags = RPM_OSTREE_DB_DIFF_EXT_ALLOW_NOENT;
|
||||
if (!rpm_ostree_db_diff_ext (repo, old_checksum, new_checksum, flags,
|
||||
&removed, &added, &modified_old, &modified_new,
|
||||
cancellable, error))
|
||||
return FALSE;
|
||||
|
||||
/* check if allow_noent kicked in */
|
||||
if (!removed)
|
||||
return TRUE; /* NB: early return */
|
||||
|
||||
g_assert_cmpuint (modified_old->len, ==, modified_new->len);
|
||||
for (guint i = 0; i < removed->len; i++)
|
||||
g_ptr_array_add (diff->removed, single_pkg_variant_new (type, removed->pdata[i]));
|
||||
for (guint i = 0; i < added->len; i++)
|
||||
@ -597,6 +613,10 @@ rpm_diff_add_base_db_diff (RpmDiff *diff,
|
||||
g_ptr_array_add (diff->downgraded,
|
||||
modified_pkg_variant_new (type, old_pkg, new_pkg));
|
||||
}
|
||||
|
||||
if (out_modified_new)
|
||||
*out_modified_new = g_steal_pointer (&modified_new);
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
static void
|
||||
@ -752,58 +772,6 @@ rpmmd_diff_guess (OstreeRepo *repo,
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
/* If we have a staged deployment, then those are our new pkgs already. All we need to do is
|
||||
* just find them in the rpmmd for advisory purposes. */
|
||||
static gboolean
|
||||
rpmmd_diff_exact (OstreeRepo *repo,
|
||||
const char *base_checksum,
|
||||
const char *layered_checksum,
|
||||
DnfSack *sack,
|
||||
RpmDiff *rpm_diff,
|
||||
GPtrArray **out_newer_packages,
|
||||
GError **error)
|
||||
{
|
||||
g_autoptr(GPtrArray) all_layered_pkgs = NULL;
|
||||
RpmOstreeDbDiffExtFlags flags = RPM_OSTREE_DB_DIFF_EXT_ALLOW_NOENT;
|
||||
if (!rpm_ostree_db_diff_ext (repo, base_checksum, layered_checksum, flags, NULL,
|
||||
&all_layered_pkgs, NULL, NULL, NULL, error))
|
||||
return FALSE;
|
||||
|
||||
if (all_layered_pkgs == NULL || /* -> older layer before we injected pkglist metadata */
|
||||
all_layered_pkgs->len == 0) /* -> no layered pkgs, e.g. override remove only */
|
||||
{
|
||||
*out_newer_packages = NULL;
|
||||
return TRUE; /* note early return */
|
||||
}
|
||||
|
||||
g_autoptr(GPtrArray) newer_packages =
|
||||
g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref);
|
||||
for (guint i = 0; i < all_layered_pkgs->len; i++)
|
||||
{
|
||||
RpmOstreePackage *pkg = all_layered_pkgs->pdata[i];
|
||||
g_autoptr(DnfPackage) dnfpkg = find_package (sack, FALSE, pkg);
|
||||
if (!dnfpkg)
|
||||
{
|
||||
/* We *should* be able to always find the pkg since we're probably using the same
|
||||
* rpmmd that was used to derive the layer in the first place. Handle gracefully if
|
||||
* somehow we don't, but log to journal. */
|
||||
sd_journal_print (LOG_WARNING, "Failed to find layered pkg %s in rpmmd.",
|
||||
rpm_ostree_package_get_nevra (pkg));
|
||||
continue;
|
||||
}
|
||||
|
||||
g_ptr_array_add (newer_packages, g_object_ref (dnfpkg));
|
||||
rpm_diff_add_layered_diff (rpm_diff, pkg, dnfpkg);
|
||||
}
|
||||
|
||||
/* canonicalize to NULL if there's nothing new */
|
||||
if (newer_packages->len == 0)
|
||||
g_clear_pointer (&newer_packages, (GDestroyNotify)g_ptr_array_unref);
|
||||
|
||||
*out_newer_packages = g_steal_pointer (&newer_packages);
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
/* convert those now to make the D-Bus API nicer and easier for clients */
|
||||
static RpmOstreeAdvisorySeverity
|
||||
str2severity (const char *str)
|
||||
@ -1058,43 +1026,39 @@ rpmostreed_update_generate_variant (OstreeDeployment *booted_deployment,
|
||||
g_auto(RpmDiff) rpm_diff = {0, };
|
||||
rpm_diff_init (&rpm_diff);
|
||||
|
||||
/* we'll need this later for advisories, so just keep it around */
|
||||
/* we'll need these later for advisories, so just keep them around */
|
||||
g_autoptr(GPtrArray) ostree_modified_new = NULL;
|
||||
if (is_new_checksum)
|
||||
{
|
||||
g_autoptr(GPtrArray) removed = NULL;
|
||||
g_autoptr(GPtrArray) added = NULL;
|
||||
g_autoptr(GPtrArray) modified_old = NULL;
|
||||
|
||||
/* Note we allow_noent here; we'll just skip over the rpm diff if there's no data */
|
||||
RpmOstreeDbDiffExtFlags flags = RPM_OSTREE_DB_DIFF_EXT_ALLOW_NOENT;
|
||||
if (!rpm_ostree_db_diff_ext (repo, current_base_checksum, new_base_checksum, flags,
|
||||
&removed, &added, &modified_old, &ostree_modified_new,
|
||||
cancellable, error))
|
||||
return FALSE;
|
||||
|
||||
/* check if allow_noent kicked in */
|
||||
if (removed)
|
||||
rpm_diff_add_base_db_diff (&rpm_diff, removed, added,
|
||||
modified_old, ostree_modified_new);
|
||||
}
|
||||
|
||||
/* now we look at the rpm-md side */
|
||||
|
||||
g_autoptr(GPtrArray) rpmmd_modified_new = NULL;
|
||||
|
||||
GHashTable *layered_pkgs = rpmostree_origin_get_packages (origin);
|
||||
/* check that it's actually layered (i.e. the requests are not all just dormant) */
|
||||
if (sack && is_new_layered && g_hash_table_size (layered_pkgs) > 0)
|
||||
if (staged_deployment)
|
||||
{
|
||||
if (staged_deployment)
|
||||
/* ok we have a staged deployment; we just need to do a simple diff and BOOM done! */
|
||||
/* XXX: we're marking all pkgs as BASE right now even though there could be layered
|
||||
* pkgs too -- we can tease those out in the future if needed */
|
||||
if (!rpm_diff_add_db_diff (&rpm_diff, repo, RPM_OSTREE_PKG_TYPE_BASE,
|
||||
current_checksum, new_checksum, &ostree_modified_new,
|
||||
cancellable, error))
|
||||
return FALSE;
|
||||
}
|
||||
else
|
||||
{
|
||||
/* no staged deployment; we do our best to come up with a diff:
|
||||
* - if a new base checksum was pulled, do a db diff of the old and new bases
|
||||
* - if there are currently any layered pkgs, lookup in sack for newer versions
|
||||
*/
|
||||
if (is_new_checksum)
|
||||
{
|
||||
/* no need to guess, we *know* what the new layered pkgs are */
|
||||
if (!rpmmd_diff_exact (repo, new_base_checksum, new_checksum, sack, &rpm_diff,
|
||||
&rpmmd_modified_new, error))
|
||||
if (!rpm_diff_add_db_diff (&rpm_diff, repo, RPM_OSTREE_PKG_TYPE_BASE,
|
||||
current_base_checksum, new_base_checksum,
|
||||
&ostree_modified_new, cancellable, error))
|
||||
return FALSE;
|
||||
}
|
||||
else
|
||||
|
||||
/* now we look at the rpm-md/layering side */
|
||||
GHashTable *layered_pkgs = rpmostree_origin_get_packages (origin);
|
||||
|
||||
/* check that it's actually layered (i.e. the requests are not all just dormant) */
|
||||
if (sack && is_new_layered && g_hash_table_size (layered_pkgs) > 0)
|
||||
{
|
||||
if (!rpmmd_diff_guess (repo, current_base_checksum, current_checksum, sack,
|
||||
&rpm_diff, &rpmmd_modified_new, error))
|
||||
|
||||
@ -34,12 +34,14 @@ vm_build_rpm layered-enh
|
||||
vm_build_rpm layered-sec-none
|
||||
vm_build_rpm layered-sec-low
|
||||
vm_build_rpm layered-sec-crit
|
||||
vm_build_rpm layered-constant # this one we won't update
|
||||
vm_rpmostree rebase vmcheckmote:vmcheck \
|
||||
--install layered-cake \
|
||||
--install layered-enh \
|
||||
--install layered-sec-none \
|
||||
--install layered-sec-low \
|
||||
--install layered-sec-crit
|
||||
--install layered-sec-crit \
|
||||
--install layered-constant
|
||||
if vm_cmd 'grep -qE -e "^StageDeployments=true" /etc/rpm-ostreed.conf'; then
|
||||
vm_cmd systemctl is-active ostree-finalize-staged.service
|
||||
fi
|
||||
@ -48,7 +50,7 @@ vm_rpmostree status -v
|
||||
vm_assert_status_jq \
|
||||
'.deployments[0]["origin"] == "vmcheckmote:vmcheck"' \
|
||||
'.deployments[0]["version"] == "v1"' \
|
||||
'.deployments[0]["packages"]|length == 5' \
|
||||
'.deployments[0]["packages"]|length == 6' \
|
||||
'.deployments[0]["packages"]|index("layered-cake") >= 0'
|
||||
echo "ok prep"
|
||||
|
||||
@ -143,6 +145,8 @@ assert_output() {
|
||||
" VMCHECK-SEC-LOW Low layered-sec-low-2.0-1.x86_64" \
|
||||
" VMCHECK-SEC-CRIT Critical layered-sec-crit-2.0-1.x86_64"
|
||||
|
||||
assert_not_file_has_content out-verbose.txt "layered-constant 1.0-1 -> 1.0-1"
|
||||
|
||||
# make sure any future call doesn't forget to create fresh ones
|
||||
rm -f out.txt out-verbose.txt
|
||||
}
|
||||
@ -209,6 +213,8 @@ assert_output2() {
|
||||
'Removed: base-pkg-baz-1.1-1.x86_64' \
|
||||
'Added: base-pkg-boo-3.7-2.11.x86_64'
|
||||
|
||||
assert_not_file_has_content out-verbose.txt "layered-constant 1.0-1 -> 1.0-1"
|
||||
|
||||
# make sure any future call doesn't forget to create fresh ones
|
||||
rm -f out.txt out-verbose.txt
|
||||
}
|
||||
@ -235,7 +241,7 @@ assert_default_deployment_is_update() {
|
||||
vm_assert_status_jq \
|
||||
'.deployments[0]["origin"] == "vmcheckmote:vmcheck"' \
|
||||
'.deployments[0]["version"] == "v2"' \
|
||||
'.deployments[0]["packages"]|length == 5' \
|
||||
'.deployments[0]["packages"]|length == 6' \
|
||||
'.deployments[0]["packages"]|index("layered-cake") >= 0'
|
||||
vm_rpmostree db list $(vm_get_pending_csum) > list.txt
|
||||
assert_file_has_content list.txt 'layered-cake-2.1-4.x86_64'
|
||||
|
||||
Loading…
Reference in New Issue
Block a user