[lxc-devel] [lxd/master] Switch more background tasks to operations

stgraber on Github lxc-bot at linuxcontainers.org
Tue Oct 30 21:03:06 UTC 2018


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20181030/50aca058/attachment.bin>
-------------- next part --------------
From 1cc810ec72349e8a3fdcd761ebf270a475b71e77 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 30 Oct 2018 16:33:02 -0400
Subject: [PATCH 1/2] lxd/daemon: Don't mention MAAS unless configured
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 | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index 08e110621a..c4de2cfbc3 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -703,18 +703,20 @@ func (d *Daemon) init() error {
 		readSavedClientCAList(d)
 
 		// Connect to MAAS
-		go func() {
-			for {
-				err = d.setupMAASController(maasAPIURL, maasAPIKey, maasMachine)
-				if err == nil {
-					logger.Info("Connected to MAAS controller")
-					break
+		if maasAPIURL != "" {
+			go func() {
+				for {
+					err = d.setupMAASController(maasAPIURL, maasAPIKey, maasMachine)
+					if err == nil {
+						logger.Info("Connected to MAAS controller", log.Ctx{"url": maasAPIURL})
+						break
+					}
+
+					logger.Warn("Unable to connect to MAAS, trying again in a minute", log.Ctx{"url": maasAPIURL, "err": err})
+					time.Sleep(time.Minute)
 				}
-
-				logger.Warn("Unable to connect to MAAS, trying again in a minute", log.Ctx{"err": err})
-				time.Sleep(time.Minute)
-			}
-		}()
+			}()
+		}
 	}
 
 	close(d.setupChan)

From 41461b3ef51fbe25075e7040daa26caa8cdbd2c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 30 Oct 2018 17:00:52 -0400
Subject: [PATCH 2/2] lxd: Register background tasks as operations
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/container_instance_types.go |  41 ++++++---
 lxd/db/operations.go            |  15 ++++
 lxd/images.go                   | 143 +++++++++++++++++++-------------
 lxd/logging.go                  |  12 ++-
 4 files changed, 142 insertions(+), 69 deletions(-)

diff --git a/lxd/container_instance_types.go b/lxd/container_instance_types.go
index a2e1c69634..5500355858 100644
--- a/lxd/container_instance_types.go
+++ b/lxd/container_instance_types.go
@@ -7,14 +7,17 @@ import (
 	"strconv"
 	"strings"
 
+	"golang.org/x/net/context"
 	"gopkg.in/yaml.v2"
 
+	"github.com/lxc/lxd/lxd/db"
 	"github.com/lxc/lxd/lxd/task"
 	"github.com/lxc/lxd/lxd/util"
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/logger"
 	"github.com/lxc/lxd/shared/version"
-	"golang.org/x/net/context"
+
+	log "github.com/lxc/lxd/shared/log15"
 )
 
 type instanceType struct {
@@ -71,27 +74,42 @@ func instanceRefreshTypesTask(d *Daemon) (task.Func, task.Schedule) {
 	// returning in case the context expires.
 	_, hasCancellationSupport := interface{}(&http.Request{}).(util.ContextAwareRequest)
 	f := func(ctx context.Context) {
-		if hasCancellationSupport {
-			instanceRefreshTypes(ctx, d)
-		} else {
-			ch := make(chan struct{})
+		opRun := func(op *operation) error {
+			if hasCancellationSupport {
+				return instanceRefreshTypes(ctx, d)
+			}
+
+			ch := make(chan error)
 			go func() {
-				instanceRefreshTypes(ctx, d)
-				ch <- struct{}{}
+				ch <- instanceRefreshTypes(ctx, d)
 			}()
 			select {
 			case <-ctx.Done():
-				return
-			case <-ch:
+				return nil
+			case err := <-ch:
+				return err
 			}
+
+			return nil
+		}
+
+		op, err := operationCreate(d.cluster, "", operationClassTask, db.OperationInstanceTypesUpdate, nil, nil, opRun, nil, nil)
+		if err != nil {
+			logger.Error("Failed to start instance types update operation", log.Ctx{"err": err})
 		}
+
+		logger.Info("Updating instance types")
+		_, err = op.Run()
+		if err != nil {
+			logger.Error("Failed to update instance types", log.Ctx{"err": err})
+		}
+		logger.Infof("Done updating instance types")
 	}
+
 	return f, task.Daily()
 }
 
 func instanceRefreshTypes(ctx context.Context, d *Daemon) error {
-	logger.Info("Updating instance types")
-
 	// Attempt to download the new definitions
 	downloadParse := func(filename string, target interface{}) error {
 		url := fmt.Sprintf("https://images.linuxcontainers.org/meta/instance-types/%s", filename)
@@ -177,7 +195,6 @@ func instanceRefreshTypes(ctx context.Context, d *Daemon) error {
 		return err
 	}
 
-	logger.Infof("Done updating instance types")
 	return nil
 }
 
diff --git a/lxd/db/operations.go b/lxd/db/operations.go
index 47c4db2d2a..49f5c64f71 100644
--- a/lxd/db/operations.go
+++ b/lxd/db/operations.go
@@ -54,6 +54,11 @@ const (
 	OperationVolumeSnapshotDelete
 	OperationVolumeSnapshotUpdate
 	OperationProjectRename
+	OperationImagesExpire
+	OperationImagesPruneLeftover
+	OperationImagesUpdate
+	OperationLogsExpire
+	OperationInstanceTypesUpdate
 )
 
 // Description return a human-readable description of the operation type.
@@ -133,6 +138,16 @@ func (t OperationType) Description() string {
 		return "Updating storage volume snapshot"
 	case OperationProjectRename:
 		return "Renaming project"
+	case OperationImagesExpire:
+		return "Cleaning up expired images"
+	case OperationImagesPruneLeftover:
+		return "Pruning leftover image files"
+	case OperationImagesUpdate:
+		return "Updating images"
+	case OperationLogsExpire:
+		return "Expiring log files"
+	case OperationInstanceTypesUpdate:
+		return "Updating instance types"
 	default:
 		return "Executing operation"
 
diff --git a/lxd/images.go b/lxd/images.go
index 7dd70da63f..dab0b4f879 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -838,8 +838,23 @@ func imagesGet(d *Daemon, r *http.Request) Response {
 
 func autoUpdateImagesTask(d *Daemon) (task.Func, task.Schedule) {
 	f := func(ctx context.Context) {
-		autoUpdateImages(ctx, d)
+		opRun := func(op *operation) error {
+			return autoUpdateImages(ctx, d)
+		}
+
+		op, err := operationCreate(d.cluster, "", operationClassTask, db.OperationImagesUpdate, nil, nil, opRun, nil, nil)
+		if err != nil {
+			logger.Error("Failed to start image update operation", log.Ctx{"err": err})
+		}
+
+		logger.Infof("Updating images")
+		_, err = op.Run()
+		if err != nil {
+			logger.Error("Failed to update images", log.Ctx{"err": err})
+		}
+		logger.Infof("Done updating images")
 	}
+
 	schedule := func() (time.Duration, error) {
 		var interval time.Duration
 		err := d.cluster.Transaction(func(tx *db.ClusterTx) error {
@@ -858,9 +873,7 @@ func autoUpdateImagesTask(d *Daemon) (task.Func, task.Schedule) {
 	return f, schedule
 }
 
-func autoUpdateImages(ctx context.Context, d *Daemon) {
-	logger.Infof("Updating images")
-
+func autoUpdateImages(ctx context.Context, d *Daemon) error {
 	projectNames := []string{}
 	err := d.cluster.Transaction(func(tx *db.ClusterTx) error {
 		projects, err := tx.ProjectList(db.ProjectFilter{})
@@ -875,18 +888,17 @@ func autoUpdateImages(ctx context.Context, d *Daemon) {
 		return nil
 	})
 	if err != nil {
-		logger.Error("Unable to retrieve project names", log.Ctx{"err": err})
-		return
+		return errors.Wrap(err, "Unable to retrieve project names")
 	}
 
 	for _, project := range projectNames {
 		err := autoUpdateImagesInProject(ctx, d, project)
 		if err != nil {
-			logger.Errorf("Unable to update images for project %s: %v", project, err)
+			return errors.Wrapf(err, "Unable to update images for project %s", project)
 		}
 	}
 
-	logger.Infof("Done updating images")
+	return nil
 }
 
 func autoUpdateImagesInProject(ctx context.Context, d *Daemon, project string) error {
@@ -1053,7 +1065,21 @@ func autoUpdateImage(d *Daemon, op *operation, id int, info *api.Image, project
 
 func pruneExpiredImagesTask(d *Daemon) (task.Func, task.Schedule) {
 	f := func(ctx context.Context) {
-		pruneExpiredImages(ctx, d)
+		opRun := func(op *operation) error {
+			return pruneExpiredImages(ctx, d)
+		}
+
+		op, err := operationCreate(d.cluster, "", operationClassTask, db.OperationImagesExpire, nil, nil, opRun, nil, nil)
+		if err != nil {
+			logger.Error("Failed to start expired image operation", log.Ctx{"err": err})
+		}
+
+		logger.Infof("Pruning expired images")
+		_, err = op.Run()
+		if err != nil {
+			logger.Error("Failed to expire images", log.Ctx{"err": err})
+		}
+		logger.Infof("Done pruning expired images")
 	}
 
 	// Skip the first run, and instead run an initial pruning synchronously
@@ -1062,8 +1088,9 @@ func pruneExpiredImagesTask(d *Daemon) (task.Func, task.Schedule) {
 	if err != nil {
 		logger.Error("Unable to fetch cluster configuration", log.Ctx{"err": err})
 	} else if expiry > 0 {
-		pruneExpiredImages(context.Background(), d)
+		f(context.Background())
 	}
+
 	first := true
 	schedule := func() (time.Duration, error) {
 		interval := 24 * time.Hour
@@ -1090,53 +1117,58 @@ func pruneExpiredImagesTask(d *Daemon) (task.Func, task.Schedule) {
 }
 
 func pruneLeftoverImages(d *Daemon) {
-	logger.Infof("Pruning leftover image files")
+	opRun := func(op *operation) error {
+		// Get all images
+		images, err := d.cluster.ImagesGet("default", false)
+		if err != nil {
+			return errors.Wrap(err, "Unable to retrieve the list of images")
+		}
 
-	// Get all images
-	images, err := d.cluster.ImagesGet("default", false)
-	if err != nil {
-		logger.Error("Unable to retrieve the list of images", log.Ctx{"err": err})
-		return
-	}
+		// Look at what's in the images directory
+		entries, err := ioutil.ReadDir(shared.VarPath("images"))
+		if err != nil {
+			return errors.Wrap(err, "Unable to list the images directory")
+		}
 
-	// Look at what's in the images directory
-	entries, err := ioutil.ReadDir(shared.VarPath("images"))
-	if err != nil {
-		logger.Error("Unable to list the images directory", log.Ctx{"err": err})
-		return
-	}
+		// Check and delete leftovers
+		for _, entry := range entries {
+			fp := strings.Split(entry.Name(), ".")[0]
+			if !shared.StringInSlice(fp, images) {
+				err = os.RemoveAll(shared.VarPath("images", entry.Name()))
+				if err != nil {
+					return errors.Wrapf(err, "Unable to remove leftover image: %v", entry.Name())
+				}
 
-	// Check and delete leftovers
-	for _, entry := range entries {
-		fp := strings.Split(entry.Name(), ".")[0]
-		if !shared.StringInSlice(fp, images) {
-			err = os.RemoveAll(shared.VarPath("images", entry.Name()))
-			if err != nil {
-				logger.Error("Unable to remove leftover image", log.Ctx{"err": err, "file": entry.Name()})
-				continue
+				logger.Debugf("Removed leftover image file: %s", entry.Name())
 			}
-
-			logger.Debugf("Removed leftover image file: %s", entry.Name())
 		}
+
+		return nil
 	}
 
+	op, err := operationCreate(d.cluster, "", operationClassTask, db.OperationImagesPruneLeftover, nil, nil, opRun, nil, nil)
+	if err != nil {
+		logger.Error("Failed to start image leftover cleanup operation", log.Ctx{"err": err})
+	}
+
+	logger.Infof("Pruning leftover image files")
+	_, err = op.Run()
+	if err != nil {
+		logger.Error("Failed to prune leftover image files", log.Ctx{"err": err})
+	}
 	logger.Infof("Done pruning leftover image files")
 }
 
-func pruneExpiredImages(ctx context.Context, d *Daemon) {
-	logger.Infof("Pruning expired images")
-
+func pruneExpiredImages(ctx context.Context, d *Daemon) error {
 	expiry, err := cluster.ConfigGetInt64(d.cluster, "images.remote_cache_expiry")
 	if err != nil {
-		logger.Error("Unable to fetch cluster configuration", log.Ctx{"err": err})
-		return
+		return errors.Wrap(err, "Unable to fetch cluster configuration")
 	}
 
 	// Get the list of expired images.
 	images, err := d.cluster.ImagesGetExpired(expiry)
 	if err != nil {
-		logger.Error("Unable to retrieve the list of expired images", log.Ctx{"err": err})
-		return
+		return errors.Wrap(err, "Unable to retrieve the list of expired images")
 	}
 
 	// Delete them
@@ -1146,7 +1178,7 @@ func pruneExpiredImages(ctx context.Context, d *Daemon) {
 		// expired now will be expired at the next run.
 		select {
 		case <-ctx.Done():
-			return
+			return nil
 		default:
 		}
 
@@ -1166,8 +1198,7 @@ func pruneExpiredImages(ctx context.Context, d *Daemon) {
 		for _, pool := range poolNames {
 			err := doDeleteImageFromPool(d.State(), fp, pool)
 			if err != nil {
-				logger.Debugf("Error deleting image %s from storage pool %s: %s", fp, pool, err)
-				continue
+				return errors.Wrapf(err, "Error deleting image %s from storage pool %s", fp, pool)
 			}
 		}
 
@@ -1175,8 +1206,8 @@ func pruneExpiredImages(ctx context.Context, d *Daemon) {
 		fname := filepath.Join(d.os.VarDir, "images", fp)
 		if shared.PathExists(fname) {
 			err = os.Remove(fname)
-			if err != nil {
-				logger.Debugf("Error deleting image file %s: %s", fname, err)
+			if err != nil && !os.IsNotExist(err) {
+				return errors.Wrapf(err, "Error deleting image file %s", fname)
 			}
 		}
 
@@ -1184,23 +1215,23 @@ func pruneExpiredImages(ctx context.Context, d *Daemon) {
 		fname = filepath.Join(d.os.VarDir, "images", fp) + ".rootfs"
 		if shared.PathExists(fname) {
 			err = os.Remove(fname)
-			if err != nil {
-				logger.Debugf("Error deleting image file %s: %s", fname, err)
+			if err != nil && !os.IsNotExist(err) {
+				return errors.Wrapf(err, "Error deleting image file %s", fname)
 			}
 		}
 
 		imgID, _, err := d.cluster.ImageGet("default", fp, false, false)
 		if err != nil {
-			logger.Debugf("Error retrieving image info %s: %s", fp, err)
+			return errors.Wrapf(err, "Error retrieving image info %s", fp)
 		}
 
 		// Remove the database entry for the image.
 		if err = d.cluster.ImageDelete(imgID); err != nil {
-			logger.Debugf("Error deleting image %s from database: %s", fp, err)
+			return errors.Wrapf(err, "Error deleting image %s from database", fp)
 		}
 	}
 
-	logger.Infof("Done pruning expired images")
+	return nil
 }
 
 func doDeleteImageFromPool(state *state.State, fingerprint string, storagePool string) error {
@@ -1268,8 +1299,8 @@ func imageDelete(d *Daemon, r *http.Request) Response {
 		fname := shared.VarPath("images", imgInfo.Fingerprint)
 		if shared.PathExists(fname) {
 			err = os.Remove(fname)
-			if err != nil {
-				logger.Debugf("Error deleting image file %s: %s", fname, err)
+			if err != nil && !os.IsNotExist(err) {
+				return errors.Wrapf(err, "Error deleting image file %s: %s", fname)
 			}
 		}
 
@@ -1340,8 +1371,8 @@ func imageDeleteFromDisk(fingerprint string) {
 	fname := shared.VarPath("images", fingerprint)
 	if shared.PathExists(fname) {
 		err := os.Remove(fname)
-		if err != nil {
-			logger.Debugf("Error deleting image file %s: %s", fname, err)
+		if err != nil && !os.IsNotExist(err) {
+			logger.Errorf("Error deleting image file %s: %s", fname, err)
 		}
 	}
 
@@ -1349,8 +1380,8 @@ func imageDeleteFromDisk(fingerprint string) {
 	fname = shared.VarPath("images", fingerprint) + ".rootfs"
 	if shared.PathExists(fname) {
 		err := os.Remove(fname)
-		if err != nil {
-			logger.Debugf("Error deleting image file %s: %s", fname, err)
+		if err != nil && !os.IsNotExist(err) {
+			logger.Errorf("Error deleting image file %s: %s", fname, err)
 		}
 	}
 }
diff --git a/lxd/logging.go b/lxd/logging.go
index 0605ebb8f4..53a70e8605 100644
--- a/lxd/logging.go
+++ b/lxd/logging.go
@@ -19,13 +19,23 @@ import (
 // and will run once every 24h.
 func expireLogsTask(state *state.State) (task.Func, task.Schedule) {
 	f := func(ctx context.Context) {
+		opRun := func(op *operation) error {
+			return expireLogs(ctx, state)
+		}
+
+		op, err := operationCreate(state.Cluster, "", operationClassTask, db.OperationLogsExpire, nil, nil, opRun, nil, nil)
+		if err != nil {
+			logger.Error("Failed to start log expiry operation", log.Ctx{"err": err})
+		}
+
 		logger.Infof("Expiring log files")
-		err := expireLogs(ctx, state)
+		_, err = op.Run()
 		if err != nil {
 			logger.Error("Failed to expire logs", log.Ctx{"err": err})
 		}
 		logger.Infof("Done expiring log files")
 	}
+
 	return f, task.Daily()
 }
 


More information about the lxc-devel mailing list