Merge ~andersson123/autopkgtest-cloud:fix-update-github-jobs into autopkgtest-cloud:master

[Tim Andersson]

[Tim Andersson (andersson123)]

Need to squash and clean up commits and also remove TimeoutStartSec from service file, include time diff in commit and run linting stuff

[Tim Andersson (andersson123)]

I’ve made some modifications to the script which are in the MP.

I fixed an issue I found before in a previous MP, incorrect urls getting passed to update-github-jobs, and I’ve made the script a bit faster too.

with my changes:

real 1m57.851s
user 0m51.147s
sys 0m2.708s

I was running the one in master, trying to get a total time, but I got bored after a while….

KeyboardInterrupt

real 115m0.538s
user 0m19.061s
sys 0m3.917s

….

lol

[Skia (hyask)]

I don't really know that code or its purpose, but I've put you an inline comment anyway.

[Skia (hyask)]

review: Needs Fixing

[Paride Legovini (paride)]

Some inline questions / comments.

review: Needs Information

[Paride Legovini (paride)]

As discussed, the logic is OK, but let's implement the time difference check as Skia suggested.

We can't rely on the object metadata to get its timestamp, as we're accessing it using url calls, not swiftclient or similar.

It's still a bit a mystery why the structure of the answers changed as some point, so we need to change the URLs, but let's accept how things are (no need to dig).

On the timeout: let's run the script manually, check how long it takes, and set the timeout to a few times that time (we can be generous, it's mostly to avoid the "stuck forever" case).

[Tim Andersson (andersson123)]

Amended, please re-review! I'm just going to double check I haven't broken anything, but I doubt I have.

[Tim Andersson (andersson123)]

Doesn't look like my latest changes have caused any breaking, awesome

[Tim Andersson (andersson123)]

CI seems to be unhappy, looking...

[Tim Andersson (andersson123)]

ah, black error, fixing

[Tim Andersson (andersson123)]

Amended, please re-review

[Paride Legovini (paride)]

A couple of minor review comments, but looks good in general.

review: Needs Fixing

[Tim Andersson (andersson123)]

Preserving your comment here paride as I'm about to push:

>>> Should log something here about the job being skipped?

I originally did that in earlier versions of my changes, but so many jobs get skipped that the logs get spammed an insane amount, so I decided against it. Let me know if that's all good with you!

[Tim Andersson (andersson123)]

Amended, please re-review! :D

[Paride Legovini (paride)]

I suspected log spam could be an issue.

One last inline question...

review: Needs Information

[Paride Legovini (paride)]

CI passed, this LGTM, but please check my previous comment on the possible need for an abs() when computing the time difference. I don't think it's needed; if you agree go ahead and merge.

review: Approve

[Tim Andersson (andersson123)]

Amended, please re-review

[Tim Andersson (andersson123)]

Amended, please re-review

[Skia (hyask)]

Great, thanks!

review: Approve

[Tim Andersson (andersson123)]

I'll wait for CI to pass (or tempfail) and then I'll merge this.

[Tim Andersson (andersson123)]

NOICE! a pass

diff --git a/charms/focal/autopkgtest-web/units/update-github-jobs.service b/charms/focal/autopkgtest-web/units/update-github-jobs.service
index a52bac5..bf71e43 100644
--- a/charms/focal/autopkgtest-web/units/update-github-jobs.service
+++ b/charms/focal/autopkgtest-web/units/update-github-jobs.service
@@ -5,6 +5,6 @@ Description=Update GitHub job status
 Type=oneshot
 User=www-data
 Group=www-data
-TimeoutStartSec=1m
+TimeoutStartSec=10m
 EnvironmentFile=/etc/environment.d/*.conf
 ExecStart=/home/ubuntu/webcontrol/update-github-jobs
diff --git a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
index 6362803..83ee598 100755
--- a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
+++ b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
@@ -1,6 +1,7 @@
 #!/usr/bin/python3
 
 import configparser
+import datetime
 import io
 import json
 import logging
@@ -15,6 +16,7 @@ from urllib.error import HTTPError
 from request.submit import Submit
 
 PENDING_DIR = "/run/autopkgtest_webcontrol/github-pending"
+MAX_DAYS_DIFF = 15
 swift_url = None
 external_url = None
 
@@ -23,7 +25,7 @@ def result_matches_job(result_url, params):
     # download result.tar and get exit code and testinfo
     for _ in range(5):
         try:
-            with urllib.request.urlopen(result_url + "/result.tar") as f:
+            with urllib.request.urlopen(result_url) as f:
                 tar_bytes = io.BytesIO(f.read())
             break
         except IOError as e:
@@ -96,6 +98,17 @@ def finish_job(jobfile, params, code, log_url):
     os.unlink(jobfile)
 
 
+def check_time_diff(object_time, job_time):
+    splitted_url = object_time.split("/")
+    # date and time of object embedded in swift object path
+    timestamp = splitted_url[4]
+    # last part of the timestamp is a storage ID which we don't need
+    timestamp = "_".join(timestamp.split("_")[0:2])
+    timestamp = datetime.datetime.strptime(timestamp, "%Y%m%d_%H%M%S")
+    job_timestamp = datetime.datetime.fromtimestamp(job_time)
+    return abs(timestamp - job_timestamp)
+
+
 def process_job(jobfile):
     try:
         with open(jobfile) as f:
@@ -137,6 +150,16 @@ def process_job(jobfile):
                 result_url = os.path.join(
                     container_url, result.strip().decode()
                 )
+                # We don't want to check the urls that aren't for result.tar
+                if "result.tar" not in result_url:
+                    continue
+                # We check the swift object and the github jobfile has a time
+                # difference of less than 15 days - otherwise we're checking
+                # swift objects from years ago and this script is super slow.
+                if check_time_diff(
+                    result.strip().decode(), mtime
+                ) > datetime.timedelta(days=MAX_DAYS_DIFF):
+                    continue
                 logging.debug(
                     "checking result %s for job %s",
                     result_url,