Merge branch 'fix-missing-etc-booth' into pcs-0.10

CHANGELOG.md

@@ -12,8 +12,14 @@
   is not false when a stonith device is in a group ([ghpull#370])
 - Misleading error message from `pcs quorum unblock` when `wait_for_all=0`
   ([rhbz#1968088])
+- Misleading error message from `pcs booth setup` and `pcs booth pull` when
+  booth config directory (`/etc/booth`) is missing ([rhbz#1791670],
+  [ghpull#411], [ghissue#225])
 
+[ghissue#225]: https://github.com/ClusterLabs/pcs/issues/225
 [ghpull#370]: https://github.com/ClusterLabs/pcs/pull/370
+[ghpull#411]: https://github.com/ClusterLabs/pcs/pull/411
+[rhbz#1791670]: https://bugzilla.redhat.com/show_bug.cgi?id=1791670
 [rhbz#1968088]: https://bugzilla.redhat.com/show_bug.cgi?id=1968088
 [rhbz#1990784]: https://bugzilla.redhat.com/show_bug.cgi?id=1990784
 [rhbz#2018969]: https://bugzilla.redhat.com/show_bug.cgi?id=2018969

pcs/common/reports/codes.py

@@ -58,6 +58,7 @@
 BOOTH_LACK_OF_SITES = M("BOOTH_LACK_OF_SITES")
 BOOTH_MULTIPLE_TIMES_IN_CIB = M("BOOTH_MULTIPLE_TIMES_IN_CIB")
 BOOTH_NOT_EXISTS_IN_CIB = M("BOOTH_NOT_EXISTS_IN_CIB")
+BOOTH_PATH_NOT_EXISTS = M("BOOTH_PATH_NOT_EXISTS")
 BOOTH_PEERS_STATUS_ERROR = M("BOOTH_PEERS_STATUS_ERROR")
 BOOTH_TICKET_DOES_NOT_EXIST = M("BOOTH_TICKET_DOES_NOT_EXIST")
 BOOTH_TICKET_DUPLICATE = M("BOOTH_TICKET_DUPLICATE")

pcs/common/reports/messages.py

@@ -6274,6 +6274,26 @@ def message(self) -> str:
         return f"Node '{self.node}' is part of local cluster"
 
 
+@dataclass(frozen=True)
+class BoothPathNotExists(ReportItemMessage):
+    """
+    Path '/etc/booth' is generated when Booth is installed, so it can be used to
+    check whether Booth is installed
+
+    path -- The path generated by booth installation
+    """
+
+    path: str
+    _code = codes.BOOTH_PATH_NOT_EXISTS
+
+    @property
+    def message(self) -> str:
+        return (
+            f"Configuration directory for booth '{self.path}' is missing. "
+            "Is booth installed?"
+        )
+
+
 @dataclass(frozen=True)
 class BoothLackOfSites(ReportItemMessage):
     """

pcs/lib/commands/booth.py

@@ -85,6 +85,12 @@ def config_setup(
     booth_conf = booth_env.create_facade(site_list, arbitrator_list)
     booth_conf.set_authfile(booth_env.key_path)
 
+    conf_dir = (
+        None
+        if booth_env.ghost_file_codes
+        else os.path.dirname(booth_env.config_path)
+    )
+
     try:
         booth_env.key.write_raw(
             tools.generate_binary_key(
@@ -109,7 +115,12 @@ def config_setup(
             )
         )
     except RawFileError as e:
-        report_processor.report(raw_file_error_report(e))
+        if conf_dir and not os.path.exists(conf_dir):
+            report_processor.report(
+                ReportItem.error(reports.messages.BoothPathNotExists(conf_dir))
+            )
+        else:
+            report_processor.report(raw_file_error_report(e))
     if report_processor.has_errors:
         raise LibraryError()
 
@@ -766,6 +777,7 @@ def pull_config(env: LibraryEnvironment, node_name, instance_name=None):
     booth_env = env.get_booth_env(instance_name)
     instance_name = booth_env.instance_name
     _ensure_live_env(env, booth_env)
+    conf_dir = os.path.dirname(booth_env.config_path)
 
     env.report_processor.report(
         ReportItem.info(
@@ -809,7 +821,12 @@ def pull_config(env: LibraryEnvironment, node_name, instance_name=None):
             )
         )
     except RawFileError as e:
-        report_processor.report(raw_file_error_report(e))
+        if not os.path.exists(conf_dir):
+            report_processor.report(
+                ReportItem.error(reports.messages.BoothPathNotExists(conf_dir))
+            )
+        else:
+            report_processor.report(raw_file_error_report(e))
     except KeyError as e:
         raise LibraryError(
             ReportItem.error(reports.messages.InvalidResponseFormat(node_name))

pcs_test/tier0/common/reports/test_messages.py

@@ -4740,6 +4740,17 @@ def test_success(self):
         )
 
 
+class BoothPathNotExists(NameBuildTest):
+    def test_success(self):
+        self.assert_message_from_report(
+            (
+                "Configuration directory for booth 'path' is missing. Is booth "
+                "installed?"
+            ),
+            reports.BoothPathNotExists("path"),
+        )
+
+
 class BoothNotExistsInCib(NameBuildTest):
     def test_success(self):
         self.assert_message_from_report(

pcs_test/tier0/lib/commands/test_booth.py

@@ -333,23 +333,22 @@ def test_files_exist_forced(self):
             overwrite_existing=True,
         )
 
-    def test_write_config_error(self):
-        error = "an error occurred"
+    def _assert_write_config_error(self, error, booth_dir_exists):
 
-        (
-            self.config.raw_file.write(
-                file_type_codes.BOOTH_KEY,
-                self.fixture_key_path(),
-                RANDOM_KEY,
-                name="raw_file.write.key",
-            ).raw_file.write(
-                file_type_codes.BOOTH_CONFIG,
-                self.fixture_cfg_path(),
-                self.fixture_cfg_content(),
-                exception_msg=error,
-                name="raw_file.write.cfg",
-            )
+        self.config.raw_file.write(
+            file_type_codes.BOOTH_KEY,
+            self.fixture_key_path(),
+            RANDOM_KEY,
+            name="raw_file.write.key",
+        )
+        self.config.raw_file.write(
+            file_type_codes.BOOTH_CONFIG,
+            self.fixture_cfg_path(),
+            self.fixture_cfg_content(),
+            exception_msg=error,
+            name="raw_file.write.cfg",
         )
+        self.config.fs.exists(self.booth_dir, booth_dir_exists)
 
         self.env_assist.assert_raise_library_error(
             lambda: commands.config_setup(
@@ -358,6 +357,10 @@ def test_write_config_error(self):
                 self.arbitrators,
             )
         )
+
+    def test_write_config_error(self):
+        error = "an error occurred"
+        self._assert_write_config_error(error, True)
         self.env_assist.assert_reports(
             [
                 fixture.error(
@@ -370,18 +373,27 @@ def test_write_config_error(self):
             ]
         )
 
-    def test_write_key_error(self):
+    def test_write_config_error_booth_dir_missing(self):
         error = "an error occurred"
+        self._assert_write_config_error(error, False)
+        self.env_assist.assert_reports(
+            [
+                fixture.error(
+                    reports.codes.BOOTH_PATH_NOT_EXISTS,
+                    path=self.booth_dir,
+                ),
+            ]
+        )
 
-        (
-            self.config.raw_file.write(
-                file_type_codes.BOOTH_KEY,
-                self.fixture_key_path(),
-                RANDOM_KEY,
-                exception_msg=error,
-                name="raw_file.write.key",
-            )
+    def _assert_write_key_error(self, error, booth_dir_exists):
+        self.config.raw_file.write(
+            file_type_codes.BOOTH_KEY,
+            self.fixture_key_path(),
+            RANDOM_KEY,
+            exception_msg=error,
+            name="raw_file.write.key",
         )
+        self.config.fs.exists(self.booth_dir, booth_dir_exists)
 
         self.env_assist.assert_raise_library_error(
             lambda: commands.config_setup(
@@ -390,6 +402,10 @@ def test_write_key_error(self):
                 self.arbitrators,
             )
         )
+
+    def test_write_key_error(self):
+        error = "an error occurred"
+        self._assert_write_key_error(error, True)
         self.env_assist.assert_reports(
             [
                 fixture.error(
@@ -402,6 +418,18 @@ def test_write_key_error(self):
             ]
         )
 
+    def test_write_key_error_booth_dir_missing(self):
+        error = "an error occurred"
+        self._assert_write_key_error(error, False)
+        self.env_assist.assert_reports(
+            [
+                fixture.error(
+                    reports.codes.BOOTH_PATH_NOT_EXISTS,
+                    path=self.booth_dir,
+                ),
+            ]
+        )
+
     def test_not_live(self):
         key_path = "/tmp/pcs_test/booth.key"
         self.config.env.set_booth(
@@ -3308,26 +3336,29 @@ def test_not_live(self):
             expected_in_processor=False,
         )
 
-    def test_write_failure(self):
-        (
-            self.config.http.booth.get_config(
-                self.name,
-                self.config_data.decode("utf-8"),
-                node_labels=[self.node_name],
-            ).raw_file.write(
-                file_type_codes.BOOTH_CONFIG,
-                self.config_path,
-                self.config_data,
-                can_overwrite=True,
-                exception_msg=self.reason,
-            )
+    def _assert_write_failure(self, booth_dir_exists):
+        self.config.http.booth.get_config(
+            self.name,
+            self.config_data.decode("utf-8"),
+            node_labels=[self.node_name],
         )
+        self.config.raw_file.write(
+            file_type_codes.BOOTH_CONFIG,
+            self.config_path,
+            self.config_data,
+            can_overwrite=True,
+            exception_msg=self.reason,
+        )
+        self.config.fs.exists(self.booth_dir, booth_dir_exists)
 
         self.env_assist.assert_raise_library_error(
             lambda: commands.pull_config(
                 self.env_assist.get_env(), self.node_name
             ),
         )
+
+    def test_write_failure(self):
+        self._assert_write_failure(True)
         self.env_assist.assert_reports(
             self.report_list[:1]
             + [
@@ -3341,6 +3372,18 @@ def test_write_failure(self):
             ]
         )
 
+    def test_write_failure_booth_dir_missing(self):
+        self._assert_write_failure(False)
+        self.env_assist.assert_reports(
+            self.report_list[:1]
+            + [
+                fixture.error(
+                    reports.codes.BOOTH_PATH_NOT_EXISTS,
+                    path=self.booth_dir,
+                ),
+            ]
+        )
+
     def test_network_failure(self):
         self.config.http.booth.get_config(
             self.name,
@@ -3506,22 +3549,24 @@ def setUp(self):
         super().setUp()
         self.reason = "reason"
 
-    def test_authfile_write_failure(self):
-        (
-            self.config.raw_file.write(
-                file_type_codes.BOOTH_KEY,
-                self.authfile_path,
-                self.authfile_data,
-                can_overwrite=True,
-                exception_msg=self.reason,
-            )
+    def _assert_authfile_write_failure(self, booth_dir_exists):
+        self.config.raw_file.write(
+            file_type_codes.BOOTH_KEY,
+            self.authfile_path,
+            self.authfile_data,
+            can_overwrite=True,
+            exception_msg=self.reason,
         )
+        self.config.fs.exists(self.booth_dir, booth_dir_exists)
 
         self.env_assist.assert_raise_library_error(
             lambda: commands.pull_config(
                 self.env_assist.get_env(), self.node_name
             ),
         )
+
+    def test_authfile_write_failure(self):
+        self._assert_authfile_write_failure(True)
         self.env_assist.assert_reports(
             self.report_list[:1]
             + [
@@ -3535,6 +3580,18 @@ def test_authfile_write_failure(self):
             ]
         )
 
+    def test_authfile_write_failure_booth_dir_missing(self):
+        self._assert_authfile_write_failure(False)
+        self.env_assist.assert_reports(
+            self.report_list[:1]
+            + [
+                fixture.error(
+                    reports.codes.BOOTH_PATH_NOT_EXISTS,
+                    path=self.booth_dir,
+                ),
+            ]
+        )
+
 
 class GetStatus(TestCase):
     def setUp(self):