[lxc-devel] [lxd/master] lxd/images: Don't keep an in-memory simplestreams cache

[stgraber on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 354 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190401/60a12ff1/attachment-0001.bin>
-------------- next part --------------
From c68f65ee69b92c45facee90bb96cfea608cdc88f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 1 Apr 2019 22:04:58 -0400
Subject: [PATCH] lxd/images: Don't keep an in-memory simplestreams cache
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/daemon.go        | 10 ---------
 lxd/daemon_config.go |  7 ------
 lxd/daemon_images.go | 53 +++++++++++++++++++++-----------------------
 3 files changed, 25 insertions(+), 45 deletions(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index 8aabd26614..23a8fd8fb4 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -715,12 +715,6 @@ func (d *Daemon) init() error {
 		return err
 	}
 
-	/* Restore simplestreams cache */
-	err = imageLoadStreamCache(d)
-	if err != nil {
-		return err
-	}
-
 	// Cleanup leftover images
 	pruneLeftoverImages(d)
 
@@ -994,10 +988,6 @@ func (d *Daemon) Stop() error {
 			"Not unmounting temporary filesystems (containers are still running)")
 	}
 
-	logger.Infof("Saving simplestreams cache")
-	trackError(imageSaveStreamCache(d.os))
-	logger.Infof("Saved simplestreams cache")
-
 	var err error
 	if n := len(errs); n > 0 {
 		format := "%v"
diff --git a/lxd/daemon_config.go b/lxd/daemon_config.go
index a3f9a63480..57b6f04533 100644
--- a/lxd/daemon_config.go
+++ b/lxd/daemon_config.go
@@ -50,11 +50,4 @@ func daemonConfigSetProxy(d *Daemon, config *cluster.Config) {
 		config.ProxyHTTP(),
 		config.ProxyIgnoreHosts(),
 	)
-
-	// Clear the simplestreams cache as it's tied to the old proxy config
-	imageStreamCacheLock.Lock()
-	for k := range imageStreamCache {
-		delete(imageStreamCache, k)
-	}
-	imageStreamCacheLock.Unlock()
 }
diff --git a/lxd/daemon_images.go b/lxd/daemon_images.go
index 3a3b22eff0..0344436a64 100644
--- a/lxd/daemon_images.go
+++ b/lxd/daemon_images.go
@@ -36,16 +36,14 @@ type imageStreamCacheEntry struct {
 	Fingerprints []string                `yaml:"fingerprints"`
 
 	expiry time.Time
-	remote lxd.ImageServer
 }
 
-var imageStreamCache = map[string]*imageStreamCacheEntry{}
 var imageStreamCacheLock sync.Mutex
 
 var imagesDownloading = map[string]chan bool{}
 var imagesDownloadingLock sync.Mutex
 
-func imageSaveStreamCache(os *sys.OS) error {
+func imageSaveStreamCache(os *sys.OS, imageStreamCache map[string]*imageStreamCacheEntry) error {
 	data, err := yaml.Marshal(&imageStreamCache)
 	if err != nil {
 		return err
@@ -59,41 +57,25 @@ func imageSaveStreamCache(os *sys.OS) error {
 	return nil
 }
 
-func imageLoadStreamCache(d *Daemon) error {
-	imageStreamCacheLock.Lock()
-	defer imageStreamCacheLock.Unlock()
+func imageGetStreamCache(d *Daemon) (map[string]*imageStreamCacheEntry, error) {
+	imageStreamCache := map[string]*imageStreamCacheEntry{}
 
 	simplestreamsPath := filepath.Join(d.os.CacheDir, "simplestreams.yaml")
 	if !shared.PathExists(simplestreamsPath) {
-		return nil
+		return imageStreamCache, nil
 	}
 
 	content, err := ioutil.ReadFile(simplestreamsPath)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	err = yaml.Unmarshal(content, imageStreamCache)
 	if err != nil {
-		return err
-	}
-
-	for url, entry := range imageStreamCache {
-		if entry.remote == nil {
-			remote, err := lxd.ConnectSimpleStreams(url, &lxd.ConnectionArgs{
-				TLSServerCert: entry.Certificate,
-				UserAgent:     version.UserAgent,
-				Proxy:         d.proxy,
-			})
-			if err != nil {
-				continue
-			}
-
-			entry.remote = remote
-		}
+		return nil, err
 	}
 
-	return nil
+	return imageStreamCache, nil
 }
 
 // ImageDownload resolves the image fingerprint and if not in the database, downloads it
@@ -115,6 +97,12 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 	// Attempt to resolve the alias
 	if protocol == "simplestreams" {
 		imageStreamCacheLock.Lock()
+		imageStreamCache, err := imageGetStreamCache(d)
+		if err != nil {
+			imageStreamCacheLock.Unlock()
+			return nil, err
+		}
+
 		entry, _ := imageStreamCache[server]
 		if entry == nil || entry.expiry.Before(time.Now()) {
 			// Add a new entry to the cache
@@ -147,9 +135,9 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 				}
 
 				// Generate cache entry
-				entry = &imageStreamCacheEntry{remote: remote, Aliases: aliases, Certificate: certificate, Fingerprints: fingerprints, expiry: time.Now().Add(time.Hour)}
+				entry = &imageStreamCacheEntry{Aliases: aliases, Certificate: certificate, Fingerprints: fingerprints, expiry: time.Now().Add(time.Hour)}
 				imageStreamCache[server] = entry
-				imageSaveStreamCache(d.os)
+				imageSaveStreamCache(d.os, imageStreamCache)
 
 				return entry, nil
 			}
@@ -170,7 +158,16 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 		} else {
 			// use the existing entry
 			logger.Debug("Using SimpleStreams cache entry", log.Ctx{"server": server, "expiry": entry.expiry})
-			remote = entry.remote
+
+			remote, err = lxd.ConnectSimpleStreams(server, &lxd.ConnectionArgs{
+				TLSServerCert: entry.Certificate,
+				UserAgent:     version.UserAgent,
+				Proxy:         d.proxy,
+			})
+			if err != nil {
+				imageStreamCacheLock.Unlock()
+				return nil, err
+			}
 		}
 		imageStreamCacheLock.Unlock()