From 19b9c4ede26a328f0012178b2501853667c97abf Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Mon, 9 Mar 2015 11:15:41 -0400 Subject: [PATCH 01/33] --to changes --- cmd/juju/ensureavailability.go | 8 +++- state/addmachine.go | 71 ++++++++++++++++++++++++++++------ 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/cmd/juju/ensureavailability.go b/cmd/juju/ensureavailability.go index 83500d4fe9b..f3c9923a5df 100644 --- a/cmd/juju/ensureavailability.go +++ b/cmd/juju/ensureavailability.go @@ -145,7 +145,13 @@ func (c *EnsureAvailabilityCommand) Init(args []string) error { placementSpecs := strings.Split(c.PlacementSpec, ",") c.Placement = make([]string, len(placementSpecs)) for i, spec := range placementSpecs { - _, err := instance.ParsePlacement(strings.TrimSpace(spec)) + p, err := instance.ParsePlacement(strings.TrimSpace(spec)) + fmt.Println(p, err) + if err == nil && p.Scope == instance.MachineScope { + // targeting machines is ok + c.Placement[i] = p.String() + continue + } if err != instance.ErrPlacementScopeMissing { // We only support unscoped placement directives. return fmt.Errorf("unsupported ensure-availability placement directive %q", spec) diff --git a/state/addmachine.go b/state/addmachine.go index 855e7410b26..30a71e91d08 100644 --- a/state/addmachine.go +++ b/state/addmachine.go @@ -618,7 +618,7 @@ func (st *State) EnsureAvailability( return nil, errors.New("cannot reduce state server count") } - intent, err := st.ensureAvailabilityIntentions(currentInfo) + intent, err := st.ensureAvailabilityIntentions(currentInfo, placement) if err != nil { return nil, err } @@ -636,11 +636,14 @@ func (st *State) EnsureAvailability( intent.promote = intent.promote[:n] } voteCount += len(intent.promote) - intent.newCount = desiredStateServerCount - voteCount - logger.Infof("%d new machines; promoting %v", intent.newCount, intent.promote) + intent.newCount = desiredStateServerCount - voteCount - len(intent.convert) + if intent.newCount < 0 { + intent.newCount = 0 + } + logger.Infof("%d new machines; promoting %v; converting %v", intent.newCount, intent.promote, intent.convert) var ops []txn.Op - ops, change, err = st.ensureAvailabilityIntentionOps(intent, currentInfo, cons, series, placement) + ops, change, err = st.ensureAvailabilityIntentionOps(intent, currentInfo, cons, series) return ops, err } if err := st.run(buildTxn); err != nil { @@ -657,6 +660,7 @@ type StateServersChanges struct { Maintained []string Promoted []string Demoted []string + Converted []string } // ensureAvailabilityIntentionOps returns operations to fulfil the desired intent. @@ -665,7 +669,6 @@ func (st *State) ensureAvailabilityIntentionOps( currentInfo *StateServerInfo, cons constraints.Value, series string, - placement []string, ) ([]txn.Op, StateServersChanges, error) { var ops []txn.Op var change StateServersChanges @@ -677,16 +680,20 @@ func (st *State) ensureAvailabilityIntentionOps( ops = append(ops, demoteStateServerOps(m)...) change.Demoted = append(change.Demoted, m.doc.Id) } + for _, m := range intent.convert { + ops = append(ops, convertStateServerOps(m)...) + change.Converted = append(change.Converted, m.doc.Id) + } // Use any placement directives that have been provided // when adding new machines, until the directives have // been all used up. Set up a helper function to do the // work required. placementCount := 0 getPlacement := func() string { - if placementCount >= len(placement) { + if placementCount >= len(intent.placement) { return "" } - result := placement[placementCount] + result := intent.placement[placementCount] placementCount++ return result } @@ -744,8 +751,10 @@ var stateServerAvailable = func(m *Machine) (bool, error) { } type ensureAvailabilityIntent struct { - newCount int - promote, maintain, demote, remove []*Machine + newCount int + placement []string + + promote, maintain, demote, remove, convert []*Machine } // ensureAvailabilityIntentions returns what we would like @@ -754,8 +763,29 @@ type ensureAvailabilityIntent struct { // demoting unavailable, voting machines; // removing unavailable, non-voting, non-vote-holding machines; // gathering available, non-voting machines that may be promoted; -func (st *State) ensureAvailabilityIntentions(info *StateServerInfo) (*ensureAvailabilityIntent, error) { +func (st *State) ensureAvailabilityIntentions(info *StateServerInfo, placement []string) (*ensureAvailabilityIntent, error) { var intent ensureAvailabilityIntent + for _, s := range placement { + p, err := instance.ParsePlacement(s) + if err == instance.ErrPlacementScopeMissing { + intent.placement = append(intent.placement, s) + continue + } + if err == nil && p.Scope == instance.MachineScope { + m, err := st.Machine(p.Directive) + if err != nil { + return nil, errors.Annotatef(err, "can't find machine for placement directive %q", s) + } + if m.IsManager() { + return nil, errors.Errorf("machine for placement directive %q is already a state server", s) + } + intent.convert = append(intent.convert, m) + intent.placement = append(intent.placement, s) + continue + } + return nil, errors.Errorf("unsupported HA placement directive %q", s) + } + for _, mid := range info.MachineIds { m, err := st.Machine(mid) if err != nil { @@ -790,10 +820,29 @@ func (st *State) ensureAvailabilityIntentions(info *StateServerInfo) (*ensureAva intent.remove = append(intent.remove, m) } } - logger.Infof("initial intentions: promote %v; maintain %v; demote %v; remove %v", intent.promote, intent.maintain, intent.demote, intent.remove) + logger.Infof("initial intentions: promote %v; maintain %v; demote %v; remove %v; convert: %v", + intent.promote, intent.maintain, intent.demote, intent.remove, intent.convert) return &intent, nil } +func convertStateServerOps(m *Machine) []txn.Op { + return []txn.Op{{ + C: machinesC, + Id: m.doc.DocID, + Update: bson.D{ + {"$addToSet", bson.D{{"jobs", JobManageEnviron}}}, + {"$set", bson.D{{"novote", false}}}, + }, + }, { + C: stateServersC, + Id: environGlobalKey, + Update: bson.D{ + {"$addToSet", bson.D{{"votingmachineids", m.doc.Id}}}, + {"$addToSet", bson.D{{"machineids", m.doc.Id}}}, + }, + }} +} + func promoteStateServerOps(m *Machine) []txn.Op { return []txn.Op{{ C: machinesC, From 504859370a4099eed0aabf8224d67507a9cca704 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Fri, 13 Mar 2015 11:13:33 -0400 Subject: [PATCH 02/33] WIP --- apiserver/params/constants.go | 4 ++++ cmd/jujud/unit.go | 9 ++++++++- state/state.go | 1 + worker/reboot/reboot.go | 11 ++++++++--- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/apiserver/params/constants.go b/apiserver/params/constants.go index ed58d273af6..f8f77667122 100644 --- a/apiserver/params/constants.go +++ b/apiserver/params/constants.go @@ -19,6 +19,10 @@ const ( // happens when running inside a container, and a hook on the parent // machine requests a reboot ShouldShutdown RebootAction = "shutdown" + // ShouldRestart instructs a machine agent to restart jujud. This usually + // happens after an upgrade or conversion to state server using + // ensure-availability. + ShouldRestart RebootAction = "restart" ) // ResolvedMode describes the way state transition errors diff --git a/cmd/jujud/unit.go b/cmd/jujud/unit.go index 81ed41e99e5..25f8976b44b 100644 --- a/cmd/jujud/unit.go +++ b/cmd/jujud/unit.go @@ -25,6 +25,7 @@ import ( "github.com/juju/juju/version" "github.com/juju/juju/worker" "github.com/juju/juju/worker/apiaddressupdater" + "github.com/juju/juju/worker/converter" workerlogger "github.com/juju/juju/worker/logger" "github.com/juju/juju/worker/proxyupdater" "github.com/juju/juju/worker/rsyslog" @@ -177,7 +178,6 @@ func (a *UnitAgent) APIWorkers() (worker.Worker, error) { } return uniter.NewUniter(uniterFacade, unitTag, st.LeadershipManager(), dataDir, hookLock), nil }) - runner.StartWorker("apiaddressupdater", func() (worker.Worker, error) { uniterFacade, err := st.Uniter() if err != nil { @@ -185,6 +185,13 @@ func (a *UnitAgent) APIWorkers() (worker.Worker, error) { } return apiaddressupdater.NewAPIAddressUpdater(uniterFacade, a), nil }) + runner.StartWorker("converter", func() (worker.Worker, error) { + uniterFacade, err := st.Uniter() + if err != nil { + return nil, errors.Trace(err) + } + return converter.NewUnitConverter(uniterFacade, entity), nil + }) runner.StartWorker("rsyslog", func() (worker.Worker, error) { return cmdutil.NewRsyslogConfigWorker(st.Rsyslog(), agentConfig, rsyslog.RsyslogModeForwarding) }) diff --git a/state/state.go b/state/state.go index c779f3b6d93..4b8bbf4ff3f 100644 --- a/state/state.go +++ b/state/state.go @@ -56,6 +56,7 @@ const ( unitsC = "units" subnetsC = "subnets" ipaddressesC = "ipaddresses" + jobsC = "jobs" // actionsC and related collections store state of Actions that // have been enqueued. diff --git a/worker/reboot/reboot.go b/worker/reboot/reboot.go index b538c301622..3ab214fa41c 100644 --- a/worker/reboot/reboot.go +++ b/worker/reboot/reboot.go @@ -17,14 +17,16 @@ import ( var logger = loggo.GetLogger("juju.worker.reboot") const RebootMessage = "preparing for reboot" +const RestartMessage = "restarting juju" var _ worker.NotifyWatchHandler = (*Reboot)(nil) // The reboot worker listens for changes to the reboot flag and -// exists with worker.ErrRebootMachine if the machine should reboot or +// exists with worker.ErrTerminateAgent if only jujud should be restarts, +// with worker.ErrRebootMachine if the machine should reboot, or // with worker.ErrShutdownMachine if it should shutdown. This will be picked // up by the machine agent as a fatal error and will do the -// right thing (reboot or shutdown) +// right thing (restart, reboot, or shutdown) type Reboot struct { tomb tomb.Tomb st *reboot.State @@ -51,7 +53,7 @@ func (r *Reboot) checkForRebootState() error { return nil } - if r.machineLock.Message() == RebootMessage { + if r.machineLock.Message() == RebootMessage || r.machineLock.Message() == RestartMessage { // Not a lock held by the machne agent in order to reboot err = r.machineLock.BreakLock() if err != nil { @@ -87,6 +89,9 @@ func (r *Reboot) Handle() error { case params.ShouldShutdown: r.machineLock.Lock(RebootMessage) return worker.ErrShutdownMachine + case params.ShouldRestart: + r.machineLock.Lock(RestartMessage) + return worker.ErrTerminateAgent } return nil } From dd6dc654c71cd6e806d12c843791cc0e021a22bc Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Fri, 13 Mar 2015 11:13:41 -0400 Subject: [PATCH 03/33] WIP --- worker/converter/converter.go | 66 +++++++++++++++++++++++++++ worker/converter/converter_test.go | 72 ++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 worker/converter/converter.go create mode 100644 worker/converter/converter_test.go diff --git a/worker/converter/converter.go b/worker/converter/converter.go new file mode 100644 index 00000000000..5eb0177d71c --- /dev/null +++ b/worker/converter/converter.go @@ -0,0 +1,66 @@ +package converter + +import ( + "github.com/juju/errors" + "github.com/juju/loggo" + + "github.com/juju/juju/api/agent" + "github.com/juju/juju/api/base" + "github.com/juju/juju/api/watcher" + "github.com/juju/juju/worker" +) + +var logger = loggo.GetLogger("juju.worker.converter") + +var _ worker.NotifyWatchHandler = (*UnitConverter)(nil) + +type Environment struct { + facade base.FacadeCaller +} + +type UnitConverter struct { + entity *agent.Entity + converter Converter +} + +type Converter interface { + WatchAPIHostPorts() (watcher.NotifyWatcher, error) +} + +func NewUnitConverter(converter Converter, entity *agent.Entity) worker.Worker { + return worker.NewNotifyWorker(&UnitConverter{ + converter: converter, + entity: entity, + }) +} + +func (c *UnitConverter) checkForManageEnvironJob() bool { + logger.Infof("checking if unit has been converted") + for _, job := range c.entity.Jobs() { + if job.NeedsState() { + return true + } + } + return false +} + +func (c *UnitConverter) SetUp() (watcher.NotifyWatcher, error) { + logger.Debugf("converter worker setup") + if c.checkForManageEnvironJob() { + return nil, errors.Errorf("already a state server, cannot convert") + } + return c.converter.WatchAPIHostPorts() +} + +func (c *UnitConverter) Handle() error { + if c.checkForManageEnvironJob() { + logger.Debugf("we have been converted *sip kool-aid*") + return worker.ErrTerminateAgent + } + return nil +} + +func (r *UnitConverter) TearDown() error { + // nothing to teardown. + return nil +} diff --git a/worker/converter/converter_test.go b/worker/converter/converter_test.go new file mode 100644 index 00000000000..b3209dc59f6 --- /dev/null +++ b/worker/converter/converter_test.go @@ -0,0 +1,72 @@ +package converter_test + +import ( + "net" + stdtesting "testing" + + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + jujutesting "github.com/juju/juju/juju/testing" + "github.com/juju/juju/network" + "github.com/juju/juju/state" + coretesting "github.com/juju/juju/testing" + "github.com/juju/juju/worker/converter" +) + +func TestPackage(t *stdtesting.T) { + coretesting.MgoTestPackage(t) +} + +type UnitConverterSuite struct { + jujutesting.JujuConnSuite +} + +var _ = gc.Suite(&UnitConverterSuite{}) + +func (s *UnitConverterSuite) SetUpTest(c *gc.C) { + s.JujuConnSuite.SetUpTest(c) + err := s.State.SetAPIHostPorts(nil) + c.Assert(err, jc.ErrorIsNil) + // By default mock these to better isolate the test from the real machine. + s.PatchValue(&network.InterfaceByNameAddrs, func(string) ([]net.Addr, error) { + return nil, nil + }) + s.PatchValue(&network.LXCNetDefaultConfig, "") +} + +func (s *UnitConverterSuite) TestStartStop(c *gc.C) { + st, _ := s.OpenAPIAsNewMachine(c, state.JobHostUnits) + worker := converter.NewUnitConverter(st.Machiner(), &apiAddressSetter{}) + worker.Kill() + c.Assert(worker.Wait(), gc.IsNil) +} + +/* +func (s *converterSuite) TestWorkerCatchesConverterEvent(c *gc.C) { + wrk, err := converter.NewConverter(s.converterState, s.AgentConfigForTag(c, s.machine.Tag()), s.lock) + c.Assert(err, jc.ErrorIsNil) + err = s.converterState.RequestConverter() + c.Assert(err, jc.ErrorIsNil) + c.Assert(wrk.Wait(), gc.Equals, worker.ErrConverterMachine) +} + +func (s *converterSuite) TestContainerCatchesParentFlag(c *gc.C) { + wrk, err := converter.NewConverter(s.ctConverterState, s.AgentConfigForTag(c, s.ct.Tag()), s.lock) + c.Assert(err, jc.ErrorIsNil) + err = s.converterState.RequestConverter() + c.Assert(err, jc.ErrorIsNil) + c.Assert(wrk.Wait(), gc.Equals, worker.ErrShutdownMachine) +} + +func (s *converterSuite) TestCleanupIsDoneOnBoot(c *gc.C) { + s.lock.Lock(converter.ConverterMessage) + + wrk, err := converter.NewConverter(s.converterState, s.AgentConfigForTag(c, s.machine.Tag()), s.lock) + c.Assert(err, jc.ErrorIsNil) + wrk.Kill() + c.Assert(wrk.Wait(), gc.IsNil) + + c.Assert(s.lock.IsLocked(), jc.IsFalse) +} +*/ From a8fd79b099ec44adfb958d614954c0af5bb89e5b Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 17 Mar 2015 09:24:25 -0400 Subject: [PATCH 04/33] WIP --- api/converter/converter.go | 37 +++ api/state.go | 6 + apiserver/converter/converter.go | 71 ++++++ apiserver/converter/converter_test.go | 328 ++++++++++++++++++++++++++ apiserver/converter/suite_test.go | 14 ++ cmd/jujud/agent/machine.go | 7 + cmd/jujud/unit.go | 8 - state/watcher.go | 6 + worker/converter/converter.go | 69 +++--- worker/converter/converter_test.go | 85 +++---- 10 files changed, 536 insertions(+), 95 deletions(-) create mode 100644 api/converter/converter.go create mode 100644 apiserver/converter/converter.go create mode 100644 apiserver/converter/converter_test.go create mode 100644 apiserver/converter/suite_test.go diff --git a/api/converter/converter.go b/api/converter/converter.go new file mode 100644 index 00000000000..bf4fb126fc4 --- /dev/null +++ b/api/converter/converter.go @@ -0,0 +1,37 @@ +// Copyright 2014 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter + +import ( + "github.com/juju/juju/api/base" + "github.com/juju/juju/api/watcher" + "github.com/juju/juju/apiserver/params" +) + +const converterAPI = "Converter" + +// Converter provides common client-side API +// functions to call into apiserver.Converter +type State struct { + facade base.FacadeCaller +} + +// NewAPIAddresser returns a new APIAddresser that makes API calls +// using caller and the specified facade name. +func NewState(caller base.APICaller) *State { + return &State{base.NewFacadeCaller(caller, converterAPI)} +} + +// WatchAPIHostPorts watches the host/port addresses of the API servers. +func (c *State) WatchForJobsChanges(tag string) (watcher.NotifyWatcher, error) { + var result params.NotifyWatchResult + args := params.Entities{ + Entities: []params.Entity{{Tag: tag}}, + } + err := c.facade.FacadeCall("WatchForJobsChanges", args, &result) + if err != nil { + return nil, err + } + return watcher.NewNotifyWatcher(c.facade.RawAPICaller(), result), nil +} diff --git a/api/state.go b/api/state.go index dca26c4517d..ee553371313 100644 --- a/api/state.go +++ b/api/state.go @@ -13,6 +13,7 @@ import ( "github.com/juju/juju/api/agent" "github.com/juju/juju/api/base" "github.com/juju/juju/api/charmrevisionupdater" + "github.com/juju/juju/api/converter" "github.com/juju/juju/api/deployer" "github.com/juju/juju/api/diskformatter" "github.com/juju/juju/api/diskmanager" @@ -318,3 +319,8 @@ func (st *State) CharmRevisionUpdater() *charmrevisionupdater.State { func (st *State) Rsyslog() *rsyslog.State { return rsyslog.NewState(st) } + +// Converter returns access to the Converter API +func (st *State) Converter() *converter.State { + return converter.NewState(st) +} diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go new file mode 100644 index 00000000000..88ea156c0c2 --- /dev/null +++ b/apiserver/converter/converter.go @@ -0,0 +1,71 @@ +// Copyright 2012, 2013 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter + +import ( + "github.com/juju/errors" + "github.com/juju/loggo" + "github.com/juju/names" + + "github.com/juju/juju/apiserver/common" + "github.com/juju/juju/apiserver/params" + "github.com/juju/juju/state" + "github.com/juju/juju/state/watcher" +) + +var logger = loggo.GetLogger("juju.apiserver.converter") + +func init() { + common.RegisterStandardFacade("Converter", 0, NewConverterAPI) +} + +type ConverterAPI struct { + st *state.State + resources *common.Resources + authorizer common.Authorizer +} + +// NewUpgraderAPI creates a new server-side UpgraderAPI facade. +func NewConverterAPI( + st *state.State, + resources *common.Resources, + authorizer common.Authorizer, +) (*ConverterAPI, error) { + if !authorizer.AuthMachineAgent() { + return nil, common.ErrPerm + } + return &ConverterAPI{ + st: st, + resources: resources, + authorizer: authorizer, + }, nil +} + +func (c *ConverterAPI) WatchForJobsChanges(args params.Entities) (params.NotifyWatchResults, error) { + result := params.NotifyWatchResults{ + Results: make([]params.NotifyWatchResult, len(args.Entities)), + } + for i, agent := range args.Entities { + tag, err := names.ParseMachineTag(agent.Tag) + if err != nil { + return params.NotifyWatchResults{}, errors.Trace(err) + } + err = common.ErrPerm + if c.authorizer.AuthOwner(tag) { + watch := c.st.WatchForJobsChanges(tag) + // Consume the initial event. Technically, API + // calls to Watch 'transmit' the initial event + // in the Watch response. But NotifyWatchers + // have no state to transmit. + if _, ok := <-watch.Changes(); ok { + result.Results[i].NotifyWatcherId = c.resources.Register(watch) + err = nil + } else { + err = watcher.EnsureErr(watch) + } + } + result.Results[i].Error = common.ServerError(err) + } + return result, nil +} diff --git a/apiserver/converter/converter_test.go b/apiserver/converter/converter_test.go new file mode 100644 index 00000000000..af4b13d9a36 --- /dev/null +++ b/apiserver/converter/converter_test.go @@ -0,0 +1,328 @@ +// Copyright 2012, 2013 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter_test + +import ( + "fmt" + + "github.com/juju/errors" + "github.com/juju/names" + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + "github.com/juju/juju/apiserver/common" + "github.com/juju/juju/apiserver/params" + apiservertesting "github.com/juju/juju/apiserver/testing" + "github.com/juju/juju/apiserver/upgrader" + jujutesting "github.com/juju/juju/juju/testing" + "github.com/juju/juju/state" + statetesting "github.com/juju/juju/state/testing" + coretesting "github.com/juju/juju/testing" + "github.com/juju/juju/version" +) + +type upgraderSuite struct { + jujutesting.JujuConnSuite + + // These are raw State objects. Use them for setup and assertions, but + // should never be touched by the API calls themselves + rawMachine *state.Machine + apiMachine *state.Machine + upgrader *upgrader.UpgraderAPI + resources *common.Resources + authorizer apiservertesting.FakeAuthorizer +} + +var _ = gc.Suite(&upgraderSuite{}) + +func (s *upgraderSuite) SetUpTest(c *gc.C) { + s.JujuConnSuite.SetUpTest(c) + s.resources = common.NewResources() + s.AddCleanup(func(_ *gc.C) { s.resources.StopAll() }) + + // Create a machine to work with + var err error + // The first machine created is the only one allowed to + // JobManageEnviron + s.apiMachine, err = s.State.AddMachine("quantal", state.JobHostUnits, + state.JobManageEnviron) + c.Assert(err, jc.ErrorIsNil) + s.rawMachine, err = s.State.AddMachine("quantal", state.JobHostUnits) + c.Assert(err, jc.ErrorIsNil) + + // The default auth is as the machine agent + s.authorizer = apiservertesting.FakeAuthorizer{ + Tag: s.rawMachine.Tag(), + } + s.upgrader, err = upgrader.NewUpgraderAPI(s.State, s.resources, s.authorizer) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *upgraderSuite) TearDownTest(c *gc.C) { + if s.resources != nil { + s.resources.StopAll() + } + s.JujuConnSuite.TearDownTest(c) +} + +func (s *upgraderSuite) TestWatchAPIVersionNothing(c *gc.C) { + // Not an error to watch nothing + results, err := s.upgrader.WatchAPIVersion(params.Entities{}) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 0) +} + +func (s *upgraderSuite) TestWatchAPIVersion(c *gc.C) { + args := params.Entities{ + Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}, + } + results, err := s.upgrader.WatchAPIVersion(args) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 1) + c.Check(results.Results[0].NotifyWatcherId, gc.Not(gc.Equals), "") + c.Check(results.Results[0].Error, gc.IsNil) + resource := s.resources.Get(results.Results[0].NotifyWatcherId) + c.Check(resource, gc.NotNil) + + w := resource.(state.NotifyWatcher) + wc := statetesting.NewNotifyWatcherC(c, s.State, w) + wc.AssertNoChange() + + err = statetesting.SetAgentVersion(s.State, version.MustParse("3.4.567.8")) + c.Assert(err, jc.ErrorIsNil) + wc.AssertOneChange() + statetesting.AssertStop(c, w) + wc.AssertClosed() +} + +func (s *upgraderSuite) TestUpgraderAPIRefusesNonMachineAgent(c *gc.C) { + anAuthorizer := s.authorizer + anAuthorizer.Tag = names.NewUnitTag("ubuntu/1") + anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) + c.Check(err, gc.NotNil) + c.Check(anUpgrader, gc.IsNil) + c.Assert(err, gc.ErrorMatches, "permission denied") +} + +func (s *upgraderSuite) TestWatchAPIVersionRefusesWrongAgent(c *gc.C) { + // We are a machine agent, but not the one we are trying to track + anAuthorizer := s.authorizer + anAuthorizer.Tag = names.NewMachineTag("12354") + anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) + c.Check(err, jc.ErrorIsNil) + args := params.Entities{ + Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}, + } + results, err := anUpgrader.WatchAPIVersion(args) + // It is not an error to make the request, but the specific item is rejected + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 1) + c.Check(results.Results[0].NotifyWatcherId, gc.Equals, "") + c.Assert(results.Results[0].Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) +} + +func (s *upgraderSuite) TestToolsNothing(c *gc.C) { + // Not an error to watch nothing + results, err := s.upgrader.Tools(params.Entities{}) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 0) +} + +func (s *upgraderSuite) TestToolsRefusesWrongAgent(c *gc.C) { + anAuthorizer := s.authorizer + anAuthorizer.Tag = names.NewMachineTag("12354") + anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) + c.Check(err, jc.ErrorIsNil) + args := params.Entities{ + Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}, + } + results, err := anUpgrader.Tools(args) + // It is not an error to make the request, but the specific item is rejected + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 1) + toolResult := results.Results[0] + c.Assert(toolResult.Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) +} + +func (s *upgraderSuite) TestToolsForAgent(c *gc.C) { + cur := version.Current + agent := params.Entity{Tag: s.rawMachine.Tag().String()} + + // The machine must have its existing tools set before we query for the + // next tools. This is so that we can grab Arch and Series without + // having to pass it in again + err := s.rawMachine.SetAgentVersion(version.Current) + c.Assert(err, jc.ErrorIsNil) + + args := params.Entities{Entities: []params.Entity{agent}} + results, err := s.upgrader.Tools(args) + c.Assert(err, jc.ErrorIsNil) + assertTools := func() { + c.Check(results.Results, gc.HasLen, 1) + c.Assert(results.Results[0].Error, gc.IsNil) + agentTools := results.Results[0].Tools + url := fmt.Sprintf("https://%s/environment/%s/tools/%s", + s.APIState.Addr(), coretesting.EnvironmentTag.Id(), version.Current) + c.Check(agentTools.URL, gc.Equals, url) + c.Check(agentTools.Version, gc.DeepEquals, cur) + } + assertTools() +} + +func (s *upgraderSuite) TestSetToolsNothing(c *gc.C) { + // Not an error to watch nothing + results, err := s.upgrader.SetTools(params.EntitiesVersion{}) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 0) +} + +func (s *upgraderSuite) TestSetToolsRefusesWrongAgent(c *gc.C) { + anAuthorizer := s.authorizer + anAuthorizer.Tag = names.NewMachineTag("12354") + anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) + c.Check(err, jc.ErrorIsNil) + args := params.EntitiesVersion{ + AgentTools: []params.EntityVersion{{ + Tag: s.rawMachine.Tag().String(), + Tools: ¶ms.Version{ + Version: version.Current, + }, + }}, + } + + results, err := anUpgrader.SetTools(args) + c.Assert(results.Results, gc.HasLen, 1) + c.Assert(results.Results[0].Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) +} + +func (s *upgraderSuite) TestSetTools(c *gc.C) { + cur := version.Current + _, err := s.rawMachine.AgentTools() + c.Assert(err, jc.Satisfies, errors.IsNotFound) + args := params.EntitiesVersion{ + AgentTools: []params.EntityVersion{{ + Tag: s.rawMachine.Tag().String(), + Tools: ¶ms.Version{ + Version: cur, + }}, + }, + } + results, err := s.upgrader.SetTools(args) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results.Results, gc.HasLen, 1) + c.Assert(results.Results[0].Error, gc.IsNil) + // Check that the new value actually got set, we must Refresh because + // it was set on a different Machine object + err = s.rawMachine.Refresh() + c.Assert(err, jc.ErrorIsNil) + realTools, err := s.rawMachine.AgentTools() + c.Assert(err, jc.ErrorIsNil) + c.Check(realTools.Version.Arch, gc.Equals, cur.Arch) + c.Check(realTools.Version.Series, gc.Equals, cur.Series) + c.Check(realTools.Version.Major, gc.Equals, cur.Major) + c.Check(realTools.Version.Minor, gc.Equals, cur.Minor) + c.Check(realTools.Version.Patch, gc.Equals, cur.Patch) + c.Check(realTools.Version.Build, gc.Equals, cur.Build) + c.Check(realTools.URL, gc.Equals, "") +} + +func (s *upgraderSuite) TestDesiredVersionNothing(c *gc.C) { + // Not an error to watch nothing + results, err := s.upgrader.DesiredVersion(params.Entities{}) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 0) +} + +func (s *upgraderSuite) TestDesiredVersionRefusesWrongAgent(c *gc.C) { + anAuthorizer := s.authorizer + anAuthorizer.Tag = names.NewMachineTag("12354") + anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) + c.Check(err, jc.ErrorIsNil) + args := params.Entities{ + Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}, + } + results, err := anUpgrader.DesiredVersion(args) + // It is not an error to make the request, but the specific item is rejected + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 1) + toolResult := results.Results[0] + c.Assert(toolResult.Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) +} + +func (s *upgraderSuite) TestDesiredVersionNoticesMixedAgents(c *gc.C) { + args := params.Entities{Entities: []params.Entity{ + {Tag: s.rawMachine.Tag().String()}, + {Tag: "machine-12345"}, + }} + results, err := s.upgrader.DesiredVersion(args) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 2) + c.Assert(results.Results[0].Error, gc.IsNil) + agentVersion := results.Results[0].Version + c.Assert(agentVersion, gc.NotNil) + c.Check(*agentVersion, gc.DeepEquals, version.Current.Number) + + c.Assert(results.Results[1].Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) + c.Assert(results.Results[1].Version, gc.IsNil) + +} + +func (s *upgraderSuite) TestDesiredVersionForAgent(c *gc.C) { + args := params.Entities{Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}} + results, err := s.upgrader.DesiredVersion(args) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 1) + c.Assert(results.Results[0].Error, gc.IsNil) + agentVersion := results.Results[0].Version + c.Assert(agentVersion, gc.NotNil) + c.Check(*agentVersion, gc.DeepEquals, version.Current.Number) +} + +func (s *upgraderSuite) bumpDesiredAgentVersion(c *gc.C) version.Number { + // In order to call SetEnvironAgentVersion we have to first SetTools on + // all the existing machines + s.apiMachine.SetAgentVersion(version.Current) + s.rawMachine.SetAgentVersion(version.Current) + newer := version.Current + newer.Patch++ + err := s.State.SetEnvironAgentVersion(newer.Number) + c.Assert(err, jc.ErrorIsNil) + cfg, err := s.State.EnvironConfig() + c.Assert(err, jc.ErrorIsNil) + vers, ok := cfg.AgentVersion() + c.Assert(ok, jc.IsTrue) + c.Check(vers, gc.Equals, newer.Number) + return newer.Number +} + +func (s *upgraderSuite) TestDesiredVersionUnrestrictedForAPIAgents(c *gc.C) { + newVersion := s.bumpDesiredAgentVersion(c) + // Grab a different Upgrader for the apiMachine + authorizer := apiservertesting.FakeAuthorizer{ + Tag: s.apiMachine.Tag(), + } + upgraderAPI, err := upgrader.NewUpgraderAPI(s.State, s.resources, authorizer) + c.Assert(err, jc.ErrorIsNil) + args := params.Entities{Entities: []params.Entity{{Tag: s.apiMachine.Tag().String()}}} + results, err := upgraderAPI.DesiredVersion(args) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 1) + c.Assert(results.Results[0].Error, gc.IsNil) + agentVersion := results.Results[0].Version + c.Assert(agentVersion, gc.NotNil) + c.Check(*agentVersion, gc.DeepEquals, newVersion) +} + +func (s *upgraderSuite) TestDesiredVersionRestrictedForNonAPIAgents(c *gc.C) { + newVersion := s.bumpDesiredAgentVersion(c) + c.Assert(newVersion, gc.Not(gc.Equals), version.Current.Number) + args := params.Entities{Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}} + results, err := s.upgrader.DesiredVersion(args) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 1) + c.Assert(results.Results[0].Error, gc.IsNil) + agentVersion := results.Results[0].Version + c.Assert(agentVersion, gc.NotNil) + c.Check(*agentVersion, gc.DeepEquals, version.Current.Number) +} diff --git a/apiserver/converter/suite_test.go b/apiserver/converter/suite_test.go new file mode 100644 index 00000000000..57e013ac94c --- /dev/null +++ b/apiserver/converter/suite_test.go @@ -0,0 +1,14 @@ +// Copyright 2013 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter_test + +import ( + stdtesting "testing" + + coretesting "github.com/juju/juju/testing" +) + +func TestAll(t *stdtesting.T) { + coretesting.MgoTestPackage(t) +} diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index 9022e4ec6b1..c9a2785c88e 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -63,6 +63,7 @@ import ( "github.com/juju/juju/worker/certupdater" "github.com/juju/juju/worker/charmrevisionworker" "github.com/juju/juju/worker/cleaner" + "github.com/juju/juju/worker/converter" "github.com/juju/juju/worker/deployer" "github.com/juju/juju/worker/diskformatter" "github.com/juju/juju/worker/diskmanager" @@ -590,6 +591,12 @@ func (a *MachineAgent) APIWorker() (worker.Worker, error) { a.startWorkerAfterUpgrade(runner, "api-post-upgrade", func() (worker.Worker, error) { return a.postUpgradeAPIWorker(st, agentConfig, entity) }) + a.startWorkerAfterUpgrade(runner, "converter", func() (worker.Worker, error) { + return converter.NewConverter( + st.Converter(), + agentConfig, + ), nil + }) return cmdutil.NewCloseWorker(logger, runner, st), nil // Note: a worker.Runner is itself a worker.Worker. } diff --git a/cmd/jujud/unit.go b/cmd/jujud/unit.go index 25f8976b44b..0a527a592e2 100644 --- a/cmd/jujud/unit.go +++ b/cmd/jujud/unit.go @@ -25,7 +25,6 @@ import ( "github.com/juju/juju/version" "github.com/juju/juju/worker" "github.com/juju/juju/worker/apiaddressupdater" - "github.com/juju/juju/worker/converter" workerlogger "github.com/juju/juju/worker/logger" "github.com/juju/juju/worker/proxyupdater" "github.com/juju/juju/worker/rsyslog" @@ -185,13 +184,6 @@ func (a *UnitAgent) APIWorkers() (worker.Worker, error) { } return apiaddressupdater.NewAPIAddressUpdater(uniterFacade, a), nil }) - runner.StartWorker("converter", func() (worker.Worker, error) { - uniterFacade, err := st.Uniter() - if err != nil { - return nil, errors.Trace(err) - } - return converter.NewUnitConverter(uniterFacade, entity), nil - }) runner.StartWorker("rsyslog", func() (worker.Worker, error) { return cmdutil.NewRsyslogConfigWorker(st.Rsyslog(), agentConfig, rsyslog.RsyslogModeForwarding) }) diff --git a/state/watcher.go b/state/watcher.go index d2dd64de58f..2a9f4a370a4 100644 --- a/state/watcher.go +++ b/state/watcher.go @@ -1310,6 +1310,12 @@ func (st *State) WatchAPIHostPorts() NotifyWatcher { return newEntityWatcher(st, stateServersC, apiHostPortsKey) } +// WatchForJobsChanges returns a NotifyWatcher that notifies +// when the set of jobs for machine change. +func (st *State) WatchForJobsChanges(m names.MachineTag) NotifyWatcher { + return newEntityWatcher(st, jobsC, m.Id()) +} + // WatchVolumeAttachment returns a watcher for observing changes // to a volume attachment. func (st *State) WatchVolumeAttachment(m names.MachineTag, v names.VolumeTag) NotifyWatcher { diff --git a/worker/converter/converter.go b/worker/converter/converter.go index 5eb0177d71c..b644a707a6e 100644 --- a/worker/converter/converter.go +++ b/worker/converter/converter.go @@ -1,66 +1,53 @@ +// Copyright 2014 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + package converter import ( - "github.com/juju/errors" "github.com/juju/loggo" + "github.com/juju/names" + "launchpad.net/tomb" - "github.com/juju/juju/api/agent" - "github.com/juju/juju/api/base" + "github.com/juju/juju/agent" + "github.com/juju/juju/api/converter" "github.com/juju/juju/api/watcher" "github.com/juju/juju/worker" ) var logger = loggo.GetLogger("juju.worker.converter") -var _ worker.NotifyWatchHandler = (*UnitConverter)(nil) - -type Environment struct { - facade base.FacadeCaller -} - -type UnitConverter struct { - entity *agent.Entity - converter Converter +// Converter ... +type Converter struct { + tomb tomb.Tomb + st *converter.State + tag names.Tag } -type Converter interface { - WatchAPIHostPorts() (watcher.NotifyWatcher, error) +type converterState interface { + WatchForJobsChanges(names.MachineTag) (watcher.NotifyWatcher, error) } -func NewUnitConverter(converter Converter, entity *agent.Entity) worker.Worker { - return worker.NewNotifyWorker(&UnitConverter{ - converter: converter, - entity: entity, +// NewConverter ... +func NewConverter( + st *converter.State, + agentConfig agent.Config, +) worker.Worker { + return worker.NewNotifyWorker(&Converter{ + st: st, + tag: agentConfig.Tag(), }) } -func (c *UnitConverter) checkForManageEnvironJob() bool { - logger.Infof("checking if unit has been converted") - for _, job := range c.entity.Jobs() { - if job.NeedsState() { - return true - } - } - return false -} - -func (c *UnitConverter) SetUp() (watcher.NotifyWatcher, error) { - logger.Debugf("converter worker setup") - if c.checkForManageEnvironJob() { - return nil, errors.Errorf("already a state server, cannot convert") - } - return c.converter.WatchAPIHostPorts() +func (c *Converter) SetUp() (watcher.NotifyWatcher, error) { + logger.Infof("Setting up Converter watcher.") + return c.st.WatchForJobsChanges(c.tag.String()) } -func (c *UnitConverter) Handle() error { - if c.checkForManageEnvironJob() { - logger.Debugf("we have been converted *sip kool-aid*") - return worker.ErrTerminateAgent - } +func (c *Converter) Handle() (err error) { + logger.Infof("Jobs for %q have been changed. Check for ManageJob. Start conversion.", c.tag.String()) return nil } -func (r *UnitConverter) TearDown() error { - // nothing to teardown. +func (c *Converter) TearDown() error { return nil } diff --git a/worker/converter/converter_test.go b/worker/converter/converter_test.go index b3209dc59f6..54b3eac5cb5 100644 --- a/worker/converter/converter_test.go +++ b/worker/converter/converter_test.go @@ -1,72 +1,65 @@ +// Copyright 2014 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + package converter_test import ( - "net" + "os" + "os/signal" + "runtime" stdtesting "testing" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" - jujutesting "github.com/juju/juju/juju/testing" - "github.com/juju/juju/network" - "github.com/juju/juju/state" - coretesting "github.com/juju/juju/testing" + "github.com/juju/juju/testing" + "github.com/juju/juju/worker" "github.com/juju/juju/worker/converter" ) func TestPackage(t *stdtesting.T) { - coretesting.MgoTestPackage(t) -} - -type UnitConverterSuite struct { - jujutesting.JujuConnSuite + gc.TestingT(t) } -var _ = gc.Suite(&UnitConverterSuite{}) +var _ = gc.Suite(&ConverterSuite{}) -func (s *UnitConverterSuite) SetUpTest(c *gc.C) { - s.JujuConnSuite.SetUpTest(c) - err := s.State.SetAPIHostPorts(nil) - c.Assert(err, jc.ErrorIsNil) - // By default mock these to better isolate the test from the real machine. - s.PatchValue(&network.InterfaceByNameAddrs, func(string) ([]net.Addr, error) { - return nil, nil - }) - s.PatchValue(&network.LXCNetDefaultConfig, "") +type ConverterSuite struct { + testing.BaseSuite + // c is a channel that will wait for the termination + // signal, to prevent signals terminating the process. + c chan os.Signal } -func (s *UnitConverterSuite) TestStartStop(c *gc.C) { - st, _ := s.OpenAPIAsNewMachine(c, state.JobHostUnits) - worker := converter.NewUnitConverter(st.Machiner(), &apiAddressSetter{}) - worker.Kill() - c.Assert(worker.Wait(), gc.IsNil) +func (s *ConverterSuite) SetUpTest(c *gc.C) { + s.BaseSuite.SetUpTest(c) + s.c = make(chan os.Signal, 1) + signal.Notify(s.c, converter.TerminationSignal) } -/* -func (s *converterSuite) TestWorkerCatchesConverterEvent(c *gc.C) { - wrk, err := converter.NewConverter(s.converterState, s.AgentConfigForTag(c, s.machine.Tag()), s.lock) - c.Assert(err, jc.ErrorIsNil) - err = s.converterState.RequestConverter() - c.Assert(err, jc.ErrorIsNil) - c.Assert(wrk.Wait(), gc.Equals, worker.ErrConverterMachine) +func (s *ConverterSuite) TearDownTest(c *gc.C) { + close(s.c) + signal.Stop(s.c) + s.BaseSuite.TearDownTest(c) } -func (s *converterSuite) TestContainerCatchesParentFlag(c *gc.C) { - wrk, err := converter.NewConverter(s.ctConverterState, s.AgentConfigForTag(c, s.ct.Tag()), s.lock) - c.Assert(err, jc.ErrorIsNil) - err = s.converterState.RequestConverter() +func (s *ConverterSuite) TestStartStop(c *gc.C) { + w := converter.NewWorker() + w.Kill() + err := w.Wait() c.Assert(err, jc.ErrorIsNil) - c.Assert(wrk.Wait(), gc.Equals, worker.ErrShutdownMachine) } -func (s *converterSuite) TestCleanupIsDoneOnBoot(c *gc.C) { - s.lock.Lock(converter.ConverterMessage) - - wrk, err := converter.NewConverter(s.converterState, s.AgentConfigForTag(c, s.machine.Tag()), s.lock) +func (s *ConverterSuite) TestSignal(c *gc.C) { + //TODO(bogdanteleaga): Inspect this further on windows + if runtime.GOOS == "windows" { + c.Skip("bug 1403084: sending this signal is not supported on windows") + } + w := converter.NewWorker() + proc, err := os.FindProcess(os.Getpid()) c.Assert(err, jc.ErrorIsNil) - wrk.Kill() - c.Assert(wrk.Wait(), gc.IsNil) - - c.Assert(s.lock.IsLocked(), jc.IsFalse) + defer proc.Release() + err = proc.Signal(converter.TerminationSignal) + c.Assert(err, jc.ErrorIsNil) + err = w.Wait() + c.Assert(err, gc.Equals, worker.ErrTerminateAgent) } -*/ From d866b4c675be4c1aa5f925f0cb56ef1b87f9ef94 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 17 Mar 2015 09:31:14 -0400 Subject: [PATCH 05/33] WIP: fixing dates --- api/converter/converter.go | 5 +- apiserver/converter/converter.go | 3 +- apiserver/converter/converter_test.go | 233 +------------------------- apiserver/converter/suite_test.go | 2 +- worker/converter/converter.go | 2 +- worker/converter/converter_test.go | 2 +- 6 files changed, 6 insertions(+), 241 deletions(-) diff --git a/api/converter/converter.go b/api/converter/converter.go index bf4fb126fc4..ca35f449d1e 100644 --- a/api/converter/converter.go +++ b/api/converter/converter.go @@ -1,4 +1,4 @@ -// Copyright 2014 Canonical Ltd. +// Copyright 2015 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. package converter @@ -17,13 +17,10 @@ type State struct { facade base.FacadeCaller } -// NewAPIAddresser returns a new APIAddresser that makes API calls -// using caller and the specified facade name. func NewState(caller base.APICaller) *State { return &State{base.NewFacadeCaller(caller, converterAPI)} } -// WatchAPIHostPorts watches the host/port addresses of the API servers. func (c *State) WatchForJobsChanges(tag string) (watcher.NotifyWatcher, error) { var result params.NotifyWatchResult args := params.Entities{ diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go index 88ea156c0c2..7623fdfa48d 100644 --- a/apiserver/converter/converter.go +++ b/apiserver/converter/converter.go @@ -1,4 +1,4 @@ -// Copyright 2012, 2013 Canonical Ltd. +// Copyright 2015 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. package converter @@ -26,7 +26,6 @@ type ConverterAPI struct { authorizer common.Authorizer } -// NewUpgraderAPI creates a new server-side UpgraderAPI facade. func NewConverterAPI( st *state.State, resources *common.Resources, diff --git a/apiserver/converter/converter_test.go b/apiserver/converter/converter_test.go index af4b13d9a36..d949bd8ae24 100644 --- a/apiserver/converter/converter_test.go +++ b/apiserver/converter/converter_test.go @@ -1,4 +1,4 @@ -// Copyright 2012, 2013 Canonical Ltd. +// Copyright 2015 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. package converter_test @@ -95,234 +95,3 @@ func (s *upgraderSuite) TestWatchAPIVersion(c *gc.C) { statetesting.AssertStop(c, w) wc.AssertClosed() } - -func (s *upgraderSuite) TestUpgraderAPIRefusesNonMachineAgent(c *gc.C) { - anAuthorizer := s.authorizer - anAuthorizer.Tag = names.NewUnitTag("ubuntu/1") - anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) - c.Check(err, gc.NotNil) - c.Check(anUpgrader, gc.IsNil) - c.Assert(err, gc.ErrorMatches, "permission denied") -} - -func (s *upgraderSuite) TestWatchAPIVersionRefusesWrongAgent(c *gc.C) { - // We are a machine agent, but not the one we are trying to track - anAuthorizer := s.authorizer - anAuthorizer.Tag = names.NewMachineTag("12354") - anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) - c.Check(err, jc.ErrorIsNil) - args := params.Entities{ - Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}, - } - results, err := anUpgrader.WatchAPIVersion(args) - // It is not an error to make the request, but the specific item is rejected - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 1) - c.Check(results.Results[0].NotifyWatcherId, gc.Equals, "") - c.Assert(results.Results[0].Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) -} - -func (s *upgraderSuite) TestToolsNothing(c *gc.C) { - // Not an error to watch nothing - results, err := s.upgrader.Tools(params.Entities{}) - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 0) -} - -func (s *upgraderSuite) TestToolsRefusesWrongAgent(c *gc.C) { - anAuthorizer := s.authorizer - anAuthorizer.Tag = names.NewMachineTag("12354") - anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) - c.Check(err, jc.ErrorIsNil) - args := params.Entities{ - Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}, - } - results, err := anUpgrader.Tools(args) - // It is not an error to make the request, but the specific item is rejected - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 1) - toolResult := results.Results[0] - c.Assert(toolResult.Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) -} - -func (s *upgraderSuite) TestToolsForAgent(c *gc.C) { - cur := version.Current - agent := params.Entity{Tag: s.rawMachine.Tag().String()} - - // The machine must have its existing tools set before we query for the - // next tools. This is so that we can grab Arch and Series without - // having to pass it in again - err := s.rawMachine.SetAgentVersion(version.Current) - c.Assert(err, jc.ErrorIsNil) - - args := params.Entities{Entities: []params.Entity{agent}} - results, err := s.upgrader.Tools(args) - c.Assert(err, jc.ErrorIsNil) - assertTools := func() { - c.Check(results.Results, gc.HasLen, 1) - c.Assert(results.Results[0].Error, gc.IsNil) - agentTools := results.Results[0].Tools - url := fmt.Sprintf("https://%s/environment/%s/tools/%s", - s.APIState.Addr(), coretesting.EnvironmentTag.Id(), version.Current) - c.Check(agentTools.URL, gc.Equals, url) - c.Check(agentTools.Version, gc.DeepEquals, cur) - } - assertTools() -} - -func (s *upgraderSuite) TestSetToolsNothing(c *gc.C) { - // Not an error to watch nothing - results, err := s.upgrader.SetTools(params.EntitiesVersion{}) - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 0) -} - -func (s *upgraderSuite) TestSetToolsRefusesWrongAgent(c *gc.C) { - anAuthorizer := s.authorizer - anAuthorizer.Tag = names.NewMachineTag("12354") - anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) - c.Check(err, jc.ErrorIsNil) - args := params.EntitiesVersion{ - AgentTools: []params.EntityVersion{{ - Tag: s.rawMachine.Tag().String(), - Tools: ¶ms.Version{ - Version: version.Current, - }, - }}, - } - - results, err := anUpgrader.SetTools(args) - c.Assert(results.Results, gc.HasLen, 1) - c.Assert(results.Results[0].Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) -} - -func (s *upgraderSuite) TestSetTools(c *gc.C) { - cur := version.Current - _, err := s.rawMachine.AgentTools() - c.Assert(err, jc.Satisfies, errors.IsNotFound) - args := params.EntitiesVersion{ - AgentTools: []params.EntityVersion{{ - Tag: s.rawMachine.Tag().String(), - Tools: ¶ms.Version{ - Version: cur, - }}, - }, - } - results, err := s.upgrader.SetTools(args) - c.Assert(err, jc.ErrorIsNil) - c.Assert(results.Results, gc.HasLen, 1) - c.Assert(results.Results[0].Error, gc.IsNil) - // Check that the new value actually got set, we must Refresh because - // it was set on a different Machine object - err = s.rawMachine.Refresh() - c.Assert(err, jc.ErrorIsNil) - realTools, err := s.rawMachine.AgentTools() - c.Assert(err, jc.ErrorIsNil) - c.Check(realTools.Version.Arch, gc.Equals, cur.Arch) - c.Check(realTools.Version.Series, gc.Equals, cur.Series) - c.Check(realTools.Version.Major, gc.Equals, cur.Major) - c.Check(realTools.Version.Minor, gc.Equals, cur.Minor) - c.Check(realTools.Version.Patch, gc.Equals, cur.Patch) - c.Check(realTools.Version.Build, gc.Equals, cur.Build) - c.Check(realTools.URL, gc.Equals, "") -} - -func (s *upgraderSuite) TestDesiredVersionNothing(c *gc.C) { - // Not an error to watch nothing - results, err := s.upgrader.DesiredVersion(params.Entities{}) - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 0) -} - -func (s *upgraderSuite) TestDesiredVersionRefusesWrongAgent(c *gc.C) { - anAuthorizer := s.authorizer - anAuthorizer.Tag = names.NewMachineTag("12354") - anUpgrader, err := upgrader.NewUpgraderAPI(s.State, s.resources, anAuthorizer) - c.Check(err, jc.ErrorIsNil) - args := params.Entities{ - Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}, - } - results, err := anUpgrader.DesiredVersion(args) - // It is not an error to make the request, but the specific item is rejected - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 1) - toolResult := results.Results[0] - c.Assert(toolResult.Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) -} - -func (s *upgraderSuite) TestDesiredVersionNoticesMixedAgents(c *gc.C) { - args := params.Entities{Entities: []params.Entity{ - {Tag: s.rawMachine.Tag().String()}, - {Tag: "machine-12345"}, - }} - results, err := s.upgrader.DesiredVersion(args) - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 2) - c.Assert(results.Results[0].Error, gc.IsNil) - agentVersion := results.Results[0].Version - c.Assert(agentVersion, gc.NotNil) - c.Check(*agentVersion, gc.DeepEquals, version.Current.Number) - - c.Assert(results.Results[1].Error, gc.DeepEquals, apiservertesting.ErrUnauthorized) - c.Assert(results.Results[1].Version, gc.IsNil) - -} - -func (s *upgraderSuite) TestDesiredVersionForAgent(c *gc.C) { - args := params.Entities{Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}} - results, err := s.upgrader.DesiredVersion(args) - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 1) - c.Assert(results.Results[0].Error, gc.IsNil) - agentVersion := results.Results[0].Version - c.Assert(agentVersion, gc.NotNil) - c.Check(*agentVersion, gc.DeepEquals, version.Current.Number) -} - -func (s *upgraderSuite) bumpDesiredAgentVersion(c *gc.C) version.Number { - // In order to call SetEnvironAgentVersion we have to first SetTools on - // all the existing machines - s.apiMachine.SetAgentVersion(version.Current) - s.rawMachine.SetAgentVersion(version.Current) - newer := version.Current - newer.Patch++ - err := s.State.SetEnvironAgentVersion(newer.Number) - c.Assert(err, jc.ErrorIsNil) - cfg, err := s.State.EnvironConfig() - c.Assert(err, jc.ErrorIsNil) - vers, ok := cfg.AgentVersion() - c.Assert(ok, jc.IsTrue) - c.Check(vers, gc.Equals, newer.Number) - return newer.Number -} - -func (s *upgraderSuite) TestDesiredVersionUnrestrictedForAPIAgents(c *gc.C) { - newVersion := s.bumpDesiredAgentVersion(c) - // Grab a different Upgrader for the apiMachine - authorizer := apiservertesting.FakeAuthorizer{ - Tag: s.apiMachine.Tag(), - } - upgraderAPI, err := upgrader.NewUpgraderAPI(s.State, s.resources, authorizer) - c.Assert(err, jc.ErrorIsNil) - args := params.Entities{Entities: []params.Entity{{Tag: s.apiMachine.Tag().String()}}} - results, err := upgraderAPI.DesiredVersion(args) - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 1) - c.Assert(results.Results[0].Error, gc.IsNil) - agentVersion := results.Results[0].Version - c.Assert(agentVersion, gc.NotNil) - c.Check(*agentVersion, gc.DeepEquals, newVersion) -} - -func (s *upgraderSuite) TestDesiredVersionRestrictedForNonAPIAgents(c *gc.C) { - newVersion := s.bumpDesiredAgentVersion(c) - c.Assert(newVersion, gc.Not(gc.Equals), version.Current.Number) - args := params.Entities{Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}} - results, err := s.upgrader.DesiredVersion(args) - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 1) - c.Assert(results.Results[0].Error, gc.IsNil) - agentVersion := results.Results[0].Version - c.Assert(agentVersion, gc.NotNil) - c.Check(*agentVersion, gc.DeepEquals, version.Current.Number) -} diff --git a/apiserver/converter/suite_test.go b/apiserver/converter/suite_test.go index 57e013ac94c..aed81d60a77 100644 --- a/apiserver/converter/suite_test.go +++ b/apiserver/converter/suite_test.go @@ -1,4 +1,4 @@ -// Copyright 2013 Canonical Ltd. +// Copyright 2015 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. package converter_test diff --git a/worker/converter/converter.go b/worker/converter/converter.go index b644a707a6e..6c46c3503d1 100644 --- a/worker/converter/converter.go +++ b/worker/converter/converter.go @@ -1,4 +1,4 @@ -// Copyright 2014 Canonical Ltd. +// Copyright 2015 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. package converter diff --git a/worker/converter/converter_test.go b/worker/converter/converter_test.go index 54b3eac5cb5..70dd6ce81c7 100644 --- a/worker/converter/converter_test.go +++ b/worker/converter/converter_test.go @@ -1,4 +1,4 @@ -// Copyright 2014 Canonical Ltd. +// Copyright 2015 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. package converter_test From 0ee8a48dd0f0bed49c640520c70241e286ca8965 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Wed, 18 Mar 2015 00:52:09 -0400 Subject: [PATCH 06/33] WIP: HA --to worker --- api/converter/converter.go | 36 ++++++++++ api/facadeversions.go | 1 + api/state.go | 6 ++ apiserver/allfacades.go | 1 + apiserver/converter/converter.go | 70 +++++++++++++++++++ apiserver/converter/converter_test.go | 97 +++++++++++++++++++++++++++ apiserver/converter/suite_test.go | 14 ++++ apiserver/params/constants.go | 4 ++ cmd/juju/ensureavailability.go | 8 ++- cmd/jujud/agent/machine.go | 6 ++ cmd/jujud/unit.go | 1 - state/addmachine.go | 71 +++++++++++++++++--- state/state.go | 1 + state/watcher.go | 6 ++ worker/converter/converter.go | 53 +++++++++++++++ worker/converter/converter_test.go | 65 ++++++++++++++++++ worker/reboot/reboot.go | 11 ++- 17 files changed, 435 insertions(+), 16 deletions(-) create mode 100644 api/converter/converter.go create mode 100644 apiserver/converter/converter.go create mode 100644 apiserver/converter/converter_test.go create mode 100644 apiserver/converter/suite_test.go create mode 100644 worker/converter/converter.go create mode 100644 worker/converter/converter_test.go diff --git a/api/converter/converter.go b/api/converter/converter.go new file mode 100644 index 00000000000..d49a2ef9de2 --- /dev/null +++ b/api/converter/converter.go @@ -0,0 +1,36 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter + +import ( + "github.com/juju/juju/api/base" + "github.com/juju/juju/api/watcher" + "github.com/juju/juju/apiserver/params" +) + +const converterAPI = "Converter" + +// Converter provides common client-side API +// functions to call into apiserver.Converter +type State struct { + facade base.FacadeCaller +} + +func NewState(caller base.APICaller) *State { + return &State{facade: base.NewFacadeCaller(caller, converterAPI)} +} + +func (c *State) WatchForJobsChanges(tag string) (watcher.NotifyWatcher, error) { + var result params.NotifyWatchResult + args := params.Entities{ + Entities: []params.Entity{{Tag: tag}}, + } + + err := c.facade.FacadeCall("WatchForJobsChanges", args, &result) + if err != nil { + return nil, err + } + + return watcher.NewNotifyWatcher(c.facade.RawAPICaller(), result), nil +} diff --git a/api/facadeversions.go b/api/facadeversions.go index 784ac277a60..4de67486e17 100644 --- a/api/facadeversions.go +++ b/api/facadeversions.go @@ -20,6 +20,7 @@ var facadeVersions = map[string]int{ "Charms": 1, "CharmRevisionUpdater": 0, "Client": 0, + "Converter": 1, "Deployer": 0, "DiskFormatter": 1, "DiskManager": 1, diff --git a/api/state.go b/api/state.go index dca26c4517d..ee553371313 100644 --- a/api/state.go +++ b/api/state.go @@ -13,6 +13,7 @@ import ( "github.com/juju/juju/api/agent" "github.com/juju/juju/api/base" "github.com/juju/juju/api/charmrevisionupdater" + "github.com/juju/juju/api/converter" "github.com/juju/juju/api/deployer" "github.com/juju/juju/api/diskformatter" "github.com/juju/juju/api/diskmanager" @@ -318,3 +319,8 @@ func (st *State) CharmRevisionUpdater() *charmrevisionupdater.State { func (st *State) Rsyslog() *rsyslog.State { return rsyslog.NewState(st) } + +// Converter returns access to the Converter API +func (st *State) Converter() *converter.State { + return converter.NewState(st) +} diff --git a/apiserver/allfacades.go b/apiserver/allfacades.go index 2b446ebfbbf..d72d9309bf4 100644 --- a/apiserver/allfacades.go +++ b/apiserver/allfacades.go @@ -15,6 +15,7 @@ import ( _ "github.com/juju/juju/apiserver/charmrevisionupdater" _ "github.com/juju/juju/apiserver/charms" _ "github.com/juju/juju/apiserver/client" + _ "github.com/juju/juju/apiserver/converter" _ "github.com/juju/juju/apiserver/deployer" _ "github.com/juju/juju/apiserver/diskformatter" _ "github.com/juju/juju/apiserver/diskmanager" diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go new file mode 100644 index 00000000000..4e45b4305a7 --- /dev/null +++ b/apiserver/converter/converter.go @@ -0,0 +1,70 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter + +import ( + "github.com/juju/errors" + "github.com/juju/loggo" + "github.com/juju/names" + + "github.com/juju/juju/apiserver/common" + "github.com/juju/juju/apiserver/params" + "github.com/juju/juju/state" + "github.com/juju/juju/state/watcher" +) + +var logger = loggo.GetLogger("juju.apiserver.converter") + +func init() { + common.RegisterStandardFacade("Converter", 1, NewConverterAPI) +} + +type ConverterAPI struct { + st *state.State + resources *common.Resources + authorizer common.Authorizer +} + +func NewConverterAPI( + st *state.State, + resources *common.Resources, + authorizer common.Authorizer, +) (*ConverterAPI, error) { + if !authorizer.AuthMachineAgent() { + return nil, common.ErrPerm + } + return &ConverterAPI{ + st: st, + resources: resources, + authorizer: authorizer, + }, nil +} + +func (c *ConverterAPI) WatchForJobsChanges(args params.Entities) (params.NotifyWatchResults, error) { + result := params.NotifyWatchResults{ + Results: make([]params.NotifyWatchResult, len(args.Entities)), + } + for i, agent := range args.Entities { + tag, err := names.ParseMachineTag(agent.Tag) + if err != nil { + return params.NotifyWatchResults{}, errors.Trace(err) + } + err = common.ErrPerm + if c.authorizer.AuthOwner(tag) { + watch := c.st.WatchForJobsChanges(tag) + // Consume the initial event. Technically, API + // calls to Watch 'transmit' the initial event + // in the Watch response. But NotifyWatchers + // have no state to transmit. + if _, ok := <-watch.Changes(); ok { + result.Results[i].NotifyWatcherId = c.resources.Register(watch) + err = nil + } else { + err = watcher.EnsureErr(watch) + } + } + result.Results[i].Error = common.ServerError(err) + } + return result, nil +} diff --git a/apiserver/converter/converter_test.go b/apiserver/converter/converter_test.go new file mode 100644 index 00000000000..d949bd8ae24 --- /dev/null +++ b/apiserver/converter/converter_test.go @@ -0,0 +1,97 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter_test + +import ( + "fmt" + + "github.com/juju/errors" + "github.com/juju/names" + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + "github.com/juju/juju/apiserver/common" + "github.com/juju/juju/apiserver/params" + apiservertesting "github.com/juju/juju/apiserver/testing" + "github.com/juju/juju/apiserver/upgrader" + jujutesting "github.com/juju/juju/juju/testing" + "github.com/juju/juju/state" + statetesting "github.com/juju/juju/state/testing" + coretesting "github.com/juju/juju/testing" + "github.com/juju/juju/version" +) + +type upgraderSuite struct { + jujutesting.JujuConnSuite + + // These are raw State objects. Use them for setup and assertions, but + // should never be touched by the API calls themselves + rawMachine *state.Machine + apiMachine *state.Machine + upgrader *upgrader.UpgraderAPI + resources *common.Resources + authorizer apiservertesting.FakeAuthorizer +} + +var _ = gc.Suite(&upgraderSuite{}) + +func (s *upgraderSuite) SetUpTest(c *gc.C) { + s.JujuConnSuite.SetUpTest(c) + s.resources = common.NewResources() + s.AddCleanup(func(_ *gc.C) { s.resources.StopAll() }) + + // Create a machine to work with + var err error + // The first machine created is the only one allowed to + // JobManageEnviron + s.apiMachine, err = s.State.AddMachine("quantal", state.JobHostUnits, + state.JobManageEnviron) + c.Assert(err, jc.ErrorIsNil) + s.rawMachine, err = s.State.AddMachine("quantal", state.JobHostUnits) + c.Assert(err, jc.ErrorIsNil) + + // The default auth is as the machine agent + s.authorizer = apiservertesting.FakeAuthorizer{ + Tag: s.rawMachine.Tag(), + } + s.upgrader, err = upgrader.NewUpgraderAPI(s.State, s.resources, s.authorizer) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *upgraderSuite) TearDownTest(c *gc.C) { + if s.resources != nil { + s.resources.StopAll() + } + s.JujuConnSuite.TearDownTest(c) +} + +func (s *upgraderSuite) TestWatchAPIVersionNothing(c *gc.C) { + // Not an error to watch nothing + results, err := s.upgrader.WatchAPIVersion(params.Entities{}) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 0) +} + +func (s *upgraderSuite) TestWatchAPIVersion(c *gc.C) { + args := params.Entities{ + Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}, + } + results, err := s.upgrader.WatchAPIVersion(args) + c.Assert(err, jc.ErrorIsNil) + c.Check(results.Results, gc.HasLen, 1) + c.Check(results.Results[0].NotifyWatcherId, gc.Not(gc.Equals), "") + c.Check(results.Results[0].Error, gc.IsNil) + resource := s.resources.Get(results.Results[0].NotifyWatcherId) + c.Check(resource, gc.NotNil) + + w := resource.(state.NotifyWatcher) + wc := statetesting.NewNotifyWatcherC(c, s.State, w) + wc.AssertNoChange() + + err = statetesting.SetAgentVersion(s.State, version.MustParse("3.4.567.8")) + c.Assert(err, jc.ErrorIsNil) + wc.AssertOneChange() + statetesting.AssertStop(c, w) + wc.AssertClosed() +} diff --git a/apiserver/converter/suite_test.go b/apiserver/converter/suite_test.go new file mode 100644 index 00000000000..aed81d60a77 --- /dev/null +++ b/apiserver/converter/suite_test.go @@ -0,0 +1,14 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter_test + +import ( + stdtesting "testing" + + coretesting "github.com/juju/juju/testing" +) + +func TestAll(t *stdtesting.T) { + coretesting.MgoTestPackage(t) +} diff --git a/apiserver/params/constants.go b/apiserver/params/constants.go index ed58d273af6..f8f77667122 100644 --- a/apiserver/params/constants.go +++ b/apiserver/params/constants.go @@ -19,6 +19,10 @@ const ( // happens when running inside a container, and a hook on the parent // machine requests a reboot ShouldShutdown RebootAction = "shutdown" + // ShouldRestart instructs a machine agent to restart jujud. This usually + // happens after an upgrade or conversion to state server using + // ensure-availability. + ShouldRestart RebootAction = "restart" ) // ResolvedMode describes the way state transition errors diff --git a/cmd/juju/ensureavailability.go b/cmd/juju/ensureavailability.go index 83500d4fe9b..f3c9923a5df 100644 --- a/cmd/juju/ensureavailability.go +++ b/cmd/juju/ensureavailability.go @@ -145,7 +145,13 @@ func (c *EnsureAvailabilityCommand) Init(args []string) error { placementSpecs := strings.Split(c.PlacementSpec, ",") c.Placement = make([]string, len(placementSpecs)) for i, spec := range placementSpecs { - _, err := instance.ParsePlacement(strings.TrimSpace(spec)) + p, err := instance.ParsePlacement(strings.TrimSpace(spec)) + fmt.Println(p, err) + if err == nil && p.Scope == instance.MachineScope { + // targeting machines is ok + c.Placement[i] = p.String() + continue + } if err != instance.ErrPlacementScopeMissing { // We only support unscoped placement directives. return fmt.Errorf("unsupported ensure-availability placement directive %q", spec) diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index 3dbaa99e801..c2cafdb669b 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -63,6 +63,7 @@ import ( "github.com/juju/juju/worker/certupdater" "github.com/juju/juju/worker/charmrevisionworker" "github.com/juju/juju/worker/cleaner" + "github.com/juju/juju/worker/converter" "github.com/juju/juju/worker/dblogpruner" "github.com/juju/juju/worker/deployer" "github.com/juju/juju/worker/diskformatter" @@ -648,6 +649,11 @@ func (a *MachineAgent) postUpgradeAPIWorker( runner.StartWorker("rsyslog", func() (worker.Worker, error) { return cmdutil.NewRsyslogConfigWorker(st.Rsyslog(), agentConfig, rsyslogMode) }) + + runner.StartWorker("converter", func() (worker.Worker, error) { + return converter.NewConverter(st.Converter(), agentConfig), nil + }) + // TODO(wallyworld) - we don't want the storage workers running yet, even with feature flag. // Will be enabled in a followup branch. enableStorageWorkers := false diff --git a/cmd/jujud/unit.go b/cmd/jujud/unit.go index 81ed41e99e5..0a527a592e2 100644 --- a/cmd/jujud/unit.go +++ b/cmd/jujud/unit.go @@ -177,7 +177,6 @@ func (a *UnitAgent) APIWorkers() (worker.Worker, error) { } return uniter.NewUniter(uniterFacade, unitTag, st.LeadershipManager(), dataDir, hookLock), nil }) - runner.StartWorker("apiaddressupdater", func() (worker.Worker, error) { uniterFacade, err := st.Uniter() if err != nil { diff --git a/state/addmachine.go b/state/addmachine.go index 5f41be1c276..189972eba47 100644 --- a/state/addmachine.go +++ b/state/addmachine.go @@ -618,7 +618,7 @@ func (st *State) EnsureAvailability( return nil, errors.New("cannot reduce state server count") } - intent, err := st.ensureAvailabilityIntentions(currentInfo) + intent, err := st.ensureAvailabilityIntentions(currentInfo, placement) if err != nil { return nil, err } @@ -636,11 +636,14 @@ func (st *State) EnsureAvailability( intent.promote = intent.promote[:n] } voteCount += len(intent.promote) - intent.newCount = desiredStateServerCount - voteCount - logger.Infof("%d new machines; promoting %v", intent.newCount, intent.promote) + intent.newCount = desiredStateServerCount - voteCount - len(intent.convert) + if intent.newCount < 0 { + intent.newCount = 0 + } + logger.Infof("%d new machines; promoting %v; converting %v", intent.newCount, intent.promote, intent.convert) var ops []txn.Op - ops, change, err = st.ensureAvailabilityIntentionOps(intent, currentInfo, cons, series, placement) + ops, change, err = st.ensureAvailabilityIntentionOps(intent, currentInfo, cons, series) return ops, err } if err := st.run(buildTxn); err != nil { @@ -657,6 +660,7 @@ type StateServersChanges struct { Maintained []string Promoted []string Demoted []string + Converted []string } // ensureAvailabilityIntentionOps returns operations to fulfil the desired intent. @@ -665,7 +669,6 @@ func (st *State) ensureAvailabilityIntentionOps( currentInfo *StateServerInfo, cons constraints.Value, series string, - placement []string, ) ([]txn.Op, StateServersChanges, error) { var ops []txn.Op var change StateServersChanges @@ -677,16 +680,20 @@ func (st *State) ensureAvailabilityIntentionOps( ops = append(ops, demoteStateServerOps(m)...) change.Demoted = append(change.Demoted, m.doc.Id) } + for _, m := range intent.convert { + ops = append(ops, convertStateServerOps(m)...) + change.Converted = append(change.Converted, m.doc.Id) + } // Use any placement directives that have been provided // when adding new machines, until the directives have // been all used up. Set up a helper function to do the // work required. placementCount := 0 getPlacement := func() string { - if placementCount >= len(placement) { + if placementCount >= len(intent.placement) { return "" } - result := placement[placementCount] + result := intent.placement[placementCount] placementCount++ return result } @@ -744,8 +751,10 @@ var stateServerAvailable = func(m *Machine) (bool, error) { } type ensureAvailabilityIntent struct { - newCount int - promote, maintain, demote, remove []*Machine + newCount int + placement []string + + promote, maintain, demote, remove, convert []*Machine } // ensureAvailabilityIntentions returns what we would like @@ -754,8 +763,29 @@ type ensureAvailabilityIntent struct { // demoting unavailable, voting machines; // removing unavailable, non-voting, non-vote-holding machines; // gathering available, non-voting machines that may be promoted; -func (st *State) ensureAvailabilityIntentions(info *StateServerInfo) (*ensureAvailabilityIntent, error) { +func (st *State) ensureAvailabilityIntentions(info *StateServerInfo, placement []string) (*ensureAvailabilityIntent, error) { var intent ensureAvailabilityIntent + for _, s := range placement { + p, err := instance.ParsePlacement(s) + if err == instance.ErrPlacementScopeMissing { + intent.placement = append(intent.placement, s) + continue + } + if err == nil && p.Scope == instance.MachineScope { + m, err := st.Machine(p.Directive) + if err != nil { + return nil, errors.Annotatef(err, "can't find machine for placement directive %q", s) + } + if m.IsManager() { + return nil, errors.Errorf("machine for placement directive %q is already a state server", s) + } + intent.convert = append(intent.convert, m) + intent.placement = append(intent.placement, s) + continue + } + return nil, errors.Errorf("unsupported HA placement directive %q", s) + } + for _, mid := range info.MachineIds { m, err := st.Machine(mid) if err != nil { @@ -790,10 +820,29 @@ func (st *State) ensureAvailabilityIntentions(info *StateServerInfo) (*ensureAva intent.remove = append(intent.remove, m) } } - logger.Infof("initial intentions: promote %v; maintain %v; demote %v; remove %v", intent.promote, intent.maintain, intent.demote, intent.remove) + logger.Infof("initial intentions: promote %v; maintain %v; demote %v; remove %v; convert: %v", + intent.promote, intent.maintain, intent.demote, intent.remove, intent.convert) return &intent, nil } +func convertStateServerOps(m *Machine) []txn.Op { + return []txn.Op{{ + C: machinesC, + Id: m.doc.DocID, + Update: bson.D{ + {"$addToSet", bson.D{{"jobs", JobManageEnviron}}}, + {"$set", bson.D{{"novote", false}}}, + }, + }, { + C: stateServersC, + Id: environGlobalKey, + Update: bson.D{ + {"$addToSet", bson.D{{"votingmachineids", m.doc.Id}}}, + {"$addToSet", bson.D{{"machineids", m.doc.Id}}}, + }, + }} +} + func promoteStateServerOps(m *Machine) []txn.Op { return []txn.Op{{ C: machinesC, diff --git a/state/state.go b/state/state.go index 13439d93100..923d7a27d62 100644 --- a/state/state.go +++ b/state/state.go @@ -56,6 +56,7 @@ const ( unitsC = "units" subnetsC = "subnets" ipaddressesC = "ipaddresses" + jobsC = "jobs" // actionsC and related collections store state of Actions that // have been enqueued. diff --git a/state/watcher.go b/state/watcher.go index b08463b9f50..3a85314d2ec 100644 --- a/state/watcher.go +++ b/state/watcher.go @@ -1370,6 +1370,12 @@ func (st *State) WatchAPIHostPorts() NotifyWatcher { return newEntityWatcher(st, stateServersC, apiHostPortsKey) } +// WatchForJobsChanges returns a NotifyWatcher that notifies +// when the set of jobs for machine change. +func (st *State) WatchForJobsChanges(m names.MachineTag) NotifyWatcher { + return newEntityWatcher(st, jobsC, m.Id()) +} + // WatchVolumeAttachment returns a watcher for observing changes // to a volume attachment. func (st *State) WatchVolumeAttachment(m names.MachineTag, v names.VolumeTag) NotifyWatcher { diff --git a/worker/converter/converter.go b/worker/converter/converter.go new file mode 100644 index 00000000000..d8202847dac --- /dev/null +++ b/worker/converter/converter.go @@ -0,0 +1,53 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter + +import ( + "github.com/juju/loggo" + "github.com/juju/names" + "launchpad.net/tomb" + + "github.com/juju/juju/agent" + "github.com/juju/juju/api/converter" + "github.com/juju/juju/api/watcher" + "github.com/juju/juju/worker" +) + +var logger = loggo.GetLogger("juju.worker.converter") + +// Converter ... +type Converter struct { + tomb tomb.Tomb + st *converter.State + tag names.Tag +} + +// NewConverter ... +func NewConverter( + st *converter.State, + agentConfig agent.Config, +) worker.Worker { + return worker.NewNotifyWorker(&Converter{ + st: st, + tag: agentConfig.Tag(), + }) +} + +func (c *Converter) SetUp() (watcher.NotifyWatcher, error) { + logger.Infof("Setting up Converter watcher.") + watcher, err := c.st.WatchForJobsChanges(c.tag.String()) + if err != nil { + logger.Errorf("%q", err) + } + return watcher, err +} + +func (c *Converter) Handle() (err error) { + logger.Infof("Jobs for %q have been changed. Check for ManageJob. Start conversion.", c.tag.String()) + return nil +} + +func (c *Converter) TearDown() error { + return nil +} diff --git a/worker/converter/converter_test.go b/worker/converter/converter_test.go new file mode 100644 index 00000000000..70dd6ce81c7 --- /dev/null +++ b/worker/converter/converter_test.go @@ -0,0 +1,65 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package converter_test + +import ( + "os" + "os/signal" + "runtime" + stdtesting "testing" + + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + "github.com/juju/juju/testing" + "github.com/juju/juju/worker" + "github.com/juju/juju/worker/converter" +) + +func TestPackage(t *stdtesting.T) { + gc.TestingT(t) +} + +var _ = gc.Suite(&ConverterSuite{}) + +type ConverterSuite struct { + testing.BaseSuite + // c is a channel that will wait for the termination + // signal, to prevent signals terminating the process. + c chan os.Signal +} + +func (s *ConverterSuite) SetUpTest(c *gc.C) { + s.BaseSuite.SetUpTest(c) + s.c = make(chan os.Signal, 1) + signal.Notify(s.c, converter.TerminationSignal) +} + +func (s *ConverterSuite) TearDownTest(c *gc.C) { + close(s.c) + signal.Stop(s.c) + s.BaseSuite.TearDownTest(c) +} + +func (s *ConverterSuite) TestStartStop(c *gc.C) { + w := converter.NewWorker() + w.Kill() + err := w.Wait() + c.Assert(err, jc.ErrorIsNil) +} + +func (s *ConverterSuite) TestSignal(c *gc.C) { + //TODO(bogdanteleaga): Inspect this further on windows + if runtime.GOOS == "windows" { + c.Skip("bug 1403084: sending this signal is not supported on windows") + } + w := converter.NewWorker() + proc, err := os.FindProcess(os.Getpid()) + c.Assert(err, jc.ErrorIsNil) + defer proc.Release() + err = proc.Signal(converter.TerminationSignal) + c.Assert(err, jc.ErrorIsNil) + err = w.Wait() + c.Assert(err, gc.Equals, worker.ErrTerminateAgent) +} diff --git a/worker/reboot/reboot.go b/worker/reboot/reboot.go index b538c301622..3ab214fa41c 100644 --- a/worker/reboot/reboot.go +++ b/worker/reboot/reboot.go @@ -17,14 +17,16 @@ import ( var logger = loggo.GetLogger("juju.worker.reboot") const RebootMessage = "preparing for reboot" +const RestartMessage = "restarting juju" var _ worker.NotifyWatchHandler = (*Reboot)(nil) // The reboot worker listens for changes to the reboot flag and -// exists with worker.ErrRebootMachine if the machine should reboot or +// exists with worker.ErrTerminateAgent if only jujud should be restarts, +// with worker.ErrRebootMachine if the machine should reboot, or // with worker.ErrShutdownMachine if it should shutdown. This will be picked // up by the machine agent as a fatal error and will do the -// right thing (reboot or shutdown) +// right thing (restart, reboot, or shutdown) type Reboot struct { tomb tomb.Tomb st *reboot.State @@ -51,7 +53,7 @@ func (r *Reboot) checkForRebootState() error { return nil } - if r.machineLock.Message() == RebootMessage { + if r.machineLock.Message() == RebootMessage || r.machineLock.Message() == RestartMessage { // Not a lock held by the machne agent in order to reboot err = r.machineLock.BreakLock() if err != nil { @@ -87,6 +89,9 @@ func (r *Reboot) Handle() error { case params.ShouldShutdown: r.machineLock.Lock(RebootMessage) return worker.ErrShutdownMachine + case params.ShouldRestart: + r.machineLock.Lock(RestartMessage) + return worker.ErrTerminateAgent } return nil } From c86072ed2417f6fa757055454e338c5386c0f6bb Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Wed, 18 Mar 2015 15:31:32 -0400 Subject: [PATCH 07/33] testing and contents of watcher --- apiserver/watcher.go | 57 +++++++++++++++++++++++++++++------ cmd/jujud/agent/machine.go | 15 +++------ rpc/server.go | 1 + worker/converter/converter.go | 44 +++++++++++++++++++-------- 4 files changed, 84 insertions(+), 33 deletions(-) diff --git a/apiserver/watcher.go b/apiserver/watcher.go index e686dd1e00f..5cace1811b6 100644 --- a/apiserver/watcher.go +++ b/apiserver/watcher.go @@ -6,6 +6,7 @@ package apiserver import ( "reflect" + "github.com/juju/errors" "github.com/juju/juju/apiserver/common" "github.com/juju/juju/apiserver/params" "github.com/juju/juju/state" @@ -38,9 +39,15 @@ func newClientAllWatcher(st *state.State, resources *common.Resources, auth comm if !auth.AuthClient() { return nil, common.ErrPerm } - watcher, ok := resources.Get(id).(*state.Multiwatcher) + r := resources.Get(id) + if r == nil { + logger.Errorf("watcher %q not found", id) + panic(errors.Trace(common.ErrUnknownWatcher)) + } + watcher, ok := r.(*state.Multiwatcher) if !ok { - return nil, common.ErrUnknownWatcher + logger.Errorf("unknown watcher type for %q: %T, expected state.Multiwatcher", id, r) + panic(common.ErrUnknownWatcher) } return &srvClientAllWatcher{ watcher: watcher, @@ -85,10 +92,17 @@ func newNotifyWatcher(st *state.State, resources *common.Resources, auth common. if !isAgent(auth) { return nil, common.ErrPerm } - watcher, ok := resources.Get(id).(state.NotifyWatcher) + r := resources.Get(id) + if r == nil { + logger.Errorf("watcher %q not found", id) + panic(common.ErrUnknownWatcher) + } + watcher, ok := r.(state.NotifyWatcher) if !ok { - return nil, common.ErrUnknownWatcher + logger.Errorf("unknown watcher type for %q: %#T, expected state.NotifyWatcher", id, r) + panic(common.ErrUnknownWatcher) } + return &srvNotifyWatcher{ watcher: watcher, id: id, @@ -129,9 +143,16 @@ func newStringsWatcher(st *state.State, resources *common.Resources, auth common if !isAgent(auth) { return nil, common.ErrPerm } - watcher, ok := resources.Get(id).(state.StringsWatcher) + + r := resources.Get(id) + if r == nil { + logger.Errorf("watcher %q not found", id) + panic(common.ErrUnknownWatcher) + } + watcher, ok := r.(state.StringsWatcher) if !ok { - return nil, common.ErrUnknownWatcher + logger.Errorf("unknown watcher type for %q: %#T, expected state.StringsWatcher", id, r) + panic(common.ErrUnknownWatcher) } return &srvStringsWatcher{ watcher: watcher, @@ -174,10 +195,18 @@ func newRelationUnitsWatcher(st *state.State, resources *common.Resources, auth if !isAgent(auth) { return nil, common.ErrPerm } - watcher, ok := resources.Get(id).(state.RelationUnitsWatcher) + + r := resources.Get(id) + if r == nil { + logger.Errorf("watcher %q not found", id) + panic(common.ErrUnknownWatcher) + } + watcher, ok := r.(state.RelationUnitsWatcher) if !ok { - return nil, common.ErrUnknownWatcher + logger.Errorf("unknown watcher type for %q: %#T, expected state.RelationUnitsWatcher", id, r) + panic(common.ErrUnknownWatcher) } + return &srvRelationUnitsWatcher{ watcher: watcher, id: id, @@ -241,10 +270,18 @@ func newMachineStorageIdsWatcher( if !isAgent(auth) { return nil, common.ErrPerm } - watcher, ok := resources.Get(id).(state.StringsWatcher) + + r := resources.Get(id) + if r == nil { + logger.Errorf("watcher %q not found", id) + panic(common.ErrUnknownWatcher) + } + watcher, ok := r.(state.StringsWatcher) if !ok { - return nil, common.ErrUnknownWatcher + logger.Errorf("unknown watcher type for %q: %#T, expected state.StringsWatcher", id, r) + panic(common.ErrUnknownWatcher) } + return &srvMachineStorageIdsWatcher{ watcher: watcher, id: id, diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index 54afc373bc0..1d9e641dd3b 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -592,13 +592,6 @@ func (a *MachineAgent) APIWorker() (worker.Worker, error) { a.startWorkerAfterUpgrade(runner, "api-post-upgrade", func() (worker.Worker, error) { return a.postUpgradeAPIWorker(st, agentConfig, entity) }) - a.startWorkerAfterUpgrade(runner, "converter", func() (worker.Worker, error) { - return converter.NewConverter( - st.Converter(), - agentConfig, - ), nil - }) - return cmdutil.NewCloseWorker(logger, runner, st), nil // Note: a worker.Runner is itself a worker.Worker. } @@ -656,9 +649,11 @@ func (a *MachineAgent) postUpgradeAPIWorker( return cmdutil.NewRsyslogConfigWorker(st.Rsyslog(), agentConfig, rsyslogMode) }) - runner.StartWorker("converter", func() (worker.Worker, error) { - return converter.NewConverter(st.Converter(), agentConfig), nil - }) + if !isEnvironManager { + runner.StartWorker("converter", func() (worker.Worker, error) { + return converter.NewConverter(entity, st.Converter(), agentConfig), nil + }) + } // TODO(wallyworld) - we don't want the storage workers running yet, even with feature flag. // Will be enabled in a followup branch. diff --git a/rpc/server.go b/rpc/server.go index e76ba3068fd..8b59ea6082c 100644 --- a/rpc/server.go +++ b/rpc/server.go @@ -549,6 +549,7 @@ func (conn *Conn) bindRequest(hdr *Header) (boundRequest, error) { // runRequest runs the given request and sends the reply. func (conn *Conn) runRequest(req boundRequest, arg reflect.Value, startTime time.Time) { defer conn.srvPending.Done() + logger.Infof("Request Args: %#v", arg) rv, err := req.Call(req.hdr.Request.Id, arg) if err != nil { err = conn.writeErrorResponse(&req.hdr, req.transformErrors(err), startTime) diff --git a/worker/converter/converter.go b/worker/converter/converter.go index cd62aebdf83..f6880870e53 100644 --- a/worker/converter/converter.go +++ b/worker/converter/converter.go @@ -4,13 +4,15 @@ package converter import ( + "github.com/juju/errors" "github.com/juju/loggo" "github.com/juju/names" - "launchpad.net/tomb" + "github.com/juju/utils" "github.com/juju/juju/agent" "github.com/juju/juju/api/converter" "github.com/juju/juju/api/watcher" + "github.com/juju/juju/state/multiwatcher" "github.com/juju/juju/worker" ) @@ -18,37 +20,53 @@ var logger = loggo.GetLogger("juju.worker.converter") // Converter ... type Converter struct { - tomb tomb.Tomb - st *converter.State - tag names.Tag + st *converter.State + ent Entity + config agent.Config } type converterState interface { WatchForJobsChanges(names.MachineTag) (watcher.NotifyWatcher, error) } +type Entity interface { + SetPassword(string) error + Jobs() []multiwatcher.MachineJob +} + // NewConverter ... func NewConverter( + ent Entity, st *converter.State, agentConfig agent.Config, ) worker.Worker { return worker.NewNotifyWorker(&Converter{ - st: st, - tag: agentConfig.Tag(), + ent: ent, + st: st, + config: agentConfig, }) } func (c *Converter) SetUp() (watcher.NotifyWatcher, error) { logger.Infof("Setting up Converter watcher.") - watcher, err := c.st.WatchForJobsChanges(c.tag.String()) - if err != nil { - logger.Errorf("%q", err) - } - return watcher, err + return c.st.WatchForJobsChanges(c.config.Tag().String()) } -func (c *Converter) Handle() (err error) { - logger.Infof("Jobs for %q have been changed. Check for ManageJob. Start conversion.", c.tag.String()) +func (c *Converter) Handle() error { + logger.Infof("Jobs for %q have been changed. Check for ManageJob.", c.config.Tag()) + for _, job := range c.ent.Jobs() { + if job.NeedsState() { + logger.Infof("Converting %q to a state server", c.config.Tag()) + pw, err := utils.RandomPassword() + if err != nil { + return errors.Trace(err) + } + if err := c.ent.SetPassword(pw); err != nil { + return errors.Trace(err) + } + // change agentConfig too? + } + } return nil } From e24e8669d21b2e357d493b30dce10a84be2275f8 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Wed, 18 Mar 2015 16:08:47 -0400 Subject: [PATCH 08/33] remove panics --- apiserver/converter/converter.go | 2 ++ apiserver/watcher.go | 20 ++++++++++---------- worker/converter/converter.go | 2 +- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go index 4e45b4305a7..4af74f369dd 100644 --- a/apiserver/converter/converter.go +++ b/apiserver/converter/converter.go @@ -46,12 +46,14 @@ func (c *ConverterAPI) WatchForJobsChanges(args params.Entities) (params.NotifyW Results: make([]params.NotifyWatchResult, len(args.Entities)), } for i, agent := range args.Entities { + logger.Infof("Watching on entity %#v", agent) tag, err := names.ParseMachineTag(agent.Tag) if err != nil { return params.NotifyWatchResults{}, errors.Trace(err) } err = common.ErrPerm if c.authorizer.AuthOwner(tag) { + logger.Infof("Watching for jobs on %#v", tag) watch := c.st.WatchForJobsChanges(tag) // Consume the initial event. Technically, API // calls to Watch 'transmit' the initial event diff --git a/apiserver/watcher.go b/apiserver/watcher.go index 5cace1811b6..df9658a3a2c 100644 --- a/apiserver/watcher.go +++ b/apiserver/watcher.go @@ -42,12 +42,12 @@ func newClientAllWatcher(st *state.State, resources *common.Resources, auth comm r := resources.Get(id) if r == nil { logger.Errorf("watcher %q not found", id) - panic(errors.Trace(common.ErrUnknownWatcher)) + return nil, errors.Trace(common.ErrUnknownWatcher) } watcher, ok := r.(*state.Multiwatcher) if !ok { logger.Errorf("unknown watcher type for %q: %T, expected state.Multiwatcher", id, r) - panic(common.ErrUnknownWatcher) + return nil, errors.Trace(common.ErrUnknownWatcher) } return &srvClientAllWatcher{ watcher: watcher, @@ -95,12 +95,12 @@ func newNotifyWatcher(st *state.State, resources *common.Resources, auth common. r := resources.Get(id) if r == nil { logger.Errorf("watcher %q not found", id) - panic(common.ErrUnknownWatcher) + return nil, errors.Trace(common.ErrUnknownWatcher) } watcher, ok := r.(state.NotifyWatcher) if !ok { logger.Errorf("unknown watcher type for %q: %#T, expected state.NotifyWatcher", id, r) - panic(common.ErrUnknownWatcher) + return nil, errors.Trace(common.ErrUnknownWatcher) } return &srvNotifyWatcher{ @@ -147,12 +147,12 @@ func newStringsWatcher(st *state.State, resources *common.Resources, auth common r := resources.Get(id) if r == nil { logger.Errorf("watcher %q not found", id) - panic(common.ErrUnknownWatcher) + return nil, errors.Trace(common.ErrUnknownWatcher) } watcher, ok := r.(state.StringsWatcher) if !ok { logger.Errorf("unknown watcher type for %q: %#T, expected state.StringsWatcher", id, r) - panic(common.ErrUnknownWatcher) + return nil, errors.Trace(common.ErrUnknownWatcher) } return &srvStringsWatcher{ watcher: watcher, @@ -199,12 +199,12 @@ func newRelationUnitsWatcher(st *state.State, resources *common.Resources, auth r := resources.Get(id) if r == nil { logger.Errorf("watcher %q not found", id) - panic(common.ErrUnknownWatcher) + return nil, errors.Trace(common.ErrUnknownWatcher) } watcher, ok := r.(state.RelationUnitsWatcher) if !ok { logger.Errorf("unknown watcher type for %q: %#T, expected state.RelationUnitsWatcher", id, r) - panic(common.ErrUnknownWatcher) + return nil, errors.Trace(common.ErrUnknownWatcher) } return &srvRelationUnitsWatcher{ @@ -274,12 +274,12 @@ func newMachineStorageIdsWatcher( r := resources.Get(id) if r == nil { logger.Errorf("watcher %q not found", id) - panic(common.ErrUnknownWatcher) + return nil, errors.Trace(common.ErrUnknownWatcher) } watcher, ok := r.(state.StringsWatcher) if !ok { logger.Errorf("unknown watcher type for %q: %#T, expected state.StringsWatcher", id, r) - panic(common.ErrUnknownWatcher) + return nil, errors.Trace(common.ErrUnknownWatcher) } return &srvMachineStorageIdsWatcher{ diff --git a/worker/converter/converter.go b/worker/converter/converter.go index f6880870e53..5301c599c95 100644 --- a/worker/converter/converter.go +++ b/worker/converter/converter.go @@ -48,7 +48,7 @@ func NewConverter( } func (c *Converter) SetUp() (watcher.NotifyWatcher, error) { - logger.Infof("Setting up Converter watcher.") + logger.Infof("Setting up Converter watcher for %s.", c.config.Tag().String()) return c.st.WatchForJobsChanges(c.config.Tag().String()) } From 12ad8d64f26aca5541bbd0f0c10cd8b45bcbcc75 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Thu, 19 Mar 2015 00:01:18 -0400 Subject: [PATCH 09/33] use correct type --- api/converter/converter.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/converter/converter.go b/api/converter/converter.go index d49a2ef9de2..7e72eed8e4a 100644 --- a/api/converter/converter.go +++ b/api/converter/converter.go @@ -22,7 +22,7 @@ func NewState(caller base.APICaller) *State { } func (c *State) WatchForJobsChanges(tag string) (watcher.NotifyWatcher, error) { - var result params.NotifyWatchResult + var result params.NotifyWatchResults args := params.Entities{ Entities: []params.Entity{{Tag: tag}}, } @@ -32,5 +32,5 @@ func (c *State) WatchForJobsChanges(tag string) (watcher.NotifyWatcher, error) { return nil, err } - return watcher.NewNotifyWatcher(c.facade.RawAPICaller(), result), nil + return watcher.NewNotifyWatcher(c.facade.RawAPICaller(), result.Results[0]), nil } From a00514f732e7e4df9a91a08d2864403814d7fa18 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 19 Mar 2015 15:20:20 -0400 Subject: [PATCH 10/33] WIP --- api/state.go | 6 ------ apiserver/converter/converter.go | 15 ++++++++++++++- cmd/jujud/agent/machine.go | 2 +- state/watcher.go | 6 ------ worker/converter/converter.go | 28 ++++++++++++++++++---------- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/api/state.go b/api/state.go index ee553371313..dca26c4517d 100644 --- a/api/state.go +++ b/api/state.go @@ -13,7 +13,6 @@ import ( "github.com/juju/juju/api/agent" "github.com/juju/juju/api/base" "github.com/juju/juju/api/charmrevisionupdater" - "github.com/juju/juju/api/converter" "github.com/juju/juju/api/deployer" "github.com/juju/juju/api/diskformatter" "github.com/juju/juju/api/diskmanager" @@ -319,8 +318,3 @@ func (st *State) CharmRevisionUpdater() *charmrevisionupdater.State { func (st *State) Rsyslog() *rsyslog.State { return rsyslog.NewState(st) } - -// Converter returns access to the Converter API -func (st *State) Converter() *converter.State { - return converter.NewState(st) -} diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go index 4af74f369dd..3daf2760428 100644 --- a/apiserver/converter/converter.go +++ b/apiserver/converter/converter.go @@ -41,6 +41,14 @@ func NewConverterAPI( }, nil } +func (c *ConverterAPI) getMachine(tag names.Tag) (*state.Machine, error) { + entity, err := c.st.FindEntity(tag) + if err != nil { + return nil, err + } + return entity.(*state.Machine), nil +} + func (c *ConverterAPI) WatchForJobsChanges(args params.Entities) (params.NotifyWatchResults, error) { result := params.NotifyWatchResults{ Results: make([]params.NotifyWatchResult, len(args.Entities)), @@ -54,7 +62,12 @@ func (c *ConverterAPI) WatchForJobsChanges(args params.Entities) (params.NotifyW err = common.ErrPerm if c.authorizer.AuthOwner(tag) { logger.Infof("Watching for jobs on %#v", tag) - watch := c.st.WatchForJobsChanges(tag) + machine, err := c.getMachine(tag) + if err != nil { + return params.NotifyWatchResults{}, errors.Trace(err) + } + + watch := machine.Watch() // Consume the initial event. Technically, API // calls to Watch 'transmit' the initial event // in the Watch response. But NotifyWatchers diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index 1d9e641dd3b..bece8024664 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -651,7 +651,7 @@ func (a *MachineAgent) postUpgradeAPIWorker( if !isEnvironManager { runner.StartWorker("converter", func() (worker.Worker, error) { - return converter.NewConverter(entity, st.Converter(), agentConfig), nil + return converter.NewConverter(entity, st.Machiner(), agentConfig), nil }) } diff --git a/state/watcher.go b/state/watcher.go index 3a85314d2ec..b08463b9f50 100644 --- a/state/watcher.go +++ b/state/watcher.go @@ -1370,12 +1370,6 @@ func (st *State) WatchAPIHostPorts() NotifyWatcher { return newEntityWatcher(st, stateServersC, apiHostPortsKey) } -// WatchForJobsChanges returns a NotifyWatcher that notifies -// when the set of jobs for machine change. -func (st *State) WatchForJobsChanges(m names.MachineTag) NotifyWatcher { - return newEntityWatcher(st, jobsC, m.Id()) -} - // WatchVolumeAttachment returns a watcher for observing changes // to a volume attachment. func (st *State) WatchVolumeAttachment(m names.MachineTag, v names.VolumeTag) NotifyWatcher { diff --git a/worker/converter/converter.go b/worker/converter/converter.go index 5301c599c95..0c60ea8a5fe 100644 --- a/worker/converter/converter.go +++ b/worker/converter/converter.go @@ -10,8 +10,9 @@ import ( "github.com/juju/utils" "github.com/juju/juju/agent" - "github.com/juju/juju/api/converter" + "github.com/juju/juju/api/machiner" "github.com/juju/juju/api/watcher" + "github.com/juju/juju/apiserver/params" "github.com/juju/juju/state/multiwatcher" "github.com/juju/juju/worker" ) @@ -20,13 +21,11 @@ var logger = loggo.GetLogger("juju.worker.converter") // Converter ... type Converter struct { - st *converter.State - ent Entity - config agent.Config -} - -type converterState interface { - WatchForJobsChanges(names.MachineTag) (watcher.NotifyWatcher, error) + st *machiner.State + ent Entity + config agent.Config + tag names.MachineTag + machine *machiner.Machine } type Entity interface { @@ -37,19 +36,28 @@ type Entity interface { // NewConverter ... func NewConverter( ent Entity, - st *converter.State, + st *machiner.State, agentConfig agent.Config, ) worker.Worker { return worker.NewNotifyWorker(&Converter{ ent: ent, st: st, config: agentConfig, + tag: agentConfig.Tag().(names.MachineTag), }) } func (c *Converter) SetUp() (watcher.NotifyWatcher, error) { logger.Infof("Setting up Converter watcher for %s.", c.config.Tag().String()) - return c.st.WatchForJobsChanges(c.config.Tag().String()) + m, err := c.st.Machine(c.tag) + if params.IsCodeNotFoundOrCodeUnauthorized(err) { + return nil, worker.ErrTerminateAgent + } else if err != nil { + return nil, errors.Trace(err) + } + + c.machine = m + return m.Watch() } func (c *Converter) Handle() error { From 04f9b1177243c579b04c6f0fb572fc9475617d78 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 19 Mar 2015 16:35:56 -0400 Subject: [PATCH 11/33] WIP: Handle firing with machine.Watch --- api/converter/converter.go | 5 +++++ api/state.go | 6 ++++++ apiserver/converter/converter.go | 4 ++-- cmd/jujud/agent/machine.go | 2 +- worker/converter/converter.go | 33 +++++++++++--------------------- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/api/converter/converter.go b/api/converter/converter.go index 7e72eed8e4a..650664f93cd 100644 --- a/api/converter/converter.go +++ b/api/converter/converter.go @@ -4,11 +4,15 @@ package converter import ( + "github.com/juju/loggo" + "github.com/juju/juju/api/base" "github.com/juju/juju/api/watcher" "github.com/juju/juju/apiserver/params" ) +var logger = loggo.GetLogger("juju.apiserver.converter") + const converterAPI = "Converter" // Converter provides common client-side API @@ -27,6 +31,7 @@ func (c *State) WatchForJobsChanges(tag string) (watcher.NotifyWatcher, error) { Entities: []params.Entity{{Tag: tag}}, } + logger.Infof("calling facade WatchForJobsChanges") err := c.facade.FacadeCall("WatchForJobsChanges", args, &result) if err != nil { return nil, err diff --git a/api/state.go b/api/state.go index dca26c4517d..ee553371313 100644 --- a/api/state.go +++ b/api/state.go @@ -13,6 +13,7 @@ import ( "github.com/juju/juju/api/agent" "github.com/juju/juju/api/base" "github.com/juju/juju/api/charmrevisionupdater" + "github.com/juju/juju/api/converter" "github.com/juju/juju/api/deployer" "github.com/juju/juju/api/diskformatter" "github.com/juju/juju/api/diskmanager" @@ -318,3 +319,8 @@ func (st *State) CharmRevisionUpdater() *charmrevisionupdater.State { func (st *State) Rsyslog() *rsyslog.State { return rsyslog.NewState(st) } + +// Converter returns access to the Converter API +func (st *State) Converter() *converter.State { + return converter.NewState(st) +} diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go index 3daf2760428..8e1b57dc4a7 100644 --- a/apiserver/converter/converter.go +++ b/apiserver/converter/converter.go @@ -54,14 +54,14 @@ func (c *ConverterAPI) WatchForJobsChanges(args params.Entities) (params.NotifyW Results: make([]params.NotifyWatchResult, len(args.Entities)), } for i, agent := range args.Entities { - logger.Infof("Watching on entity %#v", agent) + logger.Infof("watching on entity %#v", agent) tag, err := names.ParseMachineTag(agent.Tag) if err != nil { return params.NotifyWatchResults{}, errors.Trace(err) } err = common.ErrPerm if c.authorizer.AuthOwner(tag) { - logger.Infof("Watching for jobs on %#v", tag) + logger.Infof("watching for jobs on %#v", tag) machine, err := c.getMachine(tag) if err != nil { return params.NotifyWatchResults{}, errors.Trace(err) diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index bece8024664..1d9e641dd3b 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -651,7 +651,7 @@ func (a *MachineAgent) postUpgradeAPIWorker( if !isEnvironManager { runner.StartWorker("converter", func() (worker.Worker, error) { - return converter.NewConverter(entity, st.Machiner(), agentConfig), nil + return converter.NewConverter(entity, st.Converter(), agentConfig), nil }) } diff --git a/worker/converter/converter.go b/worker/converter/converter.go index 0c60ea8a5fe..59ff4f6871e 100644 --- a/worker/converter/converter.go +++ b/worker/converter/converter.go @@ -6,13 +6,11 @@ package converter import ( "github.com/juju/errors" "github.com/juju/loggo" - "github.com/juju/names" "github.com/juju/utils" "github.com/juju/juju/agent" - "github.com/juju/juju/api/machiner" + "github.com/juju/juju/api/converter" "github.com/juju/juju/api/watcher" - "github.com/juju/juju/apiserver/params" "github.com/juju/juju/state/multiwatcher" "github.com/juju/juju/worker" ) @@ -21,11 +19,9 @@ var logger = loggo.GetLogger("juju.worker.converter") // Converter ... type Converter struct { - st *machiner.State - ent Entity - config agent.Config - tag names.MachineTag - machine *machiner.Machine + st *converter.State + ent Entity + config agent.Config } type Entity interface { @@ -36,35 +32,28 @@ type Entity interface { // NewConverter ... func NewConverter( ent Entity, - st *machiner.State, + st *converter.State, agentConfig agent.Config, ) worker.Worker { return worker.NewNotifyWorker(&Converter{ ent: ent, st: st, config: agentConfig, - tag: agentConfig.Tag().(names.MachineTag), }) } func (c *Converter) SetUp() (watcher.NotifyWatcher, error) { - logger.Infof("Setting up Converter watcher for %s.", c.config.Tag().String()) - m, err := c.st.Machine(c.tag) - if params.IsCodeNotFoundOrCodeUnauthorized(err) { - return nil, worker.ErrTerminateAgent - } else if err != nil { - return nil, errors.Trace(err) - } - - c.machine = m - return m.Watch() + logger.Infof("setting up Converter watcher for %s", c.config.Tag().String()) + return c.st.WatchForJobsChanges(c.config.Tag().String()) } func (c *Converter) Handle() error { - logger.Infof("Jobs for %q have been changed. Check for ManageJob.", c.config.Tag()) + logger.Infof("environment for %q has been changed", c.config.Tag()) for _, job := range c.ent.Jobs() { + logger.Infof("job found: %q", job) + logger.Infof("job details: #v", job) if job.NeedsState() { - logger.Infof("Converting %q to a state server", c.config.Tag()) + logger.Infof("converting %q to a state server", c.config.Tag()) pw, err := utils.RandomPassword() if err != nil { return errors.Trace(err) From 43644c2d4c56c9364623cf78779de324199f1f5e Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 19 Mar 2015 17:47:55 -0400 Subject: [PATCH 12/33] WIP: added Jobs call to converter --- api/converter/converter.go | 14 ++++++++++++++ apiserver/converter/converter.go | 30 ++++++++++++++++++++++++++++++ apiserver/params/params.go | 8 ++++++++ worker/converter/converter.go | 9 ++++++++- 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/api/converter/converter.go b/api/converter/converter.go index 650664f93cd..30a55f2e48f 100644 --- a/api/converter/converter.go +++ b/api/converter/converter.go @@ -39,3 +39,17 @@ func (c *State) WatchForJobsChanges(tag string) (watcher.NotifyWatcher, error) { return watcher.NewNotifyWatcher(c.facade.RawAPICaller(), result.Results[0]), nil } + +func (c *State) Jobs(tag string) (*params.JobsResult, error) { + var results params.JobsResults + args := params.Entities{ + Entities: []params.Entity{{Tag: tag}}, + } + + logger.Infof("calling facade Jobs") + err := c.facade.FacadeCall("Jobs", args, &results) + if err != nil { + return nil, err + } + return &results.Results[0], nil +} diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go index 8e1b57dc4a7..1e4499dbdfe 100644 --- a/apiserver/converter/converter.go +++ b/apiserver/converter/converter.go @@ -11,6 +11,7 @@ import ( "github.com/juju/juju/apiserver/common" "github.com/juju/juju/apiserver/params" "github.com/juju/juju/state" + "github.com/juju/juju/state/multiwatcher" "github.com/juju/juju/state/watcher" ) @@ -49,6 +50,35 @@ func (c *ConverterAPI) getMachine(tag names.Tag) (*state.Machine, error) { return entity.(*state.Machine), nil } +func (c *ConverterAPI) Jobs(args params.Entities) (params.JobsResults, error) { + result := params.JobsResults{ + Results: make([]params.JobsResult, len(args.Entities)), + } + + for i, agent := range args.Entities { + tag, err := names.ParseMachineTag(agent.Tag) + if err != nil { + return params.JobsResults{}, errors.Trace(err) + } + + err = common.ErrPerm + if c.authorizer.AuthOwner(tag) { + logger.Infof("watching for jobs on %#v", tag) + machine, err := c.getMachine(tag) + if err != nil { + return params.JobsResults{}, errors.Trace(err) + } + machineJobs := machine.Jobs() + jobs := make([]multiwatcher.MachineJob, len(machineJobs)) + for i, job := range machineJobs { + jobs[i] = job.ToParams() + } + result.Results[i].Jobs = jobs + } + } + return result, nil +} + func (c *ConverterAPI) WatchForJobsChanges(args params.Entities) (params.NotifyWatchResults, error) { result := params.NotifyWatchResults{ Results: make([]params.NotifyWatchResult, len(args.Entities)), diff --git a/apiserver/params/params.go b/apiserver/params/params.go index 84ed40e027b..86362414e4f 100644 --- a/apiserver/params/params.go +++ b/apiserver/params/params.go @@ -543,6 +543,14 @@ type RsyslogConfigResults struct { Results []RsyslogConfigResult } +type JobsResult struct { + Jobs []multiwatcher.MachineJob `json:"Jobs"` +} + +type JobsResults struct { + Results []JobsResult +} + // DistributionGroupResult contains the result of // the DistributionGroup provisioner API call. type DistributionGroupResult struct { diff --git a/worker/converter/converter.go b/worker/converter/converter.go index 59ff4f6871e..f251a344b1d 100644 --- a/worker/converter/converter.go +++ b/worker/converter/converter.go @@ -49,7 +49,13 @@ func (c *Converter) SetUp() (watcher.NotifyWatcher, error) { func (c *Converter) Handle() error { logger.Infof("environment for %q has been changed", c.config.Tag()) - for _, job := range c.ent.Jobs() { + + jobs, err := c.st.Jobs(c.config.Tag().String()) + if err != nil { + return errors.Trace(err) + } + + for _, job := range jobs.Jobs { logger.Infof("job found: %q", job) logger.Infof("job details: #v", job) if job.NeedsState() { @@ -62,6 +68,7 @@ func (c *Converter) Handle() error { return errors.Trace(err) } // change agentConfig too? + // restart juju } } return nil From e8815526809c4c99c06ce415d40ea0fa48877b8d Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 19 Mar 2015 17:57:09 -0400 Subject: [PATCH 13/33] WIP: reboot machine, for now.. --- worker/converter/converter.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/worker/converter/converter.go b/worker/converter/converter.go index f251a344b1d..aa3862710e3 100644 --- a/worker/converter/converter.go +++ b/worker/converter/converter.go @@ -68,7 +68,9 @@ func (c *Converter) Handle() error { return errors.Trace(err) } // change agentConfig too? - // restart juju + + // restart juju, probably a better way? + return worker.ErrRebootMachine } } return nil From 33363d3f9351b64dd49fd335eb2287d3ac409521 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Mon, 23 Mar 2015 12:12:21 -0400 Subject: [PATCH 14/33] return converted machines --- apiserver/highavailability/highavailability.go | 1 + apiserver/params/params.go | 1 + cmd/juju/ensureavailability.go | 7 ++++++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/apiserver/highavailability/highavailability.go b/apiserver/highavailability/highavailability.go index c3fe1896bf5..7638b11afca 100644 --- a/apiserver/highavailability/highavailability.go +++ b/apiserver/highavailability/highavailability.go @@ -73,6 +73,7 @@ func stateServersChanges(change state.StateServersChanges) params.StateServersCh Removed: machineIdsToTags(change.Removed...), Promoted: machineIdsToTags(change.Promoted...), Demoted: machineIdsToTags(change.Demoted...), + Converted: machineIdsToTags(change.Converted...), } } diff --git a/apiserver/params/params.go b/apiserver/params/params.go index 86362414e4f..05afe4024af 100644 --- a/apiserver/params/params.go +++ b/apiserver/params/params.go @@ -663,6 +663,7 @@ type StateServersChanges struct { Removed []string `json:"removed,omitempty"` Promoted []string `json:"promoted,omitempty"` Demoted []string `json:"demoted,omitempty"` + Converted []string `json:"converted,omitempty"` } // FindToolsParams defines parameters for the FindTools method. diff --git a/cmd/juju/ensureavailability.go b/cmd/juju/ensureavailability.go index f3c9923a5df..97394337068 100644 --- a/cmd/juju/ensureavailability.go +++ b/cmd/juju/ensureavailability.go @@ -103,6 +103,10 @@ func formatSimple(value interface{}) ([]byte, error) { "demoting machines %s\n", ensureAvailabilityResult.Demoted, }, + { + "converting machines %s\n", + ensureAvailabilityResult.Converted, + }, } { if len(machineList.list) == 0 { continue @@ -146,7 +150,6 @@ func (c *EnsureAvailabilityCommand) Init(args []string) error { c.Placement = make([]string, len(placementSpecs)) for i, spec := range placementSpecs { p, err := instance.ParsePlacement(strings.TrimSpace(spec)) - fmt.Println(p, err) if err == nil && p.Scope == instance.MachineScope { // targeting machines is ok c.Placement[i] = p.String() @@ -168,6 +171,7 @@ type availabilityInfo struct { Added []string `json:"added,omitempty" yaml:"added,flow,omitempty"` Promoted []string `json:"promoted,omitempty" yaml:"promoted,flow,omitempty"` Demoted []string `json:"demoted,omitempty" yaml:"demoted,flow,omitempty"` + Converted []string `json:"converted,omitempty" yaml:"converted,flow,omitempty"` } // EnsureAvailabilityClient defines the methods @@ -219,6 +223,7 @@ func (c *EnsureAvailabilityCommand) Run(ctx *cmd.Context) error { Maintained: machineTagsToIds(ensureAvailabilityResult.Maintained...), Promoted: machineTagsToIds(ensureAvailabilityResult.Promoted...), Demoted: machineTagsToIds(ensureAvailabilityResult.Demoted...), + Converted: machineTagsToIds(ensureAvailabilityResult.Converted...), } return c.out.Write(ctx, result) } From c6553e386c1fabb5d2da33e693c591a860d9fb91 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Wed, 25 Mar 2015 10:30:36 -0400 Subject: [PATCH 15/33] implement restart and hack stateworker to restart if it fails --- agent/agent.go | 11 ++++++++++ cmd/juju/ensureavailability.go | 8 +++---- cmd/jujud/agent/machine.go | 27 ++++++++++++++++++++++-- worker/converter/converter.go | 38 +++++++++++++++++++++------------- worker/runner.go | 2 +- 5 files changed, 65 insertions(+), 21 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 045296d1423..6df080ed0d0 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -189,6 +189,10 @@ type ConfigSetterOnly interface { // SetStateServingInfo sets the information needed // to run a state server SetStateServingInfo(info params.StateServingInfo) + + // SetStateAddresses sets the addresses of state machines this machine will + // connect to. + SetStateAddresses(addrs []string) } type ConfigWriter interface { @@ -494,6 +498,13 @@ func (c *configInternal) SetAPIHostPorts(servers [][]network.HostPort) { c.apiDetails.addresses = addrs } +func (c *configInternal) SetStateAddresses(servers []string) { + if c.stateDetails == nil { + c.stateDetails = &connectionDetails{} + } + c.stateDetails.addresses = servers +} + func (c *configInternal) SetValue(key, value string) { if value == "" { delete(c.values, key) diff --git a/cmd/juju/ensureavailability.go b/cmd/juju/ensureavailability.go index 97394337068..c65a05d83bb 100644 --- a/cmd/juju/ensureavailability.go +++ b/cmd/juju/ensureavailability.go @@ -92,19 +92,19 @@ func formatSimple(value interface{}) ([]byte, error) { ensureAvailabilityResult.Added, }, { - "removing machines %s\n", + "removing machines: %s\n", ensureAvailabilityResult.Removed, }, { - "promoting machines %s\n", + "promoting machines: %s\n", ensureAvailabilityResult.Promoted, }, { - "demoting machines %s\n", + "demoting machines: %s\n", ensureAvailabilityResult.Demoted, }, { - "converting machines %s\n", + "converting machines: %s\n", ensureAvailabilityResult.Converted, }, } { diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index 1d9e641dd3b..698fc755163 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -358,6 +358,7 @@ func (a *MachineAgent) Run(*cmd.Context) error { // At this point, all workers will have been configured to start close(a.workersStarted) err := a.runner.Wait() + logger.Infof("Machine agent runner ended with %s", errors.Details(err)) switch err { case worker.ErrTerminateAgent: err = a.uninstallAgent(agentConfig) @@ -401,6 +402,11 @@ func (a *MachineAgent) executeRebootOrShutdown(action params.RebootAction) error return worker.ErrRebootMachine } +func (a *MachineAgent) RestartService() error { + name := a.CurrentConfig().Value(agent.AgentServiceName) + return service.Restart(name) +} + func (a *MachineAgent) ChangeConfig(mutate AgentConfigMutator) error { err := a.AgentConfigWriter.ChangeConfig(mutate) a.configChangedVal.Set(struct{}{}) @@ -651,7 +657,18 @@ func (a *MachineAgent) postUpgradeAPIWorker( if !isEnvironManager { runner.StartWorker("converter", func() (worker.Worker, error) { - return converter.NewConverter(entity, st.Converter(), agentConfig), nil + getEntity := func() (converter.Entity, error) { + _, entity, err := OpenAPIState(agentConfig, a) + return entity, err + } + setPW := func(pw string) { + a.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { + config.SetPassword(pw) + config.SetStateAddresses([]string{"localhost:37017"}) + return nil + }) + } + return converter.NewConverter(getEntity, setPW, a.RestartService, st.Converter(), agentConfig), nil }) } @@ -865,7 +882,13 @@ func (a *MachineAgent) updateSupportedContainers( // StateWorker returns a worker running all the workers that require // a *state.State connection. -func (a *MachineAgent) StateWorker() (worker.Worker, error) { +func (a *MachineAgent) StateWorker() (_ worker.Worker, err error) { + defer func() { + if err != nil { + logger.Errorf("State worker exiting with error %s\nRestarting jujud.", errors.Details(err)) + a.RestartService() + } + }() agentConfig := a.CurrentConfig() // Start MongoDB server and dial. diff --git a/worker/converter/converter.go b/worker/converter/converter.go index aa3862710e3..13868e93b5c 100644 --- a/worker/converter/converter.go +++ b/worker/converter/converter.go @@ -19,9 +19,11 @@ var logger = loggo.GetLogger("juju.worker.converter") // Converter ... type Converter struct { - st *converter.State - ent Entity - config agent.Config + st *converter.State + getEnt func() (Entity, error) + restart func() error + setPassword func(pw string) + config agent.Config } type Entity interface { @@ -31,14 +33,18 @@ type Entity interface { // NewConverter ... func NewConverter( - ent Entity, + getEnt func() (Entity, error), + setPW func(pw string), + restart func() error, st *converter.State, agentConfig agent.Config, ) worker.Worker { return worker.NewNotifyWorker(&Converter{ - ent: ent, - st: st, - config: agentConfig, + getEnt: getEnt, + setPassword: setPW, + restart: restart, + st: st, + config: agentConfig, }) } @@ -56,21 +62,25 @@ func (c *Converter) Handle() error { } for _, job := range jobs.Jobs { - logger.Infof("job found: %q", job) - logger.Infof("job details: #v", job) if job.NeedsState() { - logger.Infof("converting %q to a state server", c.config.Tag()) + logger.Warningf("converting %q to a state server", c.config.Tag()) pw, err := utils.RandomPassword() if err != nil { return errors.Trace(err) } - if err := c.ent.SetPassword(pw); err != nil { + ent, err := c.getEnt() + if err != nil { + logger.Errorf("error from getEntity: %s", errors.Details(err)) + return errors.Trace(err) + } + + if err := ent.SetPassword(pw); err != nil { + logger.Errorf("error trying to set password for machine agent: %s", errors.Details(err)) return errors.Trace(err) } - // change agentConfig too? + c.setPassword(pw) - // restart juju, probably a better way? - return worker.ErrRebootMachine + return c.restart() } } return nil diff --git a/worker/runner.go b/worker/runner.go index eb6b6cc47af..95b90e64004 100644 --- a/worker/runner.go +++ b/worker/runner.go @@ -4,9 +4,9 @@ package worker import ( - "errors" "time" + "github.com/juju/errors" "launchpad.net/tomb" ) From 9335e9d5eddb9488fa3245b83bbdd77c16c135a6 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Wed, 25 Mar 2015 15:11:03 -0400 Subject: [PATCH 16/33] remove hack --- cmd/jujud/agent/machine.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index 698fc755163..52ac51e6027 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -883,12 +883,6 @@ func (a *MachineAgent) updateSupportedContainers( // StateWorker returns a worker running all the workers that require // a *state.State connection. func (a *MachineAgent) StateWorker() (_ worker.Worker, err error) { - defer func() { - if err != nil { - logger.Errorf("State worker exiting with error %s\nRestarting jujud.", errors.Details(err)) - a.RestartService() - } - }() agentConfig := a.CurrentConfig() // Start MongoDB server and dial. From 38ed548758b5c5bdb55dd6e570cfe0e75aa8bb5a Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Wed, 25 Mar 2015 16:39:44 -0400 Subject: [PATCH 17/33] remove some unused changes --- apiserver/params/constants.go | 4 ---- cmd/jujud/agent/machine.go | 3 +-- rpc/server.go | 1 - worker/reboot/reboot.go | 11 +++-------- worker/runner.go | 2 +- 5 files changed, 5 insertions(+), 16 deletions(-) diff --git a/apiserver/params/constants.go b/apiserver/params/constants.go index f8f77667122..ed58d273af6 100644 --- a/apiserver/params/constants.go +++ b/apiserver/params/constants.go @@ -19,10 +19,6 @@ const ( // happens when running inside a container, and a hook on the parent // machine requests a reboot ShouldShutdown RebootAction = "shutdown" - // ShouldRestart instructs a machine agent to restart jujud. This usually - // happens after an upgrade or conversion to state server using - // ensure-availability. - ShouldRestart RebootAction = "restart" ) // ResolvedMode describes the way state transition errors diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index e78c7661f6a..77e07a9fc30 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -410,7 +410,6 @@ func (a *MachineAgent) Run(*cmd.Context) error { // At this point, all workers will have been configured to start close(a.workersStarted) err := a.runner.Wait() - logger.Infof("Machine agent runner ended with %s", errors.Details(err)) switch err { case worker.ErrTerminateAgent: err = a.uninstallAgent(agentConfig) @@ -929,7 +928,7 @@ func (a *MachineAgent) updateSupportedContainers( // StateWorker returns a worker running all the workers that require // a *state.State connection. -func (a *MachineAgent) StateWorker() (_ worker.Worker, err error) { +func (a *MachineAgent) StateWorker() (worker.Worker, error) { agentConfig := a.CurrentConfig() // Start MongoDB server and dial. diff --git a/rpc/server.go b/rpc/server.go index 8b59ea6082c..e76ba3068fd 100644 --- a/rpc/server.go +++ b/rpc/server.go @@ -549,7 +549,6 @@ func (conn *Conn) bindRequest(hdr *Header) (boundRequest, error) { // runRequest runs the given request and sends the reply. func (conn *Conn) runRequest(req boundRequest, arg reflect.Value, startTime time.Time) { defer conn.srvPending.Done() - logger.Infof("Request Args: %#v", arg) rv, err := req.Call(req.hdr.Request.Id, arg) if err != nil { err = conn.writeErrorResponse(&req.hdr, req.transformErrors(err), startTime) diff --git a/worker/reboot/reboot.go b/worker/reboot/reboot.go index 3ab214fa41c..b538c301622 100644 --- a/worker/reboot/reboot.go +++ b/worker/reboot/reboot.go @@ -17,16 +17,14 @@ import ( var logger = loggo.GetLogger("juju.worker.reboot") const RebootMessage = "preparing for reboot" -const RestartMessage = "restarting juju" var _ worker.NotifyWatchHandler = (*Reboot)(nil) // The reboot worker listens for changes to the reboot flag and -// exists with worker.ErrTerminateAgent if only jujud should be restarts, -// with worker.ErrRebootMachine if the machine should reboot, or +// exists with worker.ErrRebootMachine if the machine should reboot or // with worker.ErrShutdownMachine if it should shutdown. This will be picked // up by the machine agent as a fatal error and will do the -// right thing (restart, reboot, or shutdown) +// right thing (reboot or shutdown) type Reboot struct { tomb tomb.Tomb st *reboot.State @@ -53,7 +51,7 @@ func (r *Reboot) checkForRebootState() error { return nil } - if r.machineLock.Message() == RebootMessage || r.machineLock.Message() == RestartMessage { + if r.machineLock.Message() == RebootMessage { // Not a lock held by the machne agent in order to reboot err = r.machineLock.BreakLock() if err != nil { @@ -89,9 +87,6 @@ func (r *Reboot) Handle() error { case params.ShouldShutdown: r.machineLock.Lock(RebootMessage) return worker.ErrShutdownMachine - case params.ShouldRestart: - r.machineLock.Lock(RestartMessage) - return worker.ErrTerminateAgent } return nil } diff --git a/worker/runner.go b/worker/runner.go index 95b90e64004..eb6b6cc47af 100644 --- a/worker/runner.go +++ b/worker/runner.go @@ -4,9 +4,9 @@ package worker import ( + "errors" "time" - "github.com/juju/errors" "launchpad.net/tomb" ) From 8107dfba477bf7634ad778b6e41aadac8c7a61fd Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Thu, 26 Mar 2015 21:03:44 -0400 Subject: [PATCH 18/33] remove some unused changes --- apiserver/watcher.go | 57 ++++++++------------------------------------ cmd/jujud/unit.go | 1 + state/state.go | 1 - 3 files changed, 11 insertions(+), 48 deletions(-) diff --git a/apiserver/watcher.go b/apiserver/watcher.go index 1c6a031cacf..7856789d162 100644 --- a/apiserver/watcher.go +++ b/apiserver/watcher.go @@ -6,7 +6,6 @@ package apiserver import ( "reflect" - "github.com/juju/errors" "github.com/juju/juju/apiserver/common" "github.com/juju/juju/apiserver/params" "github.com/juju/juju/state" @@ -43,15 +42,9 @@ func newClientAllWatcher(st *state.State, resources *common.Resources, auth comm if !auth.AuthClient() { return nil, common.ErrPerm } - r := resources.Get(id) - if r == nil { - logger.Errorf("watcher %q not found", id) - return nil, errors.Trace(common.ErrUnknownWatcher) - } - watcher, ok := r.(*state.Multiwatcher) + watcher, ok := resources.Get(id).(*state.Multiwatcher) if !ok { - logger.Errorf("unknown watcher type for %q: %T, expected state.Multiwatcher", id, r) - return nil, errors.Trace(common.ErrUnknownWatcher) + return nil, common.ErrUnknownWatcher } return &srvClientAllWatcher{ watcher: watcher, @@ -96,17 +89,10 @@ func newNotifyWatcher(st *state.State, resources *common.Resources, auth common. if !isAgent(auth) { return nil, common.ErrPerm } - r := resources.Get(id) - if r == nil { - logger.Errorf("watcher %q not found", id) - return nil, errors.Trace(common.ErrUnknownWatcher) - } - watcher, ok := r.(state.NotifyWatcher) + watcher, ok := resources.Get(id).(state.NotifyWatcher) if !ok { - logger.Errorf("unknown watcher type for %q: %#T, expected state.NotifyWatcher", id, r) - return nil, errors.Trace(common.ErrUnknownWatcher) + return nil, common.ErrUnknownWatcher } - return &srvNotifyWatcher{ watcher: watcher, id: id, @@ -147,16 +133,9 @@ func newStringsWatcher(st *state.State, resources *common.Resources, auth common if !isAgent(auth) { return nil, common.ErrPerm } - - r := resources.Get(id) - if r == nil { - logger.Errorf("watcher %q not found", id) - return nil, errors.Trace(common.ErrUnknownWatcher) - } - watcher, ok := r.(state.StringsWatcher) + watcher, ok := resources.Get(id).(state.StringsWatcher) if !ok { - logger.Errorf("unknown watcher type for %q: %#T, expected state.StringsWatcher", id, r) - return nil, errors.Trace(common.ErrUnknownWatcher) + return nil, common.ErrUnknownWatcher } return &srvStringsWatcher{ watcher: watcher, @@ -199,18 +178,10 @@ func newRelationUnitsWatcher(st *state.State, resources *common.Resources, auth if !isAgent(auth) { return nil, common.ErrPerm } - - r := resources.Get(id) - if r == nil { - logger.Errorf("watcher %q not found", id) - return nil, errors.Trace(common.ErrUnknownWatcher) - } - watcher, ok := r.(state.RelationUnitsWatcher) + watcher, ok := resources.Get(id).(state.RelationUnitsWatcher) if !ok { - logger.Errorf("unknown watcher type for %q: %#T, expected state.RelationUnitsWatcher", id, r) - return nil, errors.Trace(common.ErrUnknownWatcher) + return nil, common.ErrUnknownWatcher } - return &srvRelationUnitsWatcher{ watcher: watcher, id: id, @@ -285,18 +256,10 @@ func newMachineStorageIdsWatcher( if !isAgent(auth) { return nil, common.ErrPerm } - - r := resources.Get(id) - if r == nil { - logger.Errorf("watcher %q not found", id) - return nil, errors.Trace(common.ErrUnknownWatcher) - } - watcher, ok := r.(state.StringsWatcher) + watcher, ok := resources.Get(id).(state.StringsWatcher) if !ok { - logger.Errorf("unknown watcher type for %q: %#T, expected state.StringsWatcher", id, r) - return nil, errors.Trace(common.ErrUnknownWatcher) + return nil, common.ErrUnknownWatcher } - return &srvMachineStorageIdsWatcher{ watcher: watcher, id: id, diff --git a/cmd/jujud/unit.go b/cmd/jujud/unit.go index 0a527a592e2..81ed41e99e5 100644 --- a/cmd/jujud/unit.go +++ b/cmd/jujud/unit.go @@ -177,6 +177,7 @@ func (a *UnitAgent) APIWorkers() (worker.Worker, error) { } return uniter.NewUniter(uniterFacade, unitTag, st.LeadershipManager(), dataDir, hookLock), nil }) + runner.StartWorker("apiaddressupdater", func() (worker.Worker, error) { uniterFacade, err := st.Uniter() if err != nil { diff --git a/state/state.go b/state/state.go index adb5a218779..4d3d0904047 100644 --- a/state/state.go +++ b/state/state.go @@ -56,7 +56,6 @@ const ( unitsC = "units" subnetsC = "subnets" ipaddressesC = "ipaddresses" - jobsC = "jobs" // actionsC and related collections store state of Actions that // have been enqueued. From 9b1da3742f9f2ebb2bdd5e1564f308db879bec8d Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Thu, 26 Mar 2015 21:59:58 -0400 Subject: [PATCH 19/33] move converter to jujud/agent to simplify code --- cmd/jujud/agent/converter.go | 66 ++++++++++++++++++++++ cmd/jujud/agent/machine.go | 18 ++---- worker/converter/converter.go | 91 ------------------------------ worker/converter/converter_test.go | 65 --------------------- 4 files changed, 71 insertions(+), 169 deletions(-) create mode 100644 cmd/jujud/agent/converter.go delete mode 100644 worker/converter/converter.go delete mode 100644 worker/converter/converter_test.go diff --git a/cmd/jujud/agent/converter.go b/cmd/jujud/agent/converter.go new file mode 100644 index 00000000000..70ca98567d5 --- /dev/null +++ b/cmd/jujud/agent/converter.go @@ -0,0 +1,66 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package agent + +import ( + "github.com/juju/errors" + "github.com/juju/utils" + + "github.com/juju/juju/agent" + apiconverter "github.com/juju/juju/api/converter" + "github.com/juju/juju/api/watcher" +) + +// converter is a worker that converts a unit hosting machine to a state machine. +type converter struct { + st *apiconverter.State + config agent.Config + agent *MachineAgent +} + +func (c *converter) SetUp() (watcher.NotifyWatcher, error) { + logger.Infof("setting up Converter watcher for %s", c.config.Tag().String()) + return c.st.WatchForJobsChanges(c.config.Tag().String()) +} + +func (c *converter) Handle() error { + logger.Infof("environment for %q has been changed", c.config.Tag()) + + jobs, err := c.st.Jobs(c.config.Tag().String()) + if err != nil { + return errors.Trace(err) + } + + for _, job := range jobs.Jobs { + if job.NeedsState() { + logger.Warningf("converting %q to a state server", c.config.Tag()) + pw, err := utils.RandomPassword() + if err != nil { + return errors.Trace(err) + } + c.agent.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { + config.SetPassword(pw) + config.SetStateAddresses([]string{"localhost:37017"}) + return nil + }) + _, entity, err := OpenAPIState(c.config, c.agent) + if err != nil { + logger.Errorf("can't open API state: %s", errors.Details(err)) + return errors.Trace(err) + } + + if err := entity.SetPassword(pw); err != nil { + logger.Errorf("can't set password for machine agent: %s", errors.Details(err)) + return errors.Trace(err) + } + + return c.agent.RestartService() + } + } + return nil +} + +func (c *converter) TearDown() error { + return nil +} diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index 77e07a9fc30..b0bd05fd859 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -65,7 +65,6 @@ import ( "github.com/juju/juju/worker/certupdater" "github.com/juju/juju/worker/charmrevisionworker" "github.com/juju/juju/worker/cleaner" - "github.com/juju/juju/worker/converter" "github.com/juju/juju/worker/dblogpruner" "github.com/juju/juju/worker/deployer" "github.com/juju/juju/worker/diskformatter" @@ -708,18 +707,11 @@ func (a *MachineAgent) postUpgradeAPIWorker( if !isEnvironManager { runner.StartWorker("converter", func() (worker.Worker, error) { - getEntity := func() (converter.Entity, error) { - _, entity, err := OpenAPIState(agentConfig, a) - return entity, err - } - setPW := func(pw string) { - a.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { - config.SetPassword(pw) - config.SetStateAddresses([]string{"localhost:37017"}) - return nil - }) - } - return converter.NewConverter(getEntity, setPW, a.RestartService, st.Converter(), agentConfig), nil + return worker.NewNotifyWorker(&converter{ + agent: a, + st: st.Converter(), + config: agentConfig, + }), nil }) } diff --git a/worker/converter/converter.go b/worker/converter/converter.go deleted file mode 100644 index 13868e93b5c..00000000000 --- a/worker/converter/converter.go +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2015 Canonical Ltd. -// Licensed under the AGPLv3, see LICENCE file for details. - -package converter - -import ( - "github.com/juju/errors" - "github.com/juju/loggo" - "github.com/juju/utils" - - "github.com/juju/juju/agent" - "github.com/juju/juju/api/converter" - "github.com/juju/juju/api/watcher" - "github.com/juju/juju/state/multiwatcher" - "github.com/juju/juju/worker" -) - -var logger = loggo.GetLogger("juju.worker.converter") - -// Converter ... -type Converter struct { - st *converter.State - getEnt func() (Entity, error) - restart func() error - setPassword func(pw string) - config agent.Config -} - -type Entity interface { - SetPassword(string) error - Jobs() []multiwatcher.MachineJob -} - -// NewConverter ... -func NewConverter( - getEnt func() (Entity, error), - setPW func(pw string), - restart func() error, - st *converter.State, - agentConfig agent.Config, -) worker.Worker { - return worker.NewNotifyWorker(&Converter{ - getEnt: getEnt, - setPassword: setPW, - restart: restart, - st: st, - config: agentConfig, - }) -} - -func (c *Converter) SetUp() (watcher.NotifyWatcher, error) { - logger.Infof("setting up Converter watcher for %s", c.config.Tag().String()) - return c.st.WatchForJobsChanges(c.config.Tag().String()) -} - -func (c *Converter) Handle() error { - logger.Infof("environment for %q has been changed", c.config.Tag()) - - jobs, err := c.st.Jobs(c.config.Tag().String()) - if err != nil { - return errors.Trace(err) - } - - for _, job := range jobs.Jobs { - if job.NeedsState() { - logger.Warningf("converting %q to a state server", c.config.Tag()) - pw, err := utils.RandomPassword() - if err != nil { - return errors.Trace(err) - } - ent, err := c.getEnt() - if err != nil { - logger.Errorf("error from getEntity: %s", errors.Details(err)) - return errors.Trace(err) - } - - if err := ent.SetPassword(pw); err != nil { - logger.Errorf("error trying to set password for machine agent: %s", errors.Details(err)) - return errors.Trace(err) - } - c.setPassword(pw) - - return c.restart() - } - } - return nil -} - -func (c *Converter) TearDown() error { - return nil -} diff --git a/worker/converter/converter_test.go b/worker/converter/converter_test.go deleted file mode 100644 index 70dd6ce81c7..00000000000 --- a/worker/converter/converter_test.go +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2015 Canonical Ltd. -// Licensed under the AGPLv3, see LICENCE file for details. - -package converter_test - -import ( - "os" - "os/signal" - "runtime" - stdtesting "testing" - - jc "github.com/juju/testing/checkers" - gc "gopkg.in/check.v1" - - "github.com/juju/juju/testing" - "github.com/juju/juju/worker" - "github.com/juju/juju/worker/converter" -) - -func TestPackage(t *stdtesting.T) { - gc.TestingT(t) -} - -var _ = gc.Suite(&ConverterSuite{}) - -type ConverterSuite struct { - testing.BaseSuite - // c is a channel that will wait for the termination - // signal, to prevent signals terminating the process. - c chan os.Signal -} - -func (s *ConverterSuite) SetUpTest(c *gc.C) { - s.BaseSuite.SetUpTest(c) - s.c = make(chan os.Signal, 1) - signal.Notify(s.c, converter.TerminationSignal) -} - -func (s *ConverterSuite) TearDownTest(c *gc.C) { - close(s.c) - signal.Stop(s.c) - s.BaseSuite.TearDownTest(c) -} - -func (s *ConverterSuite) TestStartStop(c *gc.C) { - w := converter.NewWorker() - w.Kill() - err := w.Wait() - c.Assert(err, jc.ErrorIsNil) -} - -func (s *ConverterSuite) TestSignal(c *gc.C) { - //TODO(bogdanteleaga): Inspect this further on windows - if runtime.GOOS == "windows" { - c.Skip("bug 1403084: sending this signal is not supported on windows") - } - w := converter.NewWorker() - proc, err := os.FindProcess(os.Getpid()) - c.Assert(err, jc.ErrorIsNil) - defer proc.Release() - err = proc.Signal(converter.TerminationSignal) - c.Assert(err, jc.ErrorIsNil) - err = w.Wait() - c.Assert(err, gc.Equals, worker.ErrTerminateAgent) -} From 98036f7444393ff581f268671accc4582983dad6 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Thu, 26 Mar 2015 22:24:25 -0400 Subject: [PATCH 20/33] remove unused tests --- apiserver/converter/converter_test.go | 97 --------------------------- apiserver/converter/suite_test.go | 14 ---- 2 files changed, 111 deletions(-) delete mode 100644 apiserver/converter/converter_test.go delete mode 100644 apiserver/converter/suite_test.go diff --git a/apiserver/converter/converter_test.go b/apiserver/converter/converter_test.go deleted file mode 100644 index d949bd8ae24..00000000000 --- a/apiserver/converter/converter_test.go +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright 2015 Canonical Ltd. -// Licensed under the AGPLv3, see LICENCE file for details. - -package converter_test - -import ( - "fmt" - - "github.com/juju/errors" - "github.com/juju/names" - jc "github.com/juju/testing/checkers" - gc "gopkg.in/check.v1" - - "github.com/juju/juju/apiserver/common" - "github.com/juju/juju/apiserver/params" - apiservertesting "github.com/juju/juju/apiserver/testing" - "github.com/juju/juju/apiserver/upgrader" - jujutesting "github.com/juju/juju/juju/testing" - "github.com/juju/juju/state" - statetesting "github.com/juju/juju/state/testing" - coretesting "github.com/juju/juju/testing" - "github.com/juju/juju/version" -) - -type upgraderSuite struct { - jujutesting.JujuConnSuite - - // These are raw State objects. Use them for setup and assertions, but - // should never be touched by the API calls themselves - rawMachine *state.Machine - apiMachine *state.Machine - upgrader *upgrader.UpgraderAPI - resources *common.Resources - authorizer apiservertesting.FakeAuthorizer -} - -var _ = gc.Suite(&upgraderSuite{}) - -func (s *upgraderSuite) SetUpTest(c *gc.C) { - s.JujuConnSuite.SetUpTest(c) - s.resources = common.NewResources() - s.AddCleanup(func(_ *gc.C) { s.resources.StopAll() }) - - // Create a machine to work with - var err error - // The first machine created is the only one allowed to - // JobManageEnviron - s.apiMachine, err = s.State.AddMachine("quantal", state.JobHostUnits, - state.JobManageEnviron) - c.Assert(err, jc.ErrorIsNil) - s.rawMachine, err = s.State.AddMachine("quantal", state.JobHostUnits) - c.Assert(err, jc.ErrorIsNil) - - // The default auth is as the machine agent - s.authorizer = apiservertesting.FakeAuthorizer{ - Tag: s.rawMachine.Tag(), - } - s.upgrader, err = upgrader.NewUpgraderAPI(s.State, s.resources, s.authorizer) - c.Assert(err, jc.ErrorIsNil) -} - -func (s *upgraderSuite) TearDownTest(c *gc.C) { - if s.resources != nil { - s.resources.StopAll() - } - s.JujuConnSuite.TearDownTest(c) -} - -func (s *upgraderSuite) TestWatchAPIVersionNothing(c *gc.C) { - // Not an error to watch nothing - results, err := s.upgrader.WatchAPIVersion(params.Entities{}) - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 0) -} - -func (s *upgraderSuite) TestWatchAPIVersion(c *gc.C) { - args := params.Entities{ - Entities: []params.Entity{{Tag: s.rawMachine.Tag().String()}}, - } - results, err := s.upgrader.WatchAPIVersion(args) - c.Assert(err, jc.ErrorIsNil) - c.Check(results.Results, gc.HasLen, 1) - c.Check(results.Results[0].NotifyWatcherId, gc.Not(gc.Equals), "") - c.Check(results.Results[0].Error, gc.IsNil) - resource := s.resources.Get(results.Results[0].NotifyWatcherId) - c.Check(resource, gc.NotNil) - - w := resource.(state.NotifyWatcher) - wc := statetesting.NewNotifyWatcherC(c, s.State, w) - wc.AssertNoChange() - - err = statetesting.SetAgentVersion(s.State, version.MustParse("3.4.567.8")) - c.Assert(err, jc.ErrorIsNil) - wc.AssertOneChange() - statetesting.AssertStop(c, w) - wc.AssertClosed() -} diff --git a/apiserver/converter/suite_test.go b/apiserver/converter/suite_test.go deleted file mode 100644 index aed81d60a77..00000000000 --- a/apiserver/converter/suite_test.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2015 Canonical Ltd. -// Licensed under the AGPLv3, see LICENCE file for details. - -package converter_test - -import ( - stdtesting "testing" - - coretesting "github.com/juju/juju/testing" -) - -func TestAll(t *stdtesting.T) { - coretesting.MgoTestPackage(t) -} From 9ac6f0c5d05a74713f1177fdbe048e337c404464 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Fri, 27 Mar 2015 16:54:54 -0400 Subject: [PATCH 21/33] new tests and get stateport from config --- cmd/juju/ensureavailability_test.go | 33 ++++++++++++++++++++++------- cmd/jujud/agent/converter.go | 29 +++++++++++++++---------- cmd/jujud/agent/machine.go | 5 ++--- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/cmd/juju/ensureavailability_test.go b/cmd/juju/ensureavailability_test.go index 230bbec6731..aa0790b8b5a 100644 --- a/cmd/juju/ensureavailability_test.go +++ b/cmd/juju/ensureavailability_test.go @@ -18,6 +18,7 @@ import ( "github.com/juju/juju/apiserver/params" "github.com/juju/juju/cmd/envcmd" "github.com/juju/juju/constraints" + "github.com/juju/juju/instance" "github.com/juju/juju/juju/testing" "github.com/juju/juju/state" coretesting "github.com/juju/juju/testing" @@ -80,9 +81,17 @@ func (f *fakeHAClient) EnsureAvailability(numStateServers int, cons constraints. numStateServers = 3 } - // If numStateServers > 1, we need to pretend that we added some machines f.result.Maintained = append(f.result.Maintained, "machine-0") - for i := 1; i < numStateServers; i++ { + + for _, p := range placement { + m, err := instance.ParsePlacement(p) + if err == nil { + f.result.Converted = append(f.result.Converted, "machine-"+m.Directive) + } + } + + // We may need to pretend that we added some machines. + for i := len(f.result.Converted) + 1; i < numStateServers; i++ { f.result.Added = append(f.result.Added, fmt.Sprintf("machine-%d", i)) } @@ -117,11 +126,6 @@ func (s *EnsureAvailabilitySuite) TestBlockEnsureAvailability(c *gc.C) { c.Check(stripped, gc.Matches, ".*TestBlockEnsureAvailability.*") } -func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityPlacementError(c *gc.C) { - _, err := s.runEnsureAvailability(c, "-n", "1", "--to", "1") - c.Assert(err, gc.ErrorMatches, `unsupported ensure-availability placement directive "1"`) -} - func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityFormatYaml(c *gc.C) { expected := map[string][]string{ "maintained": {"0"}, @@ -254,5 +258,18 @@ func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityEndToEnd(c *gc.C) { // Machine 0 is demoted because it hasn't reported its presence c.Assert(coretesting.Stdout(ctx), gc.Equals, "adding machines: 1, 2, 3\n"+ - "demoting machines 0\n\n") + "demoting machines: 0\n\n") +} + +func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityToExisting(c *gc.C) { + ctx, err := s.runEnsureAvailability(c, "--to", "1,2") + c.Assert(err, jc.ErrorIsNil) + c.Check(coretesting.Stdout(ctx), gc.Equals, + "maintaining machines: 0\n"+ + "converting machines: 1, 2\n\n") + + c.Check(s.fake.numStateServers, gc.Equals, 0) + c.Check(&s.fake.cons, jc.Satisfies, constraints.IsEmpty) + c.Check(s.fake.series, gc.Equals, "") + c.Check(len(s.fake.placement), gc.Equals, 2) } diff --git a/cmd/jujud/agent/converter.go b/cmd/jujud/agent/converter.go index 70ca98567d5..c688357221c 100644 --- a/cmd/jujud/agent/converter.go +++ b/cmd/jujud/agent/converter.go @@ -4,6 +4,8 @@ package agent import ( + "fmt" + "github.com/juju/errors" "github.com/juju/utils" @@ -14,37 +16,42 @@ import ( // converter is a worker that converts a unit hosting machine to a state machine. type converter struct { - st *apiconverter.State - config agent.Config - agent *MachineAgent + st *apiconverter.State + agent *MachineAgent } func (c *converter) SetUp() (watcher.NotifyWatcher, error) { - logger.Infof("setting up Converter watcher for %s", c.config.Tag().String()) - return c.st.WatchForJobsChanges(c.config.Tag().String()) + logger.Infof("setting up Converter watcher") + return c.st.WatchForJobsChanges(c.agent.CurrentConfig().Tag().String()) } func (c *converter) Handle() error { - logger.Infof("environment for %q has been changed", c.config.Tag()) - - jobs, err := c.st.Jobs(c.config.Tag().String()) + config := c.agent.CurrentConfig() + logger.Infof("environment for %q has been changed", config.Tag()) + jobs, err := c.st.Jobs(config.Tag().String()) if err != nil { return errors.Trace(err) } for _, job := range jobs.Jobs { if job.NeedsState() { - logger.Warningf("converting %q to a state server", c.config.Tag()) + logger.Warningf("converting %q to a state server", config.Tag()) pw, err := utils.RandomPassword() if err != nil { return errors.Trace(err) } + ssi, exists := config.StateServingInfo() + if !exists { + return errors.New("can't get state serving info from config.") + } + addr := fmt.Sprintf("localhost:%d", ssi.StatePort) + c.agent.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { config.SetPassword(pw) - config.SetStateAddresses([]string{"localhost:37017"}) + config.SetStateAddresses([]string{addr}) return nil }) - _, entity, err := OpenAPIState(c.config, c.agent) + _, entity, err := OpenAPIState(config, c.agent) if err != nil { logger.Errorf("can't open API state: %s", errors.Details(err)) return errors.Trace(err) diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index b0bd05fd859..f1113573a96 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -708,9 +708,8 @@ func (a *MachineAgent) postUpgradeAPIWorker( if !isEnvironManager { runner.StartWorker("converter", func() (worker.Worker, error) { return worker.NewNotifyWorker(&converter{ - agent: a, - st: st.Converter(), - config: agentConfig, + agent: a, + st: st.Converter(), }), nil }) } From 108e54e60ad6e84cc17e93141e885a17ddf34c21 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Wed, 1 Apr 2015 14:56:47 -0400 Subject: [PATCH 22/33] code review updates --- agent/agent.go | 2 ++ api/converter/converter.go | 49 ++++++++++++++++++++++---------- apiserver/converter/converter.go | 41 ++++++++++++++++---------- apiserver/params/params.go | 5 +++- cmd/jujud/agent/converter.go | 27 +++++++++--------- 5 files changed, 79 insertions(+), 45 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 4acad84238a..7ac9d7cf9eb 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -498,6 +498,8 @@ func (c *configInternal) SetAPIHostPorts(servers [][]network.HostPort) { c.apiDetails.addresses = addrs } +// SetStateAddresses sets the addresses of state machines this machine will +// connect to. func (c *configInternal) SetStateAddresses(servers []string) { if c.stateDetails == nil { c.stateDetails = &connectionDetails{} diff --git a/api/converter/converter.go b/api/converter/converter.go index 30a55f2e48f..e369b632b6f 100644 --- a/api/converter/converter.go +++ b/api/converter/converter.go @@ -1,55 +1,74 @@ // Copyright 2015 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. +// package converter exposes a watcher for machines to tell when they've been +// updated, and an api endpoint to get the machine's jobs. This is used by +// ensure- availability to convert existing machines in the environment into +// state servers. package converter import ( + "github.com/juju/errors" "github.com/juju/loggo" + "github.com/juju/names" "github.com/juju/juju/api/base" "github.com/juju/juju/api/watcher" "github.com/juju/juju/apiserver/params" ) -var logger = loggo.GetLogger("juju.apiserver.converter") +var logger = loggo.GetLogger("juju.api.converter") const converterAPI = "Converter" -// Converter provides common client-side API -// functions to call into apiserver.Converter +// State exposes a machine watcher and an API endpoint to get a machine's jobs. type State struct { facade base.FacadeCaller } +// NewState returns a new state object wrapping the given caller. func NewState(caller base.APICaller) *State { return &State{facade: base.NewFacadeCaller(caller, converterAPI)} } -func (c *State) WatchForJobsChanges(tag string) (watcher.NotifyWatcher, error) { - var result params.NotifyWatchResults +// WatchMachine returns a watcher that watches the machine with the given +// tag. +func (c *State) WatchMachine(tag names.MachineTag) (watcher.NotifyWatcher, error) { + var results params.NotifyWatchResults args := params.Entities{ - Entities: []params.Entity{{Tag: tag}}, + Entities: []params.Entity{{Tag: tag.String()}}, } - - logger.Infof("calling facade WatchForJobsChanges") - err := c.facade.FacadeCall("WatchForJobsChanges", args, &result) + err := c.facade.FacadeCall("WatchMachines", args, &results) if err != nil { return nil, err } - - return watcher.NewNotifyWatcher(c.facade.RawAPICaller(), result.Results[0]), nil + if len(results.Results) != 1 { + return nil, errors.Errorf("expected 1 result, got %d", len(results.Results)) + } + result := results.Results[0] + if result.Error != nil { + return nil, result.Error + } + return watcher.NewNotifyWatcher(c.facade.RawAPICaller(), result), nil } -func (c *State) Jobs(tag string) (*params.JobsResult, error) { +// Jobs returns a list of jobs for the machine with the given tag. +func (c *State) Jobs(tag names.MachineTag) (*params.JobsResult, error) { var results params.JobsResults args := params.Entities{ - Entities: []params.Entity{{Tag: tag}}, + Entities: []params.Entity{{Tag: tag.String()}}, } - logger.Infof("calling facade Jobs") err := c.facade.FacadeCall("Jobs", args, &results) if err != nil { return nil, err } - return &results.Results[0], nil + if len(results.Results) != 1 { + return nil, errors.Errorf("expected 1 result, got %d", len(results.Results)) + } + result := results.Results[0] + if result.Error != nil { + return nil, result.Error + } + return &result, nil } diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go index 1e4499dbdfe..99b662c5a6e 100644 --- a/apiserver/converter/converter.go +++ b/apiserver/converter/converter.go @@ -4,7 +4,6 @@ package converter import ( - "github.com/juju/errors" "github.com/juju/loggo" "github.com/juju/names" @@ -21,12 +20,15 @@ func init() { common.RegisterStandardFacade("Converter", 1, NewConverterAPI) } +// ConverterAPI contains methods for use with ensure-availability, watching +// machines and their jobs. type ConverterAPI struct { st *state.State resources *common.Resources authorizer common.Authorizer } +// NewConverterAPI returns a new instance of the API. func NewConverterAPI( st *state.State, resources *common.Resources, @@ -35,6 +37,7 @@ func NewConverterAPI( if !authorizer.AuthMachineAgent() { return nil, common.ErrPerm } + return &ConverterAPI{ st: st, resources: resources, @@ -43,6 +46,10 @@ func NewConverterAPI( } func (c *ConverterAPI) getMachine(tag names.Tag) (*state.Machine, error) { + if tag.Kind() != names.MachineTagKind { + return nil, common.ErrPerm + } + entity, err := c.st.FindEntity(tag) if err != nil { return nil, err @@ -50,6 +57,7 @@ func (c *ConverterAPI) getMachine(tag names.Tag) (*state.Machine, error) { return entity.(*state.Machine), nil } +// Jobs returns the jobs assigned to the given entities. func (c *ConverterAPI) Jobs(args params.Entities) (params.JobsResults, error) { result := params.JobsResults{ Results: make([]params.JobsResult, len(args.Entities)), @@ -58,15 +66,18 @@ func (c *ConverterAPI) Jobs(args params.Entities) (params.JobsResults, error) { for i, agent := range args.Entities { tag, err := names.ParseMachineTag(agent.Tag) if err != nil { - return params.JobsResults{}, errors.Trace(err) + logger.Warningf("not a machine tag: %v", agent.Tag) + result.Results[i].Error = common.ServerError(common.ErrPerm) + continue } err = common.ErrPerm if c.authorizer.AuthOwner(tag) { - logger.Infof("watching for jobs on %#v", tag) machine, err := c.getMachine(tag) if err != nil { - return params.JobsResults{}, errors.Trace(err) + logger.Warningf("can't get machine for tag %q: %v", tag, err) + result.Results[i].Error = common.ServerError(err) + continue } machineJobs := machine.Jobs() jobs := make([]multiwatcher.MachineJob, len(machineJobs)) @@ -79,22 +90,23 @@ func (c *ConverterAPI) Jobs(args params.Entities) (params.JobsResults, error) { return result, nil } -func (c *ConverterAPI) WatchForJobsChanges(args params.Entities) (params.NotifyWatchResults, error) { +func (c *ConverterAPI) WatchMachines(args params.Entities) (params.NotifyWatchResults, error) { result := params.NotifyWatchResults{ Results: make([]params.NotifyWatchResult, len(args.Entities)), } - for i, agent := range args.Entities { - logger.Infof("watching on entity %#v", agent) - tag, err := names.ParseMachineTag(agent.Tag) + for i, ent := range args.Entities { + tag, err := names.ParseMachineTag(ent.Tag) if err != nil { - return params.NotifyWatchResults{}, errors.Trace(err) + logger.Warningf("not a machine tag: %v", ent.Tag) + result.Results[i].Error = common.ServerError(common.ErrPerm) + continue } - err = common.ErrPerm if c.authorizer.AuthOwner(tag) { - logger.Infof("watching for jobs on %#v", tag) machine, err := c.getMachine(tag) if err != nil { - return params.NotifyWatchResults{}, errors.Trace(err) + logger.Warningf("can't get machine for tag %q: %v", tag, err) + result.Results[i].Error = common.ServerError(err) + continue } watch := machine.Watch() @@ -104,12 +116,11 @@ func (c *ConverterAPI) WatchForJobsChanges(args params.Entities) (params.NotifyW // have no state to transmit. if _, ok := <-watch.Changes(); ok { result.Results[i].NotifyWatcherId = c.resources.Register(watch) - err = nil } else { - err = watcher.EnsureErr(watch) + err := watcher.EnsureErr(watch) + result.Results[i].Error = common.ServerError(err) } } - result.Results[i].Error = common.ServerError(err) } return result, nil } diff --git a/apiserver/params/params.go b/apiserver/params/params.go index 8db1c1e81a0..01b3ed0ea38 100644 --- a/apiserver/params/params.go +++ b/apiserver/params/params.go @@ -543,10 +543,13 @@ type RsyslogConfigResults struct { Results []RsyslogConfigResult } +// JobsResult holds the jobs for a machine that are returned by a call to Jobs. type JobsResult struct { - Jobs []multiwatcher.MachineJob `json:"Jobs"` + Jobs []multiwatcher.MachineJob + Error *Error } +// JobsResults holds the result of a call to Jobs. type JobsResults struct { Results []JobsResult } diff --git a/cmd/jujud/agent/converter.go b/cmd/jujud/agent/converter.go index c688357221c..43437f940c4 100644 --- a/cmd/jujud/agent/converter.go +++ b/cmd/jujud/agent/converter.go @@ -4,9 +4,8 @@ package agent import ( - "fmt" - "github.com/juju/errors" + "github.com/juju/names" "github.com/juju/utils" "github.com/juju/juju/agent" @@ -14,25 +13,30 @@ import ( "github.com/juju/juju/api/watcher" ) -// converter is a worker that converts a unit hosting machine to a state machine. +// converter is a StringsWatchHandler that converts a unit hosting machine to a +// state machine. type converter struct { st *apiconverter.State agent *MachineAgent } +// SetUp implements StringsWatchHandler's SetUp method. It returns a watcher that +// checks for changes to the current machine. func (c *converter) SetUp() (watcher.NotifyWatcher, error) { - logger.Infof("setting up Converter watcher") - return c.st.WatchForJobsChanges(c.agent.CurrentConfig().Tag().String()) + logger.Infof("setting up converter watcher") + return c.st.WatchMachine(c.agent.CurrentConfig().Tag().(names.MachineTag)) } +// Handle implements StringsWatchHandler's Handle method. If the change means +// that the machine is now expected to manage the environment func (c *converter) Handle() error { config := c.agent.CurrentConfig() - logger.Infof("environment for %q has been changed", config.Tag()) - jobs, err := c.st.Jobs(config.Tag().String()) + tag := config.Tag().(names.MachineTag) + jobs, err := c.st.Jobs(tag) if err != nil { + logger.Errorf("Error getting jobs for tag %q: %v", tag, err) return errors.Trace(err) } - for _, job := range jobs.Jobs { if job.NeedsState() { logger.Warningf("converting %q to a state server", config.Tag()) @@ -40,15 +44,10 @@ func (c *converter) Handle() error { if err != nil { return errors.Trace(err) } - ssi, exists := config.StateServingInfo() - if !exists { - return errors.New("can't get state serving info from config.") - } - addr := fmt.Sprintf("localhost:%d", ssi.StatePort) c.agent.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { config.SetPassword(pw) - config.SetStateAddresses([]string{addr}) + config.SetStateAddresses([]string{"localhost:37017"}) return nil }) _, entity, err := OpenAPIState(config, c.agent) From b9894a6d6290fb227c84d3bbba405c2fab7b71a7 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Wed, 1 Apr 2015 15:42:43 -0400 Subject: [PATCH 23/33] fix comments --- apiserver/converter/converter.go | 1 + cmd/jujud/agent/converter.go | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go index 99b662c5a6e..7067b7e6cc3 100644 --- a/apiserver/converter/converter.go +++ b/apiserver/converter/converter.go @@ -90,6 +90,7 @@ func (c *ConverterAPI) Jobs(args params.Entities) (params.JobsResults, error) { return result, nil } +// WatchMachines creates a watcher for each of the specified machines. func (c *ConverterAPI) WatchMachines(args params.Entities) (params.NotifyWatchResults, error) { result := params.NotifyWatchResults{ Results: make([]params.NotifyWatchResult, len(args.Entities)), diff --git a/cmd/jujud/agent/converter.go b/cmd/jujud/agent/converter.go index 43437f940c4..8a42f83ead7 100644 --- a/cmd/jujud/agent/converter.go +++ b/cmd/jujud/agent/converter.go @@ -13,21 +13,21 @@ import ( "github.com/juju/juju/api/watcher" ) -// converter is a StringsWatchHandler that converts a unit hosting machine to a +// converter is a NotifyWatcher that converts a unit hosting machine to a // state machine. type converter struct { st *apiconverter.State agent *MachineAgent } -// SetUp implements StringsWatchHandler's SetUp method. It returns a watcher that +// SetUp implements NotifyWatcher's SetUp method. It returns a watcher that // checks for changes to the current machine. func (c *converter) SetUp() (watcher.NotifyWatcher, error) { logger.Infof("setting up converter watcher") return c.st.WatchMachine(c.agent.CurrentConfig().Tag().(names.MachineTag)) } -// Handle implements StringsWatchHandler's Handle method. If the change means +// Handle implements NotifyWatcher's Handle method. If the change means // that the machine is now expected to manage the environment func (c *converter) Handle() error { config := c.agent.CurrentConfig() From 0292076d9aeec64b8d52325c2b687e0d3b15673e Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Wed, 1 Apr 2015 16:59:28 -0400 Subject: [PATCH 24/33] review changes --- cmd/jujud/agent/converter.go | 55 ++++++++++++++++++++---------------- cmd/jujud/agent/machine.go | 1 + 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/cmd/jujud/agent/converter.go b/cmd/jujud/agent/converter.go index 8a42f83ead7..f82386fa616 100644 --- a/cmd/jujud/agent/converter.go +++ b/cmd/jujud/agent/converter.go @@ -11,23 +11,26 @@ import ( "github.com/juju/juju/agent" apiconverter "github.com/juju/juju/api/converter" "github.com/juju/juju/api/watcher" + "github.com/juju/juju/worker" ) -// converter is a NotifyWatcher that converts a unit hosting machine to a +var _ worker.NotifyWatchHandler = (*converter)(nil) + +// converter is a NotifyWatchHandler that converts a unit hosting machine to a // state machine. type converter struct { st *apiconverter.State agent *MachineAgent } -// SetUp implements NotifyWatcher's SetUp method. It returns a watcher that +// SetUp implements NotifyWatchHandler's SetUp method. It returns a watcher that // checks for changes to the current machine. func (c *converter) SetUp() (watcher.NotifyWatcher, error) { logger.Infof("setting up converter watcher") return c.st.WatchMachine(c.agent.CurrentConfig().Tag().(names.MachineTag)) } -// Handle implements NotifyWatcher's Handle method. If the change means +// Handle implements NotifyWatchHandler's Handle method. If the change means // that the machine is now expected to manage the environment func (c *converter) Handle() error { config := c.agent.CurrentConfig() @@ -38,35 +41,37 @@ func (c *converter) Handle() error { return errors.Trace(err) } for _, job := range jobs.Jobs { - if job.NeedsState() { - logger.Warningf("converting %q to a state server", config.Tag()) - pw, err := utils.RandomPassword() - if err != nil { - return errors.Trace(err) - } - - c.agent.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { - config.SetPassword(pw) - config.SetStateAddresses([]string{"localhost:37017"}) - return nil - }) - _, entity, err := OpenAPIState(config, c.agent) - if err != nil { - logger.Errorf("can't open API state: %s", errors.Details(err)) - return errors.Trace(err) - } + if !job.NeedsState() { + continue + } + logger.Warningf("converting %q to a state server", config.Tag()) + pw, err := utils.RandomPassword() + if err != nil { + return errors.Trace(err) + } - if err := entity.SetPassword(pw); err != nil { - logger.Errorf("can't set password for machine agent: %s", errors.Details(err)) - return errors.Trace(err) - } + c.agent.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { + config.SetPassword(pw) + config.SetStateAddresses([]string{"localhost:37017"}) + return nil + }) + _, entity, err := OpenAPIState(config, c.agent) + if err != nil { + logger.Errorf("can't open API state: %s", errors.Details(err)) + return errors.Trace(err) + } - return c.agent.RestartService() + if err := entity.SetPassword(pw); err != nil { + logger.Errorf("can't set password for machine agent: %s", errors.Details(err)) + return errors.Trace(err) } + + return c.agent.RestartService() } return nil } +// TearDown implements NotifyWatchHandler's TearDown method. func (c *converter) TearDown() error { return nil } diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index f1113573a96..2c58b568d63 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -642,6 +642,7 @@ func (a *MachineAgent) APIWorker() (worker.Worker, error) { a.upgradeWorkerContext.IsUpgradeRunning, ), nil }) + runner.StartWorker("upgrade-steps", a.upgradeStepsWorkerStarter(st, entity.Jobs())) // All other workers must wait for the upgrade steps to complete before starting. From 995390b83bbeebdfe9a837abd1a969615649512b Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Mon, 6 Apr 2015 00:43:35 -0400 Subject: [PATCH 25/33] big refactor & code review changes --- api/machiner/machine.go | 21 ++++ api/machiner/machiner.go | 3 +- apiserver/machine/machiner.go | 43 +++++++ cmd/jujud/agent/converter.go | 77 ------------ cmd/jujud/agent/machine.go | 40 +++++-- worker/conv2state/converter.go | 116 +++++++++++++++++++ worker/conv2state/converter_test.go | 174 ++++++++++++++++++++++++++++ worker/conv2state/fakes_test.go | 76 ++++++++++++ 8 files changed, 462 insertions(+), 88 deletions(-) delete mode 100644 cmd/jujud/agent/converter.go create mode 100644 worker/conv2state/converter.go create mode 100644 worker/conv2state/converter_test.go create mode 100644 worker/conv2state/fakes_test.go diff --git a/api/machiner/machine.go b/api/machiner/machine.go index 29635e47a9f..51def3adae0 100644 --- a/api/machiner/machine.go +++ b/api/machiner/machine.go @@ -4,6 +4,7 @@ package machiner import ( + "github.com/juju/errors" "github.com/juju/names" "github.com/juju/juju/api/common" @@ -87,3 +88,23 @@ func (m *Machine) EnsureDead() error { func (m *Machine) Watch() (watcher.NotifyWatcher, error) { return common.Watch(m.st.facade, m.tag) } + +// Jobs returns a list of jobs for the machine. +func (m *Machine) Jobs() (*params.JobsResult, error) { + var results params.JobsResults + args := params.Entities{ + Entities: []params.Entity{{Tag: m.Tag().String()}}, + } + err := m.st.facade.FacadeCall("Jobs", args, &results) + if err != nil { + return nil, errors.Annotate(err, "error from FacadeCall") + } + if len(results.Results) != 1 { + return nil, errors.Errorf("expected 1 result, got %d", len(results.Results)) + } + result := results.Results[0] + if result.Error != nil { + return nil, result.Error + } + return &result, nil +} diff --git a/api/machiner/machiner.go b/api/machiner/machiner.go index 7f7a237b222..94730fcab8f 100644 --- a/api/machiner/machiner.go +++ b/api/machiner/machiner.go @@ -4,6 +4,7 @@ package machiner import ( + "github.com/juju/errors" "github.com/juju/names" "github.com/juju/juju/api/base" @@ -38,7 +39,7 @@ func (st *State) machineLife(tag names.MachineTag) (params.Life, error) { func (st *State) Machine(tag names.MachineTag) (*Machine, error) { life, err := st.machineLife(tag) if err != nil { - return nil, err + return nil, errors.Annotate(err, "can't get life for machine") } return &Machine{ tag: tag, diff --git a/apiserver/machine/machiner.go b/apiserver/machine/machiner.go index cf43299fc23..cb7205b030c 100644 --- a/apiserver/machine/machiner.go +++ b/apiserver/machine/machiner.go @@ -7,17 +7,21 @@ package machine import ( "github.com/juju/errors" + "github.com/juju/loggo" "github.com/juju/names" "github.com/juju/juju/apiserver/common" "github.com/juju/juju/apiserver/params" "github.com/juju/juju/state" + "github.com/juju/juju/state/multiwatcher" ) func init() { common.RegisterStandardFacade("Machiner", 0, NewMachinerAPI) } +var logger = loggo.GetLogger("juju.apiserver.machine") + // MachinerAPI implements the API used by the machiner worker. type MachinerAPI struct { *common.LifeGetter @@ -52,6 +56,7 @@ func NewMachinerAPI(st *state.State, resources *common.Resources, authorizer com st: st, auth: authorizer, getCanModify: getCanModify, + getCanRead: getCanRead, }, nil } @@ -92,3 +97,41 @@ func (api *MachinerAPI) SetMachineAddresses(args params.SetMachinesAddresses) (p } return results, nil } + +// Jobs returns the jobs assigned to the given entities. +func (api *MachinerAPI) Jobs(args params.Entities) (params.JobsResults, error) { + result := params.JobsResults{ + Results: make([]params.JobsResult, len(args.Entities)), + } + + canRead, err := api.getCanRead() + if err != nil { + return result, err + } + + for i, agent := range args.Entities { + tag, err := names.ParseMachineTag(agent.Tag) + if err != nil { + result.Results[i].Error = common.ServerError(err) + continue + } + + if !canRead(tag) { + result.Results[i].Error = common.ServerError(common.ErrPerm) + continue + } + + machine, err := api.getMachine(tag) + if err != nil { + result.Results[i].Error = common.ServerError(err) + continue + } + machineJobs := machine.Jobs() + jobs := make([]multiwatcher.MachineJob, len(machineJobs)) + for i, job := range machineJobs { + jobs[i] = job.ToParams() + } + result.Results[i].Jobs = jobs + } + return result, nil +} diff --git a/cmd/jujud/agent/converter.go b/cmd/jujud/agent/converter.go deleted file mode 100644 index f82386fa616..00000000000 --- a/cmd/jujud/agent/converter.go +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2015 Canonical Ltd. -// Licensed under the AGPLv3, see LICENCE file for details. - -package agent - -import ( - "github.com/juju/errors" - "github.com/juju/names" - "github.com/juju/utils" - - "github.com/juju/juju/agent" - apiconverter "github.com/juju/juju/api/converter" - "github.com/juju/juju/api/watcher" - "github.com/juju/juju/worker" -) - -var _ worker.NotifyWatchHandler = (*converter)(nil) - -// converter is a NotifyWatchHandler that converts a unit hosting machine to a -// state machine. -type converter struct { - st *apiconverter.State - agent *MachineAgent -} - -// SetUp implements NotifyWatchHandler's SetUp method. It returns a watcher that -// checks for changes to the current machine. -func (c *converter) SetUp() (watcher.NotifyWatcher, error) { - logger.Infof("setting up converter watcher") - return c.st.WatchMachine(c.agent.CurrentConfig().Tag().(names.MachineTag)) -} - -// Handle implements NotifyWatchHandler's Handle method. If the change means -// that the machine is now expected to manage the environment -func (c *converter) Handle() error { - config := c.agent.CurrentConfig() - tag := config.Tag().(names.MachineTag) - jobs, err := c.st.Jobs(tag) - if err != nil { - logger.Errorf("Error getting jobs for tag %q: %v", tag, err) - return errors.Trace(err) - } - for _, job := range jobs.Jobs { - if !job.NeedsState() { - continue - } - logger.Warningf("converting %q to a state server", config.Tag()) - pw, err := utils.RandomPassword() - if err != nil { - return errors.Trace(err) - } - - c.agent.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { - config.SetPassword(pw) - config.SetStateAddresses([]string{"localhost:37017"}) - return nil - }) - _, entity, err := OpenAPIState(config, c.agent) - if err != nil { - logger.Errorf("can't open API state: %s", errors.Details(err)) - return errors.Trace(err) - } - - if err := entity.SetPassword(pw); err != nil { - logger.Errorf("can't set password for machine agent: %s", errors.Details(err)) - return errors.Trace(err) - } - - return c.agent.RestartService() - } - return nil -} - -// TearDown implements NotifyWatchHandler's TearDown method. -func (c *converter) TearDown() error { - return nil -} diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index 1eee3b0e938..f4602e00b9d 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -66,6 +66,7 @@ import ( "github.com/juju/juju/worker/certupdater" "github.com/juju/juju/worker/charmrevisionworker" "github.com/juju/juju/worker/cleaner" + "github.com/juju/juju/worker/conv2state" "github.com/juju/juju/worker/dblogpruner" "github.com/juju/juju/worker/deployer" "github.com/juju/juju/worker/diskmanager" @@ -452,11 +453,6 @@ func (a *MachineAgent) executeRebootOrShutdown(action params.RebootAction) error return worker.ErrRebootMachine } -func (a *MachineAgent) RestartService() error { - name := a.CurrentConfig().Value(agent.AgentServiceName) - return service.Restart(name) -} - func (a *MachineAgent) ChangeConfig(mutate AgentConfigMutator) error { err := a.AgentConfigWriter.ChangeConfig(mutate) a.configChangedVal.Set(struct{}{}) @@ -707,11 +703,8 @@ func (a *MachineAgent) postUpgradeAPIWorker( }) if !isEnvironManager { - runner.StartWorker("converter", func() (worker.Worker, error) { - return worker.NewNotifyWorker(&converter{ - agent: a, - st: st.Converter(), - }), nil + runner.StartWorker("stateconverter", func() (worker.Worker, error) { + return worker.NewNotifyWorker(conv2state.New(st.Machiner(), a)), nil }) } @@ -798,6 +791,33 @@ func (a *MachineAgent) postUpgradeAPIWorker( return cmdutil.NewCloseWorker(logger, runner, st), nil // Note: a worker.Runner is itself a worker.Worker. } +// Restart restarts the agent's service. +func (a *MachineAgent) Restart() error { + name := a.CurrentConfig().Value(agent.AgentServiceName) + return service.Restart(name) +} + +// SetPassword sets the agent's password both in the agentconfig and on the +// state server. +func (a *MachineAgent) SetPassword(pw string) error { + // Cache the config so we connect with the old one. + config := a.CurrentConfig() + + a.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { + config.SetPassword(pw) + config.SetOldPassword(config.APIInfo().Password) + return nil + }) + + _, entity, err := OpenAPIState(config, a) + if err != nil { + return errors.Annotate(err, "error opening API") + } + + err = entity.SetPassword(pw) + return errors.Annotate(err, "error setting password") +} + func (a *MachineAgent) upgradeStepsWorkerStarter( st *api.State, jobs []multiwatcher.MachineJob, diff --git a/worker/conv2state/converter.go b/worker/conv2state/converter.go new file mode 100644 index 00000000000..20be57e81c3 --- /dev/null +++ b/worker/conv2state/converter.go @@ -0,0 +1,116 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package conv2state + +import ( + "github.com/juju/errors" + "github.com/juju/loggo" + "github.com/juju/names" + "github.com/juju/utils" + + apimachiner "github.com/juju/juju/api/machiner" + "github.com/juju/juju/api/watcher" + "github.com/juju/juju/apiserver/params" + "github.com/juju/juju/worker" +) + +var logger = loggo.GetLogger("juju.worker.conv2state") + +// New returns a new notify watch handler that will convert the given machine & +// agent to a state server. +func New(m *apimachiner.State, agent Agent) worker.NotifyWatchHandler { + return &converter{machiner: wrapper{m}, agent: agent} +} + +// converter is a NotifyWatchHandler that converts a unit hosting machine to a +// state machine. +type converter struct { + agent Agent + machiner machiner + machine machine +} + +// Agent is an interface that can have its password set and be told to restart. +type Agent interface { + SetPassword(string) error + Restart() error + Tag() names.Tag +} + +// machiner is a type that creates machines from a tag. +type machiner interface { + Machine(tag names.MachineTag) (machine, error) +} + +// machine is a type that has a list of jobs and can be watched. +type machine interface { + Jobs() (*params.JobsResult, error) + Watch() (watcher.NotifyWatcher, error) +} + +// wrapper is a wrapper around api/machiner.State to match the (local) machiner +// interface. +type wrapper struct { + m *apimachiner.State +} + +// Machines implements machiner.Machine and returns a machine from the wrapper +// api/machiner. +func (w wrapper) Machine(tag names.MachineTag) (machine, error) { + m, err := w.m.Machine(tag) + if err != nil { + return nil, err + } + return m, nil +} + +// SetUp implements NotifyWatchHandler's SetUp method. It returns a watcher that +// checks for changes to the current machine. +func (c *converter) SetUp() (watcher.NotifyWatcher, error) { + m, err := c.machiner.Machine(c.agent.Tag().(names.MachineTag)) + if err != nil { + return nil, errors.Trace(err) + } + c.machine = m + return m.Watch() +} + +var utils_RandomPassword = utils.RandomPassword + +// Handle implements NotifyWatchHandler's Handle method. If the change means +// that the machine is now expected to manage the environment, we change its +// password (to set its password in mongo) and restart the agent. +func (c *converter) Handle() error { + results, err := c.machine.Jobs() + if err != nil { + return errors.Annotate(err, "can't get jobs for machine") + } + isState := false + for _, job := range results.Jobs { + if job.NeedsState() { + isState = true + break + } + } + if !isState { + return nil + } + + // We set the password on thisfrom the API in order to get credentials into + // mongo. + logger.Infof("Converting this machine to a state server.") + pw, err := utils_RandomPassword() + if err != nil { + return errors.Annotate(err, "error generating new password") + } + if err := c.agent.SetPassword(pw); err != nil { + return errors.Annotate(err, "error setting machine password") + } + return errors.Trace(c.agent.Restart()) +} + +// TearDown implements NotifyWatchHandler's TearDown method. +func (c *converter) TearDown() error { + return nil +} diff --git a/worker/conv2state/converter_test.go b/worker/conv2state/converter_test.go new file mode 100644 index 00000000000..ac2df0b88da --- /dev/null +++ b/worker/conv2state/converter_test.go @@ -0,0 +1,174 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package conv2state + +import ( + stderrs "errors" + "testing" + + "github.com/juju/errors" + "github.com/juju/names" + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + "github.com/juju/juju/apiserver/params" + "github.com/juju/juju/state/multiwatcher" +) + +var _ = gc.Suite(&Suite{}) + +type Suite struct{} + +func TestPackage(t *testing.T) { + gc.TestingT(t) +} + +func (Suite) TestSetUp(c *gc.C) { + a := &fakeAgent{tag: names.NewMachineTag("1")} + m := &fakeMachine{} + mr := &fakeMachiner{m: m} + conv := converter{machiner: mr, agent: a} + w, err := conv.SetUp() + c.Assert(err, gc.IsNil) + c.Assert(conv.machine, gc.Equals, m) + c.Assert(mr.gotTag, gc.Equals, a.tag.(names.MachineTag)) + c.Assert(w, gc.Equals, m.w) +} + +func (Suite) TestSetupMachinerErr(c *gc.C) { + a := &fakeAgent{tag: names.NewMachineTag("1")} + mr := &fakeMachiner{err: stderrs.New("foo")} + conv := converter{machiner: mr, agent: a} + w, err := conv.SetUp() + c.Assert(errors.Cause(err), gc.Equals, mr.err) + c.Assert(mr.gotTag, gc.Equals, a.tag.(names.MachineTag)) + c.Assert(w, gc.IsNil) +} + +func (Suite) TestSetupWatchErr(c *gc.C) { + a := &fakeAgent{tag: names.NewMachineTag("1")} + m := &fakeMachine{watchErr: stderrs.New("foo")} + mr := &fakeMachiner{m: m} + conv := &converter{machiner: mr, agent: a} + w, err := conv.SetUp() + c.Assert(errors.Cause(err), gc.Equals, m.watchErr) + c.Assert(mr.gotTag, gc.Equals, a.tag.(names.MachineTag)) + c.Assert(w, gc.IsNil) +} + +func (Suite) TestHandle(c *gc.C) { + pw := "password" + utils_RandomPassword = func() (string, error) { return pw, nil } + a := &fakeAgent{tag: names.NewMachineTag("1")} + jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits, multiwatcher.JobManageEnviron} + m := &fakeMachine{ + jobs: ¶ms.JobsResult{Jobs: jobs}, + } + mr := &fakeMachiner{m: m} + conv := &converter{machiner: mr, agent: a} + _, err := conv.SetUp() + c.Assert(err, gc.IsNil) + err = conv.Handle() + c.Assert(err, gc.IsNil) + c.Assert(a.password, gc.Equals, pw) + c.Assert(a.didRestart, jc.IsTrue) +} + +func (Suite) TestHandleNoManageEnviron(c *gc.C) { + pw := "password" + utils_RandomPassword = func() (string, error) { return pw, nil } + a := &fakeAgent{tag: names.NewMachineTag("1")} + jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits} + m := &fakeMachine{ + jobs: ¶ms.JobsResult{Jobs: jobs}, + } + mr := &fakeMachiner{m: m} + conv := &converter{machiner: mr, agent: a} + _, err := conv.SetUp() + c.Assert(err, gc.IsNil) + err = conv.Handle() + c.Assert(err, gc.IsNil) + c.Assert(a.password, gc.Equals, "") + c.Assert(a.didRestart, jc.IsFalse) +} + +func (Suite) TestHandleJobsError(c *gc.C) { + a := &fakeAgent{tag: names.NewMachineTag("1")} + jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits, multiwatcher.JobManageEnviron} + m := &fakeMachine{ + jobs: ¶ms.JobsResult{Jobs: jobs}, + jobsErr: errors.New("foo"), + } + mr := &fakeMachiner{m: m} + conv := &converter{machiner: mr, agent: a} + _, err := conv.SetUp() + c.Assert(err, gc.IsNil) + err = conv.Handle() + c.Assert(errors.Cause(err), gc.Equals, m.jobsErr) + c.Assert(a.password, gc.Equals, "") + c.Assert(a.didRestart, jc.IsFalse) +} + +func (Suite) TestHandlePasswordError(c *gc.C) { + pwErr := errors.New("foo") + utils_RandomPassword = func() (string, error) { return "", pwErr } + a := &fakeAgent{tag: names.NewMachineTag("1")} + jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits, multiwatcher.JobManageEnviron} + m := &fakeMachine{ + jobs: ¶ms.JobsResult{Jobs: jobs}, + } + mr := &fakeMachiner{m: m} + conv := &converter{machiner: mr, agent: a} + _, err := conv.SetUp() + c.Assert(err, gc.IsNil) + err = conv.Handle() + c.Assert(errors.Cause(err), gc.Equals, pwErr) + c.Assert(a.password, gc.Equals, "") + c.Assert(a.didRestart, jc.IsFalse) +} + +func (Suite) TestHandleSetPasswordError(c *gc.C) { + pw := "password" + utils_RandomPassword = func() (string, error) { return pw, nil } + a := &fakeAgent{ + tag: names.NewMachineTag("1"), + pwErr: errors.New("foo"), + } + jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits, multiwatcher.JobManageEnviron} + m := &fakeMachine{ + jobs: ¶ms.JobsResult{Jobs: jobs}, + } + mr := &fakeMachiner{m: m} + conv := &converter{machiner: mr, agent: a} + _, err := conv.SetUp() + c.Assert(err, gc.IsNil) + err = conv.Handle() + c.Assert(errors.Cause(err), gc.Equals, a.pwErr) + c.Assert(a.password, gc.Equals, pw) + c.Assert(a.didRestart, jc.IsFalse) +} + +func (Suite) TestHandleRestartError(c *gc.C) { + pw := "password" + utils_RandomPassword = func() (string, error) { return pw, nil } + a := &fakeAgent{ + tag: names.NewMachineTag("1"), + restartErr: errors.New("foo"), + } + jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits, multiwatcher.JobManageEnviron} + m := &fakeMachine{ + jobs: ¶ms.JobsResult{Jobs: jobs}, + } + mr := &fakeMachiner{m: m} + conv := &converter{machiner: mr, agent: a} + _, err := conv.SetUp() + c.Assert(err, gc.IsNil) + err = conv.Handle() + c.Assert(errors.Cause(err), gc.Equals, a.restartErr) + c.Assert(a.password, gc.Equals, pw) + + // We set this to true whenver the function is called, even though we're + // returning an error from it. + c.Assert(a.didRestart, jc.IsTrue) +} diff --git a/worker/conv2state/fakes_test.go b/worker/conv2state/fakes_test.go new file mode 100644 index 00000000000..8cdd028f5e9 --- /dev/null +++ b/worker/conv2state/fakes_test.go @@ -0,0 +1,76 @@ +// Copyright 2015 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package conv2state + +import ( + "github.com/juju/names" + + "github.com/juju/juju/api/watcher" + "github.com/juju/juju/apiserver/params" +) + +type fakeMachiner struct { + m machine + err error + gotTag names.MachineTag +} + +func (f *fakeMachiner) Machine(tag names.MachineTag) (machine, error) { + f.gotTag = tag + return f.m, f.err +} + +type fakeMachine struct { + jobs *params.JobsResult + jobsErr error + watchErr error + w fakeWatcher +} + +func (f fakeMachine) Jobs() (*params.JobsResult, error) { + return f.jobs, f.jobsErr +} + +func (f fakeMachine) Watch() (watcher.NotifyWatcher, error) { + if f.watchErr == nil { + return f.w, nil + } + return nil, f.watchErr +} + +type fakeWatcher struct{} + +func (fakeWatcher) Changes() <-chan struct{} { + return nil +} + +func (fakeWatcher) Stop() error { + return nil +} + +func (fakeWatcher) Err() error { + return nil +} + +type fakeAgent struct { + password string + tag names.Tag + pwErr error + restartErr error + didRestart bool +} + +func (f *fakeAgent) SetPassword(pw string) error { + f.password = pw + return f.pwErr +} + +func (f *fakeAgent) Restart() error { + f.didRestart = true + return f.restartErr +} + +func (f fakeAgent) Tag() names.Tag { + return f.tag +} From bef21cc9c113a49767d56a5e8222fa73bc8e89f8 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Mon, 6 Apr 2015 00:50:46 -0400 Subject: [PATCH 26/33] remove unused API --- api/converter/converter.go | 74 ------------------ apiserver/converter/converter.go | 127 ------------------------------- 2 files changed, 201 deletions(-) delete mode 100644 api/converter/converter.go delete mode 100644 apiserver/converter/converter.go diff --git a/api/converter/converter.go b/api/converter/converter.go deleted file mode 100644 index e369b632b6f..00000000000 --- a/api/converter/converter.go +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright 2015 Canonical Ltd. -// Licensed under the AGPLv3, see LICENCE file for details. - -// package converter exposes a watcher for machines to tell when they've been -// updated, and an api endpoint to get the machine's jobs. This is used by -// ensure- availability to convert existing machines in the environment into -// state servers. -package converter - -import ( - "github.com/juju/errors" - "github.com/juju/loggo" - "github.com/juju/names" - - "github.com/juju/juju/api/base" - "github.com/juju/juju/api/watcher" - "github.com/juju/juju/apiserver/params" -) - -var logger = loggo.GetLogger("juju.api.converter") - -const converterAPI = "Converter" - -// State exposes a machine watcher and an API endpoint to get a machine's jobs. -type State struct { - facade base.FacadeCaller -} - -// NewState returns a new state object wrapping the given caller. -func NewState(caller base.APICaller) *State { - return &State{facade: base.NewFacadeCaller(caller, converterAPI)} -} - -// WatchMachine returns a watcher that watches the machine with the given -// tag. -func (c *State) WatchMachine(tag names.MachineTag) (watcher.NotifyWatcher, error) { - var results params.NotifyWatchResults - args := params.Entities{ - Entities: []params.Entity{{Tag: tag.String()}}, - } - err := c.facade.FacadeCall("WatchMachines", args, &results) - if err != nil { - return nil, err - } - if len(results.Results) != 1 { - return nil, errors.Errorf("expected 1 result, got %d", len(results.Results)) - } - result := results.Results[0] - if result.Error != nil { - return nil, result.Error - } - return watcher.NewNotifyWatcher(c.facade.RawAPICaller(), result), nil -} - -// Jobs returns a list of jobs for the machine with the given tag. -func (c *State) Jobs(tag names.MachineTag) (*params.JobsResult, error) { - var results params.JobsResults - args := params.Entities{ - Entities: []params.Entity{{Tag: tag.String()}}, - } - - err := c.facade.FacadeCall("Jobs", args, &results) - if err != nil { - return nil, err - } - if len(results.Results) != 1 { - return nil, errors.Errorf("expected 1 result, got %d", len(results.Results)) - } - result := results.Results[0] - if result.Error != nil { - return nil, result.Error - } - return &result, nil -} diff --git a/apiserver/converter/converter.go b/apiserver/converter/converter.go deleted file mode 100644 index 7067b7e6cc3..00000000000 --- a/apiserver/converter/converter.go +++ /dev/null @@ -1,127 +0,0 @@ -// Copyright 2015 Canonical Ltd. -// Licensed under the AGPLv3, see LICENCE file for details. - -package converter - -import ( - "github.com/juju/loggo" - "github.com/juju/names" - - "github.com/juju/juju/apiserver/common" - "github.com/juju/juju/apiserver/params" - "github.com/juju/juju/state" - "github.com/juju/juju/state/multiwatcher" - "github.com/juju/juju/state/watcher" -) - -var logger = loggo.GetLogger("juju.apiserver.converter") - -func init() { - common.RegisterStandardFacade("Converter", 1, NewConverterAPI) -} - -// ConverterAPI contains methods for use with ensure-availability, watching -// machines and their jobs. -type ConverterAPI struct { - st *state.State - resources *common.Resources - authorizer common.Authorizer -} - -// NewConverterAPI returns a new instance of the API. -func NewConverterAPI( - st *state.State, - resources *common.Resources, - authorizer common.Authorizer, -) (*ConverterAPI, error) { - if !authorizer.AuthMachineAgent() { - return nil, common.ErrPerm - } - - return &ConverterAPI{ - st: st, - resources: resources, - authorizer: authorizer, - }, nil -} - -func (c *ConverterAPI) getMachine(tag names.Tag) (*state.Machine, error) { - if tag.Kind() != names.MachineTagKind { - return nil, common.ErrPerm - } - - entity, err := c.st.FindEntity(tag) - if err != nil { - return nil, err - } - return entity.(*state.Machine), nil -} - -// Jobs returns the jobs assigned to the given entities. -func (c *ConverterAPI) Jobs(args params.Entities) (params.JobsResults, error) { - result := params.JobsResults{ - Results: make([]params.JobsResult, len(args.Entities)), - } - - for i, agent := range args.Entities { - tag, err := names.ParseMachineTag(agent.Tag) - if err != nil { - logger.Warningf("not a machine tag: %v", agent.Tag) - result.Results[i].Error = common.ServerError(common.ErrPerm) - continue - } - - err = common.ErrPerm - if c.authorizer.AuthOwner(tag) { - machine, err := c.getMachine(tag) - if err != nil { - logger.Warningf("can't get machine for tag %q: %v", tag, err) - result.Results[i].Error = common.ServerError(err) - continue - } - machineJobs := machine.Jobs() - jobs := make([]multiwatcher.MachineJob, len(machineJobs)) - for i, job := range machineJobs { - jobs[i] = job.ToParams() - } - result.Results[i].Jobs = jobs - } - } - return result, nil -} - -// WatchMachines creates a watcher for each of the specified machines. -func (c *ConverterAPI) WatchMachines(args params.Entities) (params.NotifyWatchResults, error) { - result := params.NotifyWatchResults{ - Results: make([]params.NotifyWatchResult, len(args.Entities)), - } - for i, ent := range args.Entities { - tag, err := names.ParseMachineTag(ent.Tag) - if err != nil { - logger.Warningf("not a machine tag: %v", ent.Tag) - result.Results[i].Error = common.ServerError(common.ErrPerm) - continue - } - if c.authorizer.AuthOwner(tag) { - machine, err := c.getMachine(tag) - if err != nil { - logger.Warningf("can't get machine for tag %q: %v", tag, err) - result.Results[i].Error = common.ServerError(err) - continue - } - - watch := machine.Watch() - // Consume the initial event. Technically, API - // calls to Watch 'transmit' the initial event - // in the Watch response. But NotifyWatchers - // have no state to transmit. - if _, ok := <-watch.Changes(); ok { - result.Results[i].NotifyWatcherId = c.resources.Register(watch) - } else { - err := watcher.EnsureErr(watch) - result.Results[i].Error = common.ServerError(err) - } - } - } - return result, nil -} From d2827133b197a580dcf385957115d3e62aa84dc2 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Mon, 6 Apr 2015 02:18:12 -0400 Subject: [PATCH 27/33] remove references to removed code and use set password for OpenAPIState as well --- api/state.go | 6 ------ apiserver/allfacades.go | 1 - apiserver/machine/machiner_test.go | 19 ++++++++++++++++++ cmd/jujud/agent/agent.go | 31 +++++++++++++++++------------- cmd/jujud/agent/machine.go | 14 ++------------ 5 files changed, 39 insertions(+), 32 deletions(-) diff --git a/api/state.go b/api/state.go index a5006259b6c..7c35c5e45af 100644 --- a/api/state.go +++ b/api/state.go @@ -13,7 +13,6 @@ import ( "github.com/juju/juju/api/agent" "github.com/juju/juju/api/base" "github.com/juju/juju/api/charmrevisionupdater" - "github.com/juju/juju/api/converter" "github.com/juju/juju/api/deployer" "github.com/juju/juju/api/diskmanager" "github.com/juju/juju/api/environment" @@ -353,8 +352,3 @@ func (st *State) Rsyslog() *rsyslog.State { func (st *State) ServerVersion() (version.Number, bool) { return st.serverVersion, st.serverVersion != version.Zero } - -// Converter returns access to the Converter API -func (st *State) Converter() *converter.State { - return converter.NewState(st) -} diff --git a/apiserver/allfacades.go b/apiserver/allfacades.go index 072bda47f20..51e160d9ea2 100644 --- a/apiserver/allfacades.go +++ b/apiserver/allfacades.go @@ -15,7 +15,6 @@ import ( _ "github.com/juju/juju/apiserver/charmrevisionupdater" _ "github.com/juju/juju/apiserver/charms" _ "github.com/juju/juju/apiserver/client" - _ "github.com/juju/juju/apiserver/converter" _ "github.com/juju/juju/apiserver/deployer" _ "github.com/juju/juju/apiserver/diskmanager" _ "github.com/juju/juju/apiserver/environment" diff --git a/apiserver/machine/machiner_test.go b/apiserver/machine/machiner_test.go index 767fe3bc7b5..a286fdbdfe5 100644 --- a/apiserver/machine/machiner_test.go +++ b/apiserver/machine/machiner_test.go @@ -14,6 +14,7 @@ import ( apiservertesting "github.com/juju/juju/apiserver/testing" "github.com/juju/juju/network" "github.com/juju/juju/state" + "github.com/juju/juju/state/multiwatcher" statetesting "github.com/juju/juju/state/testing" ) @@ -183,6 +184,24 @@ func (s *machinerSuite) TestSetMachineAddresses(c *gc.C) { c.Assert(s.machine0.MachineAddresses(), gc.HasLen, 0) } +func (s *machinerSuite) TestJobs(c *gc.C) { + args := params.Entities{Entities: []params.Entity{ + {Tag: "machine-1"}, + {Tag: "machine-0"}, + {Tag: "machine-42"}, + }} + + result, err := s.machiner.Jobs(args) + c.Assert(err, jc.ErrorIsNil) + c.Assert(result, gc.DeepEquals, params.JobsResults{ + Results: []params.JobsResult{ + {Jobs: []multiwatcher.MachineJob{multiwatcher.JobHostUnits}}, + {Error: apiservertesting.ErrUnauthorized}, + {Error: apiservertesting.ErrUnauthorized}, + }, + }) +} + func (s *machinerSuite) TestWatch(c *gc.C) { c.Assert(s.resources.Count(), gc.Equals, 0) diff --git a/cmd/jujud/agent/agent.go b/cmd/jujud/agent/agent.go index ee313c7443d..990c42f7350 100644 --- a/cmd/jujud/agent/agent.go +++ b/cmd/jujud/agent/agent.go @@ -188,19 +188,8 @@ func OpenAPIState(agentConfig agent.Config, a Agent) (_ *api.State, _ *apiagent. if err != nil { return nil, nil, err } - // Change the configuration *before* setting the entity - // password, so that we avoid the possibility that - // we might successfully change the entity's - // password but fail to write the configuration, - // thus locking us out completely. - if err := a.ChangeConfig(func(c agent.ConfigSetter) error { - c.SetPassword(newPassword) - c.SetOldPassword(info.Password) - return nil - }); err != nil { - return nil, nil, err - } - if err := entity.SetPassword(newPassword); err != nil { + err = setAgentPassword(newPassword, info.Password, a, entity) + if err != nil { return nil, nil, err } @@ -216,6 +205,22 @@ func OpenAPIState(agentConfig agent.Config, a Agent) (_ *api.State, _ *apiagent. return st, entity, err } +func setAgentPassword(newPw, oldPw string, a Agent, entity *apiagent.Entity) error { + // Change the configuration *before* setting the entity + // password, so that we avoid the possibility that + // we might successfully change the entity's + // password but fail to write the configuration, + // thus locking us out completely. + if err := a.ChangeConfig(func(c agent.ConfigSetter) error { + c.SetPassword(newPw) + c.SetOldPassword(oldPw) + return nil + }); err != nil { + return err + } + return entity.SetPassword(newPw) +} + // OpenAPIStateUsingInfo opens the API using the given API // information, and returns the opened state and the api entity with // the given tag. diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index f4602e00b9d..46285c26226 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -800,22 +800,12 @@ func (a *MachineAgent) Restart() error { // SetPassword sets the agent's password both in the agentconfig and on the // state server. func (a *MachineAgent) SetPassword(pw string) error { - // Cache the config so we connect with the old one. config := a.CurrentConfig() - - a.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error { - config.SetPassword(pw) - config.SetOldPassword(config.APIInfo().Password) - return nil - }) - _, entity, err := OpenAPIState(config, a) if err != nil { - return errors.Annotate(err, "error opening API") + return err } - - err = entity.SetPassword(pw) - return errors.Annotate(err, "error setting password") + return setAgentPassword(pw, config.APIInfo().Password, a, entity) } func (a *MachineAgent) upgradeStepsWorkerStarter( From 525145d405e357e2546993f75b6f1bbd51a787ec Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Mon, 6 Apr 2015 02:27:41 -0400 Subject: [PATCH 28/33] more code review changes --- cmd/juju/ensureavailability.go | 1 - cmd/juju/ensureavailability_test.go | 2 +- state/addmachine.go | 10 +++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cmd/juju/ensureavailability.go b/cmd/juju/ensureavailability.go index c65a05d83bb..02d7e9624c6 100644 --- a/cmd/juju/ensureavailability.go +++ b/cmd/juju/ensureavailability.go @@ -156,7 +156,6 @@ func (c *EnsureAvailabilityCommand) Init(args []string) error { continue } if err != instance.ErrPlacementScopeMissing { - // We only support unscoped placement directives. return fmt.Errorf("unsupported ensure-availability placement directive %q", spec) } c.Placement[i] = spec diff --git a/cmd/juju/ensureavailability_test.go b/cmd/juju/ensureavailability_test.go index aa0790b8b5a..fa8aba8abcb 100644 --- a/cmd/juju/ensureavailability_test.go +++ b/cmd/juju/ensureavailability_test.go @@ -85,7 +85,7 @@ func (f *fakeHAClient) EnsureAvailability(numStateServers int, cons constraints. for _, p := range placement { m, err := instance.ParsePlacement(p) - if err == nil { + if err == nil && m.Scope == instance.MachineScope { f.result.Converted = append(f.result.Converted, "machine-"+m.Directive) } } diff --git a/state/addmachine.go b/state/addmachine.go index 8a2ee39ca80..0efce4e1027 100644 --- a/state/addmachine.go +++ b/state/addmachine.go @@ -637,10 +637,14 @@ func (st *State) EnsureAvailability( intent.promote = intent.promote[:n] } voteCount += len(intent.promote) - intent.newCount = desiredStateServerCount - voteCount - len(intent.convert) - if intent.newCount < 0 { - intent.newCount = 0 + + if n := desiredStateServerCount - voteCount; n < len(intent.convert) { + intent.convert = intent.convert[:n] } + voteCount += len(intent.convert) + + intent.newCount = desiredStateServerCount - voteCount + logger.Infof("%d new machines; promoting %v; converting %v", intent.newCount, intent.promote, intent.convert) var ops []txn.Op From 137e45ef7ad069b4e871e80ebed53a8ef7e7c62c Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Mon, 6 Apr 2015 23:16:38 -0400 Subject: [PATCH 29/33] put setpassword in OpenAPIState and remove some unused cruft --- agent/agent.go | 13 ------ api/facadeversions.go | 1 - apiserver/params/params.go | 6 +-- cmd/juju/ensureavailability.go | 7 +++- cmd/juju/ensureavailability_test.go | 8 ++-- cmd/jujud/agent/agent.go | 6 ++- state/addmachine.go | 14 ++++++- worker/conv2state/converter.go | 24 +---------- worker/conv2state/converter_test.go | 62 ++++------------------------- worker/conv2state/fakes_test.go | 7 ---- 10 files changed, 41 insertions(+), 107 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 7ac9d7cf9eb..36685adcb02 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -189,10 +189,6 @@ type ConfigSetterOnly interface { // SetStateServingInfo sets the information needed // to run a state server SetStateServingInfo(info params.StateServingInfo) - - // SetStateAddresses sets the addresses of state machines this machine will - // connect to. - SetStateAddresses(addrs []string) } type ConfigWriter interface { @@ -498,15 +494,6 @@ func (c *configInternal) SetAPIHostPorts(servers [][]network.HostPort) { c.apiDetails.addresses = addrs } -// SetStateAddresses sets the addresses of state machines this machine will -// connect to. -func (c *configInternal) SetStateAddresses(servers []string) { - if c.stateDetails == nil { - c.stateDetails = &connectionDetails{} - } - c.stateDetails.addresses = servers -} - func (c *configInternal) SetValue(key, value string) { if value == "" { delete(c.values, key) diff --git a/api/facadeversions.go b/api/facadeversions.go index 234a268e5c2..2c1d19f78f8 100644 --- a/api/facadeversions.go +++ b/api/facadeversions.go @@ -20,7 +20,6 @@ var facadeVersions = map[string]int{ "Charms": 1, "CharmRevisionUpdater": 0, "Client": 0, - "Converter": 1, "Deployer": 0, "DiskManager": 1, "Environment": 0, diff --git a/apiserver/params/params.go b/apiserver/params/params.go index 697e7ae2a5d..b2d9af15190 100644 --- a/apiserver/params/params.go +++ b/apiserver/params/params.go @@ -552,13 +552,13 @@ type RsyslogConfigResults struct { // JobsResult holds the jobs for a machine that are returned by a call to Jobs. type JobsResult struct { - Jobs []multiwatcher.MachineJob - Error *Error + Jobs []multiwatcher.MachineJob `json:"Jobs"` + Error *Error `json:"Error"` } // JobsResults holds the result of a call to Jobs. type JobsResults struct { - Results []JobsResult + Results []JobsResult `json:"Results"` } // DistributionGroupResult contains the result of diff --git a/cmd/juju/ensureavailability.go b/cmd/juju/ensureavailability.go index 02d7e9624c6..17cb8f64301 100644 --- a/cmd/juju/ensureavailability.go +++ b/cmd/juju/ensureavailability.go @@ -151,11 +151,14 @@ func (c *EnsureAvailabilityCommand) Init(args []string) error { for i, spec := range placementSpecs { p, err := instance.ParsePlacement(strings.TrimSpace(spec)) if err == nil && p.Scope == instance.MachineScope { - // targeting machines is ok + if names.IsContainerMachine(p.Directive) { + return errors.New("ensure-availability cannot be used with container placement directives") + } + // Targeting machines is ok. c.Placement[i] = p.String() continue } - if err != instance.ErrPlacementScopeMissing { + if err != instance.ErrPlacementScopeMissing || names.IsContainerMachine(p.Directive) { return fmt.Errorf("unsupported ensure-availability placement directive %q", spec) } c.Placement[i] = spec diff --git a/cmd/juju/ensureavailability_test.go b/cmd/juju/ensureavailability_test.go index fa8aba8abcb..8e5d78ddf2e 100644 --- a/cmd/juju/ensureavailability_test.go +++ b/cmd/juju/ensureavailability_test.go @@ -264,9 +264,11 @@ func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityEndToEnd(c *gc.C) { func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityToExisting(c *gc.C) { ctx, err := s.runEnsureAvailability(c, "--to", "1,2") c.Assert(err, jc.ErrorIsNil) - c.Check(coretesting.Stdout(ctx), gc.Equals, - "maintaining machines: 0\n"+ - "converting machines: 1, 2\n\n") + c.Check(coretesting.Stdout(ctx), gc.Equals, ` +maintaining machines: 0 +converting machines: 1, 2 + +`[1:]) c.Check(s.fake.numStateServers, gc.Equals, 0) c.Check(&s.fake.cons, jc.Satisfies, constraints.IsEmpty) diff --git a/cmd/jujud/agent/agent.go b/cmd/jujud/agent/agent.go index 990c42f7350..3ce57fd0ae1 100644 --- a/cmd/jujud/agent/agent.go +++ b/cmd/jujud/agent/agent.go @@ -180,7 +180,11 @@ func OpenAPIState(agentConfig agent.Config, a Agent) (_ *api.State, _ *apiagent. return nil, nil, err } - if usedOldPassword { + if !usedOldPassword { + if err := entity.SetPassword(info.Password); err != nil { + return nil, nil, errors.Annotate(err, "can't reset agent password") + } + } else { // We succeeded in connecting with the fallback // password, so we need to create a new password // for the future. diff --git a/state/addmachine.go b/state/addmachine.go index 0efce4e1027..787aba31a9c 100644 --- a/state/addmachine.go +++ b/state/addmachine.go @@ -771,12 +771,23 @@ type ensureAvailabilityIntent struct { func (st *State) ensureAvailabilityIntentions(info *StateServerInfo, placement []string) (*ensureAvailabilityIntent, error) { var intent ensureAvailabilityIntent for _, s := range placement { + // TODO(natefinch): unscoped placements shouldn't ever get here (though + // they do currently). We should fix up the CLI to always add a scope + // to placements and then we can remove the need to deal with unscoped + // placements. p, err := instance.ParsePlacement(s) if err == instance.ErrPlacementScopeMissing { intent.placement = append(intent.placement, s) continue } if err == nil && p.Scope == instance.MachineScope { + // TODO(natefinch) add env provider policy to check if conversion is + // possible (e.g. cannot be supported by Azure in HA mode). + + if names.IsContainerMachine(p.Directive) { + return nil, errors.New("container placement directives not supported") + } + m, err := st.Machine(p.Directive) if err != nil { return nil, errors.Annotatef(err, "can't find machine for placement directive %q", s) @@ -788,7 +799,7 @@ func (st *State) ensureAvailabilityIntentions(info *StateServerInfo, placement [ intent.placement = append(intent.placement, s) continue } - return nil, errors.Errorf("unsupported HA placement directive %q", s) + return nil, errors.Errorf("unsupported placement directive %q", s) } for _, mid := range info.MachineIds { @@ -838,6 +849,7 @@ func convertStateServerOps(m *Machine) []txn.Op { {"$addToSet", bson.D{{"jobs", JobManageEnviron}}}, {"$set", bson.D{{"novote", false}}}, }, + Assert: bson.D{{"jobs", bson.D{{"$nin", []MachineJob{JobManageEnviron}}}}}, }, { C: stateServersC, Id: environGlobalKey, diff --git a/worker/conv2state/converter.go b/worker/conv2state/converter.go index 20be57e81c3..eb950bb8c19 100644 --- a/worker/conv2state/converter.go +++ b/worker/conv2state/converter.go @@ -7,11 +7,11 @@ import ( "github.com/juju/errors" "github.com/juju/loggo" "github.com/juju/names" - "github.com/juju/utils" apimachiner "github.com/juju/juju/api/machiner" "github.com/juju/juju/api/watcher" "github.com/juju/juju/apiserver/params" + "github.com/juju/juju/state/multiwatcher" "github.com/juju/juju/worker" ) @@ -33,7 +33,6 @@ type converter struct { // Agent is an interface that can have its password set and be told to restart. type Agent interface { - SetPassword(string) error Restart() error Tag() names.Tag } @@ -76,8 +75,6 @@ func (c *converter) SetUp() (watcher.NotifyWatcher, error) { return m.Watch() } -var utils_RandomPassword = utils.RandomPassword - // Handle implements NotifyWatchHandler's Handle method. If the change means // that the machine is now expected to manage the environment, we change its // password (to set its password in mongo) and restart the agent. @@ -86,27 +83,10 @@ func (c *converter) Handle() error { if err != nil { return errors.Annotate(err, "can't get jobs for machine") } - isState := false - for _, job := range results.Jobs { - if job.NeedsState() { - isState = true - break - } - } - if !isState { + if !multiwatcher.AnyJobNeedsState(results.Jobs...) { return nil } - // We set the password on thisfrom the API in order to get credentials into - // mongo. - logger.Infof("Converting this machine to a state server.") - pw, err := utils_RandomPassword() - if err != nil { - return errors.Annotate(err, "error generating new password") - } - if err := c.agent.SetPassword(pw); err != nil { - return errors.Annotate(err, "error setting machine password") - } return errors.Trace(c.agent.Restart()) } diff --git a/worker/conv2state/converter_test.go b/worker/conv2state/converter_test.go index ac2df0b88da..bbefdfade28 100644 --- a/worker/conv2state/converter_test.go +++ b/worker/conv2state/converter_test.go @@ -14,16 +14,19 @@ import ( "github.com/juju/juju/apiserver/params" "github.com/juju/juju/state/multiwatcher" + coretesting "github.com/juju/juju/testing" ) var _ = gc.Suite(&Suite{}) -type Suite struct{} - func TestPackage(t *testing.T) { gc.TestingT(t) } +type Suite struct { + coretesting.BaseSuite +} + func (Suite) TestSetUp(c *gc.C) { a := &fakeAgent{tag: names.NewMachineTag("1")} m := &fakeMachine{} @@ -57,9 +60,7 @@ func (Suite) TestSetupWatchErr(c *gc.C) { c.Assert(w, gc.IsNil) } -func (Suite) TestHandle(c *gc.C) { - pw := "password" - utils_RandomPassword = func() (string, error) { return pw, nil } +func (s Suite) TestHandle(c *gc.C) { a := &fakeAgent{tag: names.NewMachineTag("1")} jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits, multiwatcher.JobManageEnviron} m := &fakeMachine{ @@ -71,13 +72,10 @@ func (Suite) TestHandle(c *gc.C) { c.Assert(err, gc.IsNil) err = conv.Handle() c.Assert(err, gc.IsNil) - c.Assert(a.password, gc.Equals, pw) c.Assert(a.didRestart, jc.IsTrue) } -func (Suite) TestHandleNoManageEnviron(c *gc.C) { - pw := "password" - utils_RandomPassword = func() (string, error) { return pw, nil } +func (s Suite) TestHandleNoManageEnviron(c *gc.C) { a := &fakeAgent{tag: names.NewMachineTag("1")} jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits} m := &fakeMachine{ @@ -89,7 +87,6 @@ func (Suite) TestHandleNoManageEnviron(c *gc.C) { c.Assert(err, gc.IsNil) err = conv.Handle() c.Assert(err, gc.IsNil) - c.Assert(a.password, gc.Equals, "") c.Assert(a.didRestart, jc.IsFalse) } @@ -106,52 +103,10 @@ func (Suite) TestHandleJobsError(c *gc.C) { c.Assert(err, gc.IsNil) err = conv.Handle() c.Assert(errors.Cause(err), gc.Equals, m.jobsErr) - c.Assert(a.password, gc.Equals, "") - c.Assert(a.didRestart, jc.IsFalse) -} - -func (Suite) TestHandlePasswordError(c *gc.C) { - pwErr := errors.New("foo") - utils_RandomPassword = func() (string, error) { return "", pwErr } - a := &fakeAgent{tag: names.NewMachineTag("1")} - jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits, multiwatcher.JobManageEnviron} - m := &fakeMachine{ - jobs: ¶ms.JobsResult{Jobs: jobs}, - } - mr := &fakeMachiner{m: m} - conv := &converter{machiner: mr, agent: a} - _, err := conv.SetUp() - c.Assert(err, gc.IsNil) - err = conv.Handle() - c.Assert(errors.Cause(err), gc.Equals, pwErr) - c.Assert(a.password, gc.Equals, "") - c.Assert(a.didRestart, jc.IsFalse) -} - -func (Suite) TestHandleSetPasswordError(c *gc.C) { - pw := "password" - utils_RandomPassword = func() (string, error) { return pw, nil } - a := &fakeAgent{ - tag: names.NewMachineTag("1"), - pwErr: errors.New("foo"), - } - jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits, multiwatcher.JobManageEnviron} - m := &fakeMachine{ - jobs: ¶ms.JobsResult{Jobs: jobs}, - } - mr := &fakeMachiner{m: m} - conv := &converter{machiner: mr, agent: a} - _, err := conv.SetUp() - c.Assert(err, gc.IsNil) - err = conv.Handle() - c.Assert(errors.Cause(err), gc.Equals, a.pwErr) - c.Assert(a.password, gc.Equals, pw) c.Assert(a.didRestart, jc.IsFalse) } -func (Suite) TestHandleRestartError(c *gc.C) { - pw := "password" - utils_RandomPassword = func() (string, error) { return pw, nil } +func (s Suite) TestHandleRestartError(c *gc.C) { a := &fakeAgent{ tag: names.NewMachineTag("1"), restartErr: errors.New("foo"), @@ -166,7 +121,6 @@ func (Suite) TestHandleRestartError(c *gc.C) { c.Assert(err, gc.IsNil) err = conv.Handle() c.Assert(errors.Cause(err), gc.Equals, a.restartErr) - c.Assert(a.password, gc.Equals, pw) // We set this to true whenver the function is called, even though we're // returning an error from it. diff --git a/worker/conv2state/fakes_test.go b/worker/conv2state/fakes_test.go index 8cdd028f5e9..797ff2edf79 100644 --- a/worker/conv2state/fakes_test.go +++ b/worker/conv2state/fakes_test.go @@ -54,18 +54,11 @@ func (fakeWatcher) Err() error { } type fakeAgent struct { - password string tag names.Tag - pwErr error restartErr error didRestart bool } -func (f *fakeAgent) SetPassword(pw string) error { - f.password = pw - return f.pwErr -} - func (f *fakeAgent) Restart() error { f.didRestart = true return f.restartErr From 7d37fdb71929f1d795d3b3ad8dd7501f9149135f Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Mon, 6 Apr 2015 23:42:22 -0400 Subject: [PATCH 30/33] tests for apiserver/highavailability --- .../highavailability/highavailability_test.go | 49 +++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/apiserver/highavailability/highavailability_test.go b/apiserver/highavailability/highavailability_test.go index 1c151cf5b63..d9856417db5 100644 --- a/apiserver/highavailability/highavailability_test.go +++ b/apiserver/highavailability/highavailability_test.go @@ -33,7 +33,7 @@ type clientSuite struct { resources *common.Resources authoriser apiservertesting.FakeAuthorizer haServer *highavailability.HighAvailabilityAPI - pinger *presence.Pinger + pingers []*presence.Pinger commontesting.BlockHelper } @@ -71,13 +71,15 @@ func (s *clientSuite) SetUpTest(c *gc.C) { c.Assert(err, jc.ErrorIsNil) // We have to ensure the agents are alive, or EnsureAvailability will // create more to replace them. - s.pinger = s.setAgentPresence(c, "0") + s.pingers = []*presence.Pinger{s.setAgentPresence(c, "0")} s.BlockHelper = commontesting.NewBlockHelper(s.APIState) s.AddCleanup(func(*gc.C) { s.BlockHelper.Close() }) } func (s *clientSuite) TearDownTest(c *gc.C) { - assertKill(c, s.pinger) + for _, pinger := range s.pingers { + assertKill(c, pinger) + } s.JujuConnSuite.TearDownTest(c) } @@ -132,6 +134,7 @@ func (s *clientSuite) TestEnsureAvailabilitySeries(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.DeepEquals, []string{"machine-0"}) c.Assert(ensureAvailabilityResult.Added, gc.DeepEquals, []string{"machine-1", "machine-2"}) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) machines, err = s.State.AllMachines() c.Assert(err, jc.ErrorIsNil) @@ -151,6 +154,7 @@ func (s *clientSuite) TestEnsureAvailabilitySeries(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.DeepEquals, []string{"machine-0", "machine-1", "machine-2"}) c.Assert(ensureAvailabilityResult.Added, gc.DeepEquals, []string{"machine-3", "machine-4"}) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) c.Assert(err, jc.ErrorIsNil) machines, err = s.State.AllMachines() @@ -169,6 +173,7 @@ func (s *clientSuite) TestEnsureAvailabilityConstraints(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.DeepEquals, []string{"machine-0"}) c.Assert(ensureAvailabilityResult.Added, gc.DeepEquals, []string{"machine-1", "machine-2"}) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) machines, err := s.State.AllMachines() c.Assert(err, jc.ErrorIsNil) @@ -195,6 +200,7 @@ func (s *clientSuite) TestBlockEnsureAvailability(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.HasLen, 0) c.Assert(ensureAvailabilityResult.Added, gc.HasLen, 0) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) machines, err := s.State.AllMachines() c.Assert(err, jc.ErrorIsNil) @@ -208,6 +214,7 @@ func (s *clientSuite) TestEnsureAvailabilityPlacement(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.DeepEquals, []string{"machine-0"}) c.Assert(ensureAvailabilityResult.Added, gc.DeepEquals, []string{"machine-1", "machine-2"}) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) machines, err := s.State.AllMachines() c.Assert(err, jc.ErrorIsNil) @@ -226,6 +233,36 @@ func (s *clientSuite) TestEnsureAvailabilityPlacement(c *gc.C) { } } +func (s *clientSuite) TestEnsureAvailabilityPlacementTo(c *gc.C) { + _, err := s.State.AddMachine("quantal", state.JobHostUnits) + c.Assert(err, jc.ErrorIsNil) + s.pingers = append(s.pingers, s.setAgentPresence(c, "1")) + + _, err = s.State.AddMachine("quantal", state.JobHostUnits) + c.Assert(err, jc.ErrorIsNil) + s.pingers = append(s.pingers, s.setAgentPresence(c, "2")) + + placement := []string{"1", "2"} + ensureAvailabilityResult, err := s.ensureAvailability(c, 3, emptyCons, defaultSeries, placement) + c.Assert(err, jc.ErrorIsNil) + c.Assert(ensureAvailabilityResult.Maintained, gc.DeepEquals, []string{"machine-0"}) + c.Assert(ensureAvailabilityResult.Added, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.DeepEquals, []string{"machine-1", "machine-2"}) + + machines, err := s.State.AllMachines() + c.Assert(err, jc.ErrorIsNil) + c.Assert(machines, gc.HasLen, 3) + expectedCons := []constraints.Value{{}, {}, {}} + expectedPlacement := []string{"", "", ""} + for i, m := range machines { + cons, err := m.Constraints() + c.Assert(err, jc.ErrorIsNil) + c.Check(cons, gc.DeepEquals, expectedCons[i]) + c.Check(m.Placement(), gc.Equals, expectedPlacement[i]) + } +} + func (s *clientSuite) TestEnsureAvailability0Preserves(c *gc.C) { // A value of 0 says either "if I'm not HA, make me HA" or "preserve my // current HA settings". @@ -234,6 +271,7 @@ func (s *clientSuite) TestEnsureAvailability0Preserves(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.DeepEquals, []string{"machine-0"}) c.Assert(ensureAvailabilityResult.Added, gc.DeepEquals, []string{"machine-1", "machine-2"}) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) machines, err := s.State.AllMachines() c.Assert(machines, gc.HasLen, 3) @@ -248,6 +286,7 @@ func (s *clientSuite) TestEnsureAvailability0Preserves(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.DeepEquals, []string{"machine-0", "machine-1"}) c.Assert(ensureAvailabilityResult.Added, gc.DeepEquals, []string{"machine-3"}) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) machines, err = s.State.AllMachines() c.Assert(machines, gc.HasLen, 4) @@ -260,6 +299,7 @@ func (s *clientSuite) TestEnsureAvailability0Preserves5(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.DeepEquals, []string{"machine-0"}) c.Assert(ensureAvailabilityResult.Added, gc.DeepEquals, []string{"machine-1", "machine-2", "machine-3", "machine-4"}) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) machines, err := s.State.AllMachines() c.Assert(machines, gc.HasLen, 5) @@ -278,6 +318,7 @@ func (s *clientSuite) TestEnsureAvailability0Preserves5(c *gc.C) { "machine-2", "machine-3"}) c.Assert(ensureAvailabilityResult.Added, gc.DeepEquals, []string{"machine-5"}) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) machines, err = s.State.AllMachines() c.Assert(machines, gc.HasLen, 6) @@ -293,6 +334,7 @@ func (s *clientSuite) TestEnsureAvailabilityErrors(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.DeepEquals, []string{"machine-0"}) c.Assert(ensureAvailabilityResult.Added, gc.DeepEquals, []string{"machine-1", "machine-2"}) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) _, err = s.ensureAvailability(c, 1, emptyCons, defaultSeries, nil) c.Assert(err, gc.ErrorMatches, "failed to create new state server machines: cannot reduce state server count") @@ -311,6 +353,7 @@ func (s *clientSuite) TestEnsureAvailabilityHostedEnvErrors(c *gc.C) { c.Assert(ensureAvailabilityResult.Maintained, gc.HasLen, 0) c.Assert(ensureAvailabilityResult.Added, gc.HasLen, 0) c.Assert(ensureAvailabilityResult.Removed, gc.HasLen, 0) + c.Assert(ensureAvailabilityResult.Converted, gc.HasLen, 0) machines, err := st2.AllMachines() c.Assert(err, jc.ErrorIsNil) From 3f0e50ad7da39f5b98e839ad33cac3da083004d0 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Mon, 6 Apr 2015 23:54:56 -0400 Subject: [PATCH 31/33] state test for ensure --to --- state/state_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/state/state_test.go b/state/state_test.go index 889e2acea5e..45ccd08eecf 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -3738,6 +3738,47 @@ func (s *StateSuite) TestEnsureAvailabilityAddsNewMachines(c *gc.C) { s.assertStateServerInfo(c, ids, ids, nil) } +func (s *StateSuite) TestEnsureAvailabilityTo(c *gc.C) { + // Don't use agent presence to decide on machine availability. + s.PatchValue(state.StateServerAvailable, func(m *state.Machine) (bool, error) { + return true, nil + }) + + ids := make([]string, 3) + m0, err := s.State.AddMachine("quantal", state.JobHostUnits, state.JobManageEnviron) + c.Assert(err, jc.ErrorIsNil) + ids[0] = m0.Id() + + // Add two non-state-server machines. + _, err = s.State.AddMachine("quantal", state.JobHostUnits) + c.Assert(err, jc.ErrorIsNil) + + _, err = s.State.AddMachine("quantal", state.JobHostUnits) + c.Assert(err, jc.ErrorIsNil) + + s.assertStateServerInfo(c, []string{"0"}, []string{"0"}, nil) + + changes, err := s.State.EnsureAvailability(3, constraints.Value{}, "quantal", []string{"1", "2"}) + c.Assert(err, jc.ErrorIsNil) + c.Assert(changes.Added, gc.HasLen, 0) + c.Assert(changes.Converted, gc.HasLen, 2) + + for i := 1; i < 3; i++ { + m, err := s.State.Machine(fmt.Sprint(i)) + c.Assert(err, jc.ErrorIsNil) + c.Assert(m.Jobs(), gc.DeepEquals, []state.MachineJob{ + state.JobHostUnits, + state.JobManageEnviron, + }) + gotCons, err := m.Constraints() + c.Assert(err, jc.ErrorIsNil) + c.Assert(gotCons, gc.DeepEquals, constraints.Value{}) + c.Assert(m.WantsVote(), jc.IsTrue) + ids[i] = m.Id() + } + s.assertStateServerInfo(c, ids, ids, nil) +} + func newUint64(i uint64) *uint64 { return &i } From 817f885581edac76abccd2f4b7512f861ee220b1 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Tue, 7 Apr 2015 10:09:02 -0400 Subject: [PATCH 32/33] add an explanation and remove unused code --- cmd/jujud/agent/agent.go | 2 ++ cmd/jujud/agent/machine.go | 11 ----------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/cmd/jujud/agent/agent.go b/cmd/jujud/agent/agent.go index 3ce57fd0ae1..415dff07ff7 100644 --- a/cmd/jujud/agent/agent.go +++ b/cmd/jujud/agent/agent.go @@ -181,6 +181,8 @@ func OpenAPIState(agentConfig agent.Config, a Agent) (_ *api.State, _ *apiagent. } if !usedOldPassword { + // Call set password with the current password. If we've recently + // become a state server, this will fix up our credentials in mongo. if err := entity.SetPassword(info.Password); err != nil { return nil, nil, errors.Annotate(err, "can't reset agent password") } diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go index 46285c26226..afab034d4fd 100644 --- a/cmd/jujud/agent/machine.go +++ b/cmd/jujud/agent/machine.go @@ -797,17 +797,6 @@ func (a *MachineAgent) Restart() error { return service.Restart(name) } -// SetPassword sets the agent's password both in the agentconfig and on the -// state server. -func (a *MachineAgent) SetPassword(pw string) error { - config := a.CurrentConfig() - _, entity, err := OpenAPIState(config, a) - if err != nil { - return err - } - return setAgentPassword(pw, config.APIInfo().Password, a, entity) -} - func (a *MachineAgent) upgradeStepsWorkerStarter( st *api.State, jobs []multiwatcher.MachineJob, From b59c254b5e77f875c71b1923685d903314893ce5 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Thu, 9 Apr 2015 06:54:59 -0400 Subject: [PATCH 33/33] fix test failures --- api/machiner/machiner_test.go | 2 +- api/networker/networker_test.go | 41 ++++++++++++++++++++ apiserver/admin_test.go | 10 ++--- apiserver/networker/networker_test.go | 56 +++++++++++++++++++++++---- apiserver/server_test.go | 4 +- cmd/juju/ensureavailability.go | 8 ++-- 6 files changed, 103 insertions(+), 18 deletions(-) diff --git a/api/machiner/machiner_test.go b/api/machiner/machiner_test.go index 2dc8a750cb7..a51fe691916 100644 --- a/api/machiner/machiner_test.go +++ b/api/machiner/machiner_test.go @@ -54,7 +54,7 @@ func (s *machinerSuite) SetUpTest(c *gc.C) { func (s *machinerSuite) TestMachineAndMachineTag(c *gc.C) { machine, err := s.machiner.Machine(names.NewMachineTag("42")) - c.Assert(err, gc.ErrorMatches, "permission denied") + c.Assert(err, gc.ErrorMatches, ".*permission denied") c.Assert(err, jc.Satisfies, params.IsCodeUnauthorized) c.Assert(machine, gc.IsNil) diff --git a/api/networker/networker_test.go b/api/networker/networker_test.go index 6261c0869bf..feb2418054c 100644 --- a/api/networker/networker_test.go +++ b/api/networker/networker_test.go @@ -5,6 +5,7 @@ package networker_test import ( "runtime" + "sort" "github.com/juju/names" jc "github.com/juju/testing/checkers" @@ -201,6 +202,38 @@ func (s *networkerSuite) TestMachineNetworkConfigNameChange(c *gc.C) { c.Assert(info, gc.IsNil) } +type orderedIfc []network.InterfaceInfo + +func (o orderedIfc) Len() int { + return len(o) +} + +func (o orderedIfc) Less(i, j int) bool { + if o[i].MACAddress < o[j].MACAddress { + return true + } + if o[i].MACAddress > o[j].MACAddress { + return false + } + if o[i].CIDR < o[j].CIDR { + return true + } + if o[i].CIDR > o[j].CIDR { + return false + } + if o[i].NetworkName < o[j].NetworkName { + return true + } + if o[i].NetworkName > o[j].NetworkName { + return false + } + return o[i].VLANTag < o[j].VLANTag +} + +func (o orderedIfc) Swap(i, j int) { + o[i], o[j] = o[j], o[i] +} + func (s *networkerSuite) TestMachineNetworkConfig(c *gc.C) { // TODO(bogdanteleaga): Find out what's the problem with this test // It seems to work on some machines @@ -245,6 +278,8 @@ func (s *networkerSuite) TestMachineNetworkConfig(c *gc.C) { InterfaceName: "eth2", Disabled: true, }} + sort.Sort(orderedIfc(expectedMachineInfo)) + expectedContainerInfo := []network.InterfaceInfo{{ MACAddress: "aa:bb:cc:dd:ee:e0", CIDR: "0.1.2.0/24", @@ -267,6 +302,8 @@ func (s *networkerSuite) TestMachineNetworkConfig(c *gc.C) { VLANTag: 42, InterfaceName: "eth1", }} + sort.Sort(orderedIfc(expectedContainerInfo)) + expectedNestedContainerInfo := []network.InterfaceInfo{{ MACAddress: "aa:bb:cc:dd:ee:d0", CIDR: "0.1.2.0/24", @@ -275,17 +312,21 @@ func (s *networkerSuite) TestMachineNetworkConfig(c *gc.C) { VLANTag: 0, InterfaceName: "eth0", }} + sort.Sort(orderedIfc(expectedNestedContainerInfo)) results, err := s.networker.MachineNetworkConfig(names.NewMachineTag("0")) c.Assert(err, jc.ErrorIsNil) + sort.Sort(orderedIfc(results)) c.Assert(results, gc.DeepEquals, expectedMachineInfo) results, err = s.networker.MachineNetworkConfig(names.NewMachineTag("0/lxc/0")) c.Assert(err, jc.ErrorIsNil) + sort.Sort(orderedIfc(results)) c.Assert(results, gc.DeepEquals, expectedContainerInfo) results, err = s.networker.MachineNetworkConfig(names.NewMachineTag("0/lxc/0/lxc/0")) c.Assert(err, jc.ErrorIsNil) + sort.Sort(orderedIfc(results)) c.Assert(results, gc.DeepEquals, expectedNestedContainerInfo) } diff --git a/apiserver/admin_test.go b/apiserver/admin_test.go index c27c8de704a..e2a87205658 100644 --- a/apiserver/admin_test.go +++ b/apiserver/admin_test.go @@ -155,7 +155,7 @@ func (s *loginSuite) TestBadLogin(c *gc.C) { defer st.Close() _, err = st.Machiner().Machine(names.NewMachineTag("0")) - c.Assert(err, gc.ErrorMatches, `unknown object type "Machiner"`) + c.Assert(err, gc.ErrorMatches, `.*unknown object type "Machiner"`) // Since these are user login tests, the nonce is empty. err = st.Login(t.tag, t.password, "") @@ -163,7 +163,7 @@ func (s *loginSuite) TestBadLogin(c *gc.C) { c.Assert(params.ErrCode(err), gc.Equals, t.code) _, err = st.Machiner().Machine(names.NewMachineTag("0")) - c.Assert(err, gc.ErrorMatches, `unknown object type "Machiner"`) + c.Assert(err, gc.ErrorMatches, `.*unknown object type "Machiner"`) }() } } @@ -181,14 +181,14 @@ func (s *loginSuite) TestLoginAsDeactivatedUser(c *gc.C) { u := s.Factory.MakeUser(c, &factory.UserParams{Password: password, Disabled: true}) _, err = st.Client().Status([]string{}) - c.Assert(err, gc.ErrorMatches, `unknown object type "Client"`) + c.Assert(err, gc.ErrorMatches, `.*unknown object type "Client"`) // Since these are user login tests, the nonce is empty. err = st.Login(u.Tag().String(), password, "") c.Assert(err, gc.ErrorMatches, "invalid entity name or password") _, err = st.Client().Status([]string{}) - c.Assert(err, gc.ErrorMatches, `unknown object type "Client"`) + c.Assert(err, gc.ErrorMatches, `.*unknown object type "Client"`) } func (s *loginV0Suite) TestLoginSetsLogIdentifier(c *gc.C) { @@ -635,7 +635,7 @@ func (s *baseLoginSuite) checkLoginWithValidator(c *gc.C, validator apiserver.Lo // Ensure not already logged in. _, err := st.Machiner().Machine(names.NewMachineTag("0")) - c.Assert(err, gc.ErrorMatches, `unknown object type "Machiner"`) + c.Assert(err, gc.ErrorMatches, `*.unknown object type "Machiner"`) adminUser := s.AdminUserTag(c) // Since these are user login tests, the nonce is empty. diff --git a/apiserver/networker/networker_test.go b/apiserver/networker/networker_test.go index 84ffeb5be2c..49914738e58 100644 --- a/apiserver/networker/networker_test.go +++ b/apiserver/networker/networker_test.go @@ -5,6 +5,7 @@ package networker_test import ( "runtime" + "sort" "github.com/juju/names" jc "github.com/juju/testing/checkers" @@ -210,6 +211,38 @@ func (s *networkerSuite) TestMachineNetworkConfigPermissions(c *gc.C) { }) } +type orderedNetwork []params.NetworkConfig + +func (o orderedNetwork) Len() int { + return len(o) +} + +func (o orderedNetwork) Less(i, j int) bool { + if o[i].MACAddress < o[j].MACAddress { + return true + } + if o[i].MACAddress > o[j].MACAddress { + return false + } + if o[i].CIDR < o[j].CIDR { + return true + } + if o[i].CIDR > o[j].CIDR { + return false + } + if o[i].NetworkName < o[j].NetworkName { + return true + } + if o[i].NetworkName > o[j].NetworkName { + return false + } + return o[i].VLANTag < o[j].VLANTag +} + +func (o orderedNetwork) Swap(i, j int) { + o[i], o[j] = o[j], o[i] +} + func (s *networkerSuite) TestMachineNetworkConfig(c *gc.C) { // TODO(bogdanteleaga): Find out what's the problem with this test // It seems to work on some machines @@ -290,16 +323,25 @@ func (s *networkerSuite) TestMachineNetworkConfig(c *gc.C) { {Tag: "machine-0-lxc-0-lxc-0"}, }} + sort.Sort(orderedNetwork(expectedMachineConfig)) + sort.Sort(orderedNetwork(expectedContainerConfig)) + sort.Sort(orderedNetwork(expectedNestedContainerConfig)) + + expected := [][]params.NetworkConfig{ + expectedMachineConfig, + expectedContainerConfig, + expectedNestedContainerConfig, + } + assert := func(f func(params.Entities) (params.MachineNetworkConfigResults, error)) { results, err := f(args) c.Assert(err, jc.ErrorIsNil) - c.Assert(results, gc.DeepEquals, params.MachineNetworkConfigResults{ - Results: []params.MachineNetworkConfigResult{ - {Error: nil, Config: expectedMachineConfig}, - {Error: nil, Config: expectedContainerConfig}, - {Error: nil, Config: expectedNestedContainerConfig}, - }, - }) + c.Assert(results.Results, gc.HasLen, 3) + for i, r := range results.Results { + c.Assert(r.Error, gc.IsNil) + sort.Sort(orderedNetwork(r.Config)) + c.Assert(r.Config, jc.DeepEquals, expected[i]) + } } assert(s.networker.MachineNetworkInfo) assert(s.networker.MachineNetworkConfig) diff --git a/apiserver/server_test.go b/apiserver/server_test.go index 2a03d5b6396..dc917ef6148 100644 --- a/apiserver/server_test.go +++ b/apiserver/server_test.go @@ -13,6 +13,7 @@ import ( stdtesting "testing" "time" + "github.com/juju/errors" "github.com/juju/loggo" "github.com/juju/names" jc "github.com/juju/testing/checkers" @@ -80,10 +81,11 @@ func (s *serverSuite) TestStop(c *gc.C) { c.Assert(err, jc.ErrorIsNil) _, err = st.Machiner().Machine(machine.MachineTag()) + err = errors.Cause(err) // The client has not necessarily seen the server shutdown yet, // so there are two possible errors. if err != rpc.ErrShutdown && err != io.ErrUnexpectedEOF { - c.Fatalf("unexpected error from request: %v", err) + c.Fatalf("unexpected error from request: %#T, expected rpc.ErrShutdown or io.ErrUnexpectedEOF", err) } // Check it can be stopped twice. diff --git a/cmd/juju/ensureavailability.go b/cmd/juju/ensureavailability.go index 17cb8f64301..cae397c042c 100644 --- a/cmd/juju/ensureavailability.go +++ b/cmd/juju/ensureavailability.go @@ -150,15 +150,15 @@ func (c *EnsureAvailabilityCommand) Init(args []string) error { c.Placement = make([]string, len(placementSpecs)) for i, spec := range placementSpecs { p, err := instance.ParsePlacement(strings.TrimSpace(spec)) + if err == nil && names.IsContainerMachine(p.Directive) { + return errors.New("ensure-availability cannot be used with container placement directives") + } if err == nil && p.Scope == instance.MachineScope { - if names.IsContainerMachine(p.Directive) { - return errors.New("ensure-availability cannot be used with container placement directives") - } // Targeting machines is ok. c.Placement[i] = p.String() continue } - if err != instance.ErrPlacementScopeMissing || names.IsContainerMachine(p.Directive) { + if err != instance.ErrPlacementScopeMissing { return fmt.Errorf("unsupported ensure-availability placement directive %q", spec) } c.Placement[i] = spec