Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place.
This is useful because sometimes, Vdsm's configuration is not good enough for the user. For example, someone may need to set various ETHTOOL_OPTS on a nic. Now, they can put a script under /usr/libexec/vdsm/after_network_setup/ that tweak their ifcfg-eth* files after they have been written by Vdsm.
However, the hook script only knows that *a* change of network configuration took place. It does not know which change took place, and has to figure this out on its own.
Enters http://gerrit.ovirt.org/20330 "allow hooks to pass down dictionaries in json format".
I'd like to discuss it here, as it introduces a new Vdsm/Hook API that is quite different than what we have for other hooks. Unlike with Vm and VmDevice creation, where Vdsm uses libvirt's xml definition internally as well as to communicate with the hooks, before/after_network_setup have to define their own means of communication.
I would like to suggest to use the same information passed on the Engine/Vdsm API, and extend its reach into the hook script. The three arguments to setupNetworks(networks, bondings, options) would be dumped as json strings, to be read by the hook script.
This option is very simple to use and implement, it gives the hook all the information that Vdsm-proper has, and allows for greatest flexibility for hook writers. This is also the down side of this idea: hook script may do all kinds of things with this information, some of them unsupportable, and they should be notified when Engine/Vdsm API changes.
In my opinion, it is a small price to pay: hooks have always had the China Store Rule - if you break something, you own it. Hook users must know what they're doing, and take care not to use deprecated bits of the API.
What is your opinion? Comments and suggestions are most welcome!
Dan.
Hello everybody, 2014/1/3 Dan Kenigsberg danken@redhat.com
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place.
Dan, thank you for starting this thread,
[...]
I'd like to discuss it here, as it introduces a new Vdsm/Hook API that is quite different than what we have for other hooks. Unlike with Vm and VmDevice creation, where Vdsm uses libvirt's xml definition internally as well as to communicate with the hooks, before/after_network_setup have to define their own means of communication.
Yes, going the xml way was the first approach, but then we realized that the cost of maintaining a new xml schema probably wasn't worth.
I would like to suggest to use the same information passed on the Engine/Vdsm API, and extend its reach into the hook script. The three arguments to setupNetworks(networks, bondings, options) would be dumped as json strings, to be read by the hook script.
In the last patch set I've extended it a little bit with suggestions in gerrit, we pass down the actual request, plus the current network configuration, whenever the unified_persistence is enabled.
{"request": { "networks": {"virtnet": {"bonding" : "bond0", "bridged": true, "vlan":27}}, "bondings": {"bond0": {"nics":["eth1","eth2"]}}, "options": {"conectivityCheck":false} }, "current": { "networks": {"ovirtmgmt": {"nic" : "eth0", "vlan":27 }} "bondings": {"bond1": {"nics":["eth3","eth4"]}} } }
This option is very simple to use and implement, it gives the hook all the information that Vdsm-proper has, and allows for greatest flexibility for hook writers. This is also the down side of this idea: hook script may do all kinds of things with this information, some of them unsupportable, and they should be notified when Engine/Vdsm API changes.
In my opinion, it is a small price to pay: hooks have always had the China Store Rule - if you break something, you own it. Hook users must know what they're doing, and take care not to use deprecated bits of the API.
What is your opinion? Comments and suggestions are most welcome!
My opinion the same as yours, but I'm new to this project,
If the chances for this API parameters to change are low: In the event of that significant changes happening, we could mitigate the problem introducing an intermediate layer to dump/load the information from the API parameters.
I'd really like to know what do others think about this.
Dan.
Cheers, Miguel Ángel Ajo.
On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place.
This is useful because sometimes, Vdsm's configuration is not good enough for the user. For example, someone may need to set various ETHTOOL_OPTS on a nic. Now, they can put a script under /usr/libexec/vdsm/after_network_setup/ that tweak their ifcfg-eth* files after they have been written by Vdsm.
However, the hook script only knows that *a* change of network configuration took place. It does not know which change took place, and has to figure this out on its own.
Enters http://gerrit.ovirt.org/20330 "allow hooks to pass down dictionaries in json format".
I'd like to discuss it here, as it introduces a new Vdsm/Hook API that is quite different than what we have for other hooks. Unlike with Vm and VmDevice creation, where Vdsm uses libvirt's xml definition internally as well as to communicate with the hooks, before/after_network_setup have to define their own means of communication.
I would like to suggest to use the same information passed on the Engine/Vdsm API, and extend its reach into the hook script. The three arguments to setupNetworks(networks, bondings, options) would be dumped as json strings, to be read by the hook script.
This option is very simple to use and implement, it gives the hook all the information that Vdsm-proper has, and allows for greatest flexibility for hook writers. This is also the down side of this idea: hook script may do all kinds of things with this information, some of them unsupportable, and they should be notified when Engine/Vdsm API changes.
In my opinion, it is a small price to pay: hooks have always had the China Store Rule - if you break something, you own it. Hook users must know what they're doing, and take care not to use deprecated bits of the API.
What is your opinion? Comments and suggestions are most welcome!
Seems like a logical thing to do. What specific mechanism do you suggest for passing the JSON strings to the hook script? If passed as arguments to the hook script we would need to consider shell escaping and argv length restrictions.
What about writing these out to a special file and adding a new getContext() call to the hooking module. A script that is unconcerned with the context would not require any changes. But a script that wants access would simply do:
ctx = hooking.getContext()
and ctx would be the contents of the special file already decoded into a native Python object for easy consumption. This could easily be extended to any hook which may want to provide some context to implementors.
One more question comes to mind: Are there any pieces of information that we would need to redact from the context (passwords or other sensitive information)?
Hi Adam
Thanks for the feedback
2014/1/3 Adam Litke alitke@redhat.com
On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place. [....]
Seems like a logical thing to do. What specific mechanism do you suggest for passing the JSON strings to the hook script? If passed as arguments to the hook script we would need to consider shell escaping and argv length restrictions.
As for the libvirt domain xml we pass to other hooks, we write a temporary
file and we set an environment variable pointing to it before calling the script
What about writing these out to a special file and adding a new getContext() call to the hooking module. A script that is unconcerned with the context would not require any changes. But a script that wants access would simply do:
ctx = hooking.getContext()
and ctx would be the contents of the special file already decoded into a native Python object for easy consumption. This could easily be extended to any hook which may want to provide some context to implementors.
That would be nice, so scripts written in python wouldn't need to look for, and parse the file.
This is an example of a simple hook:
http://gerrit.ovirt.org/#/c/20330/7/tests/functional/networkTests.py (look inside the ValidatesHook decorator)
It'd be quite simplified. We would also need a "setContext()..." to update context with changes.
One more question comes to mind: Are there any pieces of information that we would need to redact from the context (passwords or other sensitive information)?
I think there is no sensitive information as far as I know.
Greetings, Miguel Ángel.
Whichever way we decide to do this, I think the important bit is documentation - We have to make sure to update the oVirt wiki hooks pages. If users aren't aware of how to retrieve the networking config then we might as well not implement it.
That being said, I'd expose three dictionaries: What's currently configure, the current request, as well as the proposed end result. It's easy to add and I see how it would be useful to hook writers. And just to state the obvious, just like how traditional hooks can change the VM or device XML, the hook should be able to rewrite the current request contents. For example, if a user would like to take over host networking configuration, he could just write a before_setup_networks hook that would configure networking however he wants, then writes the empty dictionary to the current request, meaning that VDSM wouldn't do anything further with the current setup networks request.
Assaf Muller, Cloud Networking Engineer Red Hat
----- Original Message ----- From: "Miguel Angel" miguelangel@ajo.es To: "Adam Litke" alitke@redhat.com Cc: dsulliva@redhat.com, arch@ovirt.org, vdsm-devel@fedorahosted.org Sent: Saturday, January 4, 2014 9:08:17 PM Subject: Re: [vdsm] Smarter network_setup hooks
Hi Adam
Thanks for the feedback
2014/1/3 Adam Litke < alitke@redhat.com >
On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place. [....] Seems like a logical thing to do. What specific mechanism do you suggest for passing the JSON strings to the hook script? If passed as arguments to the hook script we would need to consider shell escaping and argv length restrictions.
As for the libvirt domain xml we pass to other hooks, we write a temporary file and we set an environment variable pointing to it before calling the script
What about writing these out to a special file and adding a new getContext() call to the hooking module. A script that is unconcerned with the context would not require any changes. But a script that wants access would simply do:
ctx = hooking.getContext()
and ctx would be the contents of the special file already decoded into a native Python object for easy consumption. This could easily be extended to any hook which may want to provide some context to implementors.
That would be nice, so scripts written in python wouldn't need to look for, and parse the file.
This is an example of a simple hook:
http://gerrit.ovirt.org/#/c/20330/7/tests/functional/networkTests.py (look inside the ValidatesHook decorator)
It'd be quite simplified. We would also need a "setContext()..." to update context with changes.
One more question comes to mind: Are there any pieces of information that we would need to redact from the context (passwords or other sensitive information)?
I think there is no sensitive information as far as I know.
Greetings, Miguel Ángel.
_______________________________________________ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
On 01/05/2014 12:05 PM, Assaf Muller wrote:
Whichever way we decide to do this, I think the important bit is documentation - We have to make sure to update the oVirt wiki hooks pages. If users aren't aware of how to retrieve the networking config then we might as well not implement it.
That being said, I'd expose three dictionaries: What's currently configure, the current request, as well as the proposed end result. It's easy to add and I see how it would be useful to hook writers. And just to state the obvious, just like how traditional hooks can change the VM or device XML, the hook should be able to rewrite the current request contents. For example, if a user would like to take over host networking configuration, he could just write a before_setup_networks hook that would configure networking however he wants, then writes the empty dictionary to the current request, meaning that VDSM wouldn't do anything further with the current setup networks request.
+1, I think the API above would be easy to consume.
Livnat
Assaf Muller, Cloud Networking Engineer Red Hat
----- Original Message ----- From: "Miguel Angel" miguelangel@ajo.es To: "Adam Litke" alitke@redhat.com Cc: dsulliva@redhat.com, arch@ovirt.org, vdsm-devel@fedorahosted.org Sent: Saturday, January 4, 2014 9:08:17 PM Subject: Re: [vdsm] Smarter network_setup hooks
Hi Adam
Thanks for the feedback
2014/1/3 Adam Litke < alitke@redhat.com >
On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place. [....] Seems like a logical thing to do. What specific mechanism do you suggest for passing the JSON strings to the hook script? If passed as arguments to the hook script we would need to consider shell escaping and argv length restrictions.
As for the libvirt domain xml we pass to other hooks, we write a temporary file and we set an environment variable pointing to it before calling the script
What about writing these out to a special file and adding a new getContext() call to the hooking module. A script that is unconcerned with the context would not require any changes. But a script that wants access would simply do:
ctx = hooking.getContext()
and ctx would be the contents of the special file already decoded into a native Python object for easy consumption. This could easily be extended to any hook which may want to provide some context to implementors.
That would be nice, so scripts written in python wouldn't need to look for, and parse the file.
This is an example of a simple hook:
http://gerrit.ovirt.org/#/c/20330/7/tests/functional/networkTests.py (look inside the ValidatesHook decorator)
It'd be quite simplified. We would also need a "setContext()..." to update context with changes.
One more question comes to mind: Are there any pieces of information that we would need to redact from the context (passwords or other sensitive information)?
I think there is no sensitive information as far as I know.
Greetings, Miguel Ángel.
vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel _______________________________________________ Arch mailing list Arch@ovirt.org http://lists.ovirt.org/mailman/listinfo/arch
2014/1/5 Livnat Peer lpeer@redhat.com
On 01/05/2014 12:05 PM, Assaf Muller wrote:
Whichever way we decide to do this, I think the important bit is
documentation - We have
to make sure to update the oVirt wiki hooks pages. If users aren't aware
of how to retrieve
the networking config then we might as well not implement it.
That being said, I'd expose three dictionaries: What's currently
configure,
the current request, as well as the proposed end result. It's easy to add and I see how it would be useful to hook writers. And just to state the
obvious,
just like how traditional hooks can change the VM or device XML, the hook should be able to rewrite the current request contents. For example, if a user would like to take over host networking
configuration,
he could just write a before_setup_networks hook that would configure networking however he wants, then writes the empty dictionary to the
current request,
meaning that VDSM wouldn't do anything further with the current setup
networks request.
I'm not sure if it's easy to get the final state without actually applying it, it's easy to get an approximate final state (just aggregating dictionaries to networks and bondings, and erasing the removed ones), but I suppose that'd be good enough :-)
May be it's good if you can provide a use case for this third "expected" final state, I can't come up with one. :-)
+1, I think the API above would be easy to consume.
Livnat
Assaf Muller, Cloud Networking Engineer Red Hat
----- Original Message ----- From: "Miguel Angel" miguelangel@ajo.es To: "Adam Litke" alitke@redhat.com Cc: dsulliva@redhat.com, arch@ovirt.org, vdsm-devel@fedorahosted.org Sent: Saturday, January 4, 2014 9:08:17 PM Subject: Re: [vdsm] Smarter network_setup hooks
Hi Adam
Thanks for the feedback
2014/1/3 Adam Litke < alitke@redhat.com >
On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place. [....] Seems like a logical thing to do. What specific mechanism do you suggest for passing the JSON strings to the hook script? If passed as arguments to the hook script we would need to consider shell escaping and argv length restrictions.
As for the libvirt domain xml we pass to other hooks, we write a
temporary file
and we set an environment variable pointing to it before calling the
script
What about writing these out to a special file and adding a new getContext() call to the hooking module. A script that is unconcerned with the context would not require any changes. But a script that wants access would simply do:
ctx = hooking.getContext()
and ctx would be the contents of the special file already decoded into a native Python object for easy consumption. This could easily be extended to any hook which may want to provide some context to implementors.
That would be nice, so scripts written in python wouldn't need to look
for, and parse
the file.
This is an example of a simple hook:
http://gerrit.ovirt.org/#/c/20330/7/tests/functional/networkTests.py(look inside the ValidatesHook decorator)
It'd be quite simplified. We would also need a "setContext()..." to
update context with changes.
One more question comes to mind: Are there any pieces of information that we would need to redact from the context (passwords or other sensitive information)?
I think there is no sensitive information as far as I know.
Greetings, Miguel Ángel.
vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel _______________________________________________ Arch mailing list Arch@ovirt.org http://lists.ovirt.org/mailman/listinfo/arch
----- Original Message -----
From: "Miguel Angel" miguelangel@ajo.es To: "Livnat Peer" lpeer@redhat.com Cc: dsulliva@redhat.com, vdsm-devel@fedorahosted.org, arch@ovirt.org Sent: Sunday, January 5, 2014 11:41:44 PM Subject: Re: [vdsm] Smarter network_setup hooks
2014/1/5 Livnat Peer < lpeer@redhat.com >
On 01/05/2014 12:05 PM, Assaf Muller wrote:
Whichever way we decide to do this, I think the important bit is documentation - We have to make sure to update the oVirt wiki hooks pages. If users aren't aware of how to retrieve the networking config then we might as well not implement it.
That being said, I'd expose three dictionaries: What's currently configure, the current request, as well as the proposed end result. It's easy to add and I see how it would be useful to hook writers. And just to state the obvious, just like how traditional hooks can change the VM or device XML, the hook should be able to rewrite the current request contents. For example, if a user would like to take over host networking configuration, he could just write a before_setup_networks hook that would configure networking however he wants, then writes the empty dictionary to the current request, meaning that VDSM wouldn't do anything further with the current setup networks request.
My original thought on this was that the hook would be able to mark a device as configured by it adding a key value like 'hooked': X. This was in order that if, for example a bond is to be configured by the hook, it would still stay in the config passed to objectivize but the configurator configure method for it would be short circuited and X would have the data that is to be put in the running config by vdsm for that device.
I didn't mature this thought much though... (holidays have kept me a bit busy)
I'm not sure if it's easy to get the final state without actually applying it, it's easy to get an approximate final state (just aggregating dictionaries to networks and bondings, and erasing the removed ones), but I suppose that'd be good enough :-)
May be it's good if you can provide a use case for this third "expected" final state, I can't come up with one. :-)
For this reason I thought of the 'hooked' value, to make the hook writer own the task of filling in the missing piece of state.
One final consideration that I didn't see arise is the need for having caps hooks. As soon as we allow setupNetworks hooks it is very necessary that we enable vdsm caps. One very easy example.
Let us say that I write a hook that makes setupNetworks specified bondings be configured in the system by libteam/teamd. In order for the changes to be made visible to the engine we need to fake that team link aggregation as a bond so that the engine can, in the future, change/delete/use it.
+1, I think the API above would be easy to consume.
Livnat
Assaf Muller, Cloud Networking Engineer Red Hat
----- Original Message ----- From: "Miguel Angel" < miguelangel@ajo.es > To: "Adam Litke" < alitke@redhat.com > Cc: dsulliva@redhat.com , arch@ovirt.org , vdsm-devel@fedorahosted.org Sent: Saturday, January 4, 2014 9:08:17 PM Subject: Re: [vdsm] Smarter network_setup hooks
Hi Adam
Thanks for the feedback
2014/1/3 Adam Litke < alitke@redhat.com >
On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place. [....] Seems like a logical thing to do. What specific mechanism do you suggest for passing the JSON strings to the hook script? If passed as arguments to the hook script we would need to consider shell escaping and argv length restrictions.
As for the libvirt domain xml we pass to other hooks, we write a temporary file and we set an environment variable pointing to it before calling the script
What about writing these out to a special file and adding a new getContext() call to the hooking module. A script that is unconcerned with the context would not require any changes. But a script that wants access would simply do:
ctx = hooking.getContext()
and ctx would be the contents of the special file already decoded into a native Python object for easy consumption. This could easily be extended to any hook which may want to provide some context to implementors.
That would be nice, so scripts written in python wouldn't need to look for, and parse the file.
This is an example of a simple hook:
http://gerrit.ovirt.org/#/c/20330/7/tests/functional/networkTests.py (look inside the ValidatesHook decorator)
It'd be quite simplified. We would also need a "setContext()..." to update context with changes.
One more question comes to mind: Are there any pieces of information that we would need to redact from the context (passwords or other sensitive information)?
I think there is no sensitive information as far as I know.
Greetings, Miguel Ángel.
vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel _______________________________________________ Arch mailing list Arch@ovirt.org http://lists.ovirt.org/mailman/listinfo/arch
vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
On Sun, Jan 05, 2014 at 08:58:07PM -0500, Antoni Segura Puimedon wrote:
----- Original Message -----
From: "Miguel Angel" miguelangel@ajo.es To: "Livnat Peer" lpeer@redhat.com Cc: dsulliva@redhat.com, vdsm-devel@fedorahosted.org, arch@ovirt.org Sent: Sunday, January 5, 2014 11:41:44 PM Subject: Re: [vdsm] Smarter network_setup hooks
2014/1/5 Livnat Peer < lpeer@redhat.com >
On 01/05/2014 12:05 PM, Assaf Muller wrote:
Whichever way we decide to do this, I think the important bit is documentation - We have to make sure to update the oVirt wiki hooks pages. If users aren't aware of how to retrieve the networking config then we might as well not implement it.
That being said, I'd expose three dictionaries: What's currently configure, the current request, as well as the proposed end result. It's easy to add and I see how it would be useful to hook writers. And just to state the obvious, just like how traditional hooks can change the VM or device XML, the hook should be able to rewrite the current request contents. For example, if a user would like to take over host networking configuration, he could just write a before_setup_networks hook that would configure networking however he wants, then writes the empty dictionary to the current request, meaning that VDSM wouldn't do anything further with the current setup networks request.
My original thought on this was that the hook would be able to mark a device as configured by it adding a key value like 'hooked': X. This was in order that if, for example a bond is to be configured by the hook, it would still stay in the config passed to objectivize but the configurator configure method for it would be short circuited and X would have the data that is to be put in the running config by vdsm for that device.
I didn't mature this thought much though... (holidays have kept me a bit busy)
Oh, few seconds ago I noted about the idea that hooks scripts would be able to remove parts of the configuration which they have already implemented, so that Vdsm proper is left unaware of them.
It's not as flexible as your "hooked=True" suggestion, as it does not alow to implement only part of a network, but I think that it is as-powerful and with clearer atomicity.
I'm not sure if it's easy to get the final state without actually applying it, it's easy to get an approximate final state (just aggregating dictionaries to networks and bondings, and erasing the removed ones), but I suppose that'd be good enough :-)
May be it's good if you can provide a use case for this third "expected" final state, I can't come up with one. :-)
For this reason I thought of the 'hooked' value, to make the hook writer own the task of filling in the missing piece of state.
Sorry, Toni, I do not understaed how hooked=True is related to the "expected" dictionary suggested by Assaf.
One final consideration that I didn't see arise is the need for having caps hooks. As soon as we allow setupNetworks hooks it is very necessary that we enable vdsm caps. One very easy example.
Let us say that I write a hook that makes setupNetworks specified bondings be configured in the system by libteam/teamd. In order for the changes to be made visible to the engine we need to fake that team link aggregation as a bond so that the engine can, in the future, change/delete/use it.
Correct. The same json-based framework devised by Miguel (as well as Adam's getContext) would come up hand when implementing it.
Dan.
On Sun, Jan 05, 2014 at 05:05:59AM -0500, Assaf Muller wrote:
Whichever way we decide to do this, I think the important bit is documentation - We have to make sure to update the oVirt wiki hooks pages. If users aren't aware of how to retrieve the networking config then we might as well not implement it.
That being said, I'd expose three dictionaries: What's currently configure, the current request, as well as the proposed end result.
I do not see why "current" is a good idea to pass from Vdsm to the hook - if the hook needs it, it could compute the current state on its own.
What do you mean by the "proposed end result"? setupNetwork API works in allows "diffs" relative to the current state. The end result, however, may even be unexpressable within the scope of our API (that's exactly what hooks are for!). And again, if Vdsm is able to calculate the end result based on the current state + diff, so can the hook itself.
It's easy to add and I see how it would be useful to hook writers. And just to state the obvious, just like how traditional hooks can change the VM or device XML, the hook should be able to rewrite the current request contents. For example, if a user would like to take over host networking configuration, he could just write a before_setup_networks hook that would configure networking however he wants, then writes the empty dictionary to the current request, meaning that VDSM wouldn't do anything further with the current setup networks request.
This would be a very good additions. Cascaded scripts would then be able to drop parts of the command, which they have implemented themselves.
Regards, Dan.
On 04/01/14 20:08 +0100, Miguel Angel wrote:
Hi Adam
Thanks for the feedback
2014/1/3 Adam Litke alitke@redhat.com
On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place. [....]
Seems like a logical thing to do. What specific mechanism do you suggest for passing the JSON strings to the hook script? If passed as arguments to the hook script we would need to consider shell escaping and argv length restrictions.
As for the libvirt domain xml we pass to other hooks, we write a temporary
file and we set an environment variable pointing to it before calling the script
What about writing these out to a special file and adding a new getContext() call to the hooking module. A script that is unconcerned with the context would not require any changes. But a script that wants access would simply do:
ctx = hooking.getContext()
and ctx would be the contents of the special file already decoded into a native Python object for easy consumption. This could easily be extended to any hook which may want to provide some context to implementors.
That would be nice, so scripts written in python wouldn't need to look for, and parse the file.
This is an example of a simple hook:
http://gerrit.ovirt.org/#/c/20330/7/tests/functional/networkTests.py (look inside the ValidatesHook decorator)
It'd be quite simplified. We would also need a "setContext()..." to update context with changes.
Yeah seems reasonable. Then hook scripts can become much simpler and easier to read and write:
ctx = hooking.getContext() ctx['foo'] = 'bar' hooking.setContext(ctx)
One more question comes to mind: Are there any pieces of information that we would need to redact from the context (passwords or other sensitive information)?
I think there is no sensitive information as far as I know.
Greetings, Miguel Ángel.
Mid-thread summary:
First some terminology so we're all on the same page: 'request' - The setupNetworks that just arrived 'running config' - If unified persistence is being used - All networking on the host as VDSM sees it 'expected' - If unified persistence is being used - The running config after the effects of the request, but before the hook is ran
I think we all agree that we have to expose the 'request' to the hook. However, there's two different approaches: 1) Allow the hook to rewrite parts of the request (For example, if two networks were sent, and the hook handled one of them, it could then delete that network from the request, so VDSM will only continue to configure the 2nd network). 2) Toni's idea with marking devices as hook-configured. So if a network is over a bridge over a VLAN over a bond over three NICs, hooks could configure only the NICs (For example) and VDSM would configure the rest, whereas the first idea means that the hook would have to configure the entire network (Bridge, VLANs, bonds, and all 3 NICs) and then remove that network from the request. The disadvantage of this method is that it would be harder to implement, and have references to the hooks flag throughout all of VDSM.
Then there's the matter of IF to expose the 'running config' and 'expected'.
My main argument is that it's very easy to expose to the hooks (But then why not expose other 'random' data that is easy for VDSM to calculate?), and that we don't understand all usages for the setupNetworks hooks. I'd rather we expose too much information than not enough and screw over hook writers.
Either way, let's get some votes in a timely manner so we could manage to implement this for 3.4.
Assaf Muller, Cloud Networking Engineer Red Hat
----- Original Message ----- From: "Adam Litke" alitke@redhat.com To: "Miguel Angel" miguelangel@ajo.es Cc: dsulliva@redhat.com, arch@ovirt.org, vdsm-devel@fedorahosted.org Sent: Monday, January 6, 2014 3:32:07 PM Subject: Re: [vdsm] Smarter network_setup hooks
On 04/01/14 20:08 +0100, Miguel Angel wrote:
Hi Adam
Thanks for the feedback
2014/1/3 Adam Litke alitke@redhat.com
On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place. [....]
Seems like a logical thing to do. What specific mechanism do you suggest for passing the JSON strings to the hook script? If passed as arguments to the hook script we would need to consider shell escaping and argv length restrictions.
As for the libvirt domain xml we pass to other hooks, we write a temporary
file and we set an environment variable pointing to it before calling the script
What about writing these out to a special file and adding a new getContext() call to the hooking module. A script that is unconcerned with the context would not require any changes. But a script that wants access would simply do:
ctx = hooking.getContext()
and ctx would be the contents of the special file already decoded into a native Python object for easy consumption. This could easily be extended to any hook which may want to provide some context to implementors.
That would be nice, so scripts written in python wouldn't need to look for, and parse the file.
This is an example of a simple hook:
http://gerrit.ovirt.org/#/c/20330/7/tests/functional/networkTests.py (look inside the ValidatesHook decorator)
It'd be quite simplified. We would also need a "setContext()..." to update context with changes.
Yeah seems reasonable. Then hook scripts can become much simpler and easier to read and write:
ctx = hooking.getContext() ctx['foo'] = 'bar' hooking.setContext(ctx)
One more question comes to mind: Are there any pieces of information that we would need to redact from the context (passwords or other sensitive information)?
I think there is no sensitive information as far as I know.
Greetings, Miguel Ángel.
_______________________________________________ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Hi Assaf, Thank you very much for the summary,
Just a few questions, there are things I don't understand yet:
2014/1/8 Assaf Muller amuller@redhat.com
Mid-thread summary:
First some terminology so we're all on the same page: 'request' - The setupNetworks that just arrived 'running config' - If unified persistence is being used - All networking on the host as VDSM sees it 'expected' - If unified persistence is being used - The running config after the effects of the request, but before the hook is ran
correct
I think we all agree that we have to expose the 'request' to the hook. However, there's two different approaches:
- Allow the hook to rewrite parts of the request (For example, if two
networks were sent, and the hook handled one of them, it could then delete that network from the request, so VDSM will only continue to configure the 2nd network).
In this case ,if the hook deleted the "2nd network", VDSM couldn't handle the network config persistence anymore, right?,
I didn't expect this use case, I only expected tweaks, etc (before or after network setup), for example, setting special hardware capabilities using ethtool or those kind of things.
- Toni's idea with marking devices as hook-configured. So if a network is
over a bridge over a VLAN over a bond over three NICs, hooks could configure only the NICs (For example) and VDSM would configure the rest, whereas the first idea means that the hook would have to configure the entire network (Bridge, VLANs, bonds, and all 3 NICs) and then remove that network from the request. The disadvantage of this method is that it would be harder to implement, and have references to the hooks flag throughout all of VDSM.
You mean that if the hook marked a certain device as "hook handled" then, VDSM would just keep this information along with the network config, etc... and won't do any setup, just config-keeping, right?
Then there's the matter of IF to expose the 'running config' and 'expected'.
My main argument is that it's very easy to expose to the hooks (But then why not expose other 'random' data that is easy for VDSM to calculate?), and that we don't understand all usages for the setupNetworks hooks. I'd rather we expose too much information than not enough and screw over hook writers.
We have something important here, the hooks don't need to be python-written, they could be bash scripts, or any other thing... that means that they wouldn't have access to get "running config", but they could calculate "expected".
So, my vote here goes for "request" + "running config".
Either way, let's get some votes in a timely manner so we could manage to implement this for 3.4.
Thanks Assaf! ;)
Assaf Muller, Cloud Networking Engineer Red Hat
----- Original Message ----- From: "Adam Litke" alitke@redhat.com To: "Miguel Angel" miguelangel@ajo.es Cc: dsulliva@redhat.com, arch@ovirt.org, vdsm-devel@fedorahosted.org Sent: Monday, January 6, 2014 3:32:07 PM Subject: Re: [vdsm] Smarter network_setup hooks
On 04/01/14 20:08 +0100, Miguel Angel wrote:
Hi Adam
Thanks for the feedback
2014/1/3 Adam Litke alitke@redhat.com
On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the implementation of setupNetworks in Vdsm: two hook points where added: before and after the setupNetworks verb takes place. [....]
Seems like a logical thing to do. What specific mechanism do you suggest for passing the JSON strings to the hook script? If passed as arguments to the hook script we would need to consider shell escaping and argv length restrictions.
As for the libvirt domain xml we pass to other hooks, we write a
temporary
file and we set an environment variable pointing to it before calling the
script
What about writing these out to a special file and adding a new getContext() call to the hooking module. A script that is unconcerned with the context would not require any changes. But a script that wants access would simply do:
ctx = hooking.getContext()
and ctx would be the contents of the special file already decoded into a native Python object for easy consumption. This could easily be extended to any hook which may want to provide some context to implementors.
That would be nice, so scripts written in python wouldn't need to look
for,
and parse the file.
This is an example of a simple hook:
http://gerrit.ovirt.org/#/c/20330/7/tests/functional/networkTests.py (look inside the ValidatesHook decorator)
It'd be quite simplified. We would also need a "setContext()..." to update context with changes.
Yeah seems reasonable. Then hook scripts can become much simpler and easier to read and write:
ctx = hooking.getContext() ctx['foo'] = 'bar' hooking.setContext(ctx)
One more question comes to mind: Are there any pieces of information that we would need to redact from the context (passwords or other sensitive information)?
I think there is no sensitive information as far as I know.
Greetings, Miguel Ángel.
vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
On Wed, Jan 08, 2014 at 03:34:24PM +0100, Miguel Angel wrote:
Hi Assaf, Thank you very much for the summary,
Just a few questions, there are things I don't understand yet:
2014/1/8 Assaf Muller amuller@redhat.com
Mid-thread summary:
First some terminology so we're all on the same page: 'request' - The setupNetworks that just arrived 'running config' - If unified persistence is being used - All networking on the host as VDSM sees it 'expected' - If unified persistence is being used - The running config after the effects of the request, but before the hook is ran
correct
I think we all agree that we have to expose the 'request' to the hook. However, there's two different approaches:
- Allow the hook to rewrite parts of the request (For example, if two
networks were sent, and the hook handled one of them, it could then delete that network from the request, so VDSM will only continue to configure the 2nd network).
In this case ,if the hook deleted the "2nd network", VDSM couldn't handle the network config persistence anymore, right?,
Right. If the before_network_setup hook decides to drop an item from the 'request', it means that neither following hooks, nor Vdsm proper, would see it. I find it as a useful and simple sematics: the hook practically says "I'm taking it from here", and from then on, it is repsonsible for everything. Implementation and persistence alike.
I didn't expect this use case, I only expected tweaks, etc (before or after network setup), for example, setting special hardware capabilities using ethtool or those kind of things.
Neither have I expected this. But it's a powerful tool that's relatively easy to implement.
- Toni's idea with marking devices as hook-configured. So if a network is
over a bridge over a VLAN over a bond over three NICs, hooks could configure only the NICs (For example) and VDSM would configure the rest, whereas the first idea means that the hook would have to configure the entire network (Bridge, VLANs, bonds, and all 3 NICs) and then remove that network from the request. The disadvantage of this method is that it would be harder to implement, and have references to the hooks flag throughout all of VDSM.
You mean that if the hook marked a certain device as "hook handled" then, VDSM would just keep this information along with the network config, etc... and won't do any setup, just config-keeping, right?
That's my understanding, too. And I share the feeling that this is error prone: Vdsm sees the config, but must remember that it should not touch it or act upon it.
Then there's the matter of IF to expose the 'running config' and 'expected'.
My main argument is that it's very easy to expose to the hooks (But then why not expose other 'random' data that is easy for VDSM to calculate?), and that we don't understand all usages for the setupNetworks hooks. I'd rather we expose too much information than not enough and screw over hook writers.
We have something important here, the hooks don't need to be python-written, they could be bash scripts, or any other thing... that means that they wouldn't have access to get "running config", but they could calculate "expected".
So, my vote here goes for "request" + "running config".
The "running config" is accessible to any process, albeit not in its Vdsm representation. All the information is available if you do `ip a` and `virsh net-list`.
I do not imagine why a hook would need the "running config"; If it does end up needing it, it could re-calculate it just as Vdsm does. Passing this as argument to each hook script is a premature optimization imho.
If I end up wrong, it would not be hard to add the "running config" to the Vdsm/hook API. Removing something from an API is a much harder task.
My vote goes to only "request".
Either way, let's get some votes in a timely manner so we could manage to implement this for 3.4.
Thanks Assaf! ;)
----- Original Message -----
From: "Dan Kenigsberg" danken@redhat.com To: "Miguel Angel" miguelangel@ajo.es Cc: dsulliva@redhat.com, arch@ovirt.org, vdsm-devel@fedorahosted.org Sent: Thursday, January 9, 2014 2:35:40 AM Subject: Re: [vdsm] Smarter network_setup hooks
On Wed, Jan 08, 2014 at 03:34:24PM +0100, Miguel Angel wrote:
Hi Assaf, Thank you very much for the summary,
Just a few questions, there are things I don't understand yet:
2014/1/8 Assaf Muller amuller@redhat.com
Mid-thread summary:
First some terminology so we're all on the same page: 'request' - The setupNetworks that just arrived 'running config' - If unified persistence is being used - All networking on the host as VDSM sees it 'expected' - If unified persistence is being used - The running config after the effects of the request, but before the hook is ran
correct
I think we all agree that we have to expose the 'request' to the hook. However, there's two different approaches:
- Allow the hook to rewrite parts of the request (For example, if two
networks were sent, and the hook handled one of them, it could then delete that network from the request, so VDSM will only continue to configure the 2nd network).
In this case ,if the hook deleted the "2nd network", VDSM couldn't handle the network config persistence anymore, right?,
Right. If the before_network_setup hook decides to drop an item from the 'request', it means that neither following hooks, nor Vdsm proper, would see it. I find it as a useful and simple sematics: the hook practically says "I'm taking it from here", and from then on, it is repsonsible for everything. Implementation and persistence alike.
I didn't expect this use case, I only expected tweaks, etc (before or after network setup), for example, setting special hardware capabilities using ethtool or those kind of things.
Neither have I expected this. But it's a powerful tool that's relatively easy to implement.
- Toni's idea with marking devices as hook-configured. So if a network
is over a bridge over a VLAN over a bond over three NICs, hooks could configure only the NICs (For example) and VDSM would configure the rest, whereas the first idea means that the hook would have to configure the entire network (Bridge, VLANs, bonds, and all 3 NICs) and then remove that network from the request. The disadvantage of this method is that it would be harder to implement, and have references to the hooks flag throughout all of VDSM.
You mean that if the hook marked a certain device as "hook handled" then, VDSM would just keep this information along with the network config, etc... and won't do any setup, just config-keeping, right?
That's my understanding, too. And I share the feeling that this is error prone: Vdsm sees the config, but must remember that it should not touch it or act upon it.
Then there's the matter of IF to expose the 'running config' and 'expected'.
My main argument is that it's very easy to expose to the hooks (But then why not expose other 'random' data that is easy for VDSM to calculate?), and that we don't understand all usages for the setupNetworks hooks. I'd rather we expose too much information than not enough and screw over hook writers.
We have something important here, the hooks don't need to be python-written, they could be bash scripts, or any other thing... that means that they wouldn't have access to get "running config", but they could calculate "expected".
So, my vote here goes for "request" + "running config".
The "running config" is accessible to any process, albeit not in its Vdsm representation. All the information is available if you do `ip a` and `virsh net-list`.
I do not imagine why a hook would need the "running config"; If it does end up needing it, it could re-calculate it just as Vdsm does. Passing this as argument to each hook script is a premature optimization imho.
If I end up wrong, it would not be hard to add the "running config" to the Vdsm/hook API. Removing something from an API is a much harder task.
My vote goes to only "request".
Only "request" for me too.
Either way, let's get some votes in a timely manner so we could manage to implement this for 3.4.
Thanks Assaf! ;)
vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Dan, your arguments conviced me, changing my vote to "request" only.
--- irc: ajo / mangelajo Miguel Angel Ajo Pelayo +34 636 52 25 69 skype: ajoajoajo
2014/1/9 Antoni Segura Puimedon asegurap@redhat.com
----- Original Message -----
From: "Dan Kenigsberg" danken@redhat.com To: "Miguel Angel" miguelangel@ajo.es Cc: dsulliva@redhat.com, arch@ovirt.org, vdsm-devel@fedorahosted.org Sent: Thursday, January 9, 2014 2:35:40 AM Subject: Re: [vdsm] Smarter network_setup hooks
On Wed, Jan 08, 2014 at 03:34:24PM +0100, Miguel Angel wrote:
Hi Assaf, Thank you very much for the summary,
Just a few questions, there are things I don't understand yet:
2014/1/8 Assaf Muller amuller@redhat.com
Mid-thread summary:
First some terminology so we're all on the same page: 'request' - The setupNetworks that just arrived 'running config' - If unified persistence is being used - All
networking
on the host as VDSM sees it 'expected' - If unified persistence is being used - The running
config
after the effects of the request, but before the hook is ran
correct
I think we all agree that we have to expose the 'request' to the
hook.
However, there's two different approaches:
- Allow the hook to rewrite parts of the request (For example, if
two
networks were sent, and the hook handled one of them, it could then delete that network from the request, so VDSM will only
continue
to configure the 2nd network).
In this case ,if the hook deleted the "2nd network", VDSM couldn't
handle
the network config persistence anymore, right?,
Right. If the before_network_setup hook decides to drop an item from the 'request', it means that neither following hooks, nor Vdsm proper, would see it. I find it as a useful and simple sematics: the hook practically says "I'm taking it from here", and from then on, it is repsonsible for everything. Implementation and persistence alike.
I didn't expect this use case, I only expected tweaks, etc (before or
after
network setup), for example, setting special hardware capabilities using ethtool or those kind of things.
Neither have I expected this. But it's a powerful tool that's relatively easy to implement.
- Toni's idea with marking devices as hook-configured. So if a
network
is over a bridge over a VLAN over a bond over three NICs, hooks could configure only the NICs (For example) and VDSM would
configure
the rest, whereas the first idea means that the hook would have to configure the entire network (Bridge, VLANs, bonds, and
all 3
NICs) and then remove that network from the request. The disadvantage of this method is that it would be harder to implement, and have references to the hooks flag throughout all of
VDSM.
You mean that if the hook marked a certain device as "hook handled"
then,
VDSM would just keep this information along with the network config, etc... and won't do any setup, just config-keeping, right?
That's my understanding, too. And I share the feeling that this is error prone: Vdsm sees the config, but must remember that it should not touch it or act upon it.
Then there's the matter of IF to expose the 'running config' and 'expected'.
My main argument is that it's very easy to expose to the hooks (But
then
why not expose other 'random' data that is easy for VDSM to
calculate?),
and that we don't understand all usages for the setupNetworks hooks.
I'd
rather we expose too much information than not enough and screw over hook writers.
We have something important here, the hooks don't need to be python-written, they could be bash scripts, or any other thing... that means that they wouldn't have access to get "running config", but
they
could calculate "expected".
So, my vote here goes for "request" + "running config".
The "running config" is accessible to any process, albeit not in its Vdsm representation. All the information is available if you do `ip a` and `virsh net-list`.
I do not imagine why a hook would need the "running config"; If it does end up needing it, it could re-calculate it just as Vdsm does. Passing this as argument to each hook script is a premature optimization imho.
If I end up wrong, it would not be hard to add the "running config" to the Vdsm/hook API. Removing something from an API is a much harder task.
My vote goes to only "request".
Only "request" for me too.
Either way, let's get some votes in a timely manner so we could
manage to
implement this for 3.4.
Thanks Assaf! ;)
vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
On Thu, Jan 09, 2014 at 12:16:04PM +0100, Miguel Angel wrote:
Dan, your arguments conviced me, changing my vote to "request" only.
Ok, so let's start with the "request" only. And let a hook script change it to notify follow-up scripts (and/or Vdsm) that more/less changes are required.
Do you think we can make it before ovirt-3.4 beta (Jan 20th) ? (I hope we do!)
Dan.
So, you mean current but removing the "current" part?
I could do that during Thursday morning, just let me know if I'm miss anything.
Cheers! Miguel Ángel
--- irc: ajo / mangelajo Miguel Angel Ajo Pelayo +34 636 52 25 69 skype: ajoajoajo
2014/1/13 Dan Kenigsberg danken@redhat.com
On Thu, Jan 09, 2014 at 12:16:04PM +0100, Miguel Angel wrote:
Dan, your arguments conviced me, changing my vote to "request" only.
Ok, so let's start with the "request" only. And let a hook script change it to notify follow-up scripts (and/or Vdsm) that more/less changes are required.
Do you think we can make it before ovirt-3.4 beta (Jan 20th) ? (I hope we do!)
Dan.
On Mon, Jan 13, 2014 at 10:48:07PM +0100, Miguel Angel wrote:
So, you mean current but removing the "current" part?
Frankly yes. Adam's "getContext" helper can wait for a followup patch. I have a couple of minor comments on gerrit.
I could do that during Thursday morning, just let me know if I'm miss anything.
Thanks!
vdsm-devel@lists.fedorahosted.org