[tripleo] please avoid creating ansible playbooks that call modules for trivial tasks
Hi, In today TripleO client development we invoke ansible playbooks instead of Mistral workflows. I believe this may only be justified for non-trivial things. Otherwise please just import python modules into the client command actions without introducing additional moving parts in the form of ansible modules (that import the same python modules as well). Ansible brings declarative DSL that sometimes can better and simpler and faster describes things to do and shape inputs data, than if the same would be done in python, in the client. What should be the trivial thing and non-trivial then? Well, I think that example [0] illustrates such a trivial thing. A new playbook [1] would only install a few packages then "returns" back into python and calls a new ansible module [2]. And that new module only wraps [3] the same code being removed from the client: from tripleo_common.image import build ... manager = build.ImageBuildManager(<args>) I see no point of adding a new playbook and a new module for that trivial example. Those 4 packages could (and should) be as well installed from the client caller code without any ansible involved in the middle IMO. [0] https://review.opendev.org/#/c/759954/ [1] https://review.opendev.org/#/c/759953/3/tripleo_ansible/playbooks/cli-overcl... [2] https://review.opendev.org/#/c/759953/3/tripleo_ansible/ansible_plugins/modu... -- Best regards, Bogdan Dobrelya, Irc #bogdando
On Thu, Oct 29, 2020 at 4:59 PM Bogdan Dobrelya <bdobreli@redhat.com> wrote:
Hi,
In today TripleO client development we invoke ansible playbooks instead of Mistral workflows. I believe this may only be justified for non-trivial things. Otherwise please just import python modules into the client command actions without introducing additional moving parts in the form of ansible modules (that import the same python modules as well).
Ansible brings declarative DSL that sometimes can better and simpler and faster describes things to do and shape inputs data, than if the same would be done in python, in the client.
What should be the trivial thing and non-trivial then?
Yes, we did create a number of playbooks/ansible modules for some trivial things starting with https://review.opendev.org/#/c/716286/ (I had a comment about it in that patch and it seems you reviewed the patch too), that could have been achieved by making a simple tripleo-common api calls. IMO, creating playbooks for every cli action and moving all logic from tripleoclient to ansible modules/playboos would have helped us in either getting rid of tripleoclient or make it very thin. We did discuss that in the victoria PTG[1]. Agreed that we would end up with some inefficient stuff (like what you've mentioned) during the transition and later, but the user experience with ansible playbooks compared to current logging in cli actions would have outweighed that. However, over the last cycle I've seen some resistance to go in that direction (i.e moving everything from tripleoclient to playbooks/role/ansible mddules), If we want to keep some stuff in tripleolcient (and some in ansible modules/playbooks), then I would agree we should not make it complex by writing playbooks/modules for things that could be achieved by simple tripleo-common api calls. [1] https://etherpad.opendev.org/p/tripleo-future-of-tripleoclient
Well, I think that example [0] illustrates such a trivial thing. A new playbook [1] would only install a few packages then "returns" back into python and calls a new ansible module [2]. And that new module only wraps [3] the same code being removed from the client:
from tripleo_common.image import build ... manager = build.ImageBuildManager(<args>)
I see no point of adding a new playbook and a new module for that trivial example. Those 4 packages could (and should) be as well installed from the client caller code without any ansible involved in the middle IMO.
[0] https://review.opendev.org/#/c/759954/
[1]
https://review.opendev.org/#/c/759953/3/tripleo_ansible/playbooks/cli-overcl...
[2]
https://review.opendev.org/#/c/759953/3/tripleo_ansible/ansible_plugins/modu...
-- Best regards, Bogdan Dobrelya, Irc #bogdando
-- Regards, Rabi Mishra
On 10/29/20 1:45 PM, Rabi Mishra wrote:
On Thu, Oct 29, 2020 at 4:59 PM Bogdan Dobrelya <bdobreli@redhat.com <mailto:bdobreli@redhat.com>> wrote:
Hi,
In today TripleO client development we invoke ansible playbooks instead of Mistral workflows. I believe this may only be justified for non-trivial things. Otherwise please just import python modules into the client command actions without introducing additional moving parts in the form of ansible modules (that import the same python modules as well).
Ansible brings declarative DSL that sometimes can better and simpler and faster describes things to do and shape inputs data, than if the same would be done in python, in the client.
What should be the trivial thing and non-trivial then?
Yes, we did create a number of playbooks/ansible modules for some trivial things starting with https://review.opendev.org/#/c/716286/ (I had a comment about it in that patch and it seems you reviewed the patch too), that could have been achieved by making a simple tripleo-common api calls.
Yes, that change belongs to a massive conversion effort that contains hundreds of patches in the topic. Also the created ansible module doesn't look trivial to me, since the result [0] of that work looked much simpler than the original parts [1], [2], [3] of workflow and its code. And "trivial" playbook for the given example only wraps the ansible module for the caller. So that approach taken for that example looks justified. [0] https://review.opendev.org/#/c/712604/7/tripleo_ansible/ansible_plugins/modu... [1] https://review.opendev.org/#/c/298682/23/tripleo_common/actions/parameters.p... [2] https://review.opendev.org/#/c/716286/4/tripleoclient/v1/overcloud_deploy.py [3] https://review.opendev.org/#/c/540398/2/workbooks/plan_management.yaml
IMO, creating playbooks for every cli action and moving all logic from tripleoclient to ansible modules/playboos would have helped us in either getting rid of tripleoclient or make it very thin. We did discuss that in the victoria PTG[1]. Agreed that we would end up with some inefficient stuff (like what you've mentioned) during the transition and later, but the user experience with ansible playbooks compared to current logging in cli actions would have outweighed that.
However, over the last cycle I've seen some resistance to go in that direction (i.e moving everything from tripleoclient to playbooks/role/ansible mddules), If we want to keep some stuff in tripleolcient (and some in ansible modules/playbooks), then I would agree we should not make it complex by writing playbooks/modules for things that could be achieved by simple tripleo-common api calls.
[1] https://etherpad.opendev.org/p/tripleo-future-of-tripleoclient
Well, I think that example [0] illustrates such a trivial thing. A new playbook [1] would only install a few packages then "returns" back into python and calls a new ansible module [2]. And that new module only wraps [3] the same code being removed from the client:
from tripleo_common.image import build ... manager = build.ImageBuildManager(<args>)
I see no point of adding a new playbook and a new module for that trivial example. Those 4 packages could (and should) be as well installed from the client caller code without any ansible involved in the middle IMO.
[0] https://review.opendev.org/#/c/759954/
[1] https://review.opendev.org/#/c/759953/3/tripleo_ansible/playbooks/cli-overcl...
[2] https://review.opendev.org/#/c/759953/3/tripleo_ansible/ansible_plugins/modu...
-- Best regards, Bogdan Dobrelya, Irc #bogdando
-- Regards, Rabi Mishra
-- Best regards, Bogdan Dobrelya, Irc #bogdando
On Thu, Oct 29, 2020 at 6:57 PM Bogdan Dobrelya <bdobreli@redhat.com> wrote:
On 10/29/20 1:45 PM, Rabi Mishra wrote:
On Thu, Oct 29, 2020 at 4:59 PM Bogdan Dobrelya <bdobreli@redhat.com <mailto:bdobreli@redhat.com>> wrote:
Hi,
In today TripleO client development we invoke ansible playbooks instead of Mistral workflows. I believe this may only be justified for non-trivial things. Otherwise please just import python modules into the client command actions without introducing additional moving parts in the form of ansible modules (that import the same python modules as well).
Ansible brings declarative DSL that sometimes can better and simpler and faster describes things to do and shape inputs data, than if the same would be done in python, in the client.
What should be the trivial thing and non-trivial then?
Yes, we did create a number of playbooks/ansible modules for some trivial things starting with https://review.opendev.org/#/c/716286/ (I had a comment about it in that patch and it seems you reviewed the patch too), that could have been achieved by making a simple tripleo-common api calls.
Yes, that change belongs to a massive conversion effort that contains hundreds of patches in the topic. Also the created ansible module doesn't look trivial to me, since the result [0] of that work looked much simpler than the original parts [1], [2], [3] of workflow and its code. And "trivial" playbook for the given example only wraps the ansible module for the caller. So that approach taken for that example looks justified.
Though we're not debating that patch here but what we do in the future, not sure why it's not trivial for you. That ansible module[0] wraps single tripleo-common api call "stack_param_utils.update_parameters", which could have been easily called from tripleoclient[2], like the example you had given.
[0]
https://review.opendev.org/#/c/712604/7/tripleo_ansible/ansible_plugins/modu...
[1]
https://review.opendev.org/#/c/298682/23/tripleo_common/actions/parameters.p...
[2]
https://review.opendev.org/#/c/716286/4/tripleoclient/v1/overcloud_deploy.py
[3] https://review.opendev.org/#/c/540398/2/workbooks/plan_management.yaml
IMO, creating playbooks for every cli action and moving all logic from tripleoclient to ansible modules/playboos would have helped us in either getting rid of tripleoclient or make it very thin. We did discuss that in the victoria PTG[1]. Agreed that we would end up with some inefficient stuff (like what you've mentioned) during the transition and later, but the user experience with ansible playbooks compared to current logging in cli actions would have outweighed that.
However, over the last cycle I've seen some resistance to go in that direction (i.e moving everything from tripleoclient to playbooks/role/ansible mddules), If we want to keep some stuff in tripleolcient (and some in ansible modules/playbooks), then I would agree we should not make it complex by writing playbooks/modules for things that could be achieved by simple tripleo-common api calls.
[1] https://etherpad.opendev.org/p/tripleo-future-of-tripleoclient
Well, I think that example [0] illustrates such a trivial thing. A
new
playbook [1] would only install a few packages then "returns" back
into
python and calls a new ansible module [2]. And that new module only wraps [3] the same code being removed from the client:
from tripleo_common.image import build ... manager = build.ImageBuildManager(<args>)
I see no point of adding a new playbook and a new module for that trivial example. Those 4 packages could (and should) be as well installed from the client caller code without any ansible involved in the middle IMO.
[0] https://review.opendev.org/#/c/759954/
[1]
https://review.opendev.org/#/c/759953/3/tripleo_ansible/playbooks/cli-overcl...
[2]
https://review.opendev.org/#/c/759953/3/tripleo_ansible/ansible_plugins/modu...
-- Best regards, Bogdan Dobrelya, Irc #bogdando
-- Regards, Rabi Mishra
-- Best regards, Bogdan Dobrelya, Irc #bogdando
-- Regards, Rabi Mishra
On Thu, Oct 29, 2020 at 5:32 AM Bogdan Dobrelya <bdobreli@redhat.com> wrote:
Hi,
In today TripleO client development we invoke ansible playbooks instead of Mistral workflows. I believe this may only be justified for non-trivial things. Otherwise please just import python modules into the client command actions without introducing additional moving parts in the form of ansible modules (that import the same python modules as well).
Ansible brings declarative DSL that sometimes can better and simpler and faster describes things to do and shape inputs data, than if the same would be done in python, in the client.
What should be the trivial thing and non-trivial then?
Well, I think that example [0] illustrates such a trivial thing. A new playbook [1] would only install a few packages then "returns" back into python and calls a new ansible module [2]. And that new module only wraps [3] the same code being removed from the client:
from tripleo_common.image import build ... manager = build.ImageBuildManager(<args>)
I see no point of adding a new playbook and a new module for that trivial example. Those 4 packages could (and should) be as well installed from the client caller code without any ansible involved in the middle IMO.
While I can agree to a certain extent, there's actually some good reasons to even move trivial bits into ansible. Personally I'm not certain the switch to using ansible under the covers from some cli actions is an improvement (questionable logging, error handling isn't great), there is a case for certain actions. As we discussed at the PTG, the overcloud image building process is one of those things that actually has to be executed on bare metal. If we wanted to continue to look at containerizing the cli, we need to be able to invoke this action from within the container but build on an external host. This is something that is trivial with the switch to an ansible playbook that isn't available when running under the pure python as it exists today. Container builds would be another example action that is required to run on a bare metal host. Additionally the movement of this invocation to an ansible module also allows the action to be moved into something like the undercloud installation as an optional action as part of the deployment itself. It's not exactly without merit in this case. I don't really care one way or another for this action, however I don't think it's as simple as saying "oh it's just a few lines of code so we shouldn't..."
[0] https://review.opendev.org/#/c/759954/
[1] https://review.opendev.org/#/c/759953/3/tripleo_ansible/playbooks/cli-overcl...
[2] https://review.opendev.org/#/c/759953/3/tripleo_ansible/ansible_plugins/modu...
-- Best regards, Bogdan Dobrelya, Irc #bogdando
On Thu, 2020-10-29 at 07:00 -0600, Alex Schultz wrote: On Thu, Oct 29, 2020 at 5:32 AM Bogdan Dobrelya < <mailto:bdobreli@redhat.com> bdobreli@redhat.com
wrote:
In today TripleO client development we invoke ansible playbooks instead of Mistral workflows. I believe this may only be justified for non-trivial things. Otherwise please just import python modules into the client command actions without introducing additional moving parts in the form of ansible modules (that import the same python modules as well). <snip> I see no point of adding a new playbook and a new module for that trivial example. Those 4 packages could (and should) be as well installed from the client caller code without any ansible involved in the middle IMO. While I can agree to a certain extent, there's actually some good reasons to even move trivial bits into ansible. Personally I'm not certain the switch to using ansible under the covers from some cli actions is an improvement (questionable logging, error handling isn't great), there is a case for certain actions. As we discussed at the PTG, the overcloud image building process is one of those things that actually has to be executed on bare metal. If we wanted to continue to look at containerizing the cli, we need to be able to invoke this action from within the container but build on an external host. This is something that is trivial with the switch to an ansible playbook that isn't available when running under the pure python as it exists today. Container builds would be another example action that is required to run on a bare metal host. Additionally the movement of this invocation to an ansible module also allows the action to be moved into something like the undercloud installation as an optional action as part of the deployment itself. It's not exactly without merit in this case. I don't really care one way or another for this action, however I don't think it's as simple as saying "oh it's just a few lines of code so we shouldn't..." What it sounds like is that there's a need for documented guidelines. A lot of changes have been made as part of a learning process and we now know a lot more about what tasks are better suited to be done directly in the client vs via ansible roles vs via ansible modules. If we can document these best practises then we can guide any new changes according to them. It seems to me that we need to consider: 1. Requirements - what needs to get done 2. Constraints - does the action need something special like access to devices or kernel API's 3. Testability - something in python or an ansible module is unit testable, whereas an ansible role is more difficult to properly test 4. Scalability - complex ansible tasks/vars scale far worse that ansible modules 5. Maintainability - many factors are involved here, but sometimes efficiency should be sacrificed for simplicity
IMHO, roles (inside collections) do provide quite good abstraction layer as they allow us to change implementation without changing the API (the variables passed to the role). If the role uses an embedded module, external one or lots of tasks to achieve the same functionality it is an implementation details. At any point in time we can refactor the role internals and create new modules, swap them, .... without messing its consumption. I cannot say the same about playbooks, especially those that call modules directly. If they call roles, we should be fine.
On 2 Nov 2020, at 11:55, Jesse Pretorius <jesse@odyssey4.me> wrote:
On Thu, 2020-10-29 at 07:00 -0600, Alex Schultz wrote:
On Thu, Oct 29, 2020 at 5:32 AM Bogdan Dobrelya < bdobreli@redhat.com
wrote:
In today TripleO client development we invoke ansible playbooks instead of Mistral workflows. I believe this may only be justified for non-trivial things. Otherwise please just import python modules into the client command actions without introducing additional moving parts in the form of ansible modules (that import the same python modules as well).
<snip>
I see no point of adding a new playbook and a new module for that trivial example. Those 4 packages could (and should) be as well installed from the client caller code without any ansible involved in the middle IMO.
While I can agree to a certain extent, there's actually some good reasons to even move trivial bits into ansible. Personally I'm not certain the switch to using ansible under the covers from some cli actions is an improvement (questionable logging, error handling isn't great), there is a case for certain actions. As we discussed at the PTG, the overcloud image building process is one of those things that actually has to be executed on bare metal. If we wanted to continue to look at containerizing the cli, we need to be able to invoke this action from within the container but build on an external host. This is something that is trivial with the switch to an ansible playbook that isn't available when running under the pure python as it exists today. Container builds would be another example action that is required to run on a bare metal host. Additionally the movement of this invocation to an ansible module also allows the action to be moved into something like the undercloud installation as an optional action as part of the deployment itself. It's not exactly without merit in this case.
I don't really care one way or another for this action, however I don't think it's as simple as saying "oh it's just a few lines of code so we shouldn't..."
What it sounds like is that there's a need for documented guidelines. A lot of changes have been made as part of a learning process and we now know a lot more about what tasks are better suited to be done directly in the client vs via ansible roles vs via ansible modules. If we can document these best practises then we can guide any new changes according to them.
It seems to me that we need to consider:
1. Requirements - what needs to get done 2. Constraints - does the action need something special like access to devices or kernel API's 3. Testability - something in python or an ansible module is unit testable, whereas an ansible role is more difficult to properly test 4. Scalability - complex ansible tasks/vars scale far worse that ansible modules 5. Maintainability - many factors are involved here, but sometimes efficiency should be sacrificed for simplicity
On Mon, 2020-11-02 at 16:40 +0000, Sorin Sbarnea wrote: IMHO, roles (inside collections) do provide quite good abstraction layer as they allow us to change implementation without changing the API (the variables passed to the role). If the role uses an embedded module, external one or lots of tasks to achieve the same functionality it is an implementation details. At any point in time we can refactor the role internals and create new modules, swap them, .... without messing its consumption. I cannot say the same about playbooks, especially those that call modules directly. If they call roles, we should be fine. Good point. That reminds me - to really make roles work more like an API, we should also probably implement tests which determine whether the API is changing in an acceptable way. We'd have to agree what 'acceptable' looks like - at the very least, I would imagine that we should be enforcing the same inputs & results with changes being additive unless there's sufficient deprecation.
I would give zuul-jobs roles a good example of big repository with well defined policies about how to write roles, including requirements for variables names and ways to document arguments. There is already work done towards adding support in ansible for declaring in/out variables for roles. They will likely look similar to how module arguments are now documented and reside inside "meta" folder. Still, until then use of README inside each role looks like a decent solution.
On 2 Nov 2020, at 18:24, Jesse Pretorius <jesse@odyssey4.me> wrote:
t reminds me - to really make roles work more like an API, we should also probably implement tests which determine whether the API is changing in an acceptable way. We'd have to agree what 'acceptable' looks like - at the very least, I would imagine that we should be enforcing the
On 2020-11-02 18:29:18 +0000 (+0000), Sorin Sbarnea wrote: [...]
I would give zuul-jobs roles a good example of big repository with well defined policies about how to write roles, including requirements for variables names and ways to document arguments. [...] use of README inside each role looks like a decent solution. [...]
Also, with the help of a Sphinx extension, it allows us to generate fancy role documentation like this: https://zuul-ci.org/docs/zuul-jobs/container-roles.html I'd imagine once standardized metadata comes along, we'll support building documentation from that too/instead. -- Jeremy Stanley
participants (6)
-
Alex Schultz
-
Bogdan Dobrelya
-
Jeremy Stanley
-
Jesse Pretorius
-
Rabi Mishra
-
Sorin Sbarnea