cmd/juju: delete .jenv if bootstrap fails

cmd/juju: delete .jenv if bootstrap fails

If bootstrap fails, and the .jenv file did
not previously exist, then Juju will attempt
to destroy the environment in case of partial
creation. If this succeeds, the .jenv file
is destroyed. If it fails, an error message
is logged; we do not currently attempt to
forcefully remove the environment.

NOTE: if an environment is already prepared, say
by running sync-tools, then a failed bootstrap
will not attempt to destroy the environment.

Fixes lp:1247152

https://code.launchpad.net/~axwalk/juju-core/juju-failed-bootstrap-destroy-jenv/+merge/204421

(do not edit description out of merge proposal)

[axw, 2014-02-03 07:10:10]

Please take a look.

[wallyworld, 2014-02-03 07:20:58]

I'm not sure about this. I think we're getting to the point where the mental
model is getting too complicated and exposes too much of Juju's internals. Why
not just go ahead and remove the jenv file automatically under the circumstances
described ie no previous jenv file, bootstrap fails, destroy files, so remove
jenv. 
What's the use case for stand alone use of the --config-only option? If the
cloud artefacts are already gone, we could just inform the user that their cloud
env has disappeared and did they want to clean up the remaining local config,
and if they say yes, just do it.

[axw, 2014-02-03 07:34:06]

On 2014/02/03 07:20:58, wallyworld wrote:
> I'm not sure about this. I think we're getting to the point where the mental
> model is getting too complicated and exposes too much of Juju's internals. Why
> not just go ahead and remove the jenv file automatically under the
circumstances
> described ie no previous jenv file, bootstrap fails, destroy files, so remove
> jenv.

If we dispose of the .jenv file without attempting to destroy the environment,
then the user cannot automatically release any partially created environment.

> What's the use case for stand alone use of the --config-only option? If the
> cloud artefacts are already gone, we could just inform the user that their
cloud
> env has disappeared and did they want to clean up the remaining local config,
> and if they say yes, just do it.

The reason it exists is to clean up if bootstrap fails *and* the auto
destroy-environment fails. It would be nice if we could automatically determine
that this can be done and just do it, but I'm not convinced it's practical.

How do you tell that the cloud artefacts are gone? By going to the provider.
What if you can't get to the provider because your config is wrong? How can Juju
tell the difference between a temporarily wrong config and a permanently wrong
one?

[wallyworld, 2014-02-03 08:03:18]

On 2014/02/03 07:34:06, axw wrote:
> On 2014/02/03 07:20:58, wallyworld wrote:
> > I'm not sure about this. I think we're getting to the point where the mental
> > model is getting too complicated and exposes too much of Juju's internals.
Why
> > not just go ahead and remove the jenv file automatically under the
> circumstances
> > described ie no previous jenv file, bootstrap fails, destroy files, so
remove
> > jenv.
> 
> If we dispose of the .jenv file without attempting to destroy the environment,
> then the user cannot automatically release any partially created environment.
> 
> > What's the use case for stand alone use of the --config-only option? If the
> > cloud artefacts are already gone, we could just inform the user that their
> cloud
> > env has disappeared and did they want to clean up the remaining local
config,
> > and if they say yes, just do it.
> 
> The reason it exists is to clean up if bootstrap fails *and* the auto
> destroy-environment fails. It would be nice if we could automatically
determine
> that this can be done and just do it, but I'm not convinced it's practical.
> 
> How do you tell that the cloud artefacts are gone? By going to the provider.
> What if you can't get to the provider because your config is wrong? How can
Juju
> tell the difference between a temporarily wrong config and a permanently wrong
> one?

I think there's 2 main use cases? 
1. Bootstrap of new env fails
In this case, we know the that jenv file did not previously exist so if
auto-destroy fails, we can (probably after asking) offer to the user that any
remaining juju configuration files be cleaned up and so leave their system as it
was prior to the initial bootstrap.
2. Someone deletes cloud artefacts outside of juju
So now the user may see that juju commands fail and they want to clean up. So
they juju destroy-env. It fails cause stuff has already been deleted. So like in
the first case above, we can just prompt them that their config will be deleted.
Here the wording of the prompt will be slightly different but the end result is
the same.

I guess there's a danger that if there's some transient failure reaching the
cloud we could remove the jenv and leave the user with cloud resources still
active. We could incorporate this into the above prompts eg "Juju cannot contact
the cloud to delete any running instances, this may be a temporary failure, did
you want to try again or just nuke your env config". If they elect to clean up
the config, we can let them know that they should ensure that nothing is left
running, Juju tried but failed so now it's up to them.

I guess my main wish is that the user be guided as to what to do and the tool
does it at a high enough level that they don't have to try and understand the
internal workings of Juju, as opposed to exiting and making them run a command
with yet another option. And like destroy-environment -y, the user could allow
juju to do what it thinks needs to be done without prompting.

[axw, 2014-02-03 08:24:17]

On 2014/02/03 08:03:18, wallyworld wrote:
> On 2014/02/03 07:34:06, axw wrote:
> > On 2014/02/03 07:20:58, wallyworld wrote:
> > > I'm not sure about this. I think we're getting to the point where the
mental
> > > model is getting too complicated and exposes too much of Juju's internals.
> Why
> > > not just go ahead and remove the jenv file automatically under the
> > circumstances
> > > described ie no previous jenv file, bootstrap fails, destroy files, so
> remove
> > > jenv.
> > 
> > If we dispose of the .jenv file without attempting to destroy the
environment,
> > then the user cannot automatically release any partially created
environment.
> > 
> > > What's the use case for stand alone use of the --config-only option? If
the
> > > cloud artefacts are already gone, we could just inform the user that their
> > cloud
> > > env has disappeared and did they want to clean up the remaining local
> config,
> > > and if they say yes, just do it.
> > 
> > The reason it exists is to clean up if bootstrap fails *and* the auto
> > destroy-environment fails. It would be nice if we could automatically
> determine
> > that this can be done and just do it, but I'm not convinced it's practical.
> > 
> > How do you tell that the cloud artefacts are gone? By going to the provider.
> > What if you can't get to the provider because your config is wrong? How can
> Juju
> > tell the difference between a temporarily wrong config and a permanently
wrong
> > one?
> 
> I think there's 2 main use cases? 
> 1. Bootstrap of new env fails
> In this case, we know the that jenv file did not previously exist so if
> auto-destroy fails, we can (probably after asking) offer to the user that any
> remaining juju configuration files be cleaned up and so leave their system as
it
> was prior to the initial bootstrap.
> 2. Someone deletes cloud artefacts outside of juju
> So now the user may see that juju commands fail and they want to clean up. So
> they juju destroy-env. It fails cause stuff has already been deleted. So like
in
> the first case above, we can just prompt them that their config will be
deleted.
> Here the wording of the prompt will be slightly different but the end result
is
> the same.
> 
> I guess there's a danger that if there's some transient failure reaching the
> cloud we could remove the jenv and leave the user with cloud resources still
> active. We could incorporate this into the above prompts eg "Juju cannot
contact
> the cloud to delete any running instances, this may be a temporary failure,
did
> you want to try again or just nuke your env config". If they elect to clean up
> the config, we can let them know that they should ensure that nothing is left
> running, Juju tried but failed so now it's up to them.
> 
> I guess my main wish is that the user be guided as to what to do and the tool
> does it at a high enough level that they don't have to try and understand the
> internal workings of Juju, as opposed to exiting and making them run a command
> with yet another option. And like destroy-environment -y, the user could allow
> juju to do what it thinks needs to be done without prompting.

Fair call about it being too low-level. I'm just not keen on adding prompting to
hide the implementation details.

I'll pare this back a bit so we attempt to destroy after a failed bootstrap, but
remove all the --config-only bits from the CL. We will probably still want to
address that at a later date.

[rog, 2014-02-03 08:32:21]

Thanks a lot for doing this - it will be much appreciated.

We should also destroy the environment if sync-tools fails and the env didn't
previously exist, I
think.

https://codereview.appspot.com/59560043/diff/1/cmd/juju/bootstrap.go
File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/59560043/diff/1/cmd/juju/bootstrap.go#newcode118
cmd/juju/bootstrap.go:118: 
I'd prefer it if result had "err" in its name here,
either by actually naming it "err", or perhaps by
naming it "resultErr".

https://codereview.appspot.com/59560043/diff/1/cmd/juju/bootstrap.go#newcode135
cmd/juju/bootstrap.go:135: 
I think this case is unusual enough that we don't actually need to special
"config-only" flag. The error message will probably contain the file name
anyway. I think a better approach would be for destroy-environment to delete the
config file if it gets a definitive "not found" error when trying to delete the
environment.

[axw, 2014-02-04 02:13:40]

Please take a look.

https://codereview.appspot.com/59560043/diff/1/cmd/juju/bootstrap.go
File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/59560043/diff/1/cmd/juju/bootstrap.go#newcode118
cmd/juju/bootstrap.go:118: 
On 2014/02/03 08:32:22, rog wrote:
> I'd prefer it if result had "err" in its name here,
> either by actually naming it "err", or perhaps by
> naming it "resultErr".

Done.

https://codereview.appspot.com/59560043/diff/1/cmd/juju/bootstrap.go#newcode135
cmd/juju/bootstrap.go:135: 
On 2014/02/03 08:32:22, rog wrote:
> I think this case is unusual enough that we don't actually need to special
> "config-only" flag. The error message will probably contain the file name
> anyway. I think a better approach would be for destroy-environment to delete
the
> config file if it gets a definitive "not found" error when trying to delete
the
> environment.

Okay. I'm going to take the --config-only bit out of the CL for now. That can be
addressed in a followup.

[axw, 2014-02-04 02:21:34]

On 2014/02/03 08:32:21, rog wrote:
> Thanks a lot for doing this - it will be much appreciated.
> 
> We should also destroy the environment if sync-tools fails and the env didn't
> previously exist, I
> think.

Done.

[dimitern, 2014-02-10 08:32:51]

LGTM