Fix 1604965: machine stays in pending state even though node has been marked failed deployment in MAAS

apiserver/instancepoller/instancepoller.go

@@ -212,6 +212,12 @@ func (a *InstancePollerAPI) SetInstanceStatus(args params.SetStatus) (params.Err
 				Since:   &now,
 			}
 			err = machine.SetInstanceStatus(s)
+			if status.Status(arg.Status) == status.ProvisioningError {
+				s.Status = status.Error
+				if err == nil {
+					err = machine.SetStatus(s)
+				}
+			}
 		}
 		result.Results[i].Error = common.ServerError(err)
 	}

apiserver/instancepoller/state.go

@@ -20,6 +20,7 @@ type StateMachine interface {
 	SetProviderAddresses(...network.Address) error
 	InstanceStatus() (status.StatusInfo, error)
 	SetInstanceStatus(status.StatusInfo) error
+	SetStatus(status.StatusInfo) error
 	String() string
 	Refresh() error
 	Life() state.Life

apiserver/provisioner/provisioner.go

@@ -809,6 +809,12 @@ func (p *ProvisionerAPI) SetInstanceStatus(args params.SetStatus) (params.ErrorR
 				Since:   &now,
 			}
 			err = machine.SetInstanceStatus(s)
+			if status.Status(arg.Status) == status.ProvisioningError {
+				s.Status = status.Error
+				if err == nil {
+					err = machine.SetStatus(s)
+				}
+			}
 		}
 		result.Results[i].Error = common.ServerError(err)
 	}

provider/maas/environ.go

@@ -54,6 +54,8 @@ var shortAttempt = utils.AttemptStrategy{
 	Delay: 200 * time.Millisecond,
 }
 
+const statusPollInterval = 5 * time.Second
+
 var (
 	ReleaseNodes         = releaseNodes
 	DeploymentStatusCall = deploymentStatusCall
@@ -948,6 +950,7 @@ func (environ *maasEnviron) StartInstance(args environs.StartInstanceParams) (
 			return nil, errors.Annotatef(err, "cannot run instances")
 		}
 	}
+
 	defer func() {
 		if err != nil {
 			if err := environ.StopInstances(inst.Id()); err != nil {

provider/maas/instance.go

@@ -51,20 +51,24 @@ func maasObjectId(maasObject *gomaasapi.MAASObject) instance.Id {
 	return instance.Id(maasObject.URI().String())
 }
 
+func normalizeStatus(statusMsg string) string {
+	return strings.ToLower(strings.TrimSpace(statusMsg))
+}
+
 func convertInstanceStatus(statusMsg, substatus string, id instance.Id) instance.InstanceStatus {
 	maasInstanceStatus := status.Empty
-	switch statusMsg {
+	switch normalizeStatus(statusMsg) {
 	case "":
 		logger.Debugf("unable to obtain status of instance %s", id)
 		statusMsg = "error in getting status"
-	case "Deployed":
+	case "deployed":
 		maasInstanceStatus = status.Running
-	case "Deploying":
+	case "deploying":
 		maasInstanceStatus = status.Allocating
 		if substatus != "" {
 			statusMsg = fmt.Sprintf("%s: %s", statusMsg, substatus)
 		}
-	case "Failed Deployment":
+	case "failed deployment":
 		maasInstanceStatus = status.ProvisioningError
 		if substatus != "" {
 			statusMsg = fmt.Sprintf("%s: %s", statusMsg, substatus)

provider/maas/interfaces_test.go

@@ -805,8 +805,8 @@ func (s *interfacesSuite) TestMAAS2NetworkInterfaces(c *gc.C) {
 		MTU:                 1500,
 		GatewayAddress:      newAddressOnSpaceWithId("storage", network.Id("3"), "10.250.19.2"),
 	}}
-
-	instance := &maas2Instance{machine: &fakeMachine{interfaceSet: exampleInterfaces}}
+	machine := &fakeMachine{interfaceSet: exampleInterfaces}
+	instance := &maas2Instance{machine: machine}
 
 	infos, err := maas2NetworkInterfaces(instance, subnetsMap)
 	c.Assert(err, jc.ErrorIsNil)
@@ -847,8 +847,8 @@ func (s *interfacesSuite) TestMAAS2InterfacesNilVLAN(c *gc.C) {
 			children: []string{"eth0.100", "eth0.250", "eth0.50"},
 		},
 	}
-
-	instance := &maas2Instance{machine: &fakeMachine{interfaceSet: exampleInterfaces}}
+	machine := &fakeMachine{interfaceSet: exampleInterfaces}
+	instance := &maas2Instance{machine: machine}
 
 	expected := []network.InterfaceInfo{{
 		DeviceIndex:       0,

provider/maas/maas2_test.go

@@ -184,6 +184,14 @@ func (c *fakeController) ReleaseMachines(args gomaasapi.ReleaseMachinesArgs) err
 	return c.NextErr()
 }
 
+type fakeMachineOnlyController struct {
+	machines []gomaasapi.Machine
+}
+
+func (f *fakeMachineOnlyController) Machines(gomaasapi.MachinesArgs) ([]gomaasapi.Machine, error) {
+	return f.machines, nil
+}
+
 type fakeBootResource struct {
 	gomaasapi.BootResource
 	name         string

provider/maas/maas2instance.go

@@ -65,7 +65,10 @@ func (mi *maas2Instance) Addresses() ([]network.Address, error) {
 // Status returns a juju status based on the maas instance returned
 // status message.
 func (mi *maas2Instance) Status() instance.InstanceStatus {
-	// TODO (babbageclunk): this should rerequest to get live status.
+	// A fresh status is not obtained here because the interface it is intended
+	// to satisfy gets a new maas2Instance before each call, using a fresh status
+	// would cause us to mask errors since this interface does not contemplate
+	// returing them.
 	statusName := mi.machine.StatusName()
 	statusMsg := mi.machine.StatusMessage()
 	return convertInstanceStatus(statusName, statusMsg, mi.Id())

provider/maas/maas2instance_test.go

@@ -19,21 +19,24 @@ type maas2InstanceSuite struct {
 var _ = gc.Suite(&maas2InstanceSuite{})
 
 func (s *maas2InstanceSuite) TestString(c *gc.C) {
-	instance := &maas2Instance{machine: &fakeMachine{hostname: "peewee", systemID: "herman"}}
+	machine := &fakeMachine{hostname: "peewee", systemID: "herman"}
+	instance := &maas2Instance{machine: machine}
 	c.Assert(instance.String(), gc.Equals, "peewee:herman")
 }
 
 func (s *maas2InstanceSuite) TestID(c *gc.C) {
-	thing := &maas2Instance{machine: &fakeMachine{systemID: "herman"}}
+	machine := &fakeMachine{systemID: "herman"}
+	thing := &maas2Instance{machine: machine}
 	c.Assert(thing.Id(), gc.Equals, instance.Id("herman"))
 }
 
 func (s *maas2InstanceSuite) TestAddresses(c *gc.C) {
-	instance := &maas2Instance{machine: &fakeMachine{ipAddresses: []string{
+	machine := &fakeMachine{ipAddresses: []string{
 		"0.0.0.0",
 		"1.2.3.4",
 		"127.0.0.1",
-	}}}
+	}}
+	instance := &maas2Instance{machine: machine}
 	expectedAddresses := []network.Address{
 		network.NewAddress("0.0.0.0"),
 		network.NewAddress("1.2.3.4"),
@@ -45,26 +48,30 @@ func (s *maas2InstanceSuite) TestAddresses(c *gc.C) {
 }
 
 func (s *maas2InstanceSuite) TestZone(c *gc.C) {
-	instance := &maas2Instance{machine: &fakeMachine{zoneName: "inflatable"}}
+	machine := &fakeMachine{zoneName: "inflatable"}
+	instance := &maas2Instance{machine: machine}
 	zone, err := instance.zone()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(zone, gc.Equals, "inflatable")
 }
 
 func (s *maas2InstanceSuite) TestStatusSuccess(c *gc.C) {
-	thing := &maas2Instance{machine: &fakeMachine{statusMessage: "Wexler", statusName: "Deploying"}}
+	machine := &fakeMachine{statusMessage: "Wexler", statusName: "Deploying"}
+	thing := &maas2Instance{machine: machine}
 	result := thing.Status()
 	c.Assert(result, jc.DeepEquals, instance.InstanceStatus{status.Allocating, "Deploying: Wexler"})
 }
 
 func (s *maas2InstanceSuite) TestStatusError(c *gc.C) {
-	thing := &maas2Instance{machine: &fakeMachine{statusMessage: "", statusName: ""}}
+	machine := &fakeMachine{statusMessage: "", statusName: ""}
+	thing := &maas2Instance{machine: machine}
 	result := thing.Status()
 	c.Assert(result, jc.DeepEquals, instance.InstanceStatus{"", "error in getting status"})
 }
 
 func (s *maas2InstanceSuite) TestHostname(c *gc.C) {
-	thing := &maas2Instance{machine: &fakeMachine{hostname: "saul-goodman"}}
+	machine := &fakeMachine{hostname: "saul-goodman"}
+	thing := &maas2Instance{machine: machine}
 	result, err := thing.hostname()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(result, gc.Equals, "saul-goodman")

worker/instancepoller/updater.go

@@ -271,6 +271,7 @@ func pollInstanceInfo(context machineContext, m machine) (instInfo instanceInfo,
 				return instanceInfo{}, err
 			}
 		}
+
 	}
 	if m.Life() != params.Dead {
 		providerAddresses, err := m.ProviderAddresses()