Merge lp:~thumper/juju-core/fix-provider-query-in-machine-env-worker into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Fix the key we are looking for.

We were looking for the wrong key to determine whether
to write system files or not.

https://codereview.appspot.com/61990043/

Fix the key we are looking for.

We were looking for the wrong key to determine whether
to write system files or not.

https://codereview.appspot.com/61990043/

[Tim Penhey (thumper)]

Reviewers: mp+205864_code.launchpad.net,

Message:
Please take a look.

Description:
Fix the key we are looking for.

We were looking for the wrong key to determine whether
to write system files or not.

https://code.launchpad.net/~thumper/juju-core/fix-provider-query-in-machine-env-worker/+merge/205864

(do not edit description out of merge proposal)

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

Affected files (+4, -1 lines):
   A [revision details]
   M worker/machineenvironmentworker/machineenvironmentworker.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-20140211202026-scgo0kk0wmhgy1oq
+New revision: <email address hidden>

Index: worker/machineenvironmentworker/machineenvironmentworker.go
=== modified
file 'worker/machineenvironmentworker/machineenvironmentworker.go'
--- worker/machineenvironmentworker/machineenvironmentworker.go 2014-01-29
04:59:18 +0000
+++ worker/machineenvironmentworker/machineenvironmentworker.go 2014-02-07
06:42:39 +0000
@@ -66,7 +66,8 @@
   // We don't write out system files for the local provider on machine zero
   // as that is the host machine.
   writeSystemFiles := !(agentConfig.Tag() == names.MachineTag("0") &&
- agentConfig.Value(agent.JujuProviderType) == provider.Local)
+ agentConfig.Value(agent.ProviderType) == provider.Local)
+ logger.Debugf("write system files: %v", writeSystemFiles)
   envWorker := &MachineEnvironmentWorker{
    api: api,
    writeSystemFiles: writeSystemFiles,

[Tim Penhey (thumper)]

On 2014/02/11 22:50:36, thumper wrote:
> Please take a look.

Please wait, I have a few more changes... (and a test fix)

https://codereview.appspot.com/61990043/

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/61990043/

[Ian Booth (wallyworld)]

LGTM

https://codereview.appspot.com/61990043/diff/20001/worker/machineenvironmentworker/machineenvironmentworker.go
File worker/machineenvironmentworker/machineenvironmentworker.go
(right):

https://codereview.appspot.com/61990043/diff/20001/worker/machineenvironmentworker/machineenvironmentworker.go#newcode69
worker/machineenvironmentworker/machineenvironmentworker.go:69:
agentConfig.Value(agent.ProviderType) == provider.Local)
I still wish this were

writeSystemFiles := agentConfig.Tag() != names.MachineTag("0") ||
agentConfig.Value(agent.JujuProviderType) != provider.Local

much easier to parse and no brackets

https://codereview.appspot.com/61990043/

[Tim Penhey (thumper)]

https://codereview.appspot.com/61990043/diff/20001/worker/machineenvironmentworker/machineenvironmentworker.go
File worker/machineenvironmentworker/machineenvironmentworker.go
(right):

https://codereview.appspot.com/61990043/diff/20001/worker/machineenvironmentworker/machineenvironmentworker.go#newcode69
worker/machineenvironmentworker/machineenvironmentworker.go:69:
agentConfig.Value(agent.ProviderType) == provider.Local)
On 2014/02/12 23:04:45, wallyworld wrote:
> I still wish this were

> writeSystemFiles := agentConfig.Tag() != names.MachineTag("0") ||
> agentConfig.Value(agent.JujuProviderType) != provider.Local

> much easier to parse and no brackets

Done.

https://codereview.appspot.com/61990043/

[Go Bot (go-bot)]

The attempt to merge lp:~thumper/juju-core/fix-provider-query-in-machine-env-worker into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.015s
ok launchpad.net/juju-core/agent 0.745s
ok launchpad.net/juju-core/agent/tools 0.239s
ok launchpad.net/juju-core/bzr 6.698s
ok launchpad.net/juju-core/cert 2.813s
ok launchpad.net/juju-core/charm 0.619s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.035s
ok launchpad.net/juju-core/cloudinit/sshinit 1.125s
ok launchpad.net/juju-core/cmd 0.293s
ok launchpad.net/juju-core/cmd/charm-admin 0.821s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 240.638s
ok launchpad.net/juju-core/cmd/jujud 60.873s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 9.401s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/constraints 0.027s
ok launchpad.net/juju-core/container 0.038s
ok launchpad.net/juju-core/container/factory 0.051s
ok launchpad.net/juju-core/container/kvm 0.243s
ok launchpad.net/juju-core/container/kvm/mock 0.038s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 0.330s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.269s
ok launchpad.net/juju-core/environs 3.058s
ok launchpad.net/juju-core/environs/bootstrap 4.589s
ok launchpad.net/juju-core/environs/cloudinit 0.702s
ok launchpad.net/juju-core/environs/config 5.105s
ok launchpad.net/juju-core/environs/configstore 0.040s
ok launchpad.net/juju-core/environs/filestorage 0.032s
ok launchpad.net/juju-core/environs/httpstorage 0.901s
ok launchpad.net/juju-core/environs/imagemetadata 0.623s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.059s
ok launchpad.net/juju-core/environs/jujutest 0.265s
ok launchpad.net/juju-core/environs/manual 9.609s
ok launchpad.net/juju-core/environs/simplestreams 0.374s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.162s
ok launchpad.net/juju-core/environs/storage 1.184s
ok launchpad.net/juju-core/environs/sync 34.153s
ok launchpad.net/juju-core/environs/testing 0.215s
ok launchpad.net/juju-core/environs/tools 6.888s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.015s
ok launchpad.net/juju-core/instance 0.028s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 22.830s
ok launchpad.net/juju-core/juju/osenv 0.020s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.015s
ok launchpad.net/juju-core/log/syslog 0.025s
ok launchp...

=== modified file 'agent/format-1.16.go'
--- agent/format-1.16.go	2014-02-04 19:12:08 +0000
+++ agent/format-1.16.go	2014-02-13 00:39:39 +0000
@@ -17,10 +17,10 @@
 const (
 	format_1_16 = "format 1.16"
 	// Old environment variables that are now stored in agent config.
-	JujuLxcBridge    = "JUJU_LXC_BRIDGE"
-	JujuProviderType = "JUJU_PROVIDER_TYPE"
-	JujuStorageDir   = "JUJU_STORAGE_DIR"
-	JujuStorageAddr  = "JUJU_STORAGE_ADDR"
+	jujuLxcBridge    = "JUJU_LXC_BRIDGE"
+	jujuProviderType = "JUJU_PROVIDER_TYPE"
+	jujuStorageDir   = "JUJU_STORAGE_DIR"
+	jujuStorageAddr  = "JUJU_STORAGE_ADDR"
 )
 
 // formatter_1_16 is the formatter for the 1.16 format.
@@ -176,19 +176,19 @@
 		environment string
 		config      string
 	}{{
-		JujuProviderType,
+		jujuProviderType,
 		ProviderType,
 	}, {
 		osenv.JujuContainerTypeEnvKey,
 		ContainerType,
 	}, {
-		JujuLxcBridge,
+		jujuLxcBridge,
 		LxcBridge,
 	}, {
-		JujuStorageDir,
+		jujuStorageDir,
 		StorageDir,
 	}, {
-		JujuStorageAddr,
+		jujuStorageAddr,
 		StorageAddr,
 	}} {
 		value := os.Getenv(name.environment)

=== modified file 'agent/format-1.16_whitebox_test.go'
--- agent/format-1.16_whitebox_test.go	2014-02-04 19:12:08 +0000
+++ agent/format-1.16_whitebox_test.go	2014-02-13 00:39:39 +0000
@@ -97,11 +97,11 @@
 }
 
 func (s *format_1_16Suite) TestMigrate(c *gc.C) {
-	s.PatchEnvironment(JujuLxcBridge, "lxc bridge")
-	s.PatchEnvironment(JujuProviderType, "provider type")
+	s.PatchEnvironment(jujuLxcBridge, "lxc bridge")
+	s.PatchEnvironment(jujuProviderType, "provider type")
 	s.PatchEnvironment(osenv.JujuContainerTypeEnvKey, "container type")
-	s.PatchEnvironment(JujuStorageDir, "storage dir")
-	s.PatchEnvironment(JujuStorageAddr, "storage addr")
+	s.PatchEnvironment(jujuStorageDir, "storage dir")
+	s.PatchEnvironment(jujuStorageAddr, "storage addr")
 
 	config := newTestConfig(c)
 	s.formatter.migrate(config)
@@ -118,7 +118,7 @@
 }
 
 func (s *format_1_16Suite) TestMigrateOnlySetsExisting(c *gc.C) {
-	s.PatchEnvironment(JujuProviderType, "provider type")
+	s.PatchEnvironment(jujuProviderType, "provider type")
 
 	config := newTestConfig(c)
 	s.formatter.migrate(config)

=== modified file 'worker/machineenvironmentworker/machineenvironmentworker.go'
--- worker/machineenvironmentworker/machineenvironmentworker.go	2014-01-29 04:59:18 +0000
+++ worker/machineenvironmentworker/machineenvironmentworker.go	2014-02-13 00:39:39 +0000
@@ -65,8 +65,9 @@
 func NewMachineEnvironmentWorker(api *environment.Facade, agentConfig agent.Config) worker.Worker {
 	// We don't write out system files for the local provider on machine zero
 	// as that is the host machine.
-	writeSystemFiles := !(agentConfig.Tag() == names.MachineTag("0") &&
-		agentConfig.Value(agent.JujuProviderType) == provider.Local)
+	writeSystemFiles := (agentConfig.Tag() != names.MachineTag("0") ||
+		agentConfig.Value(agent.ProviderType) != provider.Local)
+	logger.Debugf("write system files: %v", writeSystemFiles)
 	envWorker := &MachineEnvironmentWorker{
 		api:              api,
 		writeSystemFiles: writeSystemFiles,

=== modified file 'worker/machineenvironmentworker/machineenvironmentworker_test.go'
--- worker/machineenvironmentworker/machineenvironmentworker_test.go	2014-01-28 03:12:34 +0000
+++ worker/machineenvironmentworker/machineenvironmentworker_test.go	2014-02-13 00:39:39 +0000
@@ -211,7 +211,7 @@
 }
 
 func (mock *mockConfig) Value(key string) string {
-	if key == agent.JujuProviderType {
+	if key == agent.ProviderType {
 		return mock.provider
 	}
 	return ""