Merge lp:~hazmat/pyjuju/environment-settings into lp:~juju/pyjuju/docs

[Kapil Thangavelu]

Environment settings explicitly set.

Change environment settings from a transparent sync to an explict set/get after
initialization.

https://codereview.appspot.com/5849054/

[Kapil Thangavelu (hazmat)]

Reviewers: mp+98055_code.launchpad.net,

Message:
Please take a look.

Description:
Environment settings explicitly set.

Change environment settings from a transparent sync to an explict
set/get after
initialization.

https://code.launchpad.net/~hazmat/juju/environment-settings/+merge/98055

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   A source/drafts/environment-settings.rst

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: <email address hidden>
+New revision:
<email address hidden>

Index: source/drafts/environment-settings.rst
=== added file 'source/drafts/environment-settings.rst'
--- source/drafts/environment-settings.rst 1970-01-01 00:00:00 +0000
+++ source/drafts/environment-settings.rst 2012-03-17 05:51:10 +0000
@@ -0,0 +1,56 @@
+Environment Settings
+--------------------
+
+A Juju environment represents a group of services utilizing the
+machines of a provider. Juju's environment stores information
+regarding how to connect to a provider for the allocation of these
+machines.
+
+Juju lazily initializes this information in a post-bootstrap command
+to avoid passing provider credentials over unencrypted communciation
+channels (for example ec2 api's user-data).
+non bootstrap command.
+
+Previously Juju would sync the values in ~/.juju/environments.yaml to
+the environment's settings node in zookeeper on every command. This
+approach was problematic with multiple users interacting with an
+environment, and the values themselves weren't used to change the
+existing runtime environment, but future usage of these settings.
+
+Subsequent to the initialization of the environment settings. Juju now
+exposes a separate juju set-env command to allow for changing
+environment settings at runtime, and will no longer transparently sync
+the environments.yaml file to the environment state
+
+An example of juju-set usage::
+
+ juju set-env apt-proxy
http://us-east-1.ec2.archive.ubuntu.com.s3.amazonaws.com/ubuntu/
+
+There are several types of environment settings, which are utilized in
+different contexts. The provider credential information, used by the
+provisioning agents is kept at::
+
+ /environment/provider
+
+Other information potentially of use to a larger group of agents, is
+kept in::
+
+ /environment/settings
+
+An example of the latter might be apt-proxy information, or
+environment constraints information. Separating the storage of these
+items allows for different zk acls to be specified in accordance with
+the expected access of the different types of information.
+
+Non provider-credential settings can be specified at when using juju
+bootstrap and will be serialized into the environment via::
+
+ juju bootstrap apt-proxy=http://bit.ly ec2-instance-type=m1.large
ec2-region=eu-west-1
+
+Environment settings can also be retrieved with::
+
+ juju get-env
+
+
+
+

[William Reade (fwereade)]

Doesn't appear to address changes to provider-access settings, for which
I think the original model remains a surprisingly good fit. Otherwise
LGTM.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst
File source/drafts/environment-settings.rst (right):

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode18
source/drafts/environment-settings.rst:18: existing runtime environment,
but future usage of these settings.
Just a thought: it makes a certain amount of sense to update provider
access settings automatically; I can't forsee people changing them
without needing to do so.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode46
source/drafts/environment-settings.rst:46: bootstrap and will be
serialized into the environment via::
So, provider credentials will still be supplied lazily (good); is there
actually any reason not to keep this aspect using exactly the original
always-update behaviour? (For provider access settings only, ofc.)

https://codereview.appspot.com/5849054/

[Gustavo Niemeyer (niemeyer)]

Looks great. Details only:

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst
File source/drafts/environment-settings.rst (right):

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode12
source/drafts/environment-settings.rst:12: non bootstrap command.
Some left-over here.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode25
source/drafts/environment-settings.rst:25: An example of juju-set
usage::
s/juju-set/set-env/

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode27
source/drafts/environment-settings.rst:27: juju set-env apt-proxy
http://us-east-1.ec2.archive.ubuntu.com.s3.amazonaws.com/ubuntu/
Can we have an equals sign within each option. E.g.:

     juju set-env foo=bar foz=baz

It'd also be good to have a word about validation. set-env should blow
up with unsupported keys, probably implemented with a schema validator.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode45
source/drafts/environment-settings.rst:45: Non provider-credential
settings can be specified at when using juju
s/at //

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode48
source/drafts/environment-settings.rst:48: juju bootstrap
apt-proxy=http://bit.ly ec2-instance-type=m1.large ec2-region=eu-west-1
I hadn't thought of using raw arguments. It looks nice.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode52
source/drafts/environment-settings.rst:52: juju get-env
What does that return? JSON? Should it take arguments instead?

https://codereview.appspot.com/5849054/

[Kapil Thangavelu (hazmat)]

Thanks for the reviews guys.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst
File source/drafts/environment-settings.rst (right):

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode18
source/drafts/environment-settings.rst:18: existing runtime environment,
but future usage of these settings.
On 2012/03/17 14:49:30, fwereade wrote:
> Just a thought: it makes a certain amount of sense to update provider
access
> settings automatically; I can't forsee people changing them without
needing to
> do so.

Any sync is problematic for multi-user usage. Say we have different
accounts, sounds like a mess :-). Explicit set-env for provider creds
sounds quite reasonable to me.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode48
source/drafts/environment-settings.rst:48: juju bootstrap
apt-proxy=http://bit.ly ec2-instance-type=m1.large ec2-region=eu-west-1
On 2012/03/20 19:23:51, niemeyer wrote:
> I hadn't thought of using raw arguments. It looks nice.

it looks nice, but it might be considered inconsistent with the other
usage which is via a --constraints flag.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode52
source/drafts/environment-settings.rst:52: juju get-env
On 2012/03/20 19:23:51, niemeyer wrote:
> What does that return? JSON? Should it take arguments instead?

YAML, with a flag for json.

I'm realizing that we probably need another flag or require a key to
access provider settings. as we get to more multi user envs or delegated
usage, allowing everyone to retrieve the provider keys by default seems
suspect.

Otoh requiring a key always seems heavy handed since its an
introspection tool.

what do you think about a happy medium that provider credentials require
a --provider flag or arg. This also maps well to the proposed storage
mode.

https://codereview.appspot.com/5849054/

[William Reade (fwereade)]

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst
File source/drafts/environment-settings.rst (right):

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode48
source/drafts/environment-settings.rst:48: juju bootstrap
apt-proxy=http://bit.ly ec2-instance-type=m1.large ec2-region=eu-west-1
On 2012/03/21 08:19:05, hazmat wrote:
> On 2012/03/20 19:23:51, niemeyer wrote:
> > I hadn't thought of using raw arguments. It looks nice.

> it looks nice, but it might be considered inconsistent with the other
usage
> which is via a --constraints flag.

IMO separating the arguments into constraints/non-constraints is
unhelpfully icky; set-constraints does currently use raw args, but
that's just because there's no possibility of ambiguity: they're *all*
constraints, rather than being a mix of env settings and env
sub-settings (as it were).

https://codereview.appspot.com/5849054/

[Gustavo Niemeyer (niemeyer)]

Another round.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst
File source/drafts/environment-settings.rst (right):

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode48
source/drafts/environment-settings.rst:48: juju bootstrap
apt-proxy=http://bit.ly ec2-instance-type=m1.large ec2-region=eu-west-1
On 2012/03/21 08:33:26, fwereade wrote:
> IMO separating the arguments into constraints/non-constraints is
unhelpfully
> icky; set-constraints does currently use raw args, but that's just
because
> there's no possibility of ambiguity: they're *all* constraints, rather
than
> being a mix of env settings and env sub-settings (as it were).

Indeed. I hadn't realized that this was mixing both.

I suggest we have the environment settings as real flags for bootstrap.
For example, in the above case:

juju bootstrap --apt-proxy=http://bit.ly \
         --constraints="ec2-instance-type=m1.large"

This way it's still homogeneous, but in the other direction.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode52
source/drafts/environment-settings.rst:52: juju get-env
On 2012/03/21 08:19:05, hazmat wrote:
> YAML, with a flag for json.

Sounds good. I'd even drop json for now.

> I'm realizing that we probably need another flag or require a key to
access
> provider settings. as we get to more multi user envs or delegated
usage,
> allowing everyone to retrieve the provider keys by default seems
suspect.

I think it's fine to show whatever they've got access to. Just not
showing when they actually do have access isn't really secure, and when
we implement access control we can easily filter out provider
credentials if the user doesn't have it.

> Otoh requiring a key always seems heavy handed since its an
introspection tool.

> what do you think about a happy medium that provider credentials
require a
> --provider flag or arg. This also maps well to the proposed storage
mode.

I feel it's unnecessary. Just show everything for now. When we implement
access control, hide to those who can't see it.

https://codereview.appspot.com/5849054/

[Kapil Thangavelu (hazmat)]

On 2012/03/21 14:34:13, niemeyer wrote:
> Another round.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst
> File source/drafts/environment-settings.rst (right):

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode48
> source/drafts/environment-settings.rst:48: juju bootstrap
> apt-proxy=http://bit.ly ec2-instance-type=m1.large
ec2-region=eu-west-1
> On 2012/03/21 08:33:26, fwereade wrote:
> > IMO separating the arguments into constraints/non-constraints is
unhelpfully
> > icky; set-constraints does currently use raw args, but that's just
because
> > there's no possibility of ambiguity: they're *all* constraints,
rather than
> > being a mix of env settings and env sub-settings (as it were).

> Indeed. I hadn't realized that this was mixing both.

> I suggest we have the environment settings as real flags for
bootstrap. For
> example, in the above case:

> juju bootstrap --apt-proxy=http://bit.ly \
> --constraints="ec2-instance-type=m1.large"

> This way it's still homogeneous, but in the other direction.

sounds good.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode52
> source/drafts/environment-settings.rst:52: juju get-env
> On 2012/03/21 08:19:05, hazmat wrote:
> > YAML, with a flag for json.

> Sounds good. I'd even drop json for now.

> > I'm realizing that we probably need another flag or require a key to
access
> > provider settings. as we get to more multi user envs or delegated
usage,
> > allowing everyone to retrieve the provider keys by default seems
suspect.

> I think it's fine to show whatever they've got access to. Just not
showing when
> they actually do have access isn't really secure, and when we
implement access
> control we can easily filter out provider credentials if the user
doesn't have
> it.

> > Otoh requiring a key always seems heavy handed since its an
introspection
> tool.
> >
> > what do you think about a happy medium that provider credentials
require a
> > --provider flag or arg. This also maps well to the proposed storage
mode.

> I feel it's unnecessary. Just show everything for now. When we
implement access
> control, hide to those who can't see it.

sounds good.

https://codereview.appspot.com/5849054/

[Kapil Thangavelu (hazmat)]

Please take a look.

https://codereview.appspot.com/5849054/

[Gustavo Niemeyer (niemeyer)]

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst
File source/drafts/environment-settings.rst (right):

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode25
source/drafts/environment-settings.rst:25: An example of juju-set
usage::
On 2012/03/20 19:23:51, niemeyer wrote:
> s/juju-set/set-env/

This is still pending.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode27
source/drafts/environment-settings.rst:27: juju set-env apt-proxy
http://us-east-1.ec2.archive.ubuntu.com.s3.amazonaws.com/ubuntu/
On 2012/03/20 19:23:51, niemeyer wrote:
> Can we have an equals sign within each option. E.g.:

> juju set-env foo=bar foz=baz

> It'd also be good to have a word about validation. set-env should blow
up with
> unsupported keys, probably implemented with a schema validator.

Still pending.

https://codereview.appspot.com/5849054/diff/1/source/drafts/environment-settings.rst#newcode48
source/drafts/environment-settings.rst:48: juju bootstrap
apt-proxy=http://bit.ly ec2-instance-type=m1.large ec2-region=eu-west-1
On 2012/03/21 14:34:13, niemeyer wrote:
> On 2012/03/21 08:33:26, fwereade wrote:
> > IMO separating the arguments into constraints/non-constraints is
unhelpfully
> > icky; set-constraints does currently use raw args, but that's just
because
> > there's no possibility of ambiguity: they're *all* constraints,
rather than
> > being a mix of env settings and env sub-settings (as it were).

> Indeed. I hadn't realized that this was mixing both.

> I suggest we have the environment settings as real flags for
bootstrap. For
> example, in the above case:

> juju bootstrap --apt-proxy=http://bit.ly \
> --constraints="ec2-instance-type=m1.large"

> This way it's still homogeneous, but in the other direction.

This was also not done as suggested. apt-proxy should also be a flag,
with --apt-proxy.

https://codereview.appspot.com/5849054/

[Kapil Thangavelu (hazmat)]

Please take a look.

https://codereview.appspot.com/5849054/

[lp:~hazmat/pyjuju/environment-settings]

10. By Kapil Thangavelu on 2012-03-26

address review comments

11. By Kapil Thangavelu on 2012-03-26

add ignores

[Clint Byrum (clint-fewbar)]

This seems targetted at the wrong branch. Shouldn't it be against lp:juju/docs ?

Unmerged revisions

11. By Kapil Thangavelu on 2012-03-26

add ignores

10. By Kapil Thangavelu on 2012-03-26

address review comments

9. By Kapil Thangavelu on 2012-03-26

address review comments

8. By Kapil Thangavelu on 2012-03-21

address review comments

7. By Kapil Thangavelu on 2012-03-17

minor tweaks

6. By Kapil Thangavelu on 2012-03-17

global setting draft

=== added file '.bzrignore'
--- .bzrignore	1970-01-01 00:00:00 +0000
+++ .bzrignore	2012-03-26 13:42:27 +0000
@@ -0,0 +1,2 @@
+.emacs.desktop
+.emacs.desktop.lock

=== added file 'source/drafts/environment-settings.rst'
--- source/drafts/environment-settings.rst	1970-01-01 00:00:00 +0000
+++ source/drafts/environment-settings.rst	2012-03-26 13:42:27 +0000
@@ -0,0 +1,60 @@
+Environment Settings
+--------------------
+
+A Juju environment represents a group of services utilizing the
+machines of a provider.  Juju's environment stores information
+regarding how to connect to a provider for the allocation of these
+machines.
+
+Juju lazily stores this information in zookeeper via a post-bootstrap
+command to avoid passing provider credentials over unencrypted
+communciation channels (for example ec2 api's user-data).
+
+Previously Juju would sync the values in ~/.juju/environments.yaml to
+the environment's settings node in zookeeper on every command. This
+approach was problematic with multiple users interacting with an
+environment, and the values themselves weren't used to change the
+existing runtime environment, but future usage of these settings.
+
+Subsequent to the initialization of the environment settings, Juju now
+exposes a separate juju set-env command to allow for changing
+environment settings at runtime, and will no longer transparently sync
+the environments.yaml file to the environment state
+
+An example of juju set-env usage::
+
+   juju set-env apt-proxy http://us-east-1.ec2.archive.ubuntu.com.s3.amazonaws.com/ubuntu/
+
+Environment key/values are validated against a defined schema, and
+will return an error message on unsupported keys or values.
+
+There are several types of environment settings, which are utilized in
+different contexts. The provider credential information, used by the
+provisioning agents is kept at::
+
+    /environment/provider
+
+Other information potentially of use to a larger group of agents, is
+kept in::
+
+    /environment
+
+An example of the latter might be apt-proxy information, or
+environment constraints information. Separating the storage of these
+items allows for different zk acls to be specified in accordance with
+the expected access of the different types of information.
+
+Non provider-credential settings can be specified when using juju
+bootstrap and will be serialized into the environment via::
+
+   juju bootstrap --apt-proxy=http://bit.ly \
+                  --constraints="ec2-instance-type=m1.large ec2-region=eu-west-1"
+
+Environment settings can also be retrieved with::
+
+   juju get-env
+
+Which would return a YAML dictionary.
+
+
+