Merge autopkgtest-cloud:retry-download-all-results into autopkgtest-cloud:master

[Brian Murray]

The web frontends did not have the same test results and when I was running download-all-results I noticed that it frequently exited due to being disconnected from swift. This change wraps the urlopen for retrieving the results in a retry loop.

I cowboyed this change on both web frontends and they were able to download all the results without needing to be restarted.

[Julian Andres Klode (juliank)]

lgtm

review: Approve

[Steve Langasek (vorlon)]

Why do this with a retry within the script, vs generic failure handling at the level of the systemd unit?

[Steve Langasek (vorlon)]

review: Needs Information

[Steve Langasek (vorlon)]

Thinking about this from the other direction - if we're retrying this inside the script because it's stateful, and it's expensive to rerun the whole script instead of retrying in the inner loop, why have a limit of 5 times? Seems arbitrary. Why not retry forever?

[Brian Murray (brian-murray)]

Come to find out the script actually isn't stateful and it will only try to download results which it doesn't yet have. I was led astray by the fact that some test results are missing 'testinfo.json' and the script will try and download those results every time.

[Brian Murray (brian-murray)]

> Come to find out the script actually isn't stateful and it will only try to
> download results which it doesn't yet have. I was led astray by the fact that
> some test results are missing 'testinfo.json' and the script will try and
> download those results every time.

Having said that I added the retry part to "list_remote_container" which actually builds the dictionary of known test results. Previously if that failed for any release container then the whole script would crash and no other releases would be updated. So yes the retry belongs in here.

[Julian Andres Klode (juliank)]

Retries in the script are better anyway to avoid having to go over all the failures again, systemd retries should be there as a safety net, but don't block this.

[Steve Langasek (vorlon)]

review: Approve

diff --git a/charms/focal/autopkgtest-web/webcontrol/download-all-results b/charms/focal/autopkgtest-web/webcontrol/download-all-results
index 6696be7..ab0a1e3 100755
--- a/charms/focal/autopkgtest-web/webcontrol/download-all-results
+++ b/charms/focal/autopkgtest-web/webcontrol/download-all-results
@@ -20,6 +20,7 @@ import json
 import configparser
 import urllib.parse
 import time
+import http
 
 from distro_info import UbuntuDistroInfo
 from helpers.utils import get_test_id, init_db
@@ -41,7 +42,15 @@ def list_remote_container(container_url):
             url += f"&marker={urllib.parse.quote(start)}"
 
         LOGGER.debug('Retrieving "%s"', url)
-        resp = urlopen(url)
+        for retry in range(5):
+            try:
+                resp = urlopen(url)
+            except http.client.RemoteDisconnected as e:
+                LOGGER.debug('Got disconnected, sleeping')
+                time.sleep(5)
+                continue
+            else:
+                break
         json_string = resp.read()
         json_data = json.loads(json_string)