From 6002a128b8fb1752e332f099bcfb6613115e213e Mon Sep 17 00:00:00 2001
From: Tom Gundersen <teg@jklm.no>
Date: Thu, 4 Jun 2020 19:17:19 +0200
Subject: [PATCH] osbuild-worker: don't flush cache between jobs

Until osbuild-14, the images were unconditionally kept in the cache,
meaning the cache could grow very large. Now only the downloaded RPMs
are saved, which greatly limits how big it can grow.

Having the RPMs cached should speed up all but the first image build a
lot, so we should take advantage of that by not flushing the cache
between each build.

The cache is still flushed when the worker is stopped / restarted.

This moves the cache from /var/tmp/osbulid-worker* to
/var/cache/osbulid-worker/osbulid-worker-*. This means that each worker
gets a dedicated cache, in case there are several on one machine. In the
future we may want to combine them and only ever have one cache, but for
that we need improvements in parallel access and cache-cleanup.

Signed-off-by: Tom Gundersen <teg@jklm.no>
---
 cmd/osbuild-worker/main.go                  | 40 +++++++++++++--------
 distribution/osbuild-remote-worker@.service |  3 ++
 distribution/osbuild-worker@.service        |  3 ++
 3 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/cmd/osbuild-worker/main.go b/cmd/osbuild-worker/main.go
index da6924195..1312d3601 100644
--- a/cmd/osbuild-worker/main.go
+++ b/cmd/osbuild-worker/main.go
@@ -63,20 +63,9 @@ func (e *TargetsError) Error() string {
 	return errString
 }
 
-func RunJob(job *worker.Job, uploadFunc func(uuid.UUID, string, io.Reader) error) (*common.ComposeResult, error) {
-	tmpdir, err := ioutil.TempDir("/var/tmp", "osbuild-worker-*")
-	if err != nil {
-		return nil, fmt.Errorf("error setting up osbuild output directory: %v", err)
-	}
-	defer func() {
-		err := os.RemoveAll(tmpdir)
-		if err != nil {
-			log.Printf("Error removing temporary directory %s for job %s: %v", tmpdir, job.Id, err)
-		}
-	}()
-
-	outputDirectory := path.Join(tmpdir, "output")
-	store := path.Join(tmpdir, "store")
+func RunJob(job *worker.Job, cacheDir string, uploadFunc func(uuid.UUID, string, io.Reader) error) (*common.ComposeResult, error) {
+	outputDirectory := path.Join(cacheDir, "output")
+	store := path.Join(cacheDir, "store")
 
 	result, err := RunOSBuild(job.Manifest, store, outputDirectory, os.Stderr)
 	if err != nil {
@@ -152,6 +141,11 @@ func RunJob(job *worker.Job, uploadFunc func(uuid.UUID, string, io.Reader) error
 		}
 	}
 
+	err = os.RemoveAll(outputDirectory)
+	if err != nil {
+		log.Printf("Error removing osbuild output directory (%s): %v", outputDirectory, err)
+	}
+
 	if len(r) > 0 {
 		return result, &TargetsError{r}
 	}
@@ -176,6 +170,11 @@ func main() {
 		flag.Usage()
 	}
 
+	cacheDirectory, ok := os.LookupEnv("CACHE_DIRECTORY")
+	if !ok {
+		log.Fatal("CACHE_DIRECTORY is not set. Is the service file missing CacheDirectory=?")
+	}
+
 	var client *worker.Client
 	if unix {
 		client = worker.NewClientUnix(address)
@@ -192,6 +191,17 @@ func main() {
 		client = worker.NewClient(address, conf)
 	}
 
+	tmpdir, err := ioutil.TempDir(cacheDirectory, "osbuild-worker-*")
+	if err != nil {
+		log.Fatalf("error setting up osbuild output directory: %v", err)
+	}
+	defer func() {
+		err := os.RemoveAll(tmpdir)
+		if err != nil {
+			log.Printf("Error removing osbuild-worker cache (%s): %v", tmpdir, err)
+		}
+	}()
+
 	for {
 		fmt.Println("Waiting for a new job...")
 		job, err := client.AddJob()
@@ -202,7 +212,7 @@ func main() {
 		fmt.Printf("Running job %s\n", job.Id)
 
 		var status common.ImageBuildState
-		result, err := RunJob(job, client.UploadImage)
+		result, err := RunJob(job, tmpdir, client.UploadImage)
 		if err != nil {
 			log.Printf("  Job failed: %v", err)
 			status = common.IBFailed
diff --git a/distribution/osbuild-remote-worker@.service b/distribution/osbuild-remote-worker@.service
index 1a2eba0c9..0994e296d 100644
--- a/distribution/osbuild-remote-worker@.service
+++ b/distribution/osbuild-remote-worker@.service
@@ -10,3 +10,6 @@ Restart=on-failure
 RestartSec=10s
 CPUSchedulingPolicy=batch
 IOSchedulingClass=idle
+CacheDirectory=osbuild-worker
+# systemd >= 240 sets this, but osbuild-worker runs on earlier versions
+Environment="CACHE_DIRECTORY=/var/cache/osbuild-worker"
diff --git a/distribution/osbuild-worker@.service b/distribution/osbuild-worker@.service
index 41ec50dd4..8b12a2ec0 100644
--- a/distribution/osbuild-worker@.service
+++ b/distribution/osbuild-worker@.service
@@ -11,6 +11,9 @@ Restart=on-failure
 RestartSec=10s
 CPUSchedulingPolicy=batch
 IOSchedulingClass=idle
+CacheDirectory=osbuild-worker
+# systemd >= 240 sets this, but osbuild-worker runs on earlier versions
+Environment="CACHE_DIRECTORY=/var/cache/osbuild-worker"
 
 [Install]
 WantedBy=osbuild-composer.service