Skip to content
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

chore(ci): Add capability to build and push test images to GCR. #812

Merged
merged 9 commits into from
Dec 20, 2024

Conversation

b4handjr
Copy link
Collaborator

@b4handjr b4handjr commented Dec 18, 2024

Adds the steps needed to build and push our containerized tests to GCR.

SYNC-4559

@@ -357,6 +340,7 @@ jobs:
default: build
image:
type: string
default: docker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably isn't necessary since we're not specifying a default for either of the build jobs and we're passing this parameter from every job that calls it.

Copy link
Contributor

@emaydeck-mozilla emaydeck-mozilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. If you want, you can test this out by having it temporarily filter against your branch instead of master in the job definitions. Once your push up a commit in your branch, CircleCI should try to build and deploy the images and I can confirm whether or not they were successfully pushed to GAR.

jrconlin
jrconlin previously approved these changes Dec 18, 2024
@b4handjr b4handjr dismissed stale reviews from jrconlin and emaydeck-mozilla via e3c258b December 18, 2024 18:57
@b4handjr b4handjr force-pushed the sync-4559 branch 2 times, most recently from 9d044e2 to 1a4956c Compare December 18, 2024 19:35
@b4handjr b4handjr force-pushed the sync-4559 branch 2 times, most recently from af23d5a to 6c586fa Compare December 19, 2024 00:14
Trinaa
Trinaa previously approved these changes Dec 20, 2024
Copy link
Contributor

@Trinaa Trinaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-approving so that you are not blocked over the holidays @b4handjr.

I suggest to take this opportunity to rename the 'notification' tests, which is not a commonly recognized test category or level , to 'end-to-end' tests.

Comment on lines 482 to 483
branches:
only: master
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a departure from the way we have the PR builds setup currently. We usually build the load-test image in PR builds without deploying, so developers can be signaled before they merge if they have introduced a breaking change. I suggest to remove this filter and perhaps the one for the notification test container as well for the same reason.

@b4handjr b4handjr merged commit 6f614f9 into master Dec 20, 2024
1 check passed
@b4handjr b4handjr deleted the sync-4559 branch December 20, 2024 18:17
b4handjr added a commit that referenced this pull request Jan 9, 2025
When I was working on #812 I mislabeled the autoconnect/endpoint docker
images in the deploy step. This should fix that. I am also running those
steps on CI to make sure everything works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants