Merge lp:~thumper/juju-core/error-on-empty-jenv-file into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Have destroy-environment handle empty .jenv file

Have a particular error raised for empty environment
files, that we get when there is an emtpy .jenv file.

If we have an empty file, we currently get this output:

tim@jake:~$ touch .juju/environments/testlocal.jenv
tim@jake:~$ juju status
ERROR Unable to connect to environment "testlocal".
Please check your credentials or use 'juju bootstrap' to create a new environment.

Error details:
environment is not bootstrapped

tim@jake:~$ juju bootstrap
WARNING ignoring environments.yaml: using bootstrap config in file "/home/tim/.juju/environments/testlocal.jenv"
ERROR environment has no bootstrap configuration data

I think this is reasonable, however we also used to get the
error when trying to destroy the environment. Now we get this:

tim@jake:~$ juju destroy-environment -y testlocal
removing empty environment file

https://codereview.appspot.com/80950044/

Have destroy-environment handle empty .jenv file

Have a particular error raised for empty environment
files, that we get when there is an emtpy .jenv file.

If we have an empty file, we currently get this output:

tim@jake:~$ touch .juju/environments/testlocal.jenv
tim@jake:~$ juju status
ERROR Unable to connect to environment "testlocal".
Please check your credentials or use 'juju bootstrap' to create a new environment.

Error details:
environment is not bootstrapped

tim@jake:~$ juju bootstrap
WARNING ignoring environments.yaml: using bootstrap config in file "/home/tim/.juju/environments/testlocal.jenv"
ERROR environment has no bootstrap configuration data

I think this is reasonable, however we also used to get the
error when trying to destroy the environment. Now we get this:

tim@jake:~$ juju destroy-environment -y testlocal
removing empty environment file

https://codereview.appspot.com/80950044/

[Tim Penhey (thumper)]

Reviewers: mp+212960_code.launchpad.net,

Message:
Please take a look.

Description:
Have destroy-environment handle empty .jenv file

Have a particular error raised for empty environment
files, that we get when there is an emtpy .jenv file.

If we have an empty file, we currently get this output:

tim@jake:~$ touch .juju/environments/testlocal.jenv
tim@jake:~$ juju status
ERROR Unable to connect to environment "testlocal".
Please check your credentials or use 'juju bootstrap' to create a new
environment.

Error details:
environment is not bootstrapped

tim@jake:~$ juju bootstrap
WARNING ignoring environments.yaml: using bootstrap config in file
"/home/tim/.juju/environments/testlocal.jenv"
ERROR environment has no bootstrap configuration data

I think this is reasonable, however we also used to get the
error when trying to destroy the environment. Now we get this:

tim@jake:~$ juju destroy-environment -y testlocal
removing empty environment file

https://code.launchpad.net/~thumper/juju-core/error-on-empty-jenv-file/+merge/212960

(do not edit description out of merge proposal)

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

Affected files (+29, -1 lines):
   A [revision details]
   M cmd/juju/destroyenvironment.go
   M cmd/juju/destroyenvironment_test.go
   M environs/open.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-20140326171935-rwmkmlytkkvq26d6
+New revision: <email address hidden>

Index: environs/open.go
=== modified file 'environs/open.go'
--- environs/open.go 2014-03-12 10:59:17 +0000
+++ environs/open.go 2014-03-26 22:21:59 +0000
@@ -46,6 +46,17 @@
   ConfigFromEnvirons
  )

+// EmptyConfig indicates the .jenv file is empty.
+type EmptyConfig struct {
+ error
+}
+
+// IsEmptyConfig reports whether err is a EmptyConfig.
+func IsEmptyConfig(err error) bool {
+ _, ok := err.(EmptyConfig)
+ return ok
+}
+
  // ConfigForName returns the configuration for the environment with
  // the given name from the default environments file. If the name is
  // blank, the default environment will be used. If the configuration
@@ -70,7 +81,7 @@
    info, err := store.ReadInfo(name)
    if err == nil {
     if len(info.BootstrapConfig()) == 0 {
- return nil, ConfigFromNowhere, fmt.Errorf("environment has no
bootstrap configuration data")
+ return nil, ConfigFromNowhere, EmptyConfig{fmt.Errorf("environment has
no bootstrap configuration data")}
     }
     logger.Debugf("ConfigForName found bootstrap config %#v",
info.BootstrapConfig())
     cfg, err := config.New(config.NoDefaults, info.BootstrapConfig())

Index: cmd/juju/destroyenvironment.go
=== modified file 'cmd/juju/destroyenvironment.go'
--- cmd/juju/destroyenvironment.go 2014-03-10 20:22:44 +0000
+++ cmd/juju/destroyenvironment.go 2014-03-26 22:21:59 +0000
@@ -54,6 +54,11 @@
   }
   environ, err := environs.NewFromName(c.envName, store)
   if err != nil {
+ if environs.IsEmptyConfig(err) {
+ // Delete the .jenv file and call it done.
+ ctx.Infof("removing empty environ...

[William Reade (fwereade)]

LGTM

https://codereview.appspot.com/80950044/

=== modified file 'cmd/juju/destroyenvironment.go'
--- cmd/juju/destroyenvironment.go	2014-03-10 20:22:44 +0000
+++ cmd/juju/destroyenvironment.go	2014-03-26 22:25:49 +0000
@@ -54,6 +54,11 @@
 	}
 	environ, err := environs.NewFromName(c.envName, store)
 	if err != nil {
+		if environs.IsEmptyConfig(err) {
+			// Delete the .jenv file and call it done.
+			ctx.Infof("removing empty environment file")
+			return environs.DestroyInfo(c.envName, store)
+		}
 		return err
 	}
 	if !c.assumeYes {
 
=== modified file 'cmd/juju/destroyenvironment_test.go'
--- cmd/juju/destroyenvironment_test.go	2014-03-24 10:42:51 +0000
+++ cmd/juju/destroyenvironment_test.go	2014-03-26 22:25:49 +0000
@@ -70,6 +70,16 @@
 	c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
 }
 
+func (s *destroyEnvSuite) TestDestroyEnvironmentCommandEmptyJenv(c *gc.C) {
+	_, err := s.ConfigStore.CreateInfo("emptyenv")
+	c.Assert(err, gc.IsNil)
+
+	context, err := coretesting.RunCommand(c, new(DestroyEnvironmentCommand), []string{"-e", "emptyenv"})
+	c.Assert(err, gc.IsNil)
+
+	c.Assert(coretesting.Stderr(context), gc.Equals, "removing empty environment file\n")
+}
+
 func (s *destroyEnvSuite) TestDestroyEnvironmentCommandBroken(c *gc.C) {
 	oldinfo, err := s.ConfigStore.ReadInfo("dummyenv")
 	c.Assert(err, gc.IsNil)
 
=== modified file 'environs/open.go'
--- environs/open.go	2014-03-12 10:59:17 +0000
+++ environs/open.go	2014-03-26 22:25:49 +0000
@@ -46,6 +46,17 @@
 	ConfigFromEnvirons
 )
 
+// EmptyConfig indicates the .jenv file is empty.
+type EmptyConfig struct {
+	error
+}
+
+// IsEmptyConfig reports whether err is a EmptyConfig.
+func IsEmptyConfig(err error) bool {
+	_, ok := err.(EmptyConfig)
+	return ok
+}
+
 // ConfigForName returns the configuration for the environment with
 // the given name from the default environments file. If the name is
 // blank, the default environment will be used. If the configuration
@@ -70,7 +81,7 @@
 		info, err := store.ReadInfo(name)
 		if err == nil {
 			if len(info.BootstrapConfig()) == 0 {
-				return nil, ConfigFromNowhere, fmt.Errorf("environment has no bootstrap configuration data")
+				return nil, ConfigFromNowhere, EmptyConfig{fmt.Errorf("environment has no bootstrap configuration data")}
 			}
 			logger.Debugf("ConfigForName found bootstrap config %#v", info.BootstrapConfig())
 			cfg, err := config.New(config.NoDefaults, info.BootstrapConfig())