Review: Needs Fixing I think the sys.exit(1) will break this retry approach, as Python will just quit.
More in general I was hoping to find something better than just retrying everything. This approach is basically equivalent of doing retry ./seed-new-release ... with retry(1) is from the 'retry' package; or until ./seed-new-relese ...; do echo retrying; done In other words, it's a big retry hammer, while I was hoping to better handle the actual failure we're hitting. Maybe we should start by adding a dry-run option to the script, and then try to reproduce the failure in staging? Diff comments: > diff --git > a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release > > b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release > index 8fdd1a3..58823d6 100755 > --- > a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release > +++ > b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release > @@ -59,65 +64,81 @@ def copy_result(rel_path, old_release, new_release): > headers=headers_to_copy) > break > except (IOError, AttributeError, > swiftclient.exceptions.ClientException) as e: > - print('Error connecting to swift, re-connecting in %is: %s' % (5 > * retry, e)) > + logging.info('Error connecting to swift, re-connecting in %is: > %s' % (5 * retry, e)) > time.sleep(5 * retry) > swift_con = connect_swift() > else: > - print('Repeated failure to connect to swift') > + logging.info('Repeated failure to connect to swift') > sys.exit(1) If we end up here I think the Python interpreter will just exit, and no retry will happen. > > - > -ap = argparse.ArgumentParser() > -ap.add_argument('old_release') > -ap.add_argument('new_release') > -ap.add_argument('results_db', help='path to autopkgtest.db') > -args = ap.parse_args() > - > -# connect to Swift > -swift_con = connect_swift() > - > -# create new container > -swift_con.put_container('autopkgtest-' + args.new_release, > - headers={'X-Container-Read': '.rlistings,.r:*'}) > - > -# read existing names (needs multiple batches) > -existing = set() > -last = '' > -while True: > - print('Getting existing results starting with "%s"' % last) > - batch = [i['name'] for i in swift_con.get_container('autopkgtest-' + > args.new_release, marker=last)[1]] > - if not batch: > - break > - last = batch[-1] > - existing.update(batch) > - > -# get passing result per package/arch from database > -db_con = sqlite3.connect(args.results_db) > -for (package, arch, run_id) in db_con.execute( > - "SELECT package, arch, MAX(run_id) " > - "FROM test, result " > - "WHERE test.id = result.test_id AND release = '%s' " > - " AND (exitcode = 0 OR exitcode = 2 " > - " OR triggers = 'migration-reference/0') " > - "GROUP BY package, arch" % args.old_release): > - > - for file in 'artifacts.tar.gz', 'result.tar', 'log.gz': > - path = '/%s/%s/%s/%s/%s' % (arch, srchash(package), package, run_id, > file) > - if args.new_release + path in existing: > - print('%s%s already exists, skipping' % (args.old_release, path)) > - continue > - copy_result(path, args.old_release, args.new_release) > - > -for (package, arch, run_id) in db_con.execute( > - "SELECT package, arch, MAX(run_id) " > - "FROM test, result " > - "WHERE test.id = result.test_id AND release = '%s' " > - " AND triggers = 'migration-reference/0' " > - "GROUP BY package, arch" % args.old_release): > - > - for file in 'artifacts.tar.gz', 'result.tar', 'log.gz': > - path = '/%s/%s/%s/%s/%s' % (arch, srchash(package), package, run_id, > file) > - if args.new_release + path in existing: > - print('%s%s already exists, skipping' % (args.old_release, path)) > - continue > - copy_result(path, args.old_release, args.new_release) > +def attempt_seed_new_release(args): > + try: > + # connect to Swift > + swift_con = connect_swift() > + > + # create new container > + swift_con.put_container('autopkgtest-' + args.new_release, > + headers={'X-Container-Read': > '.rlistings,.r:*'}) > + > + # read existing names (needs multiple batches) > + existing = set() > + last = '' > + while True: > + logging.info('Getting existing results starting with "%s"' % > last) > + batch = [i['name'] for i in > swift_con.get_container('autopkgtest-' + \ > + > args.new_release, marker=last)[1]] > + if not batch: > + break > + last = batch[-1] > + existing.update(batch) > + > + # get passing result per package/arch from database > + db_con = sqlite3.connect(args.results_db) > + for (package, arch, run_id) in db_con.execute( > + "SELECT package, arch, MAX(run_id) " > + "FROM test, result " > + "WHERE test.id = result.test_id AND release = '%s' " > + " AND (exitcode = 0 OR exitcode = 2 " > + " OR triggers = 'migration-reference/0') " > + "GROUP BY package, arch" % args.old_release): > + > + for file in 'artifacts.tar.gz', 'result.tar', 'log.gz': > + path = '/%s/%s/%s/%s/%s' % (arch, srchash(package), package, > run_id, file) > + if args.new_release + path in existing: > + logging.info('%s%s already exists, skipping' % > (args.old_release, path)) > + continue > + copy_result(path, args.old_release, args.new_release) > + > + for (package, arch, run_id) in db_con.execute( > + "SELECT package, arch, MAX(run_id) " > + "FROM test, result " > + "WHERE test.id = result.test_id AND release = '%s' " > + " AND triggers = 'migration-reference/0' " > + "GROUP BY package, arch" % args.old_release): > + > + for file in 'artifacts.tar.gz', 'result.tar', 'log.gz': > + path = '/%s/%s/%s/%s/%s' % (arch, srchash(package), package, > run_id, file) > + if args.new_release + path in existing: > + logging.info('%s%s already exists, skipping' % > (args.old_release, path)) > + continue > + copy_result(path, args.old_release, args.new_release) > + return True > + except Exception as _: > + return False > + > + > +def main(args): > + while True: > + if attempt_seed_new_release(args): > + logging.info("seed-new-release has succeeded, exiting...") > + return > + logging.info("seed-new-release failed, retrying...") > + > + > +if __name__ == "__main__": > + ap = argparse.ArgumentParser() > + ap.add_argument('old_release') > + ap.add_argument('new_release') > + ap.add_argument('results_db', help='path to autopkgtest.db') > + args = ap.parse_args() > + main(args) -- https://code.launchpad.net/~andersson123/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/443597 Your team Canonical's Ubuntu QA is subscribed to branch autopkgtest-cloud:master. -- Mailing list: https://launchpad.net/~canonical-ubuntu-qa Post to : canonical-ubuntu-qa@lists.launchpad.net Unsubscribe : https://launchpad.net/~canonical-ubuntu-qa More help : https://help.launchpad.net/ListHelp