Re: [openstack-dev] [osc] bug/design flaw: Clients not cached per region

2016-10-18 Thread Adrian Turjak
In response to Jamie's comment I've abandoned my patch in favor of this one:

https://review.openstack.org/#/c/388232

It simply removes the ClientCache from the openstackclient code and
replaces it with property().


On 18/10/16 17:10, Adrian Turjak wrote:
>
> On 18/10/16 15:52, Jamie Lennox wrote:
>>
>> A comment from the cheap seats, almost all clients are using a
>> keystoneauth1 session at this point and that's where your
>> authentication information is being cached. There is essentially no
>> cost to creating a client with an existing session as auth happens on
>> demand.
>>
>> The region_name is not part of the authentication request, it's used
>> to lookup the endpoint and so is passed to Client creation.
>>
>> Given this maybe there is no longer any value in a ClientCache? It
>> was mostly useful to prevent clients doing dumb and share auth
>> amongst them. So long as the session/auth is created and saved once,
>> a client can be created per use/request with this information
>> (including region) with no real performance impact.
>>
>> Jamie
>>
> Getting rid of the cache would solve my problem, and considering it's
> all one shared session it shouldn't cause any problems.
>
> Updating region_name would still be a problem for interactive mode,
> although really that is a problem that is present right now anyway.
>
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [osc] bug/design flaw: Clients not cached per region

2016-10-17 Thread Adrian Turjak

On 18/10/16 15:52, Jamie Lennox wrote:
>
> A comment from the cheap seats, almost all clients are using a
> keystoneauth1 session at this point and that's where your
> authentication information is being cached. There is essentially no
> cost to creating a client with an existing session as auth happens on
> demand.
>
> The region_name is not part of the authentication request, it's used
> to lookup the endpoint and so is passed to Client creation.
>
> Given this maybe there is no longer any value in a ClientCache? It was
> mostly useful to prevent clients doing dumb and share auth amongst
> them. So long as the session/auth is created and saved once, a client
> can be created per use/request with this information (including
> region) with no real performance impact.
>
> Jamie
>
Getting rid of the cache would solve my problem, and considering it's
all one shared session it shouldn't cause any problems.

Updating region_name would still be a problem for interactive mode,
although really that is a problem that is present right now anyway.
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [osc] bug/design flaw: Clients not cached per region

2016-10-17 Thread Jamie Lennox
On 18 October 2016 at 12:09, Dean Troyer  wrote:

> On Mon, Oct 17, 2016 at 5:29 PM, Adrian Turjak 
> wrote:
> > What I'm wondering is can the current client cache be changed to be keyed
> > off the client_manager.region_name? That way the change is only in how
> the
> > clients are built and the code elsewhere doesn't change unless it
> actually
> > does something by manually changing region_name. This would then be a
> change
> > that would go unnoticed outside of the clientmanger and simply add a new
> > feature.
> >
> > Actually, I got distracted while writing this email and wrote a patch:
> > https://review.openstack.org/#/c/387696
> >
> > Using the test command in my first email, this works. It should simply
> work
> > with all existing cases, but the test suite should confirm that first of
> > course.
> >
> > With that change anyone can easily work exactly as before (region_name
> will
> > be set to your default region), and new features that are multi-region
> can
> > be introduced without any pain provided they know to update
> > client_manager.region_name.
>
> This is where I have a problem with this approach.  Those are
> descriptors, and make_client() is only called at first reference.  Any
> given command can not assume it will be the first one to initialize
> the client, or not.  Changing region_name like that is not going to
> reset the descriptor, that would now become a manual call.
>
> For the minimialist case of a CLI re-loading for each command issued
> (the common case it seems) this is less of an issue.  But for any
> longer-running invocation, such as interactive mode or a non-cli
> consumer of osc-lib, this quickly gets to be sub-optimal.  Keeping a
> client dict keyed off region name allows you to keep all of those
> clients (instantiated only as needed/used) around as long as you need
> them and not require re-creating them.
>
> There is also an interaction with the requests session cache that I do
> not think will be a problem, but have not yet thought through all the
> way here that we should consider.
>
> > I have been following this a little and it does sound interesting. Am
> > curious what solution is found for this. Can plugins overwrite existing
> > commands? That way if someone wanted a server create with their own
> features
> > they just make a plugin that replaces the standard command. While a bit
> of a
> > blunt solution, it does seem like the simplest, although people need to
> be
> > aware when installing plugins that they can replace/overwrite default
> > commands and be careful to install only trusted plugins.
>
> Currently there is _no_ checking done WRT the command namespaces, any
> plugin can happily stomp on any other command.  The results are
> officially undefined, mostly because the load order via setuptools is
> not deterministic or assured.  My server create plugin works, but we
> can not assure that will always be the case, which is why this is not
> released yet.
>
> The next plugin interface revision will have a notion of registering
> its commands so we can be more deliberate with the call orders and
> also collision checking.  There also needs to be some way to sanity
> check that not just any old thing that you might not be aware of is
> hooking your commands.
>
> dt
>
> --
>
> Dean Troyer
> dtro...@gmail.com
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>

A comment from the cheap seats, almost all clients are using a
keystoneauth1 session at this point and that's where your authentication
information is being cached. There is essentially no cost to creating a
client with an existing session as auth happens on demand.

The region_name is not part of the authentication request, it's used to
lookup the endpoint and so is passed to Client creation.

Given this maybe there is no longer any value in a ClientCache? It was
mostly useful to prevent clients doing dumb and share auth amongst them. So
long as the session/auth is created and saved once, a client can be created
per use/request with this information (including region) with no real
performance impact.


Jamie
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [osc] bug/design flaw: Clients not cached per region

2016-10-17 Thread Adrian Turjak


On 18/10/16 15:15, Adrian Turjak wrote:
> Although option would be to leave the cache in osc-lib untouched as a
> pure singleton and just make a new one for openstackclient that does
> support regions.

odd typo. "Although another option would be"


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [osc] bug/design flaw: Clients not cached per region

2016-10-17 Thread Adrian Turjak


On 18/10/16 14:09, Dean Troyer wrote:
> On Mon, Oct 17, 2016 at 5:29 PM, Adrian Turjak  
> wrote:
>> What I'm wondering is can the current client cache be changed to be keyed
>> off the client_manager.region_name? That way the change is only in how the
>> clients are built and the code elsewhere doesn't change unless it actually
>> does something by manually changing region_name. This would then be a change
>> that would go unnoticed outside of the clientmanger and simply add a new
>> feature.
>>
>> Actually, I got distracted while writing this email and wrote a patch:
>> https://review.openstack.org/#/c/387696
>>
>> Using the test command in my first email, this works. It should simply work
>> with all existing cases, but the test suite should confirm that first of
>> course.
>>
>> With that change anyone can easily work exactly as before (region_name will
>> be set to your default region), and new features that are multi-region can
>> be introduced without any pain provided they know to update
>> client_manager.region_name.
> This is where I have a problem with this approach.  Those are
> descriptors, and make_client() is only called at first reference.  Any
> given command can not assume it will be the first one to initialize
> the client, or not.  Changing region_name like that is not going to
> reset the descriptor, that would now become a manual call.
>
> For the minimialist case of a CLI re-loading for each command issued
> (the common case it seems) this is less of an issue.  But for any
> longer-running invocation, such as interactive mode or a non-cli
> consumer of osc-lib, this quickly gets to be sub-optimal.  Keeping a
> client dict keyed off region name allows you to keep all of those
> clients (instantiated only as needed/used) around as long as you need
> them and not require re-creating them.
>
> There is also an interaction with the requests session cache that I do
> not think will be a problem, but have not yet thought through all the
> way here that we should consider.
I'm not sure why being the first one to initialize the client or not is
a problem in anything other than interactive move. The only worry would
be in interactive mode where if one command altered
client_manger.region_name it does so globally and the next command will
reuse the set region. In all the common cases region_name will first be
set/reset from OS_REGION_NAME or clouds.yaml.

Other things that may confuse people:
client_manager.region_name = "RegionOne"
nova = client_manager.compute # reference to nova in RegionOne
# do stuff ...
client_manager.region_name = "RegionTwo"
# still a reference to nova in RegionOne
nova = client_manager.compute # only now is a reference to nova in RegionTwo


With this suggestion all I'm doing is moving your suggestion
('nova_client = self.app.client_manager.
compute[region]') down to the cache layer and having the cache still act
like a singleton but actually be a mapping of regions to clients. It
fixes my problem and needs no major refactoring elsewhere which is a
plus. Although option would be to leave the cache in osc-lib untouched
as a pure singleton and just make a new one for openstackclient that
does support regions.

For anyone using multi-region commands and the interactive mode, we
would need a different solution, but in part that could wait until there
are officially supported multi-region commands/plugins .

Ultimately I'm not fussed what the solution is, but I do think that not
having support for changing regions in the client manager or per region
clients is a waste. Plugins like ours, or future multi-region features,
will have to explicitly create the per region clients themselves, and
while not hard it does make having a central client manager less useful
or means they are duplicating the make_client functions for existing
clients like nova.

That said, I think the work being done with the OpenStackClient is
fantastic. It has been a massive effort and now especially with plugins
it is wonderful to work with. So don't take this as a complaint, I
genuinely do want to help make it better!
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [osc] bug/design flaw: Clients not cached per region

2016-10-17 Thread Dean Troyer
On Mon, Oct 17, 2016 at 5:29 PM, Adrian Turjak  wrote:
> What I'm wondering is can the current client cache be changed to be keyed
> off the client_manager.region_name? That way the change is only in how the
> clients are built and the code elsewhere doesn't change unless it actually
> does something by manually changing region_name. This would then be a change
> that would go unnoticed outside of the clientmanger and simply add a new
> feature.
>
> Actually, I got distracted while writing this email and wrote a patch:
> https://review.openstack.org/#/c/387696
>
> Using the test command in my first email, this works. It should simply work
> with all existing cases, but the test suite should confirm that first of
> course.
>
> With that change anyone can easily work exactly as before (region_name will
> be set to your default region), and new features that are multi-region can
> be introduced without any pain provided they know to update
> client_manager.region_name.

This is where I have a problem with this approach.  Those are
descriptors, and make_client() is only called at first reference.  Any
given command can not assume it will be the first one to initialize
the client, or not.  Changing region_name like that is not going to
reset the descriptor, that would now become a manual call.

For the minimialist case of a CLI re-loading for each command issued
(the common case it seems) this is less of an issue.  But for any
longer-running invocation, such as interactive mode or a non-cli
consumer of osc-lib, this quickly gets to be sub-optimal.  Keeping a
client dict keyed off region name allows you to keep all of those
clients (instantiated only as needed/used) around as long as you need
them and not require re-creating them.

There is also an interaction with the requests session cache that I do
not think will be a problem, but have not yet thought through all the
way here that we should consider.

> I have been following this a little and it does sound interesting. Am
> curious what solution is found for this. Can plugins overwrite existing
> commands? That way if someone wanted a server create with their own features
> they just make a plugin that replaces the standard command. While a bit of a
> blunt solution, it does seem like the simplest, although people need to be
> aware when installing plugins that they can replace/overwrite default
> commands and be careful to install only trusted plugins.

Currently there is _no_ checking done WRT the command namespaces, any
plugin can happily stomp on any other command.  The results are
officially undefined, mostly because the load order via setuptools is
not deterministic or assured.  My server create plugin works, but we
can not assure that will always be the case, which is why this is not
released yet.

The next plugin interface revision will have a notion of registering
its commands so we can be more deliberate with the call orders and
also collision checking.  There also needs to be some way to sanity
check that not just any old thing that you might not be aware of is
hooking your commands.

dt

-- 

Dean Troyer
dtro...@gmail.com

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [osc] bug/design flaw: Clients not cached per region

2016-10-17 Thread Adrian Turjak

On 18/10/16 03:36, Dean Troyer wrote:
> On Sun, Oct 16, 2016 at 9:11 PM, Adrian Turjak
> mailto:adri...@catalyst.net.nz>> wrote:
>
> The problem I'm running into is for some of our custom plugins we
> require the commands to run in all regions. We do this by getting the
> region list from keystone, and then doing the command in each region.
>
> ... 
>
> Is there a way to explicitly reinitialise the client_manager?
> 'client_mananger.reinitialise()' or something as easy? Maybe even a
> clear cache command?
>
>
> There is not a way to re-init the ClientManager, and really, you don't
> want to do that, but you do want a new compute client for each
> region.  OSC sets up the ClientManager client attributes with
> descriptors in common/clientmanager.py get_plugin_modules().  At this
> point we do not have any region information, so to do that you would
> have to do it later, but it is simple enough to return whatever you
> want from your plugin's make_client().  Normally that will be a client
> object for the desired API, for your plugin it could be a dict of
> client objects keyed off region_name.  Then your consuming code would
> need to deref those with:
>
>   for region in regions:
>   nova_client = self.app.client_manager.compute[region]
>   do_something(nova_client)
>  
>  
>
> While explicitly initialising clients isn't exactly difficult, it is
> something that would make more sense to simply work in the base
> openstackclient code as multi-region commands can and will be
> useful for
> a lot of people.
>
>
> Generalizing this to all of the existing plugins would be an enormous
> change, but doable.  We would need to think hard about how many of
> these things we need to handle, not just region, but cell or whatever
> else may be out there that a command would want to iterate on.
I don't think it is worth doing such a change exactly because it would
be a huge amount of work. Thats why I was hoping to have the value
returned be partly keyed off 'client_manager.region_name'. The clients
do get built off that value already, and I've tested altering that value
before the first client is built and even if my authed region was
RegionOne, if I manually set it to RegionTwo, the client will be built
for region two. The problem is that once cached, the client will always
be RegionTwo.

What I'm wondering is can the current client cache be changed to be
keyed off the client_manager.region_name? That way the change is only in
how the clients are built and the code elsewhere doesn't change unless
it actually does something by manually changing region_name. This would
then be a change that would go unnoticed outside of the clientmanger and
simply add a new feature.

Actually, I got distracted while writing this email and wrote a patch:
https://review.openstack.org/#/c/387696

Using the test command in my first email, this works. It should simply
work with all existing cases, but the test suite should confirm that
first of course.

With that change anyone can easily work exactly as before (region_name
will be set to your default region), and new features that are
multi-region can be introduced without any pain provided they know to
update client_manager.region_name.


> Note that we currently can not reliably hook existing commands to
> extend them, for example to support a 'server create' that uses --ram,
> --vcpu and --disk in place of --flavor.  That will be discussed next
> week in BCN.
I have been following this a little and it does sound interesting. Am
curious what solution is found for this. Can plugins overwrite existing
commands? That way if someone wanted a server create with their own
features they just make a plugin that replaces the standard command.
While a bit of a blunt solution, it does seem like the simplest,
although people need to be aware when installing plugins that they can
replace/overwrite default commands and be careful to install only
trusted plugins.

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [osc] bug/design flaw: Clients not cached per region

2016-10-17 Thread Dean Troyer
On Sun, Oct 16, 2016 at 9:11 PM, Adrian Turjak 
wrote:

> The problem I'm running into is for some of our custom plugins we
> require the commands to run in all regions. We do this by getting the
> region list from keystone, and then doing the command in each region.
>
...

> Is there a way to explicitly reinitialise the client_manager?
> 'client_mananger.reinitialise()' or something as easy? Maybe even a
> clear cache command?
>

There is not a way to re-init the ClientManager, and really, you don't want
to do that, but you do want a new compute client for each region.  OSC sets
up the ClientManager client attributes with descriptors in
common/clientmanager.py get_plugin_modules().  At this point we do not have
any region information, so to do that you would have to do it later, but it
is simple enough to return whatever you want from your plugin's
make_client().  Normally that will be a client object for the desired API,
for your plugin it could be a dict of client objects keyed off
region_name.  Then your consuming code would need to deref those with:

  for region in regions:
  nova_client = self.app.client_manager.compute[region]
  do_something(nova_client)



> While explicitly initialising clients isn't exactly difficult, it is
> something that would make more sense to simply work in the base
> openstackclient code as multi-region commands can and will be useful for
> a lot of people.
>

Generalizing this to all of the existing plugins would be an enormous
change, but doable.  We would need to think hard about how many of these
things we need to handle, not just region, but cell or whatever else may be
out there that a command would want to iterate on.


> Not to mention adding multi-region support to some commands so you don't
> have to change OS_REGION_NAME or your region in clouds.yaml could be
> very useful. A command to list your instances across all regions could
> be quite useful and easy to do.
>

It would, and would be near-trivial to do in a plugin with the clients
initialized as I showed above.  Note that we currently can not reliably
hook existing commands to extend them, for example to support a 'server
create' that uses --ram, --vcpu and --disk in place of --flavor.  That will
be discussed next week in BCN.

dt

-- 

Dean Troyer
dtro...@gmail.com
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


[openstack-dev] [osc] bug/design flaw: Clients not cached per region

2016-10-16 Thread Adrian Turjak
Hello OpenStack Dev,

I'm doing some work with custom plugins for openstackclient, and mostly
things are going well, but I've run into an annoying little bug/design
flaw around how per region clients are handled.

The openstackclient has a built in way for getting access to the various
openstack clients as such:
nova_client = self.app.client_manager.compute

The problem I'm running into is for some of our custom plugins we
require the commands to run in all regions. We do this by getting the
region list from keystone, and then doing the command in each region.

The expected workflow would be as such:

keystone = self.app.client_manager.identity
regions = [region.id for region in keystone.regions.list()]

for region in regions:
self.app.client_manager.region_name = region
nova_client = self.app.client_manager.compute
do_something(nova_client)


This will work for the first region, with the region_name value being
used to initialise the client the first time, but once a nova client is
created, that client is cached and isn't cached per region. When the
region_name is changed the client cache should be re-intialised or it
should cache per region per service rather than just per service.

Is there a way to explicitly reinitialise the client_manager?
'client_mananger.reinitialise()' or something as easy? Maybe even a
clear cache command?


Here is a very basic example command I've made to show exactly why this
doesn't work:
http://paste.openstack.org/show/585872/

If run in a deployment with more than one region, you will see that the
inbuilt client will list the same instances in all regions while the
explicitly created one will correctly list instances per region.


While explicitly initialising clients isn't exactly difficult, it is
something that would make more sense to simply work in the base
openstackclient code as multi-region commands can and will be useful for
a lot of people.

Not to mention adding multi-region support to some commands so you don't
have to change OS_REGION_NAME or your region in clouds.yaml could be
very useful. A command to list your instances across all regions could
be quite useful and easy to do.


Any thoughts? Suggestions?

Cheers,
Adrian Turjak


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev