Ignore --limit for localhost plays

If using the kayobe --limit <hosts> argument, only those hosts matching
the limit will have an inventory generated for Kolla Ansible. This can
cause problems for Kolla Ansible, even when using a similar limit via
--kolla-limit <hosts>, since it gathers facts for all hosts, and expects
all host variables to be defined.

This change adds a new 'ignore_limit' argument to the playbook running
methods, which allows us to override and ignore the --limit argument
provided by the user, ensuring that we always run these plays.

Change-Id: I104d6af48ddd916460ee454e17a50a2475de6bff
Story: 2004805
Task: 28960

kayobe/ansible.py

@@ -118,7 +118,7 @@ def _get_vars_files(config_path):
 
 def build_args(parsed_args, playbooks,
                extra_vars=None, limit=None, tags=None, verbose_level=None,
-               check=None):
+               check=None, ignore_limit=False):
     """Build arguments required for running Ansible playbooks."""
     cmd = ["ansible-playbook"]
     if verbose_level:
@@ -145,7 +145,7 @@ def build_args(parsed_args, playbooks,
         cmd += ["--become"]
     if check or (parsed_args.check and check is None):
         cmd += ["--check"]
-    if parsed_args.limit or limit:
+    if not ignore_limit and (parsed_args.limit or limit):
         limits = [l for l in [parsed_args.limit, limit] if l]
         cmd += ["--limit", ":&".join(limits)]
     if parsed_args.skip_tags:
@@ -159,12 +159,14 @@ def build_args(parsed_args, playbooks,
 
 def run_playbooks(parsed_args, playbooks,
                   extra_vars=None, limit=None, tags=None, quiet=False,
-                  check_output=False, verbose_level=None, check=None):
+                  check_output=False, verbose_level=None, check=None,
+                  ignore_limit=False):
     """Run a Kayobe Ansible playbook."""
     _validate_args(parsed_args, playbooks)
     cmd = build_args(parsed_args, playbooks,
                      extra_vars=extra_vars, limit=limit, tags=tags,
-                     verbose_level=verbose_level, check=check)
+                     verbose_level=verbose_level, check=check,
+                     ignore_limit=ignore_limit)
     env = os.environ.copy()
     vault.update_environment(parsed_args, env)
     # If the configuration path has been specified via --config-path, ensure

kayobe/cli/commands.py

@@ -69,17 +69,43 @@ class KayobeAnsibleMixin(object):
             verbosity_args["quiet"] = True
         return verbosity_args
 
-    def run_kayobe_playbooks(self, *args, **kwargs):
+    def run_kayobe_playbooks(self, parsed_args, *args, **kwargs):
         kwargs.update(self._get_verbosity_args())
-        return ansible.run_playbooks(*args, **kwargs)
+        return ansible.run_playbooks(parsed_args, *args, **kwargs)
 
-    def run_kayobe_playbook(self, *args, **kwargs):
+    def run_kayobe_playbook(self, parsed_args, *args, **kwargs):
         kwargs.update(self._get_verbosity_args())
-        return ansible.run_playbook(*args, **kwargs)
+        return ansible.run_playbook(parsed_args, *args, **kwargs)
 
-    def run_kayobe_config_dump(self, *args, **kwargs):
+    def run_kayobe_config_dump(self, parsed_args, *args, **kwargs):
         kwargs.update(self._get_verbosity_args())
-        return ansible.config_dump(*args, **kwargs)
+        return ansible.config_dump(parsed_args, *args, **kwargs)
+
+    def generate_kolla_ansible_config(self, parsed_args, install=False,
+                                      service_config=True,
+                                      bifrost_config=False):
+        """Generate configuration for kolla-ansible.
+
+        :param install: If True, also install kolla-ansible.
+        :param service_config: If True, generate service configuration.
+        :param bifrost_config: If True, generate bifrost configuration.
+        """
+        # We use ignore_limit here because all of these plays execute against
+        # localhost, and are typically necessary for kolla-ansible to function
+        # correctly. Previously a number of people were caught out by using
+        # --limit and having these plays skipped.
+        tags = None if install else "config"
+        playbooks = _build_playbook_list("kolla-ansible")
+        self.run_kayobe_playbooks(parsed_args, playbooks, tags=tags,
+                                  ignore_limit=True)
+        if service_config:
+            playbooks = _build_playbook_list("kolla-openstack")
+            self.run_kayobe_playbooks(parsed_args, playbooks,
+                                      ignore_limit=True)
+        if bifrost_config:
+            playbooks = _build_playbook_list("kolla-bifrost")
+            self.run_kayobe_playbooks(parsed_args, playbooks,
+                                      ignore_limit=True)
 
 
 class KollaAnsibleMixin(object):
@@ -130,9 +156,10 @@ class ControlHostBootstrap(KayobeAnsibleMixin, VaultMixin, Command):
         self.app.LOG.debug("Bootstrapping Kayobe Ansible control host")
         ansible.install_galaxy_roles(parsed_args)
         playbooks = _build_playbook_list("bootstrap")
-        self.run_kayobe_playbooks(parsed_args, playbooks)
+        self.run_kayobe_playbooks(parsed_args, playbooks, ignore_limit=True)
         playbooks = _build_playbook_list("kolla-ansible")
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="install")
+        self.run_kayobe_playbooks(parsed_args, playbooks, tags="install",
+                                  ignore_limit=True)
 
 
 class ControlHostUpgrade(KayobeAnsibleMixin, VaultMixin, Command):
@@ -151,9 +178,10 @@ class ControlHostUpgrade(KayobeAnsibleMixin, VaultMixin, Command):
         # Use force to upgrade roles.
         ansible.install_galaxy_roles(parsed_args, force=True)
         playbooks = _build_playbook_list("bootstrap")
-        self.run_kayobe_playbooks(parsed_args, playbooks)
+        self.run_kayobe_playbooks(parsed_args, playbooks, ignore_limit=True)
         playbooks = _build_playbook_list("kolla-ansible")
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="install")
+        self.run_kayobe_playbooks(parsed_args, playbooks, tags="install",
+                                  ignore_limit=True)
 
 
 class ConfigurationDump(KayobeAnsibleMixin, VaultMixin, Command):
@@ -413,9 +441,7 @@ class SeedVMProvision(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
         self.run_kayobe_playbook(parsed_args,
                                  _get_playbook_path("seed-vm-provision"))
         # Now populate the Kolla Ansible inventory.
-        self.run_kayobe_playbook(parsed_args,
+        self.generate_kolla_ansible_config(parsed_args, service_config=False)
-                                 _get_playbook_path("kolla-ansible"),
-                                 tags="config")
 
 
 class SeedVMDeprovision(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
@@ -501,8 +527,8 @@ class SeedHostConfigure(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
             "sysctl", "ip-routing", "snat", "disable-glean", "ntp", "mdadm",
             "lvm")
         self.run_kayobe_playbooks(parsed_args, playbooks, limit="seed")
-        playbooks = _build_playbook_list("kolla-ansible")
+
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="config")
+        self.generate_kolla_ansible_config(parsed_args, service_config=False)
 
         # Run kolla-ansible bootstrap-servers.
         # This command should be run as the kayobe ansible user because at this
@@ -616,11 +642,8 @@ class SeedServiceDeploy(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
 
     def take_action(self, parsed_args):
         self.app.LOG.debug("Deploying seed services")
-        playbooks = _build_playbook_list("kolla-ansible")
+        self.generate_kolla_ansible_config(parsed_args, service_config=False,
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="config")
+                                           bifrost_config=True)
-
-        playbooks = _build_playbook_list("kolla-bifrost")
-        self.run_kayobe_playbooks(parsed_args, playbooks)
 
         self.run_kolla_ansible_seed(parsed_args, "deploy-bifrost")
         playbooks = _build_playbook_list(
@@ -650,11 +673,10 @@ class SeedServiceUpgrade(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
 
     def take_action(self, parsed_args):
         self.app.LOG.debug("Upgrading seed services")
-        playbooks = _build_playbook_list("kolla-ansible")
+        self.generate_kolla_ansible_config(parsed_args, service_config=False,
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="config")
+                                           bifrost_config=True)
 
         playbooks = _build_playbook_list(
-            "kolla-bifrost",
             "seed-service-upgrade-prep")
         self.run_kayobe_playbooks(parsed_args, playbooks)
 
@@ -748,8 +770,7 @@ class OvercloudInventoryDiscover(KayobeAnsibleMixin, VaultMixin, Command):
         self.run_kayobe_playbook(parsed_args,
                                  _get_playbook_path("ip-allocation"))
         # Now populate the Kolla Ansible inventory.
-        self.run_kayobe_playbook(parsed_args, _get_playbook_path(
+        self.generate_kolla_ansible_config(parsed_args, service_config=False)
-            "kolla-ansible"), tags="config")
 
 
 class OvercloudIntrospectionDataSave(KayobeAnsibleMixin, VaultMixin, Command):
@@ -907,8 +928,8 @@ class OvercloudHostConfigure(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
             "sysctl", "disable-glean", "disable-cloud-init", "ntp", "mdadm",
             "lvm")
         self.run_kayobe_playbooks(parsed_args, playbooks, limit="overcloud")
-        playbooks = _build_playbook_list("kolla-ansible")
+
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="config")
+        self.generate_kolla_ansible_config(parsed_args, service_config=False)
 
         # Kolla-ansible bootstrap-servers.
         # The kolla-ansible bootstrap-servers command should be run as the
@@ -1071,12 +1092,7 @@ class OvercloudServiceConfigurationGenerate(KayobeAnsibleMixin,
         self.app.LOG.debug("Generating overcloud service configuration")
 
         # First prepare configuration.
-        playbooks = _build_playbook_list("kolla-ansible")
+        self.generate_kolla_ansible_config(parsed_args)
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="config")
-
-        playbooks = _build_playbook_list("kolla-openstack")
-
-        self.run_kayobe_playbooks(parsed_args, playbooks)
 
         # Run kolla-ansible prechecks before deployment.
         if not parsed_args.skip_prechecks:
@@ -1161,12 +1177,7 @@ class OvercloudServiceDeploy(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
         self.app.LOG.debug("Deploying overcloud services")
 
         # First prepare configuration.
-        playbooks = _build_playbook_list("kolla-ansible")
+        self.generate_kolla_ansible_config(parsed_args)
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="config")
-
-        playbooks = _build_playbook_list("kolla-openstack")
-
-        self.run_kayobe_playbooks(parsed_args, playbooks)
 
         # Run kolla-ansible prechecks before deployment.
         if not parsed_args.skip_prechecks:
@@ -1194,7 +1205,7 @@ class OvercloudServiceDeploy(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
         # Create an environment file for accessing the public API as the admin
         # user.
         playbooks = _build_playbook_list("public-openrc")
-        self.run_kayobe_playbooks(parsed_args, playbooks)
+        self.run_kayobe_playbooks(parsed_args, playbooks, ignore_limit=True)
 
 
 class OvercloudServiceReconfigure(KollaAnsibleMixin, KayobeAnsibleMixin,
@@ -1224,12 +1235,7 @@ class OvercloudServiceReconfigure(KollaAnsibleMixin, KayobeAnsibleMixin,
         self.app.LOG.debug("Reconfiguring overcloud services")
 
         # First prepare configuration.
-        playbooks = _build_playbook_list("kolla-ansible")
+        self.generate_kolla_ansible_config(parsed_args)
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="config")
-
-        playbooks = _build_playbook_list("kolla-openstack")
-
-        self.run_kayobe_playbooks(parsed_args, playbooks)
 
         # Run kolla-ansible prechecks before reconfiguration.
         if not parsed_args.skip_prechecks:
@@ -1257,7 +1263,7 @@ class OvercloudServiceReconfigure(KollaAnsibleMixin, KayobeAnsibleMixin,
         # Create an environment file for accessing the public API as the admin
         # user.
         playbooks = _build_playbook_list("public-openrc")
-        self.run_kayobe_playbooks(parsed_args, playbooks)
+        self.run_kayobe_playbooks(parsed_args, playbooks, ignore_limit=True)
 
 
 class OvercloudServiceUpgrade(KollaAnsibleMixin, KayobeAnsibleMixin,
@@ -1286,8 +1292,7 @@ class OvercloudServiceUpgrade(KollaAnsibleMixin, KayobeAnsibleMixin,
         self.app.LOG.debug("Upgrading overcloud services")
 
         # First prepare configuration.
-        playbooks = _build_playbook_list("kolla-ansible", "kolla-openstack")
+        self.generate_kolla_ansible_config(parsed_args, install=True)
-        self.run_kayobe_playbooks(parsed_args, playbooks)
 
         # Run kolla-ansible prechecks before upgrade.
         if not parsed_args.skip_prechecks:
@@ -1336,11 +1341,7 @@ class OvercloudServiceDestroy(KollaAnsibleMixin, KayobeAnsibleMixin,
         self.app.LOG.debug("Destroying overcloud services")
 
         # First prepare configuration.
-        playbooks = _build_playbook_list("kolla-ansible")
+        self.generate_kolla_ansible_config(parsed_args)
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="config")
-
-        playbooks = _build_playbook_list("kolla-openstack")
-        self.run_kayobe_playbooks(parsed_args, playbooks)
 
         # Run kolla-ansible destroy.
         extra_args = ["--yes-i-really-really-mean-it"]
@@ -1362,8 +1363,7 @@ class OvercloudContainerImagePull(KayobeAnsibleMixin, KollaAnsibleMixin,
         self.app.LOG.debug("Pulling overcloud container images")
 
         # First prepare configuration.
-        playbooks = _build_playbook_list("kolla-ansible")
+        self.generate_kolla_ansible_config(parsed_args, service_config=False)
-        self.run_kayobe_playbooks(parsed_args, playbooks, tags="config")
 
         # Pull updated kolla container images.
         self.run_kolla_ansible_overcloud(parsed_args, "pull")

kayobe/tests/unit/cli/test_commands.py

@@ -45,11 +45,17 @@ class TestCase(unittest.TestCase):
         self.assertEqual(0, result)
         mock_install.assert_called_once_with(parsed_args)
         expected_calls = [
-            mock.call(mock.ANY, [utils.get_data_files_path(
+            mock.call(
-                "ansible", "bootstrap.yml")]),
+                mock.ANY,
-            mock.call(mock.ANY, [
+                [utils.get_data_files_path("ansible", "bootstrap.yml")],
-                utils.get_data_files_path("ansible", "kolla-ansible.yml")
+                ignore_limit=True,
-            ], tags="install"),
+            ),
+            mock.call(
+                mock.ANY,
+                [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
+                tags="install",
+                ignore_limit=True
+            ),
         ]
         self.assertEqual(expected_calls, mock_run.call_args_list)
 
@@ -66,11 +72,17 @@ class TestCase(unittest.TestCase):
         mock_install.assert_called_once_with(parsed_args, force=True)
         mock_prune.assert_called_once_with(parsed_args)
         expected_calls = [
-            mock.call(mock.ANY, [utils.get_data_files_path(
+            mock.call(
-                "ansible", "bootstrap.yml")]),
+                mock.ANY,
-            mock.call(mock.ANY, [
+                [utils.get_data_files_path("ansible", "bootstrap.yml")],
-                utils.get_data_files_path("ansible", "kolla-ansible.yml")
+                ignore_limit=True,
-            ], tags="install"),
+            ),
+            mock.call(
+                mock.ANY,
+                [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
+                tags="install",
+                ignore_limit=True
+            ),
         ]
         self.assertEqual(expected_calls, mock_run.call_args_list)
 
@@ -461,6 +473,7 @@ class TestCase(unittest.TestCase):
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
+                ignore_limit=True,
             ),
             mock.call(
                 mock.ANY,
@@ -835,10 +848,12 @@ class TestCase(unittest.TestCase):
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
+                ignore_limit=True,
             ),
             mock.call(
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-bifrost.yml")],
+                ignore_limit=True,
             ),
             mock.call(
                 mock.ANY,
@@ -881,11 +896,16 @@ class TestCase(unittest.TestCase):
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
+                ignore_limit=True,
+            ),
+            mock.call(
+                mock.ANY,
+                [utils.get_data_files_path("ansible", "kolla-bifrost.yml")],
+                ignore_limit=True,
             ),
             mock.call(
                 mock.ANY,
                 [
-                    utils.get_data_files_path("ansible", "kolla-bifrost.yml"),
                     utils.get_data_files_path("ansible",
                                               "seed-service-upgrade-prep.yml")
                 ],
@@ -918,9 +938,11 @@ class TestCase(unittest.TestCase):
         ]
         self.assertEqual(expected_calls, mock_kolla_run.call_args_list)
 
+    @mock.patch.object(commands.KayobeAnsibleMixin,
+                       "run_kayobe_playbooks")
     @mock.patch.object(commands.KayobeAnsibleMixin,
                        "run_kayobe_playbook")
-    def test_overcloud_inventory_discover(self, mock_run):
+    def test_overcloud_inventory_discover(self, mock_run_one, mock_run):
         command = commands.OvercloudInventoryDiscover(TestApp(), [])
         parser = command.get_parser("test")
         parsed_args = parser.parse_args([])
@@ -938,10 +960,15 @@ class TestCase(unittest.TestCase):
                 mock.ANY,
                 utils.get_data_files_path("ansible", "ip-allocation.yml"),
             ),
+        ]
+        self.assertEqual(expected_calls, mock_run_one.call_args_list)
+
+        expected_calls = [
             mock.call(
                 mock.ANY,
-                utils.get_data_files_path("ansible", "kolla-ansible.yml"),
+                [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
+                ignore_limit=True,
             ),
         ]
         self.assertEqual(expected_calls, mock_run.call_args_list)
@@ -1071,6 +1098,7 @@ class TestCase(unittest.TestCase):
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
+                ignore_limit=True,
             ),
             mock.call(
                 mock.ANY,
@@ -1438,6 +1466,7 @@ class TestCase(unittest.TestCase):
             mock.call(
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
+                ignore_limit=True,
                 tags="config",
             ),
             mock.call(
@@ -1446,6 +1475,7 @@ class TestCase(unittest.TestCase):
                     utils.get_data_files_path("ansible",
                                               "kolla-openstack.yml"),
                 ],
+                ignore_limit=True,
             ),
             mock.call(
                 mock.ANY,
@@ -1470,6 +1500,7 @@ class TestCase(unittest.TestCase):
                 [
                     utils.get_data_files_path("ansible", "public-openrc.yml"),
                 ],
+                ignore_limit=True,
             ),
         ]
         self.assertEqual(expected_calls, mock_run.call_args_list)
@@ -1509,6 +1540,7 @@ class TestCase(unittest.TestCase):
             mock.call(
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
+                ignore_limit=True,
                 tags="config",
             ),
             mock.call(
@@ -1517,6 +1549,7 @@ class TestCase(unittest.TestCase):
                     utils.get_data_files_path("ansible",
                                               "kolla-openstack.yml"),
                 ],
+                ignore_limit=True,
             ),
             mock.call(
                 mock.ANY,
@@ -1541,6 +1574,7 @@ class TestCase(unittest.TestCase):
                 [
                     utils.get_data_files_path("ansible", "public-openrc.yml"),
                 ],
+                ignore_limit=True,
             ),
         ]
         self.assertEqual(expected_calls, mock_run.call_args_list)
@@ -1577,13 +1611,19 @@ class TestCase(unittest.TestCase):
         self.assertEqual(0, result)
 
         expected_calls = [
+            mock.call(
+                mock.ANY,
+                [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
+                ignore_limit=True,
+                tags=None,
+            ),
             mock.call(
                 mock.ANY,
                 [
-                    utils.get_data_files_path("ansible", "kolla-ansible.yml"),
                     utils.get_data_files_path("ansible",
                                               "kolla-openstack.yml"),
-                ]
+                ],
+                ignore_limit=True,
             ),
             mock.call(
                 mock.ANY,

kayobe/tests/unit/test_ansible.py

@@ -264,6 +264,35 @@ class TestCase(unittest.TestCase):
                                          quiet=False, env=expected_env)
         mock_vars.assert_called_once_with("/etc/kayobe")
 
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(ansible, "_get_vars_files")
+    @mock.patch.object(ansible, "_validate_args")
+    def test_run_playbooks_ignore_limit(self, mock_validate, mock_vars,
+                                        mock_run):
+        mock_vars.return_value = ["/etc/kayobe/vars-file1.yml",
+                                  "/etc/kayobe/vars-file2.yaml"]
+        parser = argparse.ArgumentParser()
+        ansible.add_args(parser)
+        vault.add_args(parser)
+        args = [
+            "-l", "group1:host",
+        ]
+        parsed_args = parser.parse_args(args)
+        ansible.run_playbooks(parsed_args, ["playbook1.yml", "playbook2.yml"],
+                              limit="foo", ignore_limit=True)
+        expected_cmd = [
+            "ansible-playbook",
+            "--inventory", "/etc/kayobe/inventory",
+            "-e", "@/etc/kayobe/vars-file1.yml",
+            "-e", "@/etc/kayobe/vars-file2.yaml",
+            "playbook1.yml",
+            "playbook2.yml",
+        ]
+        expected_env = {"KAYOBE_CONFIG_PATH": "/etc/kayobe"}
+        mock_run.assert_called_once_with(expected_cmd, check_output=False,
+                                         quiet=False, env=expected_env)
+        mock_vars.assert_called_once_with("/etc/kayobe")
+
     @mock.patch.object(utils, "run_command")
     @mock.patch.object(ansible, "_get_vars_files")
     @mock.patch.object(ansible, "_validate_args")