[gentoo-portage-dev] [PATCH 4/4] rsync: add key refresh retry (bug 649276)

[Zac Medico]

Since key refresh is prone to failure, retry using exponential
backoff with random jitter. This adds the following sync-openpgp-*
configuration settings:

sync-openpgp-key-refresh-retry-count = 40

  Maximum number of times to retry key refresh if it fails.  Between
  each  key  refresh attempt, there is an exponential delay with a
  constant multiplier and a uniform random multiplier between 0 and 1.

sync-openpgp-key-refresh-retry-delay-exp-base = 2

  The base of the exponential expression. The exponent is the number
  of previous refresh attempts.

sync-openpgp-key-refresh-retry-delay-max = 60

  Maximum  delay between each retry attempt, in units of seconds. This
  places a limit on the length of the exponential delay.

sync-openpgp-key-refresh-retry-delay-mult = 4

  Multiplier for the exponential delay.

sync-openpgp-key-refresh-retry-overall-timeout = 1200

  Combined time limit for all refresh attempts, in units of seconds.

Bug: https://bugs.gentoo.org/649276
---
 cnf/repos.conf                          |  5 ++
 man/portage.5                           | 19 ++++++++
 pym/portage/repository/config.py        | 22 +++++++++
 pym/portage/sync/modules/rsync/rsync.py | 16 ++++++-
 pym/portage/sync/syncbase.py            | 85 ++++++++++++++++++++++++++++++++-
 5 files changed, 144 insertions(+), 3 deletions(-)

diff --git a/cnf/repos.conf b/cnf/repos.conf
index 984ecd220..5759b8b43 100644
--- a/cnf/repos.conf
+++ b/cnf/repos.conf
@@ -9,6 +9,11 @@ auto-sync = yes
 sync-rsync-verify-metamanifest = yes
 sync-rsync-verify-max-age = 24
 sync-openpgp-key-path = /var/lib/gentoo/gkeys/keyrings/gentoo/release/pubring.gpg
+sync-openpgp-key-refresh-retry-count = 40
+sync-openpgp-key-refresh-retry-overall-timeout = 1200
+sync-openpgp-key-refresh-retry-delay-exp-base = 2
+sync-openpgp-key-refresh-retry-delay-max = 60
+sync-openpgp-key-refresh-retry-delay-mult = 4
 
 # for daily squashfs snapshots
 #sync-type = squashdelta
diff --git a/man/portage.5 b/man/portage.5
index 549c51c73..d3e258a43 100644
--- a/man/portage.5
+++ b/man/portage.5
@@ -1081,6 +1081,25 @@ only for protocols supporting cryptographic verification, provided
 that the respective verification option is enabled. If unset, the user's
 keyring is used.
 .TP
+.B sync\-openpgp\-key\-refresh\-retry\-count = 40
+Maximum number of times to retry key refresh if it fails. Between each
+key refresh attempt, there is an exponential delay with a constant
+multiplier and a uniform random multiplier between 0 and 1.
+.TP
+.B sync\-openpgp\-key\-refresh\-retry\-delay\-exp\-base = 2
+The base of the exponential expression. The exponent is the number of
+previous refresh attempts.
+.TP
+.B sync\-openpgp\-key\-refresh\-retry\-delay\-max = 60
+Maximum delay between each retry attempt, in units of seconds. This
+places a limit on the length of the exponential delay.
+.TP
+.B sync\-openpgp\-key\-refresh\-retry\-delay\-mult = 4
+Multiplier for the exponential delay.
+.TP
+.B sync\-openpgp\-key\-refresh\-retry\-overall\-timeout = 1200
+Combined time limit for all refresh attempts, in units of seconds.
+.TP
 .B sync-rsync-vcs-ignore = true|false
 Ignore vcs directories that may be present in the repository. It is the
 user's responsibility to set sync-rsync-extra-opts to protect vcs
diff --git a/pym/portage/repository/config.py b/pym/portage/repository/config.py
index b5db4855f..1d897bb90 100644
--- a/pym/portage/repository/config.py
+++ b/pym/portage/repository/config.py
@@ -87,6 +87,11 @@ class RepoConfig(object):
 		'update_changelog', '_eapis_banned', '_eapis_deprecated',
 		'_masters_orig', 'module_specific_options', 'manifest_required_hashes',
 		'sync_openpgp_key_path',
+		'sync_openpgp_key_refresh_retry_count',
+		'sync_openpgp_key_refresh_retry_delay_max',
+		'sync_openpgp_key_refresh_retry_delay_exp_base',
+		'sync_openpgp_key_refresh_retry_delay_mult',
+		'sync_openpgp_key_refresh_retry_overall_timeout',
 		)
 
 	def __init__(self, name, repo_opts, local_config=True):
@@ -186,6 +191,13 @@ class RepoConfig(object):
 		self.sync_openpgp_key_path = repo_opts.get(
 			'sync-openpgp-key-path', None)
 
+		for k in ('sync_openpgp_key_refresh_retry_count',
+			'sync_openpgp_key_refresh_retry_delay_max',
+			'sync_openpgp_key_refresh_retry_delay_exp_base',
+			'sync_openpgp_key_refresh_retry_delay_mult',
+			'sync_openpgp_key_refresh_retry_overall_timeout'):
+			setattr(self, k, repo_opts.get(k.replace('_', '-'), None))
+
 		self.module_specific_options = {}
 
 		# Not implemented.
@@ -523,6 +535,11 @@ class RepoConfigLoader(object):
 							'force', 'masters', 'priority', 'strict_misc_digests',
 							'sync_depth', 'sync_hooks_only_on_change',
 							'sync_openpgp_key_path',
+							'sync_openpgp_key_refresh_retry_count',
+							'sync_openpgp_key_refresh_retry_delay_max',
+							'sync_openpgp_key_refresh_retry_delay_exp_base',
+							'sync_openpgp_key_refresh_retry_delay_mult',
+							'sync_openpgp_key_refresh_retry_overall_timeout',
 							'sync_type', 'sync_umask', 'sync_uri', 'sync_user',
 							'module_specific_options'):
 							v = getattr(repos_conf_opts, k, None)
@@ -946,6 +963,11 @@ class RepoConfigLoader(object):
 		bool_keys = ("strict_misc_digests",)
 		str_or_int_keys = ("auto_sync", "clone_depth", "format", "location",
 			"main_repo", "priority", "sync_depth", "sync_openpgp_key_path",
+			"sync_openpgp_key_refresh_retry_count",
+			"sync_openpgp_key_refresh_retry_delay_max",
+			"sync_openpgp_key_refresh_retry_delay_exp_base",
+			"sync_openpgp_key_refresh_retry_delay_mult",
+			"sync_openpgp_key_refresh_retry_overall_timeout",
 			"sync_type", "sync_umask", "sync_uri", 'sync_user')
 		str_tuple_keys = ("aliases", "eclass_overrides", "force")
 		repo_config_tuple_keys = ("masters",)
diff --git a/pym/portage/sync/modules/rsync/rsync.py b/pym/portage/sync/modules/rsync/rsync.py
index ac841545d..763f41699 100644
--- a/pym/portage/sync/modules/rsync/rsync.py
+++ b/pym/portage/sync/modules/rsync/rsync.py
@@ -7,6 +7,7 @@ import time
 import signal
 import socket
 import datetime
+import functools
 import io
 import re
 import random
@@ -22,7 +23,9 @@ good = create_color_func("GOOD")
 bad = create_color_func("BAD")
 warn = create_color_func("WARN")
 from portage.const import VCS_DIRS, TIMESTAMP_FORMAT, RSYNC_PACKAGE_ATOM
+from portage.util._eventloop.global_event_loop import global_event_loop
 from portage.util import writemsg, writemsg_stdout
+from portage.util.futures.futures import TimeoutError
 from portage.sync.getaddrinfo_validate import getaddrinfo_validate
 from _emerge.UserQuery import UserQuery
 from portage.sync.syncbase import NewBase
@@ -139,14 +142,23 @@ class RsyncSync(NewBase):
 			# will not be performed and the user will have to fix it and try again,
 			# so we may as well bail out before actual rsync happens.
 			if openpgp_env is not None and self.repo.sync_openpgp_key_path is not None:
+
 				try:
 					out.einfo('Using keys from %s' % (self.repo.sync_openpgp_key_path,))
 					with io.open(self.repo.sync_openpgp_key_path, 'rb') as f:
 						openpgp_env.import_key(f)
 					out.ebegin('Refreshing keys from keyserver')
-					openpgp_env.refresh_keys()
+					retry_decorator = self._key_refresh_retry_decorator()
+					if retry_decorator is None:
+						openpgp_env.refresh_keys()
+					else:
+						loop = global_event_loop()
+						func_coroutine = functools.partial(loop.run_in_executor,
+							None, openpgp_env.refresh_keys)
+						decorated_func = retry_decorator(func_coroutine)
+						loop.run_until_complete(decorated_func())
 					out.eend(0)
-				except GematoException as e:
+				except (GematoException, TimeoutError) as e:
 					writemsg_level("!!! Manifest verification impossible due to keyring problem:\n%s\n"
 							% (e,),
 							level=logging.ERROR, noiselevel=-1)
diff --git a/pym/portage/sync/syncbase.py b/pym/portage/sync/syncbase.py
index 43b667fb0..9515dd8d8 100644
--- a/pym/portage/sync/syncbase.py
+++ b/pym/portage/sync/syncbase.py
@@ -6,12 +6,14 @@ Base class for performing sync operations.
 This class contains common initialization code and functions.
 '''
 
-
+from __future__ import unicode_literals
 import logging
 import os
 
 import portage
 from portage.util import writemsg_level
+from portage.util.backoff import RandomExponentialBackoff
+from portage.util.futures.retry import retry
 from . import _SUBMODULE_PATH_MAP
 
 class SyncBase(object):
@@ -106,6 +108,87 @@ class SyncBase(object):
 		'''Get information about the head commit'''
 		raise NotImplementedError
 
+	def _key_refresh_retry_decorator(self):
+		'''
+		Return a retry decorator, or None if retry is disabled.
+
+		If retry fails, the function reraises the exception raised
+		by the decorated function. If retry times out and no exception
+		is available to reraise, the function raises TimeoutError.
+		'''
+		errors = []
+
+		if self.repo.sync_openpgp_key_refresh_retry_count is None:
+			return None
+		try:
+			retry_count = int(self.repo.sync_openpgp_key_refresh_retry_count)
+		except Exception as e:
+			errors.append('sync-openpgp-key-refresh-retry-count: {}'.format(e))
+		else:
+			if retry_count <= 0:
+				return None
+
+		if self.repo.sync_openpgp_key_refresh_retry_overall_timeout is None:
+			retry_overall_timeout = None
+		else:
+			try:
+				retry_overall_timeout = float(self.repo.sync_openpgp_key_refresh_retry_overall_timeout)
+			except Exception as e:
+				errors.append('sync-openpgp-key-refresh-retry-overall-timeout: {}'.format(e))
+			else:
+				if retry_overall_timeout < 0:
+					errors.append('sync-openpgp-key-refresh-retry-overall-timeout: '
+						'value must be greater than or equal to zero: {}'.format(retry_overall_timeout))
+				elif retry_overall_timeout == 0:
+					retry_overall_timeout = None
+
+		if self.repo.sync_openpgp_key_refresh_retry_delay_mult is None:
+			retry_delay_mult = None
+		else:
+			try:
+				retry_delay_mult = float(self.repo.sync_openpgp_key_refresh_retry_delay_mult)
+			except Exception as e:
+				errors.append('sync-openpgp-key-refresh-retry-delay-mult: {}'.format(e))
+			else:
+				if retry_delay_mult <= 0:
+					errors.append('sync-openpgp-key-refresh-retry-mult: '
+						'value must be greater than zero: {}'.format(retry_delay_mult))
+
+		if self.repo.sync_openpgp_key_refresh_retry_delay_exp_base is None:
+			retry_delay_exp_base = None
+		else:
+			try:
+				retry_delay_exp_base = float(self.repo.sync_openpgp_key_refresh_retry_delay_exp_base)
+			except Exception as e:
+				errors.append('sync-openpgp-key-refresh-retry-delay-exp: {}'.format(e))
+			else:
+				if retry_delay_exp_base <= 0:
+					errors.append('sync-openpgp-key-refresh-retry-delay-exp: '
+						'value must be greater than zero: {}'.format(retry_delay_mult))
+
+		if errors:
+			lines = []
+			lines.append('')
+			lines.append('!!! Retry disabled for openpgp key refresh:')
+			lines.append('')
+			for msg in errors:
+				lines.append('    {}'.format(msg))
+			lines.append('')
+
+			for line in lines:
+				writemsg_level("{}\n".format(line),
+					level=logging.ERROR, noiselevel=-1)
+
+			return None
+
+		return retry(
+			reraise=True,
+			try_max=retry_count,
+			overall_timeout=(retry_overall_timeout if retry_overall_timeout > 0 else None),
+			delay_func=RandomExponentialBackoff(
+				multiplier=(1 if retry_delay_mult is None else retry_delay_mult),
+				base=(2 if retry_delay_exp_base is None else retry_delay_exp_base)))
+
 
 class NewBase(SyncBase):
 	'''Subclasses Syncbase adding a new() and runs it
-- 
2.13.6