prune: Don't prune tagged builds

Follow-up to coreos#243. Become aware of tags and make sure not to prune
builds that are tagged. I went the path of making tagged builds *not*
counting towards the `--keep=N` limit since that felt the most natural
to me.

One tricky bit is the OSTree repo pruning. The CLI is not really geared
towards this kind of pruning since we now could have older (tagged)
commits that we care about while some newer commits on that same branch
we don't care about. For now, I just swapped it to use
`--keep-younger-than`. Though this does align well with another
improvement I'd like to make to `prune` to learn timestamp-based pruning
instead.

coreos-assembler

@@ -34,7 +34,7 @@ if [ -e /sys/fs/selinux/status ]; then
 fi
 
 cmd=${1:-}
-build_commands="init fetch build buildextend-ec2 buildextend-openstack tag restamp run prune clean"
+build_commands="init fetch build buildextend-ec2 buildextend-openstack tag bump-timestamp run prune clean"
 other_commands="shell"
 utility_commands="gf-oemid virt-install oscontainer"
 if [ -z "${cmd}" ]; then

src/cmd-restamp → src/cmd-bump-timestamp

@@ -7,7 +7,7 @@ dn=$(dirname "$0")
 
 print_help() {
     cat 1>&2 <<'EOF'
-Usage: coreos-assembler restamp
+Usage: coreos-assembler bump-timestamp
 
   Update the `timestamp` key of `builds.json`.
 EOF

src/cmd-prune

@@ -13,8 +13,8 @@ print_help() {
 Usage: coreos-assembler prune --help
        coreos-assembler prune [--keep=N]
 
-  Delete older build artifacts. By default, only the last 3 builds are kept.
-  This can be overridden with the `--keep` option.
+  Delete older untagged build artifacts. By default, only the last 3 untagged
+  builds are kept.  This can be overridden with the `--keep` option.
 EOF
 }
 

src/cmdlib.py

@@ -35,5 +35,7 @@ def fatal(msg):
     raise SystemExit(1)
 
 
-def rfc3339_time():
-    return datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ")
+def rfc3339_time(t=None):
+    if t is None:
+        t = datetime.utcnow()
+    return t.strftime("%Y-%m-%dT%H:%M:%SZ")

src/prune_builds

@@ -10,21 +10,25 @@ import sys
 import json
 import shutil
 import argparse
-import tempfile
 import subprocess
+import collections
 
 import dateutil.parser
+from datetime import timedelta
 
 sys.path.insert(0, '/usr/lib/coreos-assembler')
 from cmdlib import write_json, rfc3339_time
 
+Build = collections.namedtuple('Build', ['id', 'timestamp',
+                                         'ostree_timestamp'])
+
 # Let's just hardcode this here for now
 DEFAULT_KEEP_LAST_N = 3
 
 parser = argparse.ArgumentParser()
 parser.add_argument("--workdir", required=True, help="Path to workdir")
 parser.add_argument("--keep-last-n", type=int, default=DEFAULT_KEEP_LAST_N,
-                    help="Number of builds to keep (0 for all)")
+                    help="Number of untagged builds to keep (0 for all)")
 parser.add_argument("--insert-only",
                     help="Append a new latest build, do not prune",
                     action='store')
@@ -58,7 +62,10 @@ if args.insert_only:
 
 skip_pruning = (args.keep_last_n == 0)
 
-# first, regen builds.json
+# collect all builds being pointed to by tags
+tagged_builds = set([tag['target'] for tag in builddata.get('tags', [])])
+
+# first, pick up all the builds from the dir itself
 with os.scandir(builds_dir) as it:
     for entry in it:
 
@@ -81,15 +88,44 @@ with os.scandir(builds_dir) as it:
         # Older versions only had ostree-timestamp
         ts = j.get('build-timestamp') or j['ostree-timestamp']
         t = dateutil.parser.parse(ts)
-        builds.append((entry.name, t))
+        ostree_ts = j['ostree-timestamp']
+        ostree_t = dateutil.parser.parse(ostree_ts)
+        builds.append(Build(id=entry.name, timestamp=t,
+                            ostree_timestamp=ostree_t))
+
+# just get the trivial case out of the way
+if len(builds) == 0:
+    print("No builds to prune!")
+    sys.exit(0)
 
-builds = sorted(builds, key=lambda x: x[1], reverse=True)
+# sort by timestamp, newest first
+builds = sorted(builds, key=lambda x: x.timestamp, reverse=True)
+
+new_builds = []
 builds_to_delete = []
-if not skip_pruning and len(builds) > args.keep_last_n:
-    builds_to_delete = builds[args.keep_last_n:]
-    del builds[args.keep_last_n:]
 
-builddata['builds'] = [x[0] for x in builds]
+if skip_pruning:
+    new_builds = builds
+else:
+    n = args.keep_last_n
+    assert(n > 0)
+    for build in builds:
+        # skip tagged builds and don't count them towards the limit
+        if build.id in tagged_builds:
+            print(f"Skipping tagged build {build.id}")
+            new_builds.append(build)
+            continue
+
+        if n == 0:
+            builds_to_delete.append(build)
+        else:
+            new_builds.append(build)
+            n = n - 1
+
+# either we didn't prune so it's the same builds, or we did, and keep_last_n>0
+assert(len(new_builds) > 0)
+
+builddata['builds'] = [x.id for x in new_builds]
 builddata['timestamp'] = rfc3339_time()
 write_json(builds_json, builddata)
 
@@ -99,20 +135,22 @@ if skip_pruning:
 
 # now delete other build dirs not in the manifest
 
-for build_dir, _ in builds_to_delete:
-    shutil.rmtree(os.path.join(builds_dir, build_dir))
-
+for build in builds_to_delete:
+    shutil.rmtree(os.path.join(builds_dir, build.id))
 
 # and finally prune OSTree repos
-def ostree_prune(repo_name, sudo=False):
-    repo = os.path.join(args.workdir, repo_name)
-    argv = []
-    if sudo:
-        argv.extend(['sudo', '--'])
-    print(f"Pruning {repo_name}")
-    argv.extend(["ostree", "prune", "--repo", repo, "--refs-only",
-                f"--depth={args.keep_last_n-1}"])
-    subprocess.run(argv, check=True)
-
-
-ostree_prune('repo')
+print(f"Pruning repo")
+repo = os.path.join(args.workdir, 'repo')
+
+# For now, we just use the --keep-younger-than CLI here. Doing this more
+# accurately would require enhancing the `prune` CLI (or just use the API
+# directly?). Or we could also manually figure out the depth of the oldest
+# build and then use that as the arg to `--depth`.
+oldest_ostree_t = new_builds[-1].ostree_timestamp
+# In a quick test, it seems like --keep-younger-than=x actually keeps commits
+# with timestamps exactly equal to x, but the name is a bit tricky so let's
+# be safe and just pick a time 1h before that so we're doubly sure. It might
+# keep a few other older commits, but meh...
+younger_than = rfc3339_time(oldest_ostree_t - timedelta(hours=1))
+subprocess.run(["ostree", "prune", "--repo", repo, "--refs-only",
+               f"--keep-younger-than={younger_than}"], check=True)