Skip to content

Commit

Permalink
eos-updater: Avoid scheduler for offline updates
Browse files Browse the repository at this point in the history
Currently eos-updater asks the download scheduler (mogwai) before
fetching an update, so that the user's configured update schedule can be
respected. However that system is designed to help users with limited
Internet connections; it doesn't make sense to use it when the update
comes from local sources, such as the local network or a USB drive. So
this commit makes changes to eos-updater so that offline OS updates can
happen regardless of whether or not automatic updates are turned on.

The way this is implemented is that in the Poll() stage, we first check
offline sources for updates, and then only check the Internet if none
were found offline. If we simply check all the sources at once as we
previously were, you'd have to make significant changes to the Fetch()
stage, because the scheduler can't make a decision when the offline and
online results are mixed together.

There are two notable implications of this implementation:

1. This means that even if the LAN or USB drive has an older update than
what's available on the Internet, we update from it first (then on the
next check for updates the Internet would be used). In my opinion, this
is desired behavior because after the offline update is complete, the
delta for the online update may be smaller.

2. Before this commit if the fetch from an offline source fails we try
to fetch from the Internet (if they both provide the latest commit).
After this commit, if the offline fetch fails the whole update fails.
This might be less than ideal but I don't think it will matter in
practice. The alternative would be to rearchitect eos-updater or the
libostree API used for the fetch in significant ways.

A mounted filesystem or a peer on the network could conceivably prevent
you from updating, but that's true both before and after this commit;
see ostreedev/ostree#1527

https://phabricator.endlessm.com/T23972
  • Loading branch information
mwleeds committed Oct 17, 2018
1 parent 7e0c7bb commit 2fa3f03
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 19 deletions.
13 changes: 10 additions & 3 deletions eos-updater/data.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,20 @@ struct EosUpdaterData
*/
gchar **overridden_urls;

/* The results from ostree_repo_find_remotes_async(), which contain all the
* possible sources of the given refs, including internet, LAN and USB sources
* (depending on what OstreeRepoFinders were enabled in the poll stage).
/* The results from ostree_repo_find_remotes_async(), which contain different
* possible sources of the given refs. If LAN/USB OstreeRepoFinders were
* configured at the poll stage, and any updates were found in them, this
* array contains only results from those sources. Otherwise it contains
* results from the Internet.
* This needs to be passed from poll() to fetch().
* May be NULL if using the fallback code in poll(). */
OstreeRepoFinderResult **results;

/* This is TRUE if the results array above only contains offline (LAN/USB)
* sources for refs, which implies that the fetch can be done without
* consulting the update scheduler. */
gboolean offline_results_only;

/* The object to pass to the tasks performed by the updater, in order to be
* able to cancel them. Upon cancellation (which is done by the Cancel()
* method), the object is renewed (unreffed + replaced by a new instance). */
Expand Down
7 changes: 7 additions & 0 deletions eos-updater/fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,13 @@ check_scheduler (FetchData *fetch_data,
return TRUE;
}

/* If we are using offline (LAN/USB) sources, don’t check the scheduler at all. */
if (fetch_data->data->offline_results_only)
{
g_message ("Fetch: not checking with download scheduler for offline updates");
return TRUE;
}

/* Otherwise, connect to the download scheduler. If we can’t connect, fail
* safe, and don’t download on a potentially metered connection. */
if (!schedule_download (fetch_data, context, cancellable, error))
Expand Down
4 changes: 4 additions & 0 deletions eos-updater/poll-common.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ eos_update_info_new (const gchar *checksum,
const gchar *old_refspec,
const gchar *version,
const gchar * const *urls,
gboolean offline_results_only,
OstreeRepoFinderResult **results)
{
EosUpdateInfo *info;
Expand All @@ -235,6 +236,7 @@ eos_update_info_new (const gchar *checksum,
info->old_refspec = g_strdup (old_refspec);
info->version = g_strdup (version);
info->urls = g_strdupv ((gchar **) urls);
info->offline_results_only = offline_results_only;
info->results = g_steal_pointer (&results);

return info;
Expand Down Expand Up @@ -1185,6 +1187,8 @@ metadata_fetch_finished (GObject *object,
g_clear_pointer (&data->results, ostree_repo_finder_result_freev);
data->results = g_steal_pointer (&info->results);

data->offline_results_only = info->offline_results_only;

/* Everything is happy thusfar */
/* if we have a checksum for the remote upgrade candidate
* and it's ≠ what we're currently booted into, advertise it as such.
Expand Down
2 changes: 2 additions & 0 deletions eos-updater/poll-common.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ struct _EosUpdateInfo
gchar *old_refspec;
gchar *version;
gchar **urls;
gboolean offline_results_only;

OstreeRepoFinderResult **results; /* (owned) (array zero-terminated=1) */
};
Expand All @@ -85,6 +86,7 @@ eos_update_info_new (const gchar *csum,
const gchar *old_refspec,
const gchar *version,
const gchar * const *urls,
gboolean offline_results_only,
OstreeRepoFinderResult **results);

GDateTime *
Expand Down
67 changes: 51 additions & 16 deletions eos-updater/poll.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,16 @@ object_unref0 (gpointer obj)
g_object_unref (obj);
}

static GPtrArray *
static void
get_finders (SourcesConfig *config,
GMainContext *context,
GPtrArray **out_offline_finders,
GPtrArray **out_online_finders,
OstreeRepoFinderAvahi **out_finder_avahi)
{
g_autoptr(OstreeRepoFinderAvahi) finder_avahi = NULL;
g_autoptr(GPtrArray) finders = g_ptr_array_new_full (config->download_order->len,
object_unref0);
g_autoptr(GPtrArray) offline_finders = g_ptr_array_new_full (0, object_unref0);
g_autoptr(GPtrArray) online_finders = g_ptr_array_new_full (0, object_unref0);
g_autoptr(GError) local_error = NULL;
gsize i;

Expand All @@ -176,19 +178,19 @@ get_finders (SourcesConfig *config,
switch (g_array_index (config->download_order, EosUpdaterDownloadSource, i))
{
case EOS_UPDATER_DOWNLOAD_MAIN:
g_ptr_array_add (finders, ostree_repo_finder_config_new ());
g_ptr_array_add (online_finders, ostree_repo_finder_config_new ());
break;

case EOS_UPDATER_DOWNLOAD_LAN:
/* strv_to_download_order() already checks for duplicated download_order entries */
g_assert (finder_avahi == NULL);
finder_avahi = ostree_repo_finder_avahi_new (context);
g_ptr_array_add (finders, g_object_ref (finder_avahi));
g_ptr_array_add (offline_finders, g_object_ref (finder_avahi));
break;

case EOS_UPDATER_DOWNLOAD_VOLUME:
/* TODO: How to make this one testable? */
g_ptr_array_add (finders, ostree_repo_finder_mount_new (NULL));
g_ptr_array_add (offline_finders, ostree_repo_finder_mount_new (NULL));
break;

default:
Expand All @@ -200,8 +202,13 @@ get_finders (SourcesConfig *config,
{
g_autoptr(OstreeRepoFinderOverride) finder_override = ostree_repo_finder_override_new ();

g_ptr_array_set_size (finders, 0); /* override everything */
g_ptr_array_add (finders, g_object_ref (finder_override));
g_ptr_array_set_size (offline_finders, 0); /* override everything */
g_ptr_array_set_size (online_finders, 0); /* override everything */

/* We don't know if the URIs are online or offline; assume online so we
* don't accidentally bypass the scheduler */
g_ptr_array_add (online_finders, g_object_ref (finder_override));

g_clear_object (&finder_avahi);

for (i = 0; config->override_uris[i] != NULL; i++)
Expand All @@ -211,7 +218,10 @@ get_finders (SourcesConfig *config,
}
}

g_ptr_array_add (finders, NULL); /* NULL terminator */
if (offline_finders->len > 0)
g_ptr_array_add (offline_finders, NULL); /* NULL terminator */
if (online_finders->len > 0)
g_ptr_array_add (online_finders, NULL); /* NULL terminator */

/* TODO: Stop this at some point; think of a better way to store it and
* control its lifecycle. */
Expand All @@ -222,15 +232,18 @@ get_finders (SourcesConfig *config,
if (local_error != NULL)
{
g_warning ("Avahi finder failed; removing it: %s", local_error->message);
g_ptr_array_remove (finders, finder_avahi);
g_ptr_array_remove (offline_finders, finder_avahi);
g_clear_object (&finder_avahi);
g_clear_error (&local_error);
}

if (out_finder_avahi != NULL)
*out_finder_avahi = g_steal_pointer (&finder_avahi);

return g_steal_pointer (&finders);
if (out_offline_finders != NULL)
*out_offline_finders = g_steal_pointer (&offline_finders);
if (out_online_finders != NULL)
*out_online_finders = g_steal_pointer (&online_finders);
}

typedef OstreeRepoFinderAvahi RepoFinderAvahiRunning;
Expand Down Expand Up @@ -554,11 +567,13 @@ metadata_fetch_new (OstreeRepo *repo,
g_auto(OstreeRepoFinderResultv) results = NULL;
g_autoptr(EosUpdateInfo) info = NULL;
g_auto(UpdateRefInfo) update_ref_info;
g_autoptr(GPtrArray) finders = NULL; /* (element-type OstreeRepoFinder) */
g_autoptr(GPtrArray) offline_finders = NULL; /* (element-type OstreeRepoFinder) */
g_autoptr(GPtrArray) online_finders = NULL; /* (element-type OstreeRepoFinder) */
g_autoptr(RepoFinderAvahiRunning) finder_avahi = NULL;
gboolean offline_results_only = TRUE;

finders = get_finders (config, context, &finder_avahi);
if (finders->len == 0)
get_finders (config, context, &offline_finders, &online_finders, &finder_avahi);
if (offline_finders->len == 0 && online_finders->len == 0)
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"All configured update sources failed to initialize.");
Expand All @@ -570,14 +585,32 @@ metadata_fetch_new (OstreeRepo *repo,
/* The upgrade refspec here is either the booted refspec if
* there were new commits on the branch of the booted refspec, or
* the checkpoint refspec. */
if (!check_for_update_following_checkpoint_if_allowed (repo,
if (offline_finders->len > 0 &&
!check_for_update_following_checkpoint_if_allowed (repo,
&update_ref_info,
finders,
offline_finders,
context,
cancellable,
error))
return NULL;

/* If checking for updates offline failed, check online */
if (update_ref_info.commit == NULL)
{
offline_results_only = FALSE;

update_ref_info_clear (&update_ref_info);

if (online_finders->len > 0 &&
!check_for_update_following_checkpoint_if_allowed (repo,
&update_ref_info,
online_finders,
context,
cancellable,
error))
return NULL;
}

if (update_ref_info.commit != NULL)
{
info = eos_update_info_new (update_ref_info.checksum,
Expand All @@ -586,6 +619,7 @@ metadata_fetch_new (OstreeRepo *repo,
update_ref_info.refspec,
update_ref_info.version,
NULL,
offline_results_only,
g_steal_pointer (&update_ref_info.results));
metrics_report_successful_poll (info);
return g_steal_pointer (&info);
Expand Down Expand Up @@ -633,6 +667,7 @@ metadata_fetch_from_main (OstreeRepo *repo,
update_ref_info.refspec,
update_ref_info.version,
NULL,
FALSE,
NULL);

*out_info = g_steal_pointer (&info);
Expand Down

0 comments on commit 2fa3f03

Please sign in to comment.