Merge lp:~axwalk/juju-core/lp1306902-ha-upgrade-take2 into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

HA: upgrade upstart and initiate replset

The machine agent will now upgrade the
Mongo upstart script and initiate the
replicaset when upgrading from a version
prior to 1.19.0.

Tested upgrading 1.18.1 to 1.19.1 with and
without HA enabled; tested straight bootstrap
to 1.19.1 with and without HA enabled.

(Supersedes https://codereview.appspot.com/88350043/)

https://codereview.appspot.com/88790043/

HA: upgrade upstart and initiate replset

The machine agent will now upgrade the
Mongo upstart script and initiate the
replicaset when upgrading from a version
prior to 1.19.0.

Tested upgrading 1.18.1 to 1.19.1 with and
without HA enabled; tested straight bootstrap
to 1.19.1 with and without HA enabled.

(Supersedes https://codereview.appspot.com/88350043/)

https://codereview.appspot.com/88790043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+216245_code.launchpad.net,

Message:
Please take a look.

Description:
HA: upgrade upstart and initiate replset

The machine agent will now upgrade the
Mongo upstart script and initiate the
replicaset when upgrading from a version
prior to 1.19.0.

Tested upgrading 1.18.1 to 1.19.1 with and
without HA enabled; tested straight bootstrap
to 1.19.1 with and without HA enabled.

(Supersedes https://codereview.appspot.com/88350043/)

https://code.launchpad.net/~axwalk/juju-core/lp1306902-ha-upgrade-take2/+merge/216245

(do not edit description out of merge proposal)

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

Affected files (+153, -66 lines):
   A [revision details]
   M agent/format-1.18.go
   M agent/mongo/mongo.go
   M cmd/jujud/bootstrap.go
   M cmd/jujud/machine.go
   M cmd/jujud/machine_test.go
   M worker/peergrouper/desired_test.go
   M worker/peergrouper/initiate.go
   M worker/peergrouper/initiate_test.go
   A worker/peergrouper/suite_test.go

[Roger Peppe (rogpeppe)]

LGTM with two minor suggestions.

https://codereview.appspot.com/88790043/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/88790043/diff/1/cmd/jujud/machine.go#newcode438
cmd/jujud/machine.go:438: func (a *MachineAgent)
ensureMongoServer(agentConfig agent.Config) error {
can we put this after StateWorker, so functions read generally from
higher level to lower level?

https://codereview.appspot.com/88790043/diff/1/cmd/jujud/machine.go#newcode468
cmd/jujud/machine.go:468: }
perhaps refresh agentConfig here?

(I actually think that ChangeConfig should probably always return the
new version of the agent configuration)

https://codereview.appspot.com/88790043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/88790043/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/88790043/diff/1/cmd/jujud/machine.go#newcode438
cmd/jujud/machine.go:438: func (a *MachineAgent)
ensureMongoServer(agentConfig agent.Config) error {
On 2014/04/17 18:20:23, rog wrote:
> can we put this after StateWorker, so functions read generally from
higher level
> to lower level?

Done.

https://codereview.appspot.com/88790043/diff/1/cmd/jujud/machine.go#newcode468
cmd/jujud/machine.go:468: }
On 2014/04/17 18:20:23, rog wrote:
> perhaps refresh agentConfig here?

> (I actually think that ChangeConfig should probably always return the
new
> version of the agent configuration)

Done.

https://codereview.appspot.com/88790043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/88790043/

=== modified file 'agent/format-1.18.go'
--- agent/format-1.18.go	2014-04-11 17:51:58 +0000
+++ agent/format-1.18.go	2014-04-18 04:41:05 +0000
@@ -46,6 +46,7 @@
 	StateServerKey  string `yaml:",omitempty"`
 	APIPort         int    `yaml:",omitempty"`
 	StatePort       int    `yaml:",omitempty"`
+	SharedSecret    string `yaml:",omitempty"`
 }
 
 func init() {
@@ -99,10 +100,11 @@
 	}
 	if len(format.StateServerKey) != 0 {
 		config.servingInfo = &params.StateServingInfo{
-			Cert:       format.StateServerCert,
-			PrivateKey: format.StateServerKey,
-			APIPort:    format.APIPort,
-			StatePort:  format.StatePort,
+			Cert:         format.StateServerCert,
+			PrivateKey:   format.StateServerKey,
+			APIPort:      format.APIPort,
+			StatePort:    format.StatePort,
+			SharedSecret: format.SharedSecret,
 		}
 		// There's a private key, then we need the state port,
 		// which wasn't always in the  1.18 format. If it's not present
@@ -144,6 +146,7 @@
 		format.StateServerKey = config.servingInfo.PrivateKey
 		format.APIPort = config.servingInfo.APIPort
 		format.StatePort = config.servingInfo.StatePort
+		format.SharedSecret = config.servingInfo.SharedSecret
 	}
 	if config.stateDetails != nil {
 		format.StateAddresses = config.stateDetails.addresses

=== modified file 'agent/mongo/mongo.go'
--- agent/mongo/mongo.go	2014-04-17 12:33:55 +0000
+++ agent/mongo/mongo.go	2014-04-18 04:41:05 +0000
@@ -192,14 +192,11 @@
 	}
 	logMongoVersion(mongoPath)
 
-	// TODO(natefinch) 2014-04-12 https://launchpad.net/bugs/1306902
-	// remove this once we support upgrading to HA
-	if upstartConf.Installed() {
-		return nil
+	if err := upstartServiceStop(&upstartConf.Service); err != nil {
+		return fmt.Errorf("failed to stop mongo: %v", err)
 	}
-
 	if err := makeJournalDirs(dbDir); err != nil {
-		return fmt.Errorf("Error creating journal directories: %v", err)
+		return fmt.Errorf("error creating journal directories: %v", err)
 	}
 	return upstartConfInstall(upstartConf)
 }

=== modified file 'cmd/jujud/bootstrap.go'
--- cmd/jujud/bootstrap.go	2014-04-17 16:26:01 +0000
+++ cmd/jujud/bootstrap.go	2014-04-18 04:41:05 +0000
@@ -18,7 +18,6 @@
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/instance"
-	"launchpad.net/juju-core/provider"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api/params"
 	"launchpad.net/juju-core/worker/peergrouper"
@@ -171,14 +170,15 @@
 	dialInfo.Addrs = []string{
 		net.JoinHostPort("127.0.0.1", fmt.Sprint(servingInfo.StatePort)),
 	}
+
 	logger.Debugf("calling ensureMongoServer")
-	providerType := agentConfig.Value(agent.ProviderType)
-	withHA := providerType != provider.Local
+	withHA := shouldEnableHA(agentConfig)
 	err = ensureMongoServer(
 		agentConfig.DataDir(),
 		agentConfig.Value(agent.Namespace),
 		servingInfo,
-		withHA)
+		withHA,
+	)
 	if err != nil {
 		return err
 	}

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2014-04-17 17:30:48 +0000
+++ cmd/jujud/machine.go	2014-04-18 04:41:05 +0000
@@ -5,6 +5,7 @@
 
 import (
 	"fmt"
+	"net"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -397,76 +398,19 @@
 	return nil
 }
 
-func (a *MachineAgent) ensureMongoAdminUser(agentConfig agent.Config) (added bool, err error) {
-	stateInfo, ok1 := agentConfig.StateInfo()
-	servingInfo, ok2 := agentConfig.StateServingInfo()
-	if !ok1 || !ok2 {
-		return false, fmt.Errorf("no state serving info configuration")
-	}
-	dialInfo, err := state.DialInfo(stateInfo, state.DefaultDialOpts())
-	if err != nil {
-		return false, err
-	}
-	if len(dialInfo.Addrs) > 1 {
-		logger.Infof("more than one state server; admin user must exist")
-		return false, nil
-	}
-	return ensureMongoAdminUser(mongo.EnsureAdminUserParams{
-		DialInfo:  dialInfo,
-		Namespace: agentConfig.Value(agent.Namespace),
-		DataDir:   agentConfig.DataDir(),
-		Port:      servingInfo.StatePort,
-		User:      stateInfo.Tag,
-		Password:  stateInfo.Password,
-	})
-}
-
 // StateJobs returns a worker running all the workers that require
 // a *state.State connection.
 func (a *MachineAgent) StateWorker() (worker.Worker, error) {
 	agentConfig := a.CurrentConfig()
-
-	servingInfo, ok := agentConfig.StateServingInfo()
-	if !ok {
-		return nil, fmt.Errorf("state worker was started with no state serving info")
-	}
-	providerType := agentConfig.Value(agent.ProviderType)
-	namespace := agentConfig.Value(agent.Namespace)
-	withHA := providerType != provider.Local
-	err := ensureMongoServer(
-		agentConfig.DataDir(),
-		namespace,
-		servingInfo,
-		withHA,
-	)
-	if err != nil {
+	if err := a.ensureMongoServer(agentConfig); err != nil {
 		return nil, err
 	}
-
 	st, m, err := openState(agentConfig)
-	if errors.IsUnauthorized(err) {
-		// TODO(axw) remove this when we no longer need
-		// to upgrade from pre-HA-capable environments.
-		logger.Debugf("failed to open state, reattempt after ensuring admin user exists: %v", err)
-		added, ensureErr := a.ensureMongoAdminUser(agentConfig)
-		if ensureErr != nil {
-			err = ensureErr
-		}
-		if !added {
-			// No user added, so it's probably a genuine unauthorized error.
-			return nil, err
-		}
-		st, m, err = openState(agentConfig)
-	}
 	if err != nil {
 		return nil, err
 	}
 	reportOpenedState(st)
 
-	// TODO(rog) call maybeInitiateMongoServer to upgrade mongo
-	// from old environments. We'll need to acquire a non-localhost
-	// address for the current instance before we do.
-
 	singularStateConn := singularStateConn{st.MongoSession(), m}
 	runner := newRunner(connectionIsFatal(st), moreImportant)
 	singularRunner, err := newSingularRunner(runner, singularStateConn)
@@ -476,6 +420,7 @@
 
 	// Take advantage of special knowledge here in that we will only ever want
 	// the storage provider on one machine, and that is the "bootstrap" node.
+	providerType := agentConfig.Value(agent.ProviderType)
 	if (providerType == provider.Local || provider.IsManual(providerType)) && m.Id() == bootstrapMachineId {
 		a.startWorkerAfterUpgrade(runner, "local-storage", func() (worker.Worker, error) {
 			// TODO(axw) 2013-09-24 bug #1229507
@@ -540,6 +485,129 @@
 	return newCloseWorker(runner, st), nil
 }
 
+// ensureMongoServer ensures that mongo is installed and running,
+// and ready for opening a state connection.
+func (a *MachineAgent) ensureMongoServer(agentConfig agent.Config) error {
+	servingInfo, ok := agentConfig.StateServingInfo()
+	if !ok {
+		return fmt.Errorf("state worker was started with no state serving info")
+	}
+	namespace := agentConfig.Value(agent.Namespace)
+	withHA := shouldEnableHA(agentConfig)
+
+	// When upgrading from a pre-HA-capable environment,
+	// we must add machine-0 to the admin database and
+	// initiate its replicaset.
+	//
+	// TODO(axw) remove this when we no longer need
+	// to upgrade from pre-HA-capable environments.
+	var shouldInitiateMongoServer bool
+	var addrs []instance.Address
+	if isPreHAVersion(agentConfig.UpgradedToVersion()) {
+		_, err := a.ensureMongoAdminUser(agentConfig)
+		if err != nil {
+			return err
+		}
+		if servingInfo.SharedSecret == "" {
+			servingInfo.SharedSecret, err = mongo.GenerateSharedSecret()
+			if err != nil {
+				return err
+			}
+			if err = a.ChangeConfig(func(config agent.ConfigSetter) {
+				config.SetStateServingInfo(servingInfo)
+			}); err != nil {
+				return err
+			}
+			agentConfig = a.CurrentConfig()
+		}
+		st, m, err := openState(agentConfig)
+		if err != nil {
+			return err
+		}
+		if err := st.SetStateServingInfo(servingInfo); err != nil {
+			st.Close()
+			return fmt.Errorf("cannot set state serving info: %v", err)
+		}
+		st.Close()
+		addrs = m.Addresses()
+		shouldInitiateMongoServer = withHA
+	}
+
+	// ensureMongoServer installs/upgrades the upstart config as necessary.
+	if err := ensureMongoServer(
+		agentConfig.DataDir(),
+		namespace,
+		servingInfo,
+		withHA,
+	); err != nil {
+		return err
+	}
+	if !shouldInitiateMongoServer {
+		return nil
+	}
+
+	// Initiate the replicaset for upgraded environments.
+	//
+	// TODO(axw) remove this when we no longer need
+	// to upgrade from pre-HA-capable environments.
+	stateInfo, ok := agentConfig.StateInfo()
+	if !ok {
+		return fmt.Errorf("state worker was started with no state serving info")
+	}
+	dialInfo, err := state.DialInfo(stateInfo, state.DefaultDialOpts())
+	if err != nil {
+		return err
+	}
+	peerAddr := mongo.SelectPeerAddress(addrs)
+	if peerAddr == "" {
+		return fmt.Errorf("no appropriate peer address found in %q", addrs)
+	}
+	return maybeInitiateMongoServer(peergrouper.InitiateMongoParams{
+		DialInfo:       dialInfo,
+		MemberHostPort: net.JoinHostPort(peerAddr, fmt.Sprint(servingInfo.StatePort)),
+		User:           stateInfo.Tag,
+		Password:       stateInfo.Password,
+	})
+}
+
+func (a *MachineAgent) ensureMongoAdminUser(agentConfig agent.Config) (added bool, err error) {
+	stateInfo, ok1 := agentConfig.StateInfo()
+	servingInfo, ok2 := agentConfig.StateServingInfo()
+	if !ok1 || !ok2 {
+		return false, fmt.Errorf("no state serving info configuration")
+	}
+	dialInfo, err := state.DialInfo(stateInfo, state.DefaultDialOpts())
+	if err != nil {
+		return false, err
+	}
+	if len(dialInfo.Addrs) > 1 {
+		logger.Infof("more than one state server; admin user must exist")
+		return false, nil
+	}
+	return ensureMongoAdminUser(mongo.EnsureAdminUserParams{
+		DialInfo:  dialInfo,
+		Namespace: agentConfig.Value(agent.Namespace),
+		DataDir:   agentConfig.DataDir(),
+		Port:      servingInfo.StatePort,
+		User:      stateInfo.Tag,
+		Password:  stateInfo.Password,
+	})
+}
+
+func isPreHAVersion(v version.Number) bool {
+	return v.Compare(version.MustParse("1.19.0")) < 0
+}
+
+// shouldEnableHA reports whether HA should be enabled.
+//
+// Eventually this should always be true, and ideally
+// it should be true before 1.20 is released or we'll
+// have more upgrade scenarios on our hands.
+func shouldEnableHA(agentConfig agent.Config) bool {
+	providerType := agentConfig.Value(agent.ProviderType)
+	return providerType != provider.Local
+}
+
 func openState(agentConfig agent.Config) (_ *state.State, _ *state.Machine, err error) {
 	info, ok := agentConfig.StateInfo()
 	if !ok {

=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go	2014-04-17 17:30:48 +0000
+++ cmd/jujud/machine_test.go	2014-04-18 04:41:05 +0000
@@ -59,6 +59,7 @@
 	agentSuite
 	singularRecord *singularRunnerRecord
 	lxctesting.TestSuite
+	fakeEnsureMongo fakeEnsure
 }
 
 func (s *commonMachineSuite) SetUpSuite(c *gc.C) {
@@ -96,6 +97,10 @@
 	testing.PatchValue(&peergrouperNew, func(st *state.State) (worker.Worker, error) {
 		return newDummyWorker(), nil
 	})
+
+	s.fakeEnsureMongo = fakeEnsure{}
+	s.PatchValue(&ensureMongoServer, s.fakeEnsureMongo.fakeEnsureMongo)
+	s.PatchValue(&maybeInitiateMongoServer, s.fakeEnsureMongo.fakeInitiateMongo)
 }
 
 func fakeCmd(path string) {
@@ -123,6 +128,10 @@
 	inst, md := jujutesting.AssertStartInstance(c, s.Conn.Environ, m.Id())
 	c.Assert(m.SetProvisioned(inst.Id(), state.BootstrapNonce, md), gc.IsNil)
 
+	// Add an address for the tests in case the maybeInitiateMongoServer
+	// codepath is exercised.
+	s.setFakeMachineAddresses(c, m)
+
 	// Set up the new machine.
 	err = m.SetAgentVersion(vers)
 	c.Assert(err, gc.IsNil)
@@ -327,7 +336,7 @@
 	return ctx, func() { newDeployContext = orig }
 }
 
-func (s *MachineSuite) setFakeMachineAddresses(c *gc.C, machine *state.Machine) {
+func (s *commonMachineSuite) setFakeMachineAddresses(c *gc.C, machine *state.Machine) {
 	addrs := []instance.Address{
 		instance.NewAddress("0.1.2.3", instance.NetworkUnknown),
 	}
@@ -347,7 +356,6 @@
 	usefulVersion.Series = "quantal" // to match the charm created below
 	envtesting.AssertUploadFakeToolsVersions(c, s.Conn.Environ.Storage(), usefulVersion)
 	m, _, _ := s.primeAgent(c, version.Current, state.JobManageEnviron)
-	s.setFakeMachineAddresses(c, m)
 	op := make(chan dummy.Operation, 200)
 	dummy.Listen(op)
 
@@ -405,7 +413,6 @@
 	usefulVersion.Series = "quantal" // to match the charm created below
 	envtesting.AssertUploadFakeToolsVersions(c, s.Conn.Environ.Storage(), usefulVersion)
 	m, _, _ := s.primeAgent(c, version.Current, state.JobManageEnviron)
-	s.setFakeMachineAddresses(c, m)
 	a := s.newAgent(c, m)
 	defer a.Stop()
 	go func() {
@@ -469,7 +476,6 @@
 	usefulVersion.Series = "quantal"
 	envtesting.AssertUploadFakeToolsVersions(c, s.Conn.Environ.Storage(), usefulVersion)
 	m, _, _ := s.primeAgent(c, version.Current, state.JobManageEnviron)
-	s.setFakeMachineAddresses(c, m)
 	calledChan := make(chan struct{}, 1)
 	s.PatchValue(&useMultipleCPUs, func() { calledChan <- struct{}{} })
 	// Now, start the agent, and observe that a JobManageEnviron agent
@@ -489,7 +495,6 @@
 	c.Check(a.Stop(), gc.IsNil)
 	// However, an agent that just JobHostUnits doesn't call UseMultipleCPUs
 	m2, _, _ := s.primeAgent(c, version.Current, state.JobHostUnits)
-	s.setFakeMachineAddresses(c, m2)
 	a2 := s.newAgent(c, m2)
 	defer a2.Stop()
 	go func() {
@@ -923,9 +928,12 @@
 	c.Fatalf("timeout while waiting for agent config to change")
 }
 
-func (s *MachineSuite) TestMachineAgentEnsureAdminUser(c *gc.C) {
-	m, _, _ := s.primeAgent(c, version.Current, state.JobManageEnviron)
-	err := s.State.MongoSession().DB("admin").RemoveUser(m.Tag())
+func (s *MachineSuite) TestMachineAgentUpgradeMongo(c *gc.C) {
+	m, agentConfig, _ := s.primeAgent(c, version.Current, state.JobManageEnviron)
+	agentConfig.SetUpgradedToVersion(version.MustParse("1.18.0"))
+	err := agentConfig.Write()
+	c.Assert(err, gc.IsNil)
+	err = s.State.MongoSession().DB("admin").RemoveUser(m.Tag())
 	c.Assert(err, gc.IsNil)
 
 	s.PatchValue(&ensureMongoAdminUser, func(p mongo.EnsureAdminUserParams) (bool, error) {
@@ -954,6 +962,8 @@
 		c.Fatalf("state not opened")
 	}
 	s.waitStopped(c, state.JobManageEnviron, a, done)
+	c.Assert(s.fakeEnsureMongo.ensureCount, gc.Equals, 1)
+	c.Assert(s.fakeEnsureMongo.initiateCount, gc.Equals, 1)
 }
 
 // MachineWithCharmsSuite provides infrastructure for tests which need to

=== modified file 'worker/peergrouper/desired_test.go'
--- worker/peergrouper/desired_test.go	2014-04-17 16:27:08 +0000
+++ worker/peergrouper/desired_test.go	2014-04-18 04:41:05 +0000
@@ -8,21 +8,15 @@
 	"sort"
 	"strconv"
 	"strings"
-	stdtesting "testing"
 
 	jc "github.com/juju/testing/checkers"
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/replicaset"
-	coretesting "launchpad.net/juju-core/testing"
 	"launchpad.net/juju-core/testing/testbase"
 )
 
-func TestPackage(t *stdtesting.T) {
-	coretesting.MgoTestPackage(t)
-}
-
 type desiredPeerGroupSuite struct {
 	testbase.LoggingSuite
 }

=== modified file 'worker/peergrouper/initiate.go'
--- worker/peergrouper/initiate.go	2014-04-15 16:37:08 +0000
+++ worker/peergrouper/initiate.go	2014-04-18 04:41:05 +0000
@@ -39,20 +39,20 @@
 		return nil
 	}
 	p.DialInfo.Direct = true
+
+	// TODO(rog) remove this code when we no longer need to upgrade
+	// from pre-HA-capable environments.
+	if p.User != "" {
+		p.DialInfo.Username = p.User
+		p.DialInfo.Password = p.Password
+	}
+
 	session, err := mgo.DialWithInfo(p.DialInfo)
 	if err != nil {
 		return fmt.Errorf("can't dial mongo to initiate replicaset: %v", err)
 	}
 	defer session.Close()
 
-	// TODO(rog) remove this code when we no longer need to upgrade
-	// from pre-HA-capable environments.
-	if p.User != "" {
-		err := session.DB("admin").Login(p.User, p.Password)
-		if err != nil {
-			logger.Errorf("cannot login to admin db as %q, password %q, falling back: %v", p.User, p.Password, err)
-		}
-	}
 	cfg, err := replicaset.CurrentConfig(session)
 	if err == nil && len(cfg.Members) > 0 {
 		logger.Infof("replica set configuration already found: %#v", cfg)

=== modified file 'worker/peergrouper/initiate_test.go'
--- worker/peergrouper/initiate_test.go	2014-04-14 19:23:22 +0000
+++ worker/peergrouper/initiate_test.go	2014-04-18 04:41:05 +0000
@@ -25,6 +25,7 @@
 	inst := &coretesting.MgoInstance{Params: []string{"--replSet", "juju"}}
 	err = inst.Start(true)
 	c.Assert(err, gc.IsNil)
+	defer inst.Destroy()
 
 	info := inst.DialInfo()
 	args := peergrouper.InitiateMongoParams{

=== added file 'worker/peergrouper/suite_test.go'
--- worker/peergrouper/suite_test.go	1970-01-01 00:00:00 +0000
+++ worker/peergrouper/suite_test.go	2014-04-18 04:41:05 +0000
@@ -0,0 +1,14 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package peergrouper_test
+
+import (
+	stdtesting "testing"
+
+	"launchpad.net/juju-core/testing"
+)
+
+func TestPackage(t *stdtesting.T) {
+	testing.MgoTestPackage(t)
+}