feat: Add support for excluding specific roles in the graph view #219
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
As in #218 I split the additional test logic into a separate test function |
|
Can you take a look at the merge conflicts? I merged your other PR! |
haidaraM
reviewed
Dec 11, 2024
tests/test_cli.py
Outdated
Comment on lines
47
to
54
| test_name = request.node.name | ||
| if test_name == "test_cli_exclude_roles[exclude_roles_with_file]": | ||
| content = "fake_role\ndisplay_some_facts" | ||
| exclude_role_file = tmp_path / exclude_roles_option[1] | ||
| exclude_role_file.write_text(content) | ||
| exclude_roles_option[1] = str(exclude_role_file) | ||
|
|
||
| if test_name == "test_cli_exclude_roles[exclude_roles_with_dir]": |
There was a problem hiding this comment.
Better to split this test instead of branching based on the test name. The test should not explicitly depend on its own name as this makes things brittle.
There was a problem hiding this comment.
Yes, that makes sense. I will fix this.
ansibleplaybookgrapher/cli.py
Outdated
Comment on lines
366
to
367
| if path.is_dir(): | ||
| raise IsADirectoryError(f"{path} is a directory") |
There was a problem hiding this comment.
Multiple things:
- We should provide a better error message here.
- More importantly, the roles can be stored in different places and I can have a role named after a folder in the current directory. This will fails in this case while running the playbook would work.
There was a problem hiding this comment.
I see. I fix this bug by allowing directory paths as arguments for the option. If the path exists it will assume that the basename of the given path is the role, which should be excluded. Idk if that makes sense. Let me know your opinion :)
As far as I remember you would need to tag every task which includes the role in order to exclude all role nodes in the graph, which can be quiet hard to keep track of. The following svg is the result of the the command
|
|
Ahh, I think this is a bug in the grapher because Ansible itself excludes the whole role. No need to tag individual tasks. Example with this playbook: ---
- hosts: localhost
roles:
- role: fake_role
tags:
- exclude
- role: display_some_facts
I would rather fix the existing behavior than adding another way to exclude the roles. |
|
Hmm by double checking your example, it works as expected: the role with the Here are the two examples: ---
title: "Ansible Playbook Grapher: WITHOUT '--skip-tags exclude'"
---
%%{ init: { "flowchart": { "curve": "bumpX" } } }%%
flowchart LR
%% Start of the playbook 'tmp.yaml'
playbook_ffc385f8@{ shape: rounded, label: "tmp.yaml" }
%% Start of the play 'Play: all (0)'
play_efbfd9ce@{ shape: rect, label: "Play: all (0)" }
style play_efbfd9ce stroke:#bc103e,fill:#bc103e,color:#ffffff
playbook_ffc385f8 --> |"1"| play_efbfd9ce
linkStyle 0 stroke:#bc103e,color:#bc103e
%% Start of the role 'fake_role'
play_efbfd9ce --> |"1"| role_3f1bb482
linkStyle 1 stroke:#bc103e,color:#bc103e
role_3f1bb482@{ shape: stadium, label: "[role] fake_role" }
style role_3f1bb482 fill:#bc103e,color:#ffffff,stroke:#bc103e
task_60f0442b@{ shape: rect, label: "fake_role : Debug 1" }
style task_60f0442b stroke:#bc103e,fill:#ffffff
role_3f1bb482 --> |"1 [when: ansible_distribution == 'Debian']"| task_60f0442b
linkStyle 2 stroke:#bc103e,color:#bc103e
task_4fa8ba4a@{ shape: rect, label: "fake_role : Debug 2" }
style task_4fa8ba4a stroke:#bc103e,fill:#ffffff
role_3f1bb482 --> |"2 [when: ansible_distribution == 'Debian']"| task_4fa8ba4a
linkStyle 3 stroke:#bc103e,color:#bc103e
task_fac89b68@{ shape: rect, label: "fake_role : Debug 3 with double quote "here" in the name" }
style task_fac89b68 stroke:#bc103e,fill:#ffffff
role_3f1bb482 --> |"3 [when: ansible_distribution == 'Debian']"| task_fac89b68
linkStyle 4 stroke:#bc103e,color:#bc103e
%% End of the role 'fake_role'
%% Start of the role 'display_some_facts'
play_efbfd9ce --> |"2"| role_23c162f9
linkStyle 5 stroke:#bc103e,color:#bc103e
role_23c162f9@{ shape: stadium, label: "[role] display_some_facts" }
style role_23c162f9 fill:#bc103e,color:#ffffff,stroke:#bc103e
task_cae7c317@{ shape: rect, label: "display_some_facts : ansible_architecture" }
style task_cae7c317 stroke:#bc103e,fill:#ffffff
role_23c162f9 --> |"1"| task_cae7c317
linkStyle 6 stroke:#bc103e,color:#bc103e
task_bbc0fdfe@{ shape: rect, label: "display_some_facts : ansible_date_time" }
style task_bbc0fdfe stroke:#bc103e,fill:#ffffff
role_23c162f9 --> |"2"| task_bbc0fdfe
linkStyle 7 stroke:#bc103e,color:#bc103e
task_57b0e1e9@{ shape: rect, label: "display_some_facts : Specific included task for Debian" }
style task_57b0e1e9 stroke:#bc103e,fill:#ffffff
role_23c162f9 --> |"3"| task_57b0e1e9
linkStyle 8 stroke:#bc103e,color:#bc103e
%% End of the role 'display_some_facts'
%% Start of the block ''
block_2c2f23b7@{ shape: rect, label: "[block]" }
style block_2c2f23b7 fill:#bc103e,color:#ffffff,stroke:#bc103e
play_efbfd9ce --> |"3"| block_2c2f23b7
linkStyle 9 stroke:#bc103e,color:#bc103e
subgraph subgraph_block_2c2f23b7[" "]
%% Start of the role 'fake_role'
block_2c2f23b7 --> |"1"| role_860344d8
linkStyle 10 stroke:#bc103e,color:#bc103e
role_860344d8@{ shape: stadium, label: "[role] fake_role" }
style role_860344d8 fill:#bc103e,color:#ffffff,stroke:#bc103e
task_188469f6@{ shape: rect, label: "fake_role : Debug 1" }
style task_188469f6 stroke:#bc103e,fill:#ffffff
role_860344d8 --> |"1"| task_188469f6
linkStyle 11 stroke:#bc103e,color:#bc103e
task_2730be21@{ shape: rect, label: "fake_role : Debug 2" }
style task_2730be21 stroke:#bc103e,fill:#ffffff
role_860344d8 --> |"2"| task_2730be21
linkStyle 12 stroke:#bc103e,color:#bc103e
task_67b6cb64@{ shape: rect, label: "fake_role : Debug 3 with double quote "here" in the name" }
style task_67b6cb64 stroke:#bc103e,fill:#ffffff
role_860344d8 --> |"3"| task_67b6cb64
linkStyle 13 stroke:#bc103e,color:#bc103e
%% End of the role 'fake_role'
end
%% End of the block ''
%% Start of the role 'nested_include_role'
play_efbfd9ce --> |"4"| role_d4590057
linkStyle 14 stroke:#bc103e,color:#bc103e
role_d4590057@{ shape: stadium, label: "[role] nested_include_role" }
style role_d4590057 fill:#bc103e,color:#ffffff,stroke:#bc103e
post_task_360d3719@{ shape: rect, label: "nested_include_role : Ensure postgresql is at the latest version" }
style post_task_360d3719 stroke:#bc103e,fill:#ffffff
role_d4590057 --> |"1"| post_task_360d3719
linkStyle 15 stroke:#bc103e,color:#bc103e
post_task_b59ce20b@{ shape: rect, label: "nested_include_role : Ensure that postgresql is started" }
style post_task_b59ce20b stroke:#bc103e,fill:#ffffff
role_d4590057 --> |"2"| post_task_b59ce20b
linkStyle 16 stroke:#bc103e,color:#bc103e
%% Start of the role 'display_some_facts'
role_d4590057 --> |"3 [when: x is not defined]"| role_0655223e
linkStyle 17 stroke:#bc103e,color:#bc103e
role_0655223e@{ shape: stadium, label: "[role] display_some_facts" }
style role_0655223e fill:#bc103e,color:#ffffff,stroke:#bc103e
post_task_40fe98e9@{ shape: rect, label: "display_some_facts : ansible_architecture" }
style post_task_40fe98e9 stroke:#bc103e,fill:#ffffff
role_0655223e --> |"1"| post_task_40fe98e9
linkStyle 18 stroke:#bc103e,color:#bc103e
post_task_47995892@{ shape: rect, label: "display_some_facts : ansible_date_time" }
style post_task_47995892 stroke:#bc103e,fill:#ffffff
role_0655223e --> |"2"| post_task_47995892
linkStyle 19 stroke:#bc103e,color:#bc103e
post_task_9eb53afc@{ shape: rect, label: "display_some_facts : Specific included task for Debian" }
style post_task_9eb53afc stroke:#bc103e,fill:#ffffff
role_0655223e --> |"3"| post_task_9eb53afc
linkStyle 20 stroke:#bc103e,color:#bc103e
%% End of the role 'display_some_facts'
%% Start of the role 'fake_role'
role_d4590057 --> |"4"| role_d358c510
linkStyle 21 stroke:#bc103e,color:#bc103e
role_d358c510@{ shape: stadium, label: "[role] fake_role" }
style role_d358c510 fill:#bc103e,color:#ffffff,stroke:#bc103e
post_task_7022ddb2@{ shape: rect, label: "fake_role : Debug 1" }
style post_task_7022ddb2 stroke:#bc103e,fill:#ffffff
role_d358c510 --> |"1"| post_task_7022ddb2
linkStyle 22 stroke:#bc103e,color:#bc103e
post_task_abff6ec0@{ shape: rect, label: "fake_role : Debug 2" }
style post_task_abff6ec0 stroke:#bc103e,fill:#ffffff
role_d358c510 --> |"2"| post_task_abff6ec0
linkStyle 23 stroke:#bc103e,color:#bc103e
post_task_5ffac23e@{ shape: rect, label: "fake_role : Debug 3 with double quote "here" in the name" }
style post_task_5ffac23e stroke:#bc103e,fill:#ffffff
role_d358c510 --> |"3"| post_task_5ffac23e
linkStyle 24 stroke:#bc103e,color:#bc103e
%% End of the role 'fake_role'
%% End of the role 'nested_include_role'
%% End of the play 'Play: all (0)'
%% End of the playbook 'tmp.yaml'
---
title: "Ansible Playbook Grapher: WITH '--skip-tags exclude'"
---
%%{ init: { "flowchart": { "curve": "bumpX" } } }%%
flowchart LR
%% Start of the playbook 'tmp.yaml'
playbook_12738888@{ shape: rounded, label: "tmp.yaml" }
%% Start of the play 'Play: all (0)'
play_93dcdb76@{ shape: rect, label: "Play: all (0)" }
style play_93dcdb76 stroke:#bc1019,fill:#bc1019,color:#ffffff
playbook_12738888 --> |"1"| play_93dcdb76
linkStyle 0 stroke:#bc1019,color:#bc1019
%% Start of the role 'display_some_facts'
play_93dcdb76 --> |"1"| role_23e99ac0
linkStyle 1 stroke:#bc1019,color:#bc1019
role_23e99ac0@{ shape: stadium, label: "[role] display_some_facts" }
style role_23e99ac0 fill:#bc1019,color:#ffffff,stroke:#bc1019
task_4ce5c841@{ shape: rect, label: "display_some_facts : ansible_architecture" }
style task_4ce5c841 stroke:#bc1019,fill:#ffffff
role_23e99ac0 --> |"1"| task_4ce5c841
linkStyle 2 stroke:#bc1019,color:#bc1019
task_c47ed260@{ shape: rect, label: "display_some_facts : ansible_date_time" }
style task_c47ed260 stroke:#bc1019,fill:#ffffff
role_23e99ac0 --> |"2"| task_c47ed260
linkStyle 3 stroke:#bc1019,color:#bc1019
task_bb05270b@{ shape: rect, label: "display_some_facts : Specific included task for Debian" }
style task_bb05270b stroke:#bc1019,fill:#ffffff
role_23e99ac0 --> |"3"| task_bb05270b
linkStyle 4 stroke:#bc1019,color:#bc1019
%% End of the role 'display_some_facts'
%% Start of the block ''
block_34f154c1@{ shape: rect, label: "[block]" }
style block_34f154c1 fill:#bc1019,color:#ffffff,stroke:#bc1019
play_93dcdb76 --> |"2"| block_34f154c1
linkStyle 5 stroke:#bc1019,color:#bc1019
subgraph subgraph_block_34f154c1[" "]
%% Start of the role 'fake_role'
block_34f154c1 --> |"1"| role_c01777cc
linkStyle 6 stroke:#bc1019,color:#bc1019
role_c01777cc@{ shape: stadium, label: "[role] fake_role" }
style role_c01777cc fill:#bc1019,color:#ffffff,stroke:#bc1019
task_b934a61e@{ shape: rect, label: "fake_role : Debug 1" }
style task_b934a61e stroke:#bc1019,fill:#ffffff
role_c01777cc --> |"1"| task_b934a61e
linkStyle 7 stroke:#bc1019,color:#bc1019
task_740b838f@{ shape: rect, label: "fake_role : Debug 2" }
style task_740b838f stroke:#bc1019,fill:#ffffff
role_c01777cc --> |"2"| task_740b838f
linkStyle 8 stroke:#bc1019,color:#bc1019
task_3ab4344e@{ shape: rect, label: "fake_role : Debug 3 with double quote "here" in the name" }
style task_3ab4344e stroke:#bc1019,fill:#ffffff
role_c01777cc --> |"3"| task_3ab4344e
linkStyle 9 stroke:#bc1019,color:#bc1019
%% End of the role 'fake_role'
end
%% End of the block ''
%% Start of the role 'nested_include_role'
play_93dcdb76 --> |"3"| role_6a412dcc
linkStyle 10 stroke:#bc1019,color:#bc1019
role_6a412dcc@{ shape: stadium, label: "[role] nested_include_role" }
style role_6a412dcc fill:#bc1019,color:#ffffff,stroke:#bc1019
post_task_96e05c9b@{ shape: rect, label: "nested_include_role : Ensure postgresql is at the latest version" }
style post_task_96e05c9b stroke:#bc1019,fill:#ffffff
role_6a412dcc --> |"1"| post_task_96e05c9b
linkStyle 11 stroke:#bc1019,color:#bc1019
post_task_e7f39029@{ shape: rect, label: "nested_include_role : Ensure that postgresql is started" }
style post_task_e7f39029 stroke:#bc1019,fill:#ffffff
role_6a412dcc --> |"2"| post_task_e7f39029
linkStyle 12 stroke:#bc1019,color:#bc1019
%% Start of the role 'display_some_facts'
role_6a412dcc --> |"3 [when: x is not defined]"| role_e2cc6858
linkStyle 13 stroke:#bc1019,color:#bc1019
role_e2cc6858@{ shape: stadium, label: "[role] display_some_facts" }
style role_e2cc6858 fill:#bc1019,color:#ffffff,stroke:#bc1019
post_task_4ab26f23@{ shape: rect, label: "display_some_facts : ansible_architecture" }
style post_task_4ab26f23 stroke:#bc1019,fill:#ffffff
role_e2cc6858 --> |"1"| post_task_4ab26f23
linkStyle 14 stroke:#bc1019,color:#bc1019
post_task_40001c4c@{ shape: rect, label: "display_some_facts : ansible_date_time" }
style post_task_40001c4c stroke:#bc1019,fill:#ffffff
role_e2cc6858 --> |"2"| post_task_40001c4c
linkStyle 15 stroke:#bc1019,color:#bc1019
post_task_3a7e0036@{ shape: rect, label: "display_some_facts : Specific included task for Debian" }
style post_task_3a7e0036 stroke:#bc1019,fill:#ffffff
role_e2cc6858 --> |"3"| post_task_3a7e0036
linkStyle 16 stroke:#bc1019,color:#bc1019
%% End of the role 'display_some_facts'
%% Start of the role 'fake_role'
role_6a412dcc --> |"4"| role_894810f3
linkStyle 17 stroke:#bc1019,color:#bc1019
role_894810f3@{ shape: stadium, label: "[role] fake_role" }
style role_894810f3 fill:#bc1019,color:#ffffff,stroke:#bc1019
post_task_3e48974c@{ shape: rect, label: "fake_role : Debug 1" }
style post_task_3e48974c stroke:#bc1019,fill:#ffffff
role_894810f3 --> |"1"| post_task_3e48974c
linkStyle 18 stroke:#bc1019,color:#bc1019
post_task_e3e4fe03@{ shape: rect, label: "fake_role : Debug 2" }
style post_task_e3e4fe03 stroke:#bc1019,fill:#ffffff
role_894810f3 --> |"2"| post_task_e3e4fe03
linkStyle 19 stroke:#bc1019,color:#bc1019
post_task_0f673cf7@{ shape: rect, label: "fake_role : Debug 3 with double quote "here" in the name" }
style post_task_0f673cf7 stroke:#bc1019,fill:#ffffff
role_894810f3 --> |"3"| post_task_0f673cf7
linkStyle 20 stroke:#bc1019,color:#bc1019
%% End of the role 'fake_role'
%% End of the role 'nested_include_role'
%% End of the play 'Play: all (0)'
%% End of the playbook 'tmp.yaml'
If you want to exclude the same roles used in different contexts, |
Yes, I wanted to implement a feature, with which you can easily exclude a role from every context without adding a lot of tags. For example, if someone is working on a large codebase with a role that is being referenced by a lot of other roles or tasks, it would be cumbersome to add a lot of tags instead of just completely excluding the role from the graph view with an option. |
Makes sense! |
haidaraM
reviewed
Dec 16, 2024
|
And thanks for your contributions :-) |
Thank you for the review! I fixed the inconsistencies and changed the assertion. Let me know when there are other improvements 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Premise
When running the ansible-playbook-grapher on a project folder, where a certain role is referenced by a lot of other roles or tasks then the resulting graph might be confusing especially when grouping the roles by their names (a lot of arrows crossing each other). So far there was no option to exclude specific roles from the graph view, so I implemented a feature for this.
Changes
--exclude-rolesin README.mdcli.pyand adjustgrapher.pyandparser.pyaccordinglygraph_model.pyto get all roles of a composite role (needed for the additional tests)Screenshots
without
--exclude-roleswith
--exclude-roles fake_role