Merge lp:~thumper/juju-core/ubuntu-profile-upgrade-step into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Add upgrade job for ~ubuntu/.profile

The ubuntu user's .profile is updated to source the
~ubuntu/.juju-profile to get the proxy information.

The .juju-profile is written out by the machine agent
as it starts up if it is missing, but the .profile
needs to happen for any machines that were created by
older agents.

https://codereview.appspot.com/78660043/

Add upgrade job for ~ubuntu/.profile

The ubuntu user's .profile is updated to source the
~ubuntu/.juju-profile to get the proxy information.

The .juju-profile is written out by the machine agent
as it starts up if it is missing, but the .profile
needs to happen for any machines that were created by
older agents.

https://codereview.appspot.com/78660043/

[Tim Penhey (thumper)]

Reviewers: mp+212083_code.launchpad.net,

Message:
Please take a look.

Description:
Add upgrade job for ~ubuntu/.profile

The ubuntu user's .profile is updated to source the
~ubuntu/.juju-profile to get the proxy information.

The .juju-profile is written out by the machine agent
as it starts up if it is missing, but the .profile
needs to happen for any machines that were created by
older agents.

https://code.launchpad.net/~thumper/juju-core/ubuntu-profile-upgrade-step/+merge/212083

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/78660043/

Affected files (+134, -7 lines):
   A [revision details]
   A upgrades/dotprofile.go
   A upgrades/dotprofile_test.go
   M upgrades/export_test.go
   M upgrades/steps118.go

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/78660043/

[Ian Booth (wallyworld)]

LGTM. I wonder if we should to grep for more than just ".juju-profile",
but I guess that is all we need.

https://codereview.appspot.com/78660043/

[Tim Penhey (thumper)]

On 2014/03/21 04:56:00, wallyworld wrote:
> LGTM. I wonder if we should to grep for more than just
".juju-profile", but I
> guess that is all we need.

Thanks. The only thing that should have put .juju-profile into the
/home/ubuntu/.profile should have been us.

This is exactly what the cloud-init script does too.

https://codereview.appspot.com/78660043/

=== added file 'upgrades/dotprofile.go'
--- upgrades/dotprofile.go	1970-01-01 00:00:00 +0000
+++ upgrades/dotprofile.go	2014-03-21 04:47:53 +0000
@@ -0,0 +1,37 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package upgrades
+
+import (
+	"fmt"
+
+	"launchpad.net/juju-core/utils/exec"
+)
+
+// As of the middle of the 1.17 cycle, the proxy settings are written out to
+// /home/ubuntu/.juju-proxy both by cloud-init and the machine environ worker.
+// An older version of juju that has been upgraded will get the proxy settings
+// written out to the .juju-proxy file, but the .profile for the ubuntu user
+// wouldn't have been updated to source this file.
+//
+// This upgrade step is to add the line to source the file if it is missing
+// from the file.
+func ensureUbuntuDotProfileSourcesProxyFile(context Context) error {
+	// We look to see if the proxy line is there already as the manual
+	// provider may have had it aleady. The ubuntu user may not exist
+	// (local provider only).
+	command := fmt.Sprintf(""+
+		`([ ! -e %s/.profile ] || grep -q '.juju-proxy' %s/.profile) || `+
+		`printf '\n# Added by juju\n[ -f "$HOME/.juju-proxy" ] && . "$HOME/.juju-proxy"\n' >> %s/.profile`,
+		ubuntuHome, ubuntuHome, ubuntuHome)
+	logger.Tracef("command: %s", command)
+	result, err := exec.RunCommands(exec.RunParams{
+		Commands: command,
+	})
+	if err != nil {
+		return err
+	}
+	logger.Tracef("stdout: %s", result.Stdout)
+	return nil
+}

=== added file 'upgrades/dotprofile_test.go'
--- upgrades/dotprofile_test.go	1970-01-01 00:00:00 +0000
+++ upgrades/dotprofile_test.go	2014-03-21 04:47:53 +0000
@@ -0,0 +1,82 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package upgrades_test
+
+import (
+	"io/ioutil"
+	"path"
+
+	"github.com/juju/loggo"
+	jc "github.com/juju/testing/checkers"
+	gc "launchpad.net/gocheck"
+
+	"launchpad.net/juju-core/testing"
+	"launchpad.net/juju-core/upgrades"
+)
+
+type ensureDotProfileSuite struct {
+	testing.FakeHomeSuite
+	home string
+	ctx  upgrades.Context
+}
+
+var _ = gc.Suite(&ensureDotProfileSuite{})
+
+func (s *ensureDotProfileSuite) SetUpTest(c *gc.C) {
+	s.FakeHomeSuite.SetUpTest(c)
+
+	loggo.GetLogger("juju.upgrade").SetLogLevel(loggo.TRACE)
+
+	s.home = c.MkDir()
+	s.PatchValue(upgrades.UbuntuHome, s.home)
+	s.ctx = &mockContext{}
+}
+
+const expectedLine = `
+# Added by juju
+[ -f "$HOME/.juju-proxy" ] && . "$HOME/.juju-proxy"
+`
+
+func (s *ensureDotProfileSuite) writeDotProfile(c *gc.C, content string) {
+	dotProfile := path.Join(s.home, ".profile")
+	err := ioutil.WriteFile(dotProfile, []byte(content), 0644)
+	c.Assert(err, gc.IsNil)
+}
+
+func (s *ensureDotProfileSuite) assertProfile(c *gc.C, content string) {
+	dotProfile := path.Join(s.home, ".profile")
+	data, err := ioutil.ReadFile(dotProfile)
+	c.Assert(err, gc.IsNil)
+	c.Assert(string(data), gc.Equals, content)
+}
+
+func (s *ensureDotProfileSuite) TestSourceAdded(c *gc.C) {
+	s.writeDotProfile(c, "")
+	err := upgrades.EnsureUbuntuDotProfileSourcesProxyFile(s.ctx)
+	c.Assert(err, gc.IsNil)
+	s.assertProfile(c, expectedLine)
+}
+
+func (s *ensureDotProfileSuite) TestIdempotent(c *gc.C) {
+	s.writeDotProfile(c, "")
+	err := upgrades.EnsureUbuntuDotProfileSourcesProxyFile(s.ctx)
+	c.Assert(err, gc.IsNil)
+	err = upgrades.EnsureUbuntuDotProfileSourcesProxyFile(s.ctx)
+	c.Assert(err, gc.IsNil)
+	s.assertProfile(c, expectedLine)
+}
+
+func (s *ensureDotProfileSuite) TestProfileUntouchedIfJujuProxyInSource(c *gc.C) {
+	content := "source .juju-proxy\n"
+	s.writeDotProfile(c, content)
+	err := upgrades.EnsureUbuntuDotProfileSourcesProxyFile(s.ctx)
+	c.Assert(err, gc.IsNil)
+	s.assertProfile(c, content)
+}
+
+func (s *ensureDotProfileSuite) TestSkippedIfDotProfileDoesntExist(c *gc.C) {
+	err := upgrades.EnsureUbuntuDotProfileSourcesProxyFile(s.ctx)
+	c.Assert(err, gc.IsNil)
+	c.Assert(path.Join(s.home, ".profile"), jc.DoesNotExist)
+}

=== modified file 'upgrades/export_test.go'
--- upgrades/export_test.go	2014-03-18 14:50:47 +0000
+++ upgrades/export_test.go	2014-03-21 04:47:53 +0000
@@ -13,10 +13,11 @@
 	IsLocalEnviron = &isLocalEnviron
 
 	// 118 upgrade functions
-	StepsFor118                          = stepsFor118
-	EnsureLockDirExistsAndUbuntuWritable = ensureLockDirExistsAndUbuntuWritable
-	EnsureSystemSSHKey                   = ensureSystemSSHKey
-	UpdateRsyslogPort                    = updateRsyslogPort
-	ProcessDeprecatedEnvSettings         = processDeprecatedEnvSettings
-	MigrateLocalProviderAgentConfig      = migrateLocalProviderAgentConfig
+	StepsFor118                            = stepsFor118
+	EnsureLockDirExistsAndUbuntuWritable   = ensureLockDirExistsAndUbuntuWritable
+	EnsureSystemSSHKey                     = ensureSystemSSHKey
+	EnsureUbuntuDotProfileSourcesProxyFile = ensureUbuntuDotProfileSourcesProxyFile
+	UpdateRsyslogPort                      = updateRsyslogPort
+	ProcessDeprecatedEnvSettings           = processDeprecatedEnvSettings
+	MigrateLocalProviderAgentConfig        = migrateLocalProviderAgentConfig
 )

=== modified file 'upgrades/steps118.go'
--- upgrades/steps118.go	2014-03-18 09:21:53 +0000
+++ upgrades/steps118.go	2014-03-21 04:47:53 +0000
@@ -8,7 +8,7 @@
 	return []Step{
 		&upgradeStep{
 			description: "make $DATADIR/locks owned by ubuntu:ubuntu",
-			targets:     []Target{HostMachine},
+			targets:     []Target{AllMachines},
 			run:         ensureLockDirExistsAndUbuntuWritable,
 		},
 		&upgradeStep{
@@ -36,5 +36,10 @@
 			targets:     []Target{StateServer},
 			run:         migrateLocalProviderAgentConfig,
 		},
+		&upgradeStep{
+			description: "make /home/ubuntu/.profile source .juju-proxy file",
+			targets:     []Target{AllMachines},
+			run:         ensureUbuntuDotProfileSourcesProxyFile,
+		},
 	}
 }

=== modified file 'upgrades/steps118_test.go'
--- upgrades/steps118_test.go	2014-03-13 21:58:22 +0000
+++ upgrades/steps118_test.go	2014-03-21 04:47:53 +0000
@@ -23,6 +23,7 @@
 	"install rsyslog-gnutls",
 	"remove deprecated environment config settings",
 	"migrate local provider agent config",
+	"make /home/ubuntu/.profile source .juju-proxy file",
 }
 
 func (s *steps118Suite) TestUpgradeOperationsContent(c *gc.C) {