fix: Drop support for ansible 2.8

.github/workflows/testing.yaml

@@ -15,7 +15,7 @@ jobs:
     strategy:
       matrix:
         python-version: [ 3.7, 3.8 ]
-        ansible-version: [ 2.8.6, 2.9.0, 2.9.14 ]
+        ansible-version: [ 2.9.0, 2.9.14 ]
 
     steps:
       - name: Checkout code

Makefile

@@ -23,10 +23,7 @@ test_install: build
 	@./test_install.sh $(VIRTUALENV_DIR) $(ANSIBLE_VERSION)
 
 test:
-	# Ansible 2.8 CLI sets some global variables causing the tests to fail if the cli tests are run before
-	# the grapher tests. It works in Ansible 2.9. So here we explicitly set the tests order.
-	# TODO: Remove pytest arguments when we drop support for Ansible 2.8
-	cd tests && pytest test_grapher.py test_cli.py test_postprocessor.py
+	cd tests && pytest
 
 clean:
 	@echo "Cleaning..."

README.md

@@ -11,8 +11,8 @@ Inspired by [Ansible Inventory Grapher](https://github.com/willthames/ansible-in
 
 ## Prerequisites
  - Python 3
- - **Ansible** >= 2.8: If you still use an older version of Ansible, create a virtual environment and install ansible-playbook-grapher. 
- **pip will install a version of Ansible >= 2.8 if not already installed.** I try to respect [Red Hat Ansible Engine Life Cycle](https://access.redhat.com/support/policy/updates/ansible-engine) for the supported Ansible version. 
+ - **Ansible** >= 2.9: If you still use an older version of Ansible, create a virtual environment and install ansible-playbook-grapher. 
+ **pip will install a version of Ansible >= 2.9 if not already installed.** I try to respect [Red Hat Ansible Engine Life Cycle](https://access.redhat.com/support/policy/updates/ansible-engine) for the supported Ansible version. 
  - **Graphviz**: The tool used to generate the graph in SVG. 
    ```shell script
    $ sudo apt-get install graphviz # or yum install or brew install

ansibleplaybookgrapher/cli.py

@@ -4,7 +4,7 @@
 from abc import ABC
 
 from ansible.cli import CLI
-from ansible.errors import AnsibleOptionsError
+from ansible.cli.arguments import option_helpers
 from ansible.release import __version__ as ansible_version
 from ansible.utils.display import Display
 from packaging import version
@@ -13,25 +13,23 @@
 from ansibleplaybookgrapher.grapher import PlaybookGrapher
 from ansibleplaybookgrapher.postprocessor import PostProcessor
 
-# We need to know if we are using ansible 2.8 because the CLI has been refactored in
+# At some time, we needed to know if we are using ansible 2.8 because the CLI has been refactored in this PR:
 # https://github.com/ansible/ansible/pull/50069
 IS_ANSIBLE_2_9_X = version.parse(ansible_version) >= version.parse("2.9")
 
 
 def get_cli_class():
     """
-    Utility function to return the class to use as CLI depending on Ansible version
+    Utility function to return the class to use as CLI depending on Ansible version.
     :return:
     """
-    if IS_ANSIBLE_2_9_X:
-        return PlaybookGrapherCLI29
-    else:
-        return PlaybookGrapherCLI28
+
+    return PlaybookGrapherCLI
 
 
 class GrapherCLI(CLI, ABC):
     """
-    An abstract class to provide to be implemented by the different Grapher CLIs.
+    An abstract class to be implemented by the different Grapher CLIs.
     """
 
     def run(self):
@@ -60,77 +58,16 @@ def run(self):
         return svg_path
 
 
-class PlaybookGrapherCLI28(GrapherCLI):
+class PlaybookGrapherCLI(GrapherCLI):
     """
-    The dedicated playbook CLI for Ansible 2.8.
+    The dedicated playbook CLI for Ansible 2.9 and above (for the moment)
     """
 
     def __init__(self, args, callback=None):
-        super(PlaybookGrapherCLI28, self).__init__(args=args, callback=callback)
-        # we keep the old options as instance attribute for backward compatibility for the grapher
-        # Ansible 2.8 has removed it and use a global context instead
-        self.options = None
-
-    def _add_my_options(self):
-        """
-        Method to add some options specific to the grapher
-        :return:
-        """
-        self.parser.add_option('-i', '--inventory', dest='inventory', action="append",
-                               help="specify inventory host path or comma separated host list.")
-
-        self.parser.add_option("--include-role-tasks", dest="include_role_tasks", action='store_true', default=False,
-                               help="Include the tasks of the role in the graph.")
-
-        self.parser.add_option("-s", "--save-dot-file", dest="save_dot_file", action='store_true', default=False,
-                               help="Save the dot file used to generate the graph.")
-
-        self.parser.add_option("-o", "--ouput-file-name", dest='output_filename',
-                               help="Output filename without the '.svg' extension. Default: <playbook>.svg")
-
-        self.parser.version = "%s %s (with ansible %s)" % (__prog__, __version__, ansible_version)
-
-    def init_parser(self, usage="", desc=None, epilog=None):
-        super(PlaybookGrapherCLI28, self).init_parser(usage="%s [options] playbook.yml" % __prog__,
-                                                      desc="Make graphs from your Ansible Playbooks.", epilog=epilog)
-        self._add_my_options()
-
-        from ansible.cli.arguments import optparse_helpers as opt_help
-
-        opt_help.add_subset_options(self.parser)
-        opt_help.add_vault_options(self.parser)
-        opt_help.add_runtask_options(self.parser)
-
-    def post_process_args(self, options, args):
-        options, args = super(PlaybookGrapherCLI28, self).post_process_args(options, args)
-
-        if len(args) == 0:
-            raise AnsibleOptionsError("You must specify a playbook file to graph.")
-
-        if len(args) > 1:
-            raise AnsibleOptionsError("You must specify only one playbook file to graph.")
-
-        # init the options
-        self.options = options
-        self.options.playbook_filename = args[0]
-
-        if self.options.output_filename is None:
-            # use the playbook name (without the extension) as output filename
-            self.options.output_filename = os.path.splitext(ntpath.basename(self.options.playbook_filename))[0]
-
-        return options, args
-
-
-class PlaybookGrapherCLI29(GrapherCLI):
-    """
-    The dedicated playbook CLI for Ansible 2.9 and above.
-    Note: Use this class as the main CLI when we drop support for ansible < 2.9
-    """
-
-    def __init__(self, args, callback=None):
-        super(PlaybookGrapherCLI29, self).__init__(args=args, callback=callback)
-        # we keep the old options as instance attribute for backward compatibility for the grapher
-        # Ansible 2.8 has removed it and use a global context instead
+        super(PlaybookGrapherCLI, self).__init__(args=args, callback=callback)
+        # We keep the old options as instance attribute for backward compatibility for the grapher CLI.
+        # From Ansible 2.8, they remove this instance attribute 'options' and use a global context instead.
+        # But this may change in the future: https://github.com/ansible/ansible/blob/bcb64054edaa7cf636bd38b8ab0259f6fb93f3f9/lib/ansible/context.py#L8
         self.options = None
 
     def _add_my_options(self):
@@ -158,20 +95,19 @@ def _add_my_options(self):
 
         self.parser.add_argument('playbook_filename', help='Playbook to graph', metavar='playbook')
 
+        # Use ansible helper to add some default options also
+        option_helpers.add_subset_options(self.parser)
+        option_helpers.add_vault_options(self.parser)
+        option_helpers.add_runtask_options(self.parser)
+
     def init_parser(self, usage="", desc=None, epilog=None):
-        super(PlaybookGrapherCLI29, self).init_parser(usage="%s [options] playbook.yml" % __prog__,
-                                                      desc="Make graphs from your Ansible Playbooks.", epilog=epilog)
+        super(PlaybookGrapherCLI, self).init_parser(usage="%s [options] playbook.yml" % __prog__,
+                                                    desc="Make graphs from your Ansible Playbooks.", epilog=epilog)
 
         self._add_my_options()
 
-        # add ansible specific options
-        from ansible.cli.arguments import option_helpers as opt_help
-        opt_help.add_subset_options(self.parser)
-        opt_help.add_vault_options(self.parser)
-        opt_help.add_runtask_options(self.parser)
-
     def post_process_args(self, options):
-        options = super(PlaybookGrapherCLI29, self).post_process_args(options)
+        options = super(PlaybookGrapherCLI, self).post_process_args(options)
 
         # init the options
         self.options = options

requirements.txt

@@ -1,4 +1,4 @@
-ansible>=2.8.0
+ansible>=2.9.0
 graphviz<1
 colour<1
 lxml<5

tests/test_cli.py

@@ -102,8 +102,6 @@ def test_cli_include_role_tasks(include_role_tasks_option, expected):
                           (['-t', 'tag1', '-t', 'tag2'], ['tag1', 'tag2']),
                           (['-t', 'tag1,tag2'], ['tag1', 'tag2'])],
                          ids=['no_tags_provided', 'one-tag', 'multiple-tags', 'multiple-tags2'])
-@pytest.mark.xfail(not IS_ANSIBLE_2_9_X, reason="This will fail in ansible 2.8 due to some global variables.")
-# TODO: Remove xfail when we drop support for Ansible 2.8
 def test_cli_tags(tags_option, expected):
     """
 
@@ -127,8 +125,6 @@ def test_cli_tags(tags_option, expected):
                           (['--skip-tags', 'tag1', '--skip-tags', 'tag2'], ['tag1', 'tag2']),
                           (['--skip-tags', 'tag1,tag2'], ['tag1', 'tag2'])],
                          ids=['no_skip_tags_provided', 'one-skip-tag', 'multiple-skip-tags', 'multiple-skip-tags2'])
-@pytest.mark.xfail(not IS_ANSIBLE_2_9_X, reason="This will fail in ansible 2.8 due to some global variables.")
-# TODO: Remove xfail when we drop support for Ansible 2.8
 def test_skip_tags(skip_tags_option, expected):
     """
 

tests/test_grapher.py

@@ -6,7 +6,7 @@
 from pyquery import PyQuery
 
 from ansibleplaybookgrapher import __prog__
-from ansibleplaybookgrapher.cli import get_cli_class, IS_ANSIBLE_2_9_X
+from ansibleplaybookgrapher.cli import get_cli_class
 from tests import FIXTURES_DIR
 
 
@@ -219,8 +219,6 @@ def test_tags(request):
     _common_tests(svg_path=svg_path, playbook_path=playbook_path, plays_number=1, pre_tasks_number=1)
 
 
-@pytest.mark.xfail(not IS_ANSIBLE_2_9_X, reason="This will fail in ansible 2.8 due to some global variables.")
-# TODO: Remove xfail when we drop support for Ansible 2.8
 def test_skip_tags(request):
     """
     Test a playbook by only graphing a specific tasks based on the given tags