search

dmulyalin / salt-nornir

SALTSTACK Nornir based proxy minion, execution, state and runner modules
MIT License
27 stars 3 forks source link

[FEATURE REQUEST] tighter structure with hcache data #36

Open bodleytunes opened 1 year ago

bodleytunes commented 1 year ago

When saving data to the Hcache from something like TTP parsing, the nesting includes "run_ttp" as a key.

Is there a way to have the dict it creates in the hcache without the run_ttp key to make it a little easier to navigate / more readable?

           tacacs:
                    ----------
                    run_ttp:
                        |_
                          ----------
                          group:
                              |_
                                ----------
                                name:
                                    TACACS
dmulyalin commented 1 year ago

Its a good idea but not feasible to implement without significant code and tests rework, which is not justifiable for such a small convenience improvement.

run_ttp is the name of the task that set by dataprocessor after ttp parsing, task name is mandatory attribute, to remove it from task results will have to introduce non trivial logic in several places in the codebase.

bodleytunes commented 1 year ago

My only slight issue with it is the structure is not always the same. If I run it from the command line like salt nrp1 nr.cli xyz then it gives a slightly different structure than when I run the equivalent command from the workflow steps, which can sometimes cause a bit of head scratching when you try to reference the variable where you think it should be nested, say from a template, and it is in a slightly different position, so when you test from the cli its giving different context than when running the state/workflow. Can't recall if its to do with the result.0 part moving around?

dmulyalin commented 1 year ago

ic, could you provide samples of the discrepancies and what was done to produce the results, maybe sample workflow or cli command, would be glad to have a look and see if those can be made to look alike.

bodleytunes commented 1 year ago

OK looking at it there isn't much difference...

From Workflow step:

          tacacs_ips_workflow:
                    ----------
                    run_ttp:
                        ----------
                        changed:
                            False
                        connection_retry:
                            0
                        diff:
                        exception:
                            None
                        failed:
                            False
                        result:
                            |_
                              ----------
                              tacacs:
                                  - 10.0.0.1
                                  - 10.0.0.2
                                  - 10.0.0.3
                              tacacs_group:
                                  TACACS-SERVERS
                              vrf:
                                  MANAGEMENT
                        task_retry:
                            0

From running in the cli

                tacacs_ips_cli:
                    ----------
                    run_ttp:
                        |_
                          ----------
                              tacacs:
                                  - 10.0.0.1
                                  - 10.0.0.2
                                  - 10.0.0.3
                          tacacs_group:
                              TACACS-SERVERS
                          vrf:
                              MANAGEMENT

workflow has run_ttp.result.0.tacacs

cli has a ever so slightly simpler structure of run_ttp.0.tacacs

I assume this is probably because its needed for reporting with the workflows.

bodleytunes commented 1 year ago

Looking at it its probably nothing worth really bothering about as you said seems like it could be a design change for no real world gains. As long as I remember the differences then should be fine, its just it tripped me up a couple of times, but I've learned now the differences.

dmulyalin commented 1 year ago

workflow uses add_details=True for all steps by default, if you run your nr.cli run_ttp task with add_details=True you should get same exact result as workflow does.

add_details docs

bodleytunes commented 1 year ago

makes sense thanks!

On Tue, 22 Aug 2023 at 13:13, Denis @.***> wrote:

workflow uses add_details=True for all steps by default, if you run your nr.cli run_ttp task with add_details=True you should get same exact result as workflow does.

add_details docs https://salt-nornir.readthedocs.io/en/latest/Nornir%20Execution%20Module.html#add-details

— Reply to this email directly, view it on GitHub https://github.com/dmulyalin/salt-nornir/issues/36#issuecomment-1688067487, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXFZ3YRHIJFCVOSMH6C2TXWSO57ANCNFSM6AAAAAA2AGDTIM . You are receiving this because you authored the thread.Message ID: @.***>

bodleytunes commented 1 year ago

And visa versa works? add_details=False on workflow results in different structure?

On Tue, 22 Aug 2023 at 13:34, Jon Clayton @.***> wrote:

makes sense thanks!

On Tue, 22 Aug 2023 at 13:13, Denis @.***> wrote:

workflow uses add_details=True for all steps by default, if you run your nr.cli run_ttp task with add_details=True you should get same exact result as workflow does.

add_details docs https://salt-nornir.readthedocs.io/en/latest/Nornir%20Execution%20Module.html#add-details

— Reply to this email directly, view it on GitHub https://github.com/dmulyalin/salt-nornir/issues/36#issuecomment-1688067487, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXFZ3YRHIJFCVOSMH6C2TXWSO57ANCNFSM6AAAAAA2AGDTIM . You are receiving this because you authored the thread.Message ID: @.***>

bodleytunes commented 1 year ago

Oh sorry I see its False by default according to docs

On Tue, 22 Aug 2023 at 13:35, Jon Clayton @.***> wrote:

And visa versa works? add_details=False on workflow results in different structure?

On Tue, 22 Aug 2023 at 13:34, Jon Clayton @.***> wrote:

makes sense thanks!

On Tue, 22 Aug 2023 at 13:13, Denis @.***> wrote:

workflow uses add_details=True for all steps by default, if you run your nr.cli run_ttp task with add_details=True you should get same exact result as workflow does.

add_details docs https://salt-nornir.readthedocs.io/en/latest/Nornir%20Execution%20Module.html#add-details

— Reply to this email directly, view it on GitHub https://github.com/dmulyalin/salt-nornir/issues/36#issuecomment-1688067487, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXFZ3YRHIJFCVOSMH6C2TXWSO57ANCNFSM6AAAAAA2AGDTIM . You are receiving this because you authored the thread.Message ID: @.***>

dmulyalin commented 1 year ago

worlflow has to use add_details=True to be able to reason whether task was successful or failed, so its not controllable from workflow steps arguments.

bodleytunes commented 1 year ago

OK thought that might be the case :)

On Tue, 22 Aug 2023 at 13:51, Denis @.***> wrote:

worlflow has to use add_details=True to be able to reason whether task was successful or failed, so its not controllable from workflow steps arguments.

— Reply to this email directly, view it on GitHub https://github.com/dmulyalin/salt-nornir/issues/36#issuecomment-1688125884, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXFZ2PIIMBOZG7SRWYY6LXWSTNRANCNFSM6AAAAAA2AGDTIM . You are receiving this because you authored the thread.Message ID: @.***>

dmulyalin commented 1 year ago

in other words, workflow sets add_details=True for all nr.xyz step unconditionally - https://github.com/dmulyalin/salt-nornir/blob/1fc3b1660988b3da3621129192888f7d3398d09d/salt_nornir/states/nornir_proxy_state_module.py#L308