Fix 1370896 - juju has conf files in /var/log/juju on instances

cmd/jujud/util/util.go

@@ -237,7 +237,7 @@ var NewRsyslogConfigWorker = func(st *apirsyslog.State, agentConfig agent.Config
 	if err != nil {
 		return nil, err
 	}
-	return rsyslog.NewRsyslogConfigWorker(st, mode, tag, namespace, addrs)
+	return rsyslog.NewRsyslogConfigWorker(st, mode, tag, namespace, addrs, agentConfig.DataDir())
 }
 
 // ParamsStateServingInfoToStateStateServingInfo converts a

upgrades/export_test.go

@@ -42,4 +42,8 @@ var (
 	AddEnvironmentUUIDToAgentConfig = addEnvironmentUUIDToAgentConfig
 	AddDefaultStoragePools          = addDefaultStoragePools
 	MoveBlocksFromEnvironToState    = moveBlocksFromEnvironToState
+
+	// 124 upgrade functions
+	MoveSyslogConfig = moveSyslogConfig
+	CopyFile         = copyFile
 )

upgrades/operations.go

@@ -56,6 +56,10 @@ var upgradeOperations = func() []Operation {
 			version.MustParse("1.23.0"),
 			stepsFor123(),
 		},
+		upgradeToVersion{
+			version.MustParse("1.24.0"),
+			stepsFor124(),
+		},
 	}
 	return steps
 }

upgrades/steps124.go

@@ -3,12 +3,19 @@
 
 package upgrades
 
-import "github.com/juju/juju/state"
+import (
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/juju/juju/state"
+)
 
 // stateStepsFor124 returns upgrade steps for Juju 1.24 that manipulate state directly.
 func stateStepsFor124() []Step {
-	var steps []Step
-	steps = append(steps,
+	return []Step{
 		&upgradeStep{
 			description: "add block device documents for existing machines",
 			targets:     []Target{DatabaseMaster},
@@ -22,6 +29,80 @@ func stateStepsFor124() []Step {
 				return state.AddInstanceIdFieldOfIPAddresses(context.State())
 			},
 		},
-	)
-	return steps
+	}
+}
+
+// stepsFor124 returns upgrade steps for Juju 1.24 that only need the API.
+func stepsFor124() []Step {
+	return []Step{
+		&upgradeStep{
+			description: "move syslog config from LogDir to DataDir",
+			targets:     []Target{AllMachines},
+			run:         moveSyslogConfig,
+		},
+	}
+}
+
+func moveSyslogConfig(context Context) error {
+	config := context.AgentConfig()
+	logdir := config.LogDir()
+	datadir := config.DataDir()
+
+	// these values were copied from
+	// github.com/juju/juju/utils/syslog/config.go
+	// Yes this is bad, but it is only needed once every for an
+	// upgrade, so it didn't seem worth exporting those values.
+	files := []string{
+		"ca-cert.pem",
+		"rsyslog-cert.pem",
+		"rsyslog-key.pem",
+		"logrotate.conf",
+		"logrotate.run",
+	}
+	var errs []string
+	for _, f := range files {
+		oldpath := filepath.Join(logdir, f)
+		newpath := filepath.Join(datadir, f)
+		if err := copyFile(newpath, oldpath); err != nil {
+			errs = append(errs, err.Error())
+			continue
+		}
+		if err := os.Remove(oldpath); err != nil && !os.IsNotExist(err) {
+			errs = append(errs, err.Error())
+		}
+	}
+	if len(errs) > 0 {
+		return fmt.Errorf("error(s) while moving old syslog config files: %s", strings.Join(errs, "\n"))
+	}
+	return nil
+}
+
+// copyFile copies a file from one location to another.  It won't overwrite
+// existing files and will return nil in this case.  This is used instead of
+// os.Rename because os.Rename won't work across partitions.
+func copyFile(to, from string) error {
+	logger.Debugf("Copying %q to %q", from, to)
+	orig, err := os.Open(from)
+	if os.IsNotExist(err) {
+		logger.Debugf("Old file %q does not exist, skipping.", from)
+		// original doesn't exist, that's fine.
+		return nil
+	}
+	if err != nil {
+		return err
+	}
+	defer orig.Close()
+	info, err := orig.Stat()
+	if err != nil {
+		return err
+	}
+	target, err := os.OpenFile(to, os.O_CREATE|os.O_WRONLY|os.O_EXCL, info.Mode())
+	if os.IsExist(err) {
+		return nil
+	}
+	defer target.Close()
+	if _, err := io.Copy(target, orig); err != nil {
+		return err
+	}
+	return nil
 }

upgrades/steps124_test.go

@@ -4,9 +4,16 @@
 package upgrades_test
 
 import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 
+	"github.com/juju/juju/agent"
 	"github.com/juju/juju/testing"
+	"github.com/juju/juju/upgrades"
 	"github.com/juju/juju/version"
 )
 
@@ -23,3 +30,151 @@ func (s *steps124Suite) TestStateStepsFor124(c *gc.C) {
 	}
 	assertStateSteps(c, version.MustParse("1.24.0"), expected)
 }
+
+func (s *steps124Suite) TestStepsFor124(c *gc.C) {
+	expected := []string{
+		"move syslog config from LogDir to DataDir",
+	}
+	assertSteps(c, version.MustParse("1.24.0"), expected)
+}
+
+func (s *steps124Suite) TestCopyFileNew(c *gc.C) {
+	src := c.MkDir()
+	dest := c.MkDir()
+	srcdata := []byte("new data!")
+
+	// test that a file in src dir and not in dest dir gets copied.
+
+	newSrc := filepath.Join(src, "new")
+	err := ioutil.WriteFile(newSrc, srcdata, 0644)
+	c.Assert(err, gc.IsNil)
+
+	newDest := filepath.Join(dest, "new")
+
+	err = upgrades.CopyFile(newDest, newSrc)
+	c.Assert(err, gc.IsNil)
+
+	srcb, err := ioutil.ReadFile(newSrc)
+	c.Assert(err, gc.IsNil)
+	destb, err := ioutil.ReadFile(newDest)
+	c.Assert(err, gc.IsNil)
+	// convert to string and use Equals because we'll get a better failure message
+	c.Assert(string(destb), gc.Equals, string(srcb))
+}
+
+func (s *steps124Suite) TestCopyFileExisting(c *gc.C) {
+	src := c.MkDir()
+	dest := c.MkDir()
+	srcdata := []byte("new data!")
+	destdata := []byte("old data!")
+
+	exSrc := filepath.Join(src, "existing")
+	exDest := filepath.Join(dest, "existing")
+
+	err := ioutil.WriteFile(exSrc, srcdata, 0644)
+	c.Assert(err, gc.IsNil)
+	err = ioutil.WriteFile(exDest, destdata, 0644)
+	c.Assert(err, gc.IsNil)
+
+	err = upgrades.CopyFile(exDest, exSrc)
+	c.Assert(err, gc.IsNil)
+
+	// assert we haven't changed the destination
+	b, err := ioutil.ReadFile(exDest)
+
+	c.Assert(err, gc.IsNil)
+	// convert to string because we'll get a better failure message
+	c.Assert(string(b), gc.Equals, string(destdata))
+}
+
+func (s *steps124Suite) TestMoveSyslogConfigDefault(c *gc.C) {
+	logdir := c.MkDir()
+	datadir := c.MkDir()
+	data := []byte("data!")
+	files := []string{
+		"ca-cert.pem",
+		"rsyslog-cert.pem",
+		"rsyslog-key.pem",
+		"logrotate.conf",
+		"logrotate.run",
+	}
+	for _, f := range files {
+		err := ioutil.WriteFile(filepath.Join(logdir, f), data, 0644)
+		c.Assert(err, gc.IsNil)
+	}
+
+	ctx := fakeContext{cfg: fakeConfig{logdir: logdir, datadir: datadir}}
+	err := upgrades.MoveSyslogConfig(ctx)
+	c.Assert(err, gc.IsNil)
+
+	for _, f := range files {
+		_, err := os.Stat(filepath.Join(datadir, f))
+		c.Assert(err, gc.IsNil)
+		_, err = os.Stat(filepath.Join(logdir, f))
+		c.Assert(err, jc.Satisfies, os.IsNotExist)
+	}
+}
+
+func (s *steps124Suite) TestMoveSyslogConfig(c *gc.C) {
+	logdir := c.MkDir()
+	datadir := c.MkDir()
+	data := []byte("data!")
+	files := []string{
+		"logrotate.conf",
+		"logrotate.run",
+	}
+
+	// ensure that we don't overwrite an existing file in datadir, and don't
+	// error out if one of the files exists in datadir but not logdir.
+
+	err := ioutil.WriteFile(filepath.Join(logdir, "logrotate.conf"), data, 0644)
+	c.Assert(err, gc.IsNil)
+
+	err = ioutil.WriteFile(filepath.Join(datadir, "logrotate.run"), data, 0644)
+	c.Assert(err, gc.IsNil)
+
+	differentData := []byte("different")
+	existing := filepath.Join(datadir, "logrotate.conf")
+	err = ioutil.WriteFile(existing, differentData, 0644)
+	c.Assert(err, gc.IsNil)
+
+	ctx := fakeContext{cfg: fakeConfig{logdir: logdir, datadir: datadir}}
+	err = upgrades.MoveSyslogConfig(ctx)
+	c.Assert(err, gc.IsNil)
+
+	for _, f := range files {
+		_, err := os.Stat(filepath.Join(datadir, f))
+		c.Assert(err, gc.IsNil)
+		_, err = os.Stat(filepath.Join(logdir, f))
+		c.Assert(err, jc.Satisfies, os.IsNotExist)
+	}
+
+	b, err := ioutil.ReadFile(existing)
+	c.Assert(err, gc.IsNil)
+	// convert to string because we'll get a better failure message
+	c.Assert(string(b), gc.Not(gc.Equals), string(existing))
+
+}
+
+type fakeContext struct {
+	upgrades.Context
+	cfg fakeConfig
+}
+
+func (f fakeContext) AgentConfig() agent.ConfigSetter {
+	return f.cfg
+}
+
+type fakeConfig struct {
+	agent.ConfigSetter
+	logdir  string
+	datadir string
+}
+
+func (f fakeConfig) LogDir() string {
+	return f.logdir
+}
+
+func (f fakeConfig) DataDir() string {
+	return f.datadir
+}

upgrades/upgrade_test.go

@@ -671,7 +671,7 @@ func (s *upgradeSuite) TestStateUpgradeOperationsVersions(c *gc.C) {
 
 func (s *upgradeSuite) TestUpgradeOperationsVersions(c *gc.C) {
 	versions := extractUpgradeVersions(c, (*upgrades.UpgradeOperations)())
-	c.Assert(versions, gc.DeepEquals, []string{"1.18.0", "1.22.0", "1.23.0"})
+	c.Assert(versions, gc.DeepEquals, []string{"1.18.0", "1.22.0", "1.23.0", "1.24.0"})
 }
 
 func extractUpgradeVersions(c *gc.C, ops []upgrades.Operation) []string {