-
Notifications
You must be signed in to change notification settings - Fork 23
CLOUDP-339241 - re-enable multi arch smoke tests #539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
MCK 1.6.0 Release NotesNew Features
Bug Fixes
Other Changes
|
411ad0d to
0937335
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR re-enables multi-architecture (IBM Z and IBM Power) smoke tests for release builds by migrating the meko-test image from the public Quay.io registry to a staging ECR repository. This prevents test images from being publicly available to customers during release processes.
Key changes include:
- Added support for IBM Power and IBM Z architectures with podman builder configuration
- Updated repository URLs from
quay.io/mongodb/mongodb-kubernetes-teststo268558157000.dkr.ecr.us-east-1.amazonaws.com/staging/mongodb-kubernetes-testsfor release scenarios - Enhanced build pipeline to support both Docker and Podman builders for different architectures
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/release/tests/build_info_test.py |
Updates test configurations to include IBM Power and Z architectures with staging ECR repositories |
scripts/release/pipeline.py |
Adds new image constants and builder support, updates argument parsing for skip-if-exists flag |
scripts/release/build/build_info.py |
Adds builder constants and integrates builder field into ImageInfo configuration |
scripts/release/atomic_pipeline.py |
Implements podman build support alongside existing Docker functionality |
build_info.json |
Defines multi-arch image configurations with staging repositories for release scenarios |
.evergreen.yml |
Updates build variants and tasks to support separate IBM Power and Z builds |
.evergreen-release.yml |
Creates dedicated release variants for multi-arch smoke testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
04bdb0a to
0dbe506
Compare
0dbe506 to
19d5534
Compare
| if build_configuration.builder == BUILDER_PODMAN: | ||
| logger.info( | ||
| f"Building image with podman, tags {tags} for platforms={build_configuration.platforms}, dockerfile args: {build_args}" | ||
| ) | ||
| try: | ||
| build_command = [ | ||
| "sudo", | ||
| "podman", | ||
| "buildx", | ||
| "build", | ||
| "--progress", | ||
| "plain", | ||
| build_path, | ||
| "-f", | ||
| build_configuration.dockerfile_path, | ||
| ] | ||
| for tag in tags: | ||
| build_command.extend(["-t", tag]) | ||
| for key, value in build_args.items(): | ||
| build_command.extend(["--build-arg", f"{key}={value}"]) | ||
|
|
||
| result = subprocess.run(build_command, capture_output=True, text=True, check=True) | ||
| logger.debug(result.stdout) | ||
|
|
||
| for tag in tags: | ||
| push_command = ["sudo", "podman", "push", "--authfile=/root/.config/containers/auth.json", tag] | ||
| result = subprocess.run(push_command, capture_output=True, text=True, check=True) | ||
| logger.debug(result.stdout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic seems better suited for execute_docker_build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not execute_docker_build because it's using docker ;) but I agree this can be encapsulated in separate method with similar signature as execute_docker_build. Or you meant something else?
| build_command = [ | ||
| "sudo", | ||
| "podman", | ||
| "buildx", | ||
| "build", | ||
| "--progress", | ||
| "plain", | ||
| build_path, | ||
| "-f", | ||
| build_configuration.dockerfile_path, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the platform flag is not used (except for the tag). I know it is implied by the machine it runs on, but for clarity we should set it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the platform flag being used before:
sudo podman buildx build --progress plain . -f Dockerfile -t "${REGISTRY}/mongodb-kubernetes-tests:${OPERATOR_VERSION}-$(arch)" --build-arg PYTHON_VERSION="${PYTHON_VERSION}"
| - release-rhel9-power-small # This is required for CISA attestation https://jira.mongodb.org/browse/DEVPROD-17780 | ||
| - release-rhel9-power-large # This is required for CISA attestation https://jira.mongodb.org/browse/DEVPROD-17780 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a private test image, why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well it's not a big deal, but we are using this test image to verify that our released images are passing smoke tests. In theory somebody from outside could interfere with test image build process and acquire stored credentials for quay.io for instance
| export CUSTOM_OM_VERSION | ||
|
|
||
| # During release the meko-tests image is pushed to a staging registry. We don't want to push it to production registry | ||
| export MEKO_TESTS_REGISTRY="268558157000.dkr.ecr.us-east-1.amazonaws.com/staging" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this logic be in evg-private-context? We will never use quay for test image, so I think this can be simplified
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, this can be also in evg-private-context. In the future evg-private-context should read repository settings from build_info.json instead of having them hardcoded. If you have nothing against this I would rather target this in separate PR and use build_info.json properly.
| architecture_suffix=True, | ||
| ), | ||
| "meko-tests-ibm-z": ImageInfo( | ||
| repositories=["268558157000.dkr.ecr.us-east-1.amazonaws.com/dev/mongodb-kubernetes-tests"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: It would be better to use constants for the repetitive dev and staging repos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, there is one nit and another issue the Lucian already mentioned:
https://github.com/mongodb/mongodb-kubernetes/pull/539/files#r2490868628
Summary
Key changes include:
quay.io/mongodb/mongodb-kubernetes-teststo268558157000.dkr.ecr.us-east-1.amazonaws.com/staging/mongodb-kubernetes-testsfor release scenarios. This will preventmeko-testimage to be available also for our customersProof of Work
Staging job is successful -> https://spruce.mongodb.com/version/69033a1e9701b900075d742f (ignore ibm z which is disabled in updated code)
Release job is successful -> https://spruce.mongodb.com/version/69036de79701b900075df4de (ignore GKE code snippets tasks, they need different fix. create_chart_release_pr will fail due to missing tag)
We are not ready yet to enable smoke tests for IBM Z, because our evergreen machines are unstable -> https://jira.mongodb.org/browse/DEVPROD-23283
Checklist
skip-changeloglabel if not needed