mzcompose: Fix various issues

misc/python/materialize/cli/mzcompose.py

@@ -163,10 +163,13 @@ def main(argv: list[str]) -> None:
     )
     shtab.add_argument_to(completion_parser, "shell", parent=parser)
 
-    # shtab can't handle Enum choices (indexes with [0]). Convert to strings.
+    # shtab can't handle Enum classes as choices (it indexes with [0]).
+    # Convert to a list of the members: shtab renders them via str(), and
+    # argparse still accepts the type-converted member, which converting to
+    # name strings would not (the member never compares equal to its name).
     for action in parser._actions:  # noqa: SLF001
         if isinstance(action.choices, type) and issubclass(action.choices, enum.Enum):
-            action.choices = [e.name for e in action.choices]
+            action.choices = list(action.choices)
 
     args = parser.parse_args(argv)
     if args.file:
@@ -538,7 +541,9 @@ def run(self, args: argparse.Namespace) -> None:
                 ],
                 env={"PGCLIENTENCODING": "utf-8"},
             )
-        elif image == "materialize/postgres":
+        # Check the service name before the image: postgres-metadata uses the
+        # materialize/postgres image but listens on 26257 as root.
+        elif image == "cockroachdb/cockroach" or args.service == "postgres-metadata":
             assert not args.mz_system
             deps = composition.repo.resolve_dependencies(
                 [composition.repo.images["psql"]]
@@ -548,17 +553,19 @@ def run(self, args: argparse.Namespace) -> None:
                 [
                     "-h",
                     service.get("hostname", args.service),
+                    "-p",
+                    "26257",
                     "-U",
-                    "postgres",
-                    "postgres",
+                    "root",
+                    "root",
                 ],
                 docker_args=[
                     "--interactive",
                     f"--network={composition.project_name}_default",
                 ],
-                env={"PGPASSWORD": "postgres", "PGCLIENTENCODING": "utf-8"},
+                env={"PGCLIENTENCODING": "utf-8"},
             )
-        elif image == "cockroachdb/cockroach" or args.service == "postgres-metadata":
+        elif image == "materialize/postgres":
             assert not args.mz_system
             deps = composition.repo.resolve_dependencies(
                 [composition.repo.images["psql"]]
@@ -568,16 +575,15 @@ def run(self, args: argparse.Namespace) -> None:
                 [
                     "-h",
                     service.get("hostname", args.service),
-                    "-p" "26257",
                     "-U",
-                    "root",
-                    "root",
+                    "postgres",
+                    "postgres",
                 ],
                 docker_args=[
                     "--interactive",
                     f"--network={composition.project_name}_default",
                 ],
-                env={"PGCLIENTENCODING": "utf-8"},
+                env={"PGPASSWORD": "postgres", "PGCLIENTENCODING": "utf-8"},
             )
         elif image == "mysql":
             assert not args.mz_system
@@ -772,15 +778,20 @@ def run(self, args: argparse.Namespace) -> Any:
         # `run` command while the latter asks for help on the named `workflow`.
 
         KNOWN_OPTIONS_WITH_ARGUMENT = [
-            "-d",
-            "--detach",
-            "--name",
+            "--cap-add",
+            "--cap-drop",
             "--entrypoint",
             "-e",
+            "--env",
+            "--env-from-file",
             "-l",
             "--label",
+            "--name",
             "-p",
             "--publish",
+            "--pull",
+            "-u",
+            "--user",
             "-v",
             "--volume",
             "-w",

misc/python/materialize/mzcompose/composition.py

@@ -529,8 +529,12 @@ def emit(is_stdout: bool, text: str) -> None:
                         check=check,
                         stdout=stdout,
                         stderr=stderr,
+                        # NOTE: isinstance against typing.IO is always False for
+                        # real file objects, so dispatch on str instead: strings
+                        # go through `input`, everything else (file objects,
+                        # subprocess.DEVNULL) is passed as the stdin stream.
                         input=stdin if isinstance(stdin, str) else None,
-                        stdin=stdin if isinstance(stdin, IO) else None,
+                        stdin=stdin if not isinstance(stdin, str) else None,
                         text=True,
                         bufsize=1,
                         env=environment,
@@ -1912,9 +1916,14 @@ def test_parts(self, parts: list[T], process_func: Callable[[T], Any]) -> None:
                 )
                 from materialize.test_analytics.test_analytics_db import TestAnalyticsDb
 
-                test_analytics_config = create_test_analytics_config(self)
-                test_analytics = TestAnalyticsDb(test_analytics_config)
-                test_analytics.database_connector.submit_update_statements()
+                try:
+                    test_analytics_config = create_test_analytics_config(self)
+                    test_analytics = TestAnalyticsDb(test_analytics_config)
+                    test_analytics.database_connector.submit_update_statements()
+                except Exception as e:
+                    # An analytics failure must not mask the actual test
+                    # result, which is raised below.
+                    print(f"Failed to submit test analytics data: {e}")
         if len(exceptions) > 1:
             print(f"Further exceptions were raised:\n{exceptions[1:]}")
         if exceptions:

misc/python/materialize/mzcompose/services/balancerd.py

@@ -50,9 +50,7 @@ def __init__(
             if https_resolver_template is not None:
                 command.append(f"--https-resolver-template={https_resolver_template}")
         if frontegg_resolver_template is not None:
-            command.append(
-                f"--frontegg-reesolver-template={frontegg_resolver_template}"
-            )
+            command.append(f"--frontegg-resolver-template={frontegg_resolver_template}")
 
         depends_graph: dict[str, ServiceDependency] = {
             s: {"condition": "service_started"} for s in depends_on

misc/python/materialize/mzcompose/services/debezium.py

@@ -44,7 +44,10 @@ def __init__(
             "kafka": {"condition": "service_healthy"},
             "schema-registry": {"condition": "service_healthy"},
         }
-        environment.append(f"CONNECT_REST_ADVERTISED_HOST_NAME={name}")
+        # Concatenate instead of appending: the default list is shared across
+        # instances, so appending would leak one instance's advertised host
+        # name into every other instance's environment.
+        environment = environment + [f"CONNECT_REST_ADVERTISED_HOST_NAME={name}"]
         if redpanda:
             depends_on = {"redpanda": {"condition": "service_healthy"}}
         super().__init__(

misc/python/materialize/mzcompose/services/dnsmasq.py

@@ -37,7 +37,6 @@ def __init__(
         self,
         name: str = "dnsmasq",
         mzbuild: str = "dnsmasq",
-        command: list[str] | None = None,
         depends_on: list[str] = [],
         networks: (
             dict[str, dict[str, list[str]]]

misc/python/materialize/mzcompose/services/environmentd.py

@@ -19,9 +19,6 @@ def __init__(
         self,
         name: str = "environmentd",
         mzbuild: str = "environmentd",
-        https_resolver_template: str | None = None,
-        frontegg_resolver_template: str | None = None,
-        static_resolver_addr: str | None = None,
     ) -> None:
         config: ServiceConfig = {
             "mzbuild": mzbuild,

misc/python/materialize/mzcompose/services/materialized.py

@@ -50,7 +50,6 @@ def __init__(self, image: str | None = None):
         name = "materialized"
 
         config: ServiceConfig = {
-            "mzbuild": name,
             "ports": [6875, 6874, 6876, 6877, 6878, 26257],
             "healthcheck": {
                 "test": ["CMD", "curl", "-f", "localhost:6878/api/readyz"],
@@ -59,6 +58,10 @@ def __init__(self, image: str | None = None):
                 "start_period": "600s",
             },
         }
+        if image is not None:
+            config["image"] = image
+        else:
+            config["mzbuild"] = name
 
         super().__init__(name=name, config=config)
 
@@ -187,6 +190,10 @@ def __init__(
             system_parameter_defaults = get_default_system_parameters(
                 system_parameter_version or image_version
             )
+        else:
+            # Copy so the writes below don't leak into the caller's dict,
+            # which is often shared across multiple Materialized instances.
+            system_parameter_defaults = dict(system_parameter_defaults)
 
         system_parameter_defaults["default_cluster_replication_factor"] = str(
             default_replication_factor

misc/python/materialize/mzcompose/services/orchestratord.py

@@ -19,9 +19,6 @@ def __init__(
         self,
         name: str = "orchestratord",
         mzbuild: str = "orchestratord",
-        https_resolver_template: str | None = None,
-        frontegg_resolver_template: str | None = None,
-        static_resolver_addr: str | None = None,
     ) -> None:
         config: ServiceConfig = {
             "mzbuild": mzbuild,

misc/python/materialize/mzcompose/services/sql_logic_test.py

@@ -29,7 +29,8 @@ def __init__(
                 "MZ_SOFT_ASSERTIONS=1",
                 "LD_PRELOAD=libeatmydata.so",
             ]
-        environment += [
+        # Concatenate instead of appending so the caller's list is not mutated.
+        environment = environment + [
             "MZ_SYSTEM_PARAMETER_DEFAULT="
             + ";".join(
                 [

misc/python/materialize/mzcompose/services/sql_server.py

@@ -49,10 +49,13 @@ def __init__(
                 # healthy while msdb is still recovering, causing error
                 # 904 ("cannot be autostarted during server shutdown or
                 # startup").
+                # NOTE: -b is what turns the RAISERROR into a nonzero exit
+                # code; without it sqlcmd exits 0 and the check degrades to
+                # a plain connectivity probe.
                 "test": (
                     "SQLCMD=/opt/mssql-tools18/bin/sqlcmd; "
                     '[ -x "$$SQLCMD" ] || SQLCMD=/opt/mssql-tools/bin/sqlcmd; '
-                    f"\"$$SQLCMD\" -C -S localhost -U sa -P '{sa_password}' "
+                    f"\"$$SQLCMD\" -b -C -S localhost -U sa -P '{sa_password}' "
                     "-Q \"SET NOCOUNT ON; IF EXISTS (SELECT 1 FROM sys.databases WHERE state_desc != 'ONLINE') RAISERROR('not ready', 16, 1)\""
                 ),
                 # Recovering can take a while

misc/python/materialize/mzcompose/services/test_certs.py

@@ -21,7 +21,7 @@ def __init__(
         name: str = "test-certs",
     ) -> None:
         super().__init__(
-            name="test-certs",
+            name=name,
             config={
                 # Container must stay alive indefinitely to be considered
                 # healthy by `docker compose up --wait`.

misc/python/materialize/mzcompose/services/testdrive.py

@@ -97,7 +97,10 @@ def __init__(
                 "AWS_SESSION_TOKEN",
             ]
 
-        environment += [
+        # Concatenate instead of appending so the caller's list is not
+        # mutated: constructing Testdrive twice from the same list would
+        # otherwise accumulate duplicate entries.
+        environment = environment + [
             f"CLUSTER_REPLICA_SIZES={json.dumps(cluster_replica_size)}",
             "MZ_CI_LICENSE_KEY",
             "LD_PRELOAD=libeatmydata.so",
@@ -120,6 +123,11 @@ def __init__(
                 *(["--materialize-use-https"] if materialize_use_https else []),
                 # Faster retries
             ]
+        else:
+            # Copy so the appends below don't mutate the caller's list:
+            # constructing Testdrive twice from the same list would otherwise
+            # accumulate duplicate flags, which abort testdrive at startup.
+            entrypoint = list(entrypoint)
 
         entrypoint.append(f"--backoff-factor={backoff_factor}")
 

misc/python/materialize/mzcompose/test_result.py

@@ -74,7 +74,11 @@ def __init__(
 def try_determine_errors_from_cmd_execution(
     e: CommandFailureCausedUIError, test_context: str | None
 ) -> list[TestFailureDetails]:
-    output = e.stderr or e.stdout
+    # Combine both streams: testdrive prints the actual error text to stdout
+    # while the "^^^ +++" markers and the error report go to stderr, so
+    # either stream alone is missing half the picture.
+    output_parts = [s for s in [e.stdout, e.stderr] if s]
+    output = "\n".join(output_parts) if output_parts else None
 
     if "running docker compose failed" in str(e):
         return [determine_error_from_docker_compose_failure(e, output, test_context)]
@@ -149,8 +153,10 @@ def extract_error_chunks_from_output(output: str) -> list[str]:
     if pos == -1:
         return []
 
-    error_output = output[: pos - 1]
-    error_chunks = error_output.split("^^^ +++")
+    error_output = output[:pos]
+    # Testdrive prints "^^^ +++" *before* each error, so everything up to the
+    # first marker is regular output, not an error.
+    error_chunks = error_output.split("^^^ +++")[1:]
 
     return [chunk.strip() for chunk in error_chunks if len(chunk.strip()) > 0]
 

misc/python/materialize/test_analytics/config/test_analytics_db_config.py

@@ -22,7 +22,7 @@ def create_test_analytics_config(c: Composition) -> MzDbConfig:
     if app_password is not None:
         try:
             hostname = get_cloud_hostname(c, app_password=app_password)
-        except ui.CommandFailureCausedUIError as e:
+        except ui.UIError as e:
             # TODO: Remove when database-issues#8592 is fixed
             print(f"Failed to get cloud hostname ({e}), using fallback value")
             hostname = "7vifiksqeftxc6ld3r6zvc8n2.lb.us-east-1.aws.materialize.cloud"