Ensure process limits are correctly set with using juju-db snap

sdg0919 · Dec 9, 2020 · 9ae76f4 · 9ae76f4
1 parent cfde215
commit 9ae76f4
Show file tree

Hide file tree

Showing 7 changed files with 127 additions and 44 deletions.
diff --git a/mongo/mongo.go b/mongo/mongo.go
@@ -30,7 +30,6 @@ import (
 	"github.com/juju/juju/packaging"
 	"github.com/juju/juju/packaging/dependency"
 	"github.com/juju/juju/service"
-	"github.com/juju/juju/service/common"
 	"github.com/juju/juju/service/snap"
 )
 
@@ -426,6 +425,9 @@ type EnsureServerParams struct {
 	// DataDir is the machine agent data directory.
 	DataDir string
 
+	// LogDir is where mongo logs go.
+	LogDir string
+
 	// Namespace is the machine agent's namespace, which is used to
 	// generate a unique service name for Mongo.
 	Namespace string
@@ -484,9 +486,12 @@ func ensureServer(args EnsureServerParams, mongoKernelTweaks map[string]string)
 		args.DataDir, args.StatePort,
 	)
 
-	setupDataDirectory(args, usingMongoFromSnap)
+	err = setupDataDirectory(args, usingMongoFromSnap)
+	if err != nil {
+		return zeroVersion, errors.Trace(err)
+	}
 
-	if err := installMongod(mongoDep, hostSeries, dataDir); err != nil {
+	if err := installMongod(mongoDep, usingMongoFromSnap, hostSeries, dataDir); err != nil {
 		// This isn't treated as fatal because the Juju MongoDB
 		// package is likely to be already installed anyway. There
 		// could just be a temporary issue with apt-get/yum/whatever
@@ -617,7 +622,7 @@ func setupDataDirectory(args EnsureServerParams, usingMongoFromSnap bool) error
 		return errors.Annotatef(err, "cannot write mongod shared secret to %v", sharedSecretPath(args.DataDir))
 	}
 
-	if err := os.MkdirAll(logPath(dbDir, usingMongoFromSnap), 0644); err != nil {
+	if err := os.MkdirAll(logPath(args.LogDir, dbDir, usingMongoFromSnap), 0644); err != nil {
 		return errors.Annotate(err, "cannot create mongodb logging directory")
 	}
 
@@ -681,25 +686,21 @@ func logVersion(mongoPath string) {
 	logger.Debugf("using mongod: %s --version: %q", mongoPath, output)
 }
 
-func installMongod(mongoDep packaging.Dependency, hostSeries, dataDir string) error {
-	// If we are not forcing a mongo snap via a feature flag, install the
-	// package list (which may also include snaps for focal+) provided by
-	// the mongo dependency for our series.
-	if !featureflag.Enabled(feature.MongoDbSnap) {
-		return packaging.InstallDependency(mongoDep, hostSeries)
-	}
-
+func installMongod(mongoDep packaging.Dependency, usingMongoFromSnap bool, hostSeries, dataDir string) (err error) {
 	snapName := ServiceName
-	jujuDbLocalSnapPattern := regexp.MustCompile(`juju-db_[0-9]+\.snap`)
-
-	// If we're installing a local snap, then provide an absolute path
-	// as a snap <name>. snap install <name> will then do the Right Thing (TM).
-	files, err := ioutil.ReadDir(path.Join(dataDir, "snap"))
-	if err == nil {
-		for _, fullFileName := range files {
-			_, fileName := path.Split(fullFileName.Name())
-			if jujuDbLocalSnapPattern.MatchString(fileName) {
-				snapName = fullFileName.Name()
+
+	if usingMongoFromSnap {
+		jujuDbLocalSnapPattern := regexp.MustCompile(`juju-db_[0-9]+\.snap`)
+
+		// If we're installing a local snap, then provide an absolute path
+		// as a snap <name>. snap install <name> will then do the Right Thing (TM).
+		files, err := ioutil.ReadDir(path.Join(dataDir, "snap"))
+		if err == nil {
+			for _, fullFileName := range files {
+				_, fileName := path.Split(fullFileName.Name())
+				if jujuDbLocalSnapPattern.MatchString(fileName) {
+					snapName = fullFileName.Name()
+				}
 			}
 		}
 	}
@@ -711,14 +712,31 @@ func installMongod(mongoDep packaging.Dependency, hostSeries, dataDir string) er
 			EnableAtStartup: true,
 		},
 	}
-	conf := common.Conf{Desc: ServiceName + " snap"}
+	conf := newConf(&ConfigArgs{})
+	conf.Desc = ServiceName + " snap"
 	snapChannel := fmt.Sprintf("%s/%s", SnapTrack, SnapRisk)
-	service, err := snap.NewService(snapName, ServiceName, conf, snap.Command, snapChannel, "", backgroundServices, prerequisites)
+	snapSvc, err := snap.NewService(snapName, ServiceName, conf, snap.Command, snapChannel, "", backgroundServices, prerequisites)
 	if err != nil {
 		return errors.Trace(err)
 	}
 
-	return service.Install()
+	if usingMongoFromSnap {
+		defer func() {
+			err = snapSvc.ConfigOverride()
+			if err == nil {
+				err = snapSvc.Restart()
+			}
+		}()
+	}
+
+	// If we are not forcing a mongo snap via a feature flag, install the
+	// package list (which may also include snaps for focal+) provided by
+	// the mongo dependency for our series.
+	if !featureflag.Enabled(feature.MongoDbSnap) {
+		return packaging.InstallDependency(mongoDep, hostSeries)
+	}
+
+	return snapSvc.Install()
 }
 
 // DbDir returns the dir where mongo storage is.

diff --git a/mongo/mongo_test.go b/mongo/mongo_test.go
@@ -70,6 +70,7 @@ func makeEnsureServerParams(dataDir string) mongo.EnsureServerParams {
 		SharedSecret: testInfo.SharedSecret,
 
 		DataDir: dataDir,
+		LogDir:  dataDir,
 	}
 }
 

diff --git a/mongo/service.go b/mongo/service.go
@@ -168,10 +168,13 @@ func sharedSecretPath(dataDir string) string {
 	return filepath.Join(dataDir, SharedSecretFile)
 }
 
-func logPath(dataDir string, usingMongoFromSnap bool) string {
+func logPath(logDir, dataDir string, usingMongoFromSnap bool) string {
 	if usingMongoFromSnap {
 		return filepath.Join(dataDir, "logs", "mongodb.log")
 	}
+	if logDir != "" {
+		return logDir
+	}
 	return mongoLogPath
 }
 
@@ -493,7 +496,7 @@ func generateConfig(mongoPath string, oplogSizeMB int, version Version, usingMon
 		// have the same permissions.
 		mongoArgs.Syslog = false
 		mongoArgs.LogAppend = true
-		mongoArgs.LogPath = logPath(args.DataDir, true)
+		mongoArgs.LogPath = logPath(args.LogDir, args.DataDir, true)
 		mongoArgs.BindToAllIP = true // TODO(tsm): disable when not needed
 	}
 

diff --git a/service/snap/export_test.go b/service/snap/export_test.go
@@ -7,3 +7,7 @@ var (
 	DefaultChannel           = defaultChannel
 	DefaultConfinementPolicy = defaultConfinementPolicy
 )
+
+func SetServiceConfigDir(s *Service, dir string) {
+	s.configDir = dir
+}
diff --git a/service/snap/snap.go b/service/snap/snap.go
@@ -6,7 +6,10 @@ package snap
 
 import (
 	"fmt"
+	"io/ioutil"
+	"os"
 	"os/exec"
+	"path/filepath"
 	"regexp"
 	"runtime"
 	"strings"
@@ -17,6 +20,7 @@ import (
 	"github.com/juju/utils/shell"
 
 	"github.com/juju/juju/service/common"
+	"github.com/juju/juju/service/systemd"
 )
 
 const (
@@ -186,6 +190,7 @@ type Service struct {
 	executable     string
 	app            App
 	conf           common.Conf
+	configDir      string
 }
 
 // NewService returns a new Service defined by `conf`, with the name `serviceName`.
@@ -222,6 +227,7 @@ func NewService(mainSnap string, serviceName string, conf common.Conf, snapPath
 		executable:     snapPath,
 		app:            app,
 		conf:           conf,
+		configDir:      systemd.EtcSystemdDir,
 	}
 
 	return svc, nil
@@ -378,6 +384,29 @@ func (s Service) InstallCommands() ([]string, error) {
 	return commands, nil
 }
 
+// ConfigOverride writes a systemd override to enable the
+// specified limits to be used by the snap.
+func (s Service) ConfigOverride() error {
+	if len(s.conf.Limit) == 0 {
+		return nil
+	}
+	for _, backgroundService := range s.app.BackgroundServices {
+		unitOptions := systemd.ServiceLimits(s.conf)
+		data, err := ioutil.ReadAll(systemd.UnitSerialize(unitOptions))
+		if err != nil {
+			return errors.Trace(err)
+		}
+		overridesDir := fmt.Sprintf("%s/snap.%s.%s.service.d", s.configDir, s.name, backgroundService.Name)
+		if err := os.MkdirAll(overridesDir, 0755); err != nil {
+			return errors.Trace(err)
+		}
+		if err := ioutil.WriteFile(filepath.Join(overridesDir, "overrides.conf"), data, 0644); err != nil {
+			return errors.Trace(err)
+		}
+	}
+	return nil
+}
+
 func confimentParameterAsString(confinementPolicy string) string {
 	if confinementPolicy != "" {
 		return fmt.Sprintf("--%v", confinementPolicy)

diff --git a/service/snap/snap_test.go b/service/snap/snap_test.go
@@ -4,7 +4,9 @@
 package snap_test
 
 import (
+	"io/ioutil"
 	"os/exec"
+	"path/filepath"
 	"strings"
 
 	"github.com/juju/testing"
@@ -73,18 +75,18 @@ func (*validationSuite) TestNewApp(c *gc.C) {
 	})
 }
 
-type externalCommandsSuite struct {
+type snapSuite struct {
 	testing.IsolationSuite
 }
 
-var _ = gc.Suite(&externalCommandsSuite{})
+var _ = gc.Suite(&snapSuite{})
 
-func (*externalCommandsSuite) TestSnapCommandIsAValidCommand(c *gc.C) {
+func (*snapSuite) TestSnapCommandIsAValidCommand(c *gc.C) {
 	_, err := exec.LookPath(snap.Command)
 	c.Check(err, gc.NotNil)
 }
 
-func (*externalCommandsSuite) TestSnapListCommandreValidShellCommand(c *gc.C) {
+func (*snapSuite) TestSnapListCommandreValidShellCommand(c *gc.C) {
 	listCommand := snap.ListCommand()
 	listCommandParts := strings.Fields(listCommand)
 
@@ -100,3 +102,23 @@ func (*externalCommandsSuite) TestSnapListCommandreValidShellCommand(c *gc.C) {
 	}
 	c.Check(err, gc.NotNil)
 }
+
+func (*snapSuite) TestConfigOverride(c *gc.C) {
+	conf := common.Conf{Limit: map[string]string{"nofile": "64000"}}
+	svc, err := snap.NewService("juju-db", "", conf, snap.Command, "latest", "strict", []snap.BackgroundService{{
+		Name: "daemon",
+	}}, nil)
+	c.Assert(err, jc.ErrorIsNil)
+
+	dir := c.MkDir()
+	snap.SetServiceConfigDir(&svc, dir)
+	err = svc.ConfigOverride()
+	c.Assert(err, jc.ErrorIsNil)
+	data, err := ioutil.ReadFile(filepath.Join(dir, "snap.juju-db.daemon.service.d/overrides.conf"))
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(string(data), gc.Equals, `
+[Service]
+LimitNOFILE=64000
+
+`[1:])
+}
diff --git a/service/systemd/conf.go b/service/systemd/conf.go
@@ -185,20 +185,9 @@ func iterSortedKeys(in map[string]string) []string {
 	return out
 }
 
-func serializeService(conf common.Conf) []*unit.UnitOption {
+// ServiceLimits converts the limits in conf to systemd unit options.
+func ServiceLimits(conf common.Conf) []*unit.UnitOption {
 	var unitOptions []*unit.UnitOption
-
-	// TODO(ericsnow) Support "Type" (e.g. "forking")? For now we just
-	// use the default, "simple".
-
-	for k, v := range conf.Env {
-		unitOptions = append(unitOptions, &unit.UnitOption{
-			Section: "Service",
-			Name:    "Environment",
-			Value:   fmt.Sprintf(`"%s=%s"`, k, v),
-		})
-	}
-
 	for _, k := range iterSortedKeys(conf.Limit) {
 		v := conf.Limit[k]
 		if v == "unlimited" {
@@ -211,6 +200,23 @@ func serializeService(conf common.Conf) []*unit.UnitOption {
 			Value:   v,
 		})
 	}
+	return unitOptions
+}
+
+func serializeService(conf common.Conf) []*unit.UnitOption {
+	var unitOptions []*unit.UnitOption
+
+	// TODO(ericsnow) Support "Type" (e.g. "forking")? For now we just
+	// use the default, "simple".
+
+	for k, v := range conf.Env {
+		unitOptions = append(unitOptions, &unit.UnitOption{
+			Section: "Service",
+			Name:    "Environment",
+			Value:   fmt.Sprintf(`"%s=%s"`, k, v),
+		})
+	}
+	unitOptions = append(unitOptions, ServiceLimits(conf)...)
 
 	if conf.ExecStart != "" {
 		unitOptions = append(unitOptions, &unit.UnitOption{
-Original file line number
+Diff line change
@@ Expand Up @@
     		SharedSecret: testInfo.SharedSecret,
     		DataDir: dataDir,
+    		LogDir:  dataDir,
     	}
     }
@@ Expand Down @@