diff --git a/misc/python/materialize/cli/mzcompose.py b/misc/python/materialize/cli/mzcompose.py index 63508191d133a..c438e09ee9e05 100644 --- a/misc/python/materialize/cli/mzcompose.py +++ b/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", diff --git a/misc/python/materialize/mzcompose/composition.py b/misc/python/materialize/mzcompose/composition.py index 82558cc2a0e2b..7914b16fea482 100644 --- a/misc/python/materialize/mzcompose/composition.py +++ b/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: diff --git a/misc/python/materialize/mzcompose/services/balancerd.py b/misc/python/materialize/mzcompose/services/balancerd.py index b7e9f4db454eb..8220fa80349f3 100644 --- a/misc/python/materialize/mzcompose/services/balancerd.py +++ b/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 diff --git a/misc/python/materialize/mzcompose/services/debezium.py b/misc/python/materialize/mzcompose/services/debezium.py index 73a7e255e4fc3..9482fd0d70a11 100644 --- a/misc/python/materialize/mzcompose/services/debezium.py +++ b/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__( diff --git a/misc/python/materialize/mzcompose/services/dnsmasq.py b/misc/python/materialize/mzcompose/services/dnsmasq.py index d47a6786ebe38..fbd8489f0e6a6 100644 --- a/misc/python/materialize/mzcompose/services/dnsmasq.py +++ b/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]]] diff --git a/misc/python/materialize/mzcompose/services/environmentd.py b/misc/python/materialize/mzcompose/services/environmentd.py index 0919c86de54f7..1b2ba9c6e4950 100644 --- a/misc/python/materialize/mzcompose/services/environmentd.py +++ b/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, diff --git a/misc/python/materialize/mzcompose/services/materialized.py b/misc/python/materialize/mzcompose/services/materialized.py index 56ff6918c7f56..e19821e79a064 100644 --- a/misc/python/materialize/mzcompose/services/materialized.py +++ b/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 diff --git a/misc/python/materialize/mzcompose/services/orchestratord.py b/misc/python/materialize/mzcompose/services/orchestratord.py index d5031cdfa6b89..2db223224b77e 100644 --- a/misc/python/materialize/mzcompose/services/orchestratord.py +++ b/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, diff --git a/misc/python/materialize/mzcompose/services/sql_logic_test.py b/misc/python/materialize/mzcompose/services/sql_logic_test.py index 5f6508adc9531..53584d6f55494 100644 --- a/misc/python/materialize/mzcompose/services/sql_logic_test.py +++ b/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( [ diff --git a/misc/python/materialize/mzcompose/services/sql_server.py b/misc/python/materialize/mzcompose/services/sql_server.py index da343c820c28e..5216a4f012dab 100644 --- a/misc/python/materialize/mzcompose/services/sql_server.py +++ b/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 diff --git a/misc/python/materialize/mzcompose/services/test_certs.py b/misc/python/materialize/mzcompose/services/test_certs.py index 0237e18cf5a21..3c6b76f2e149a 100644 --- a/misc/python/materialize/mzcompose/services/test_certs.py +++ b/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`. diff --git a/misc/python/materialize/mzcompose/services/testdrive.py b/misc/python/materialize/mzcompose/services/testdrive.py index 081658d2804e9..2e834372a741c 100644 --- a/misc/python/materialize/mzcompose/services/testdrive.py +++ b/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}") diff --git a/misc/python/materialize/mzcompose/test_result.py b/misc/python/materialize/mzcompose/test_result.py index ad9f700ad91c2..222c7ec6396cc 100644 --- a/misc/python/materialize/mzcompose/test_result.py +++ b/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] diff --git a/misc/python/materialize/test_analytics/config/test_analytics_db_config.py b/misc/python/materialize/test_analytics/config/test_analytics_db_config.py index 4a4cb34cdc852..533b14b3830d0 100644 --- a/misc/python/materialize/test_analytics/config/test_analytics_db_config.py +++ b/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"