Remove support for PyJuju and rapi from charm.

Remove support for PyJuju and rapi from charm.

- Removes the support for running rapi/pyjuju.
- Removes the agent used to communicate with zookeeper.
- Removes anything pertaining to zookeeper.
- Attempts to clean up docs and such to make sense with pure juju-core.


Note:

- The functional test run was taking 62min to complete for me. The timeout is
bumped to 80min to help allow it to complete. This was against ec2.

https://code.launchpad.net/~rharding/charms/precise/juju-gui/remove-pyjuju/+merge/201510

(do not edit description out of merge proposal)

[rharding, 2014-01-14 15:40:07]

Please take a look.

[frankban, 2014-01-14 17:26:36]

Thanks a lot for getting rid of all this pyJuju stuff Rick!
This branch is nice: please take a look at my comments below.
LGTM with changes.

https://codereview.appspot.com/51470044/diff/1/README.md
File README.md (right):

https://codereview.appspot.com/51470044/diff/1/README.md#newcode76
README.md:76: core) or "admin" (for pyjuju). The password is the same as your
Juju
We can remove the pyjuju reference here.

https://codereview.appspot.com/51470044/diff/1/README.md#newcode105
README.md:105: For both Juju Core and PyJuju, you must simply do the following
steps.  Note
And here too...

https://codereview.appspot.com/51470044/diff/1/README.md#newcode162
README.md:162: #### pyjuju ####
We can get rid of this paragraph and of all the juju-jitsu stuff \o/

https://codereview.appspot.com/51470044/diff/1/config/haproxy.cfg.template
File config/haproxy.cfg.template (right):

https://codereview.appspot.com/51470044/diff/1/config/haproxy.cfg.template#ne...
config/haproxy.cfg.template:25: # Replace "/ws/" with "/" in any request path.
Nice.

https://codereview.appspot.com/51470044/diff/1/hooks/backend.py
File hooks/backend.py (left):

https://codereview.appspot.com/51470044/diff/1/hooks/backend.py#oldcode262
hooks/backend.py:262: raise ValueError('Unable to use staging with go backend')
I am very happy we removed this ValueError for incompatible options.

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode87
hooks/utils.py:87: Serializer, su,
Please keep "su" in its own line.

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode148
hooks/utils.py:148: return api_addresses.split()[0]
So you decided to stop supporting also old releases of juju-core.
I am not sure about this: at least we should document it more explicitly.

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode320
hooks/utils.py:320: password = password if password else None
Maybe rewrite those three lines like the following?
if not password:
    password = 'admin' if sandbox else None

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode362
hooks/utils.py:362: # In PyJuju environments, use the same certificate for both
HTTPS and
Please remove this reference to PyJuju.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test
File tests/20-functional.test (right):

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:45: STAGING_SERVICES = ('haproxy', 'mediawiki',
'memcached', 'mysql', 'wordpress')
STAGING_SERVICES seems to be no longer required.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:46: juju_version().major == 0
Can't we remove this line?
Moreover, can't we remove the juju_version function entirely?

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:69: # XXX 2012-11-29 frankban bug=872264:
This XXX comment is no longer required. We can remove all the cleanup_services
machinery.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:78: if options is None:
This seems not right. The cleanup_services list was only required in pyJuju, and
now we are using it in juju-core. IIRC this whole function is a wrapper for
juju_deploy, and was required to workaround the pyJuju bug descibed in the XXX
above. AFAICT now it is no longer required for this function to return the list
of services to be stopped.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:180: # XXX 2013-11-27 frankban bug=872264:
Please remove these comments. The comment above suggests we could also remove
this method all together.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:262: # XXX 2013-11-27 frankban bug=872264:
These comments must go away.

[rharding, 2014-01-15 00:03:16]

I'm fighing lbox. When I updated this it put it over at a new url.

https://codereview.appspot.com/52440043

https://codereview.appspot.com/51470044/diff/1/README.md
File README.md (right):

https://codereview.appspot.com/51470044/diff/1/README.md#newcode76
README.md:76: core) or "admin" (for pyjuju). The password is the same as your
Juju
On 2014/01/14 17:26:36, frankban wrote:
> We can remove the pyjuju reference here.

Thanks, removed.

https://codereview.appspot.com/51470044/diff/1/README.md#newcode105
README.md:105: For both Juju Core and PyJuju, you must simply do the following
steps.  Note
On 2014/01/14 17:26:36, frankban wrote:
> And here too...

Removed

https://codereview.appspot.com/51470044/diff/1/README.md#newcode162
README.md:162: #### pyjuju ####
On 2014/01/14 17:26:36, frankban wrote:
> We can get rid of this paragraph and of all the juju-jitsu stuff \o/


Removed

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode87
hooks/utils.py:87: Serializer, su,
On 2014/01/14 17:26:36, frankban wrote:
> Please keep "su" in its own line.

Mistake, thanks for the catch.

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode148
hooks/utils.py:148: return api_addresses.split()[0]
On 2014/01/14 17:26:36, frankban wrote:
> So you decided to stop supporting also old releases of juju-core.
> I am not sure about this: at least we should document it more explicitly.

Can you talk about this more? My understanding is that we got rid of the agent
bit and that was the conf file we've removed as part of that. Is the machiner
agent file something different?

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode320
hooks/utils.py:320: password = password if password else None
On 2014/01/14 17:26:36, frankban wrote:
> Maybe rewrite those three lines like the following?
> if not password:
>     password = 'admin' if sandbox else None

Tweaked.

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode362
hooks/utils.py:362: # In PyJuju environments, use the same certificate for both
HTTPS and
On 2014/01/14 17:26:36, frankban wrote:
> Please remove this reference to PyJuju.

Removed

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test
File tests/20-functional.test (right):

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:45: STAGING_SERVICES = ('haproxy', 'mediawiki',
'memcached', 'mysql', 'wordpress')
On 2014/01/14 17:26:36, frankban wrote:
> STAGING_SERVICES seems to be no longer required.

Wow, how did lint not catch that? Thanks.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:46: juju_version().major == 0
On 2014/01/14 17:26:36, frankban wrote:
> Can't we remove this line?
> Moreover, can't we remove the juju_version function entirely?

Oh hmm, I didn't notice it was only used for pyjuju vs juju-core distinction.
Removing.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:78: if options is None:
On 2014/01/14 17:26:36, frankban wrote:
> This seems not right. The cleanup_services list was only required in pyJuju,
and
> now we are using it in juju-core. IIRC this whole function is a wrapper for
> juju_deploy, and was required to workaround the pyJuju bug descibed in the XXX
> above. AFAICT now it is no longer required for this function to return the
list
> of services to be stopped.

Sorry, I meant to ask about this one. The bug was noted as wrapping and adding a
cleanup. It does set self.service_name and only returns the unit_info so I
wasn't sure if it made sense to keep it as it kept a specific api. 

It sounds like no, all calls to self.deploy_gui should just be updated to
juju_deploy_gui. I'll update.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:180: # XXX 2013-11-27 frankban bug=872264:
On 2014/01/14 17:26:36, frankban wrote:
> Please remove these comments. The comment above suggests we could also remove
> this method all together.

Removed

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:262: # XXX 2013-11-27 frankban bug=872264:
On 2014/01/14 17:26:36, frankban wrote:
> These comments must go away.

gone

[frankban, 2014-01-15 08:58:40]

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode148
hooks/utils.py:148: return api_addresses.split()[0]
On 2014/01/15 00:03:16, rharding wrote:
> On 2014/01/14 17:26:36, frankban wrote:
> > So you decided to stop supporting also old releases of juju-core.
> > I am not sure about this: at least we should document it more explicitly.
> 
> Can you talk about this more? My understanding is that we got rid of the agent
> bit and that was the conf file we've removed as part of that. Is the machiner
> agent file something different? 

Those are two different concepts:
- the pyJuju API agent is the one that was started in the GUI node by running
the rapi-delta branch. The GUI used it as WebSocket server, and the agent itself
was connected to the bootstrap node Zookeper instance. Your branch get rid of
this machinery, and that's ok;
- the machiner agent file is a YAML created by juju-core which includes the
information required by the machiner worker. The API addresses are part of that
information. It must be considered an internal juju-core detail, and for this
reason we introduced the JUJU_API_ADDRESSES env var, included in the hooks'
execution context. But old versions of juju-core don't support this, so the
function strategy was:
  - try to use the environment variable;
  - if it is not present, parse the machiner agent file.
I am comfortable with that strategy, and I'd be inclined to preserve that
behavior. Anyway, as I mentioned, I can be ok in dropping support for old
juju-core releases if we clearly document it.