Skip to content

Commit

Permalink
ci: pull cargo test out of the build CI job
Browse files Browse the repository at this point in the history
Building the ci-cargo-test Docker image is one of the slowest parts of
the CI build job because it requires shipping around a 1.5GiB image
that's only used by one other job.

Instead, use a third builder agent to run `cargo nextest` directly. This
is a bit more wasteful in terms of overall CPU usage (we have to
recompile the world from scratch twice), but should make CI *much*
faster because the main build job will complete much faster, which lets
the rest of the tests get started much sooner.

Fix MaterializeInc#13010.
  • Loading branch information
benesch committed Dec 5, 2022
1 parent 04c38f7 commit 0275079
Show file tree
Hide file tree
Showing 14 changed files with 74 additions and 213 deletions.
12 changes: 12 additions & 0 deletions .config/nextest.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[profile.default]
slow-timeout = { period = "60s", terminate-after = 2 }

[[profile.default.overrides]]
filter = "package(mz-environmentd)"
threads-required = 8
slow-timeout = { period = "120s", terminate-after = 2 }

[profile.ci]
junit = { path = "junit_cargo-test.xml" }
fail-fast = false
failure-output = "immediate-final"
1 change: 1 addition & 0 deletions bin/lint
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ copyright_files=$(grep -vE \
-e '^netlify\.toml$' \
-e '^rustfmt\.toml$' \
-e '^clippy\.toml$' \
-e '^\.config/nextest\.toml$' \
-e '(^|/)yarn\.lock$' \
-e '(^|/)requirements.*\.txt$' \
-e '\.(md|json|asc|png|jpe?g|svg|avro|avsc|pb|ico|html|so)$' \
Expand Down
2 changes: 1 addition & 1 deletion ci/builder/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ RUN mkdir rust \
&& rm -rf rust \
&& cargo install --root /usr/local --version "=1.40.5" cargo-deb \
&& cargo install --root /usr/local --version "=0.12.2" cargo-deny \
&& cargo install --root /usr/local --version "=0.1.12" cargo2junit \
&& cargo install --root /usr/local --version "=0.9.44" cargo-nextest \
&& cargo install --root /usr/local --version "=0.1.34" cargo-udeps --features=vendored-openssl \
&& cargo install --root /usr/local --version "=0.2.15" --no-default-features --features=s3,openssl/vendored sccache

Expand Down
8 changes: 0 additions & 8 deletions ci/test/cargo-test/image/.gitignore

This file was deleted.

28 changes: 0 additions & 28 deletions ci/test/cargo-test/image/Dockerfile

This file was deleted.

19 changes: 0 additions & 19 deletions ci/test/cargo-test/image/mzbuild.yml

This file was deleted.

38 changes: 0 additions & 38 deletions ci/test/cargo-test/image/run-tests

This file was deleted.

80 changes: 44 additions & 36 deletions ci/test/cargo-test/mzcompose.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,50 +7,58 @@
# the Business Source License, use of this software will be governed
# by the Apache License, Version 2.0.

from materialize import ROOT, ci_util, spawn
from materialize.mzcompose import Composition, Service
import os

from materialize import spawn
from materialize.mzcompose import Composition, WorkflowArgumentParser
from materialize.mzcompose.services import Kafka, Postgres, SchemaRegistry, Zookeeper

SERVICES = [
Zookeeper(),
Kafka(),
Kafka(
# We need a stable port to advertise, so pick one that is unlikely to
# conflict with a Kafka cluster running on the local machine.
port="30123:30123",
allow_host_ports=True,
extra_environment=[
f"KAFKA_ADVERTISED_LISTENERS=HOST://localhost:30123,PLAINTEXT://kafka:9092",
"KAFKA_LISTENER_SECURITY_PROTOCOL_MAP=HOST:PLAINTEXT,PLAINTEXT:PLAINTEXT",
],
),
SchemaRegistry(),
Postgres(image="postgres:14.2"),
Service(
name="ci-cargo-test",
config={
"mzbuild": "ci-cargo-test",
"environment": [
"ZOOKEEPER_ADDR=zookeeper:2181",
"KAFKA_ADDRS=kafka:9092",
"SCHEMA_REGISTRY_URL=http://schema-registry:8081",
"POSTGRES_URL=postgres://postgres:postgres@postgres",
"MZ_SOFT_ASSERTIONS=1",
"MZ_PERSIST_EXTERNAL_STORAGE_TEST_S3_BUCKET=mtlz-test-persist-1d-lifecycle-delete",
"MZ_PERSIST_EXTERNAL_STORAGE_TEST_POSTGRES_URL=postgres://postgres:postgres@postgres",
"AWS_DEFAULT_REGION",
"AWS_ACCESS_KEY_ID",
"AWS_SECRET_ACCESS_KEY",
"AWS_SESSION_TOKEN",
],
"volumes": ["../../../:/workdir"],
"ulimits": {
"core": 0,
},
},
),
]


def workflow_default(c: Composition) -> None:
def workflow_default(c: Composition, parser: WorkflowArgumentParser) -> None:
parser.add_argument("args", nargs="*")
args = parser.parse_args()
c.start_and_wait_for_tcp(["zookeeper", "kafka", "schema-registry", "postgres"])
try:
c.run("ci-cargo-test", "run-tests")
finally:
junit_report = ci_util.junit_report_filename("cargo-test")
spawn.runv(
["cargo2junit"],
stdin=(ROOT / "results.json").open("rb"),
stdout=junit_report.open("wb"),
# Heads up: this intentionally runs on the host rather than in a Docker
# image. See #13010.
postgres_url = (
f"postgres://postgres:postgres@localhost:{c.default_port('postgres')}"
)
spawn.runv(
[
"cargo",
"build",
"--bin",
"storaged",
"--bin",
"computed",
]
)
spawn.runv(
["cargo", "nextest", "run", "--profile=ci", *args.args],
env=dict(
os.environ,
ZOOKEEPER_ADDR=f"localhost:{c.default_port('zookeeper')}",
KAFKA_ADDRS=f"localhost:30123",
SCHEMA_REGISTRY_URL=f"http://localhost:{c.default_port('schema-registry')}",
POSTGRES_URL=postgres_url,
MZ_SOFT_ASSERTIONS="1",
MZ_PERSIST_EXTERNAL_STORAGE_TEST_S3_BUCKET="mtlz-test-persist-1d-lifecycle-delete",
MZ_PERSIST_EXTERNAL_STORAGE_TEST_POSTGRES_URL=postgres_url,
),
ci_util.upload_junit_report("cargo-test", junit_report)
)
5 changes: 2 additions & 3 deletions ci/test/pipeline.template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,17 +113,16 @@ steps:

- id: cargo-test
label: Cargo test
depends_on: build-x86_64
timeout_in_minutes: 30
artifact_paths: junit_cargo-test_*.xml
artifact_paths: target/nextest/ci/junit_cargo-test.xml
env:
AWS_DEFAULT_REGION: "us-east-2"
plugins:
- ./ci/plugins/scratch-aws-access: ~
- ./ci/plugins/mzcompose:
composition: cargo-test
agents:
queue: linux-x86_64
queue: builder-linux-x86_64

- id: miri-test
label: Miri test
Expand Down
4 changes: 0 additions & 4 deletions doc/developer/mzbuild.md
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,6 @@ publish: true
has a custom Cargo build script, as Rust crates without a build script do
not have a build directory.

* `type: cargo-test` builds a special image that simulates `cargo test`. This
plugin is very special-cased at the moment, and unlikely to be generally
useful.

* `publish` (bool) specifies whether the image should be automatically published
to Docker Hub by CI. Non-publishable images can still be *used* by users and
CI, but they must always be built from source. Use sparingly. The default is
Expand Down
72 changes: 0 additions & 72 deletions misc/python/materialize/mzbuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,76 +341,6 @@ def inputs(self) -> Set[str]:
return super().inputs() | set(inp for dep in deps for inp in dep.inputs())


# TODO(benesch): make this less hardcoded and custom.
class CargoTest(CargoPreImage):
"""A pre-image action that builds all test binaries in the Cargo workspace.
.. todo:: This action does not generalize well.
Its implementation currently hardcodes many details about the
dependencies of various Rust crates. Ideally these dependencies would
instead be captured by configuration in `mzbuild.yml`.
"""

def __init__(self, rd: RepositoryDetails, path: Path, config: Dict[str, Any]):
super().__init__(rd, path)

def run(self) -> None:
super().run()

# NOTE(benesch): The two invocations of `cargo test --no-run` here
# deserve some explanation. The first invocation prints error messages
# to stdout in a human readable form. If that succeeds, the second
# invocation instructs Cargo to dump the locations of the test binaries
# it built in a machine readable form. Without the first invocation, the
# error messages would also be sent to the output file in JSON, and the
# user would only see a vague "could not compile <package>" error.
args = [*self.rd.cargo("test", rustflags=[]), "--locked", "--no-run"]
spawn.runv(args, cwd=self.rd.root)
output = spawn.capture(args + ["--message-format=json"], cwd=self.rd.root)

tests = []
for line in output.split("\n"):
if line.strip() == "":
continue
message = json.loads(line)
if message.get("profile", {}).get("test", False):
crate_name = message["package_id"].split()[0]
target_kind = "".join(message["target"]["kind"])
if target_kind == "proc-macro":
continue
slug = crate_name + "." + target_kind
if target_kind != "lib":
slug += "." + message["target"]["name"]
crate_path_match = re.search(
r"\(path\+file://(.*)\)", message["package_id"]
)
if not crate_path_match:
raise ValueError(f'invalid package_id: {message["package_id"]}')
crate_path = Path(crate_path_match.group(1)).relative_to(
self.rd.root.resolve()
)
executable = self.rd.rewrite_builder_path_for_host(
Path(message["executable"])
)
tests.append((executable, slug, crate_path))

os.makedirs(self.path / "tests" / "examples")
with open(self.path / "tests" / "manifest", "w") as manifest:
for (executable, slug, crate_path) in tests:
shutil.copy(executable, self.path / "tests" / slug)
spawn.runv(
[*self.rd.tool("strip"), self.path / "tests" / slug],
cwd=self.rd.root,
)
package = slug.replace(".", "::")
manifest.write(f"{slug} {package} {crate_path}\n")
shutil.copytree(self.rd.root / "misc" / "shlib", self.path / "shlib")

def inputs(self) -> Set[str]:
crates = self.rd.cargo_workspace.crates.values()
return super().inputs() | set(inp for crate in crates for inp in crate.inputs())


class Image:
"""A Docker image whose build and dependencies are managed by mzbuild.
Expand Down Expand Up @@ -442,8 +372,6 @@ def __init__(self, rd: RepositoryDetails, path: Path):
typ = pre_image.pop("type", None)
if typ == "cargo-build":
self.pre_images.append(CargoBuild(self.rd, self.path, pre_image))
elif typ == "cargo-test":
self.pre_images.append(CargoTest(self.rd, self.path, pre_image))
elif typ == "copy":
self.pre_images.append(Copy(self.rd, self.path, pre_image))
else:
Expand Down
4 changes: 3 additions & 1 deletion misc/python/materialize/mzcompose/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,8 @@ def __init__(
name: str = "kafka",
image: str = "confluentinc/cp-kafka",
tag: str = DEFAULT_CONFLUENT_PLATFORM_VERSION,
port: int = 9092,
port: Union[str, int] = 9092,
allow_host_ports: bool = False,
auto_create_topics: bool = False,
broker_id: int = 1,
offsets_topic_replication_factor: int = 1,
Expand Down Expand Up @@ -343,6 +344,7 @@ def __init__(
config: ServiceConfig = {
"image": f"{image}:{tag}",
"ports": [port],
"allow_host_ports": allow_host_ports,
"environment": [
*environment,
f"KAFKA_AUTO_CREATE_TOPICS_ENABLE={auto_create_topics}",
Expand Down
Loading

0 comments on commit 0275079

Please sign in to comment.