Merge lp:~axwalk/juju-core/manual-provisioning-followup into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

environs/manual: log machine removal on error

Log the fact that the machine is being removed
from state if an error occurred during manual
provisioning.

https://codereview.appspot.com/13306044/

environs/manual: log machine removal on error

Log the fact that the machine is being removed
from state if an error occurred during manual
provisioning.

https://codereview.appspot.com/13306044/

[Andrew Wilkins (axwalk)]

Reviewers: mp+183097_code.launchpad.net,

Message:
Please take a look.

Description:
environs/manual: log machine removal on error

Log the fact that the machine is being removed
from state if an error occurred during manual
provisioning.

https://code.launchpad.net/~axwalk/juju-core/manual-provisioning-followup/+merge/183097

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M environs/manual/provisioner.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20130830023839-2er74ebh5lr6m46p
+New revision: <email address hidden>

Index: environs/manual/provisioner.go
=== modified file 'environs/manual/provisioner.go'
--- environs/manual/provisioner.go 2013-08-29 03:58:19 +0000
+++ environs/manual/provisioner.go 2013-08-30 06:59:40 +0000
@@ -53,6 +53,7 @@
  func ProvisionMachine(args ProvisionMachineArgs) (m *state.Machine, err
error) {
   defer func() {
    if m != nil && err != nil {
+ logger.Errorf("provisioning failed, removing machine %v", m)
     m.EnsureDead()
     m.Remove()
     m = nil
@@ -155,6 +156,7 @@
  func injectMachine(args injectMachineArgs) (m *state.Machine, err error) {
   defer func() {
    if m != nil && err != nil {
+ logger.Errorf("injecting into state failed, removing machine %v", m)
     m.EnsureDead()
     m.Remove()
    }

[Ian Booth (wallyworld)]

LGTM, please add the error to the logged message

https://codereview.appspot.com/13306044/diff/1/environs/manual/provisioner.go
File environs/manual/provisioner.go (right):

https://codereview.appspot.com/13306044/diff/1/environs/manual/provisioner.go#newcode56
environs/manual/provisioner.go:56: logger.Errorf("provisioning failed,
removing machine %v", m)
can we please include the error in the log

https://codereview.appspot.com/13306044/diff/1/environs/manual/provisioner.go#newcode159
environs/manual/provisioner.go:159: logger.Errorf("injecting into state
failed, removing machine %v", m)
can we please include the error in the log

https://codereview.appspot.com/13306044/

[Andrew Wilkins (axwalk)]

On 2013/09/03 01:33:44, wallyworld wrote:
> LGTM, please add the error to the logged message

https://codereview.appspot.com/13306044/diff/1/environs/manual/provisioner.go
> File environs/manual/provisioner.go (right):

https://codereview.appspot.com/13306044/diff/1/environs/manual/provisioner.go#newcode56
> environs/manual/provisioner.go:56: logger.Errorf("provisioning failed,
removing
> machine %v", m)
> can we please include the error in the log

https://codereview.appspot.com/13306044/diff/1/environs/manual/provisioner.go#newcode159
> environs/manual/provisioner.go:159: logger.Errorf("injecting into
state failed,
> removing machine %v", m)
> can we please include the error in the log

Done. I had thought it superfluous, as the error will be logged by the
command code. But I suppose we might use this elsewhere in the future,
and the error might not get logged.

https://codereview.appspot.com/13306044/

=== modified file 'environs/manual/agent.go'
--- environs/manual/agent.go	2013-09-02 00:44:52 +0000
+++ environs/manual/agent.go	2013-09-03 01:56:59 +0000
@@ -81,7 +81,7 @@
 	if err != nil {
 		return "", err
 	}
-	mcfg.MachineEnvironment[osenv.JujuProviderType] = provider.Manual
+	mcfg.MachineEnvironment[osenv.JujuProviderType] = provider.Null
 	cloudcfg := corecloudinit.New()
 	if cloudcfg, err = cloudinit.Configure(mcfg, cloudcfg); err != nil {
 		return "", err

=== modified file 'environs/manual/provisioner.go'
--- environs/manual/provisioner.go	2013-08-29 03:58:19 +0000
+++ environs/manual/provisioner.go	2013-09-03 01:56:59 +0000
@@ -53,6 +53,7 @@
 func ProvisionMachine(args ProvisionMachineArgs) (m *state.Machine, err error) {
 	defer func() {
 		if m != nil && err != nil {
+			logger.Errorf("provisioning failed, removing machine %v: %v", m, err)
 			m.EnsureDead()
 			m.Remove()
 			m = nil
@@ -155,6 +156,7 @@
 func injectMachine(args injectMachineArgs) (m *state.Machine, err error) {
 	defer func() {
 		if m != nil && err != nil {
+			logger.Errorf("injecting into state failed, removing machine %v: %v", m, err)
 			m.EnsureDead()
 			m.Remove()
 		}

=== modified file 'provider/provider.go'
--- provider/provider.go	2013-08-30 01:48:39 +0000
+++ provider/provider.go	2013-09-03 01:56:59 +0000
@@ -12,5 +12,5 @@
 	MAAS      = "maas"
 	Azure     = "azure"
 	OpenStack = "openstack"
-	Manual    = "manual"
+	Null      = "null"
 )