Skip to content

Provide cockroach in test env#2855

Merged
AlCutter merged 1 commit intogoogle:masterfrom
AlCutter:mo_roach
Nov 29, 2022
Merged

Provide cockroach in test env#2855
AlCutter merged 1 commit intogoogle:masterfrom
AlCutter:mo_roach

Conversation

@AlCutter
Copy link
Copy Markdown
Member

Install CockroachDB in the testbase image.

Checklist

@AlCutter AlCutter requested a review from phbnf November 28, 2022 18:14
@AlCutter AlCutter marked this pull request as ready for review November 28, 2022 18:14
@AlCutter AlCutter requested a review from a team as a code owner November 28, 2022 18:14
@AlCutter AlCutter requested review from roger2hk and removed request for phbnf November 28, 2022 18:15
Copy link
Copy Markdown
Contributor

@roger2hk roger2hk left a comment

Choose a reason for hiding this comment

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

LGTM but we have to keep the tech debt (repo complexity) in mind.

@JAORMX
Copy link
Copy Markdown
Contributor

JAORMX commented Nov 29, 2022

@roger2hk normally that binary is fetched directly at the test setup. However, it seems that the test infra no longer allows egress access. So doing this was necessary.

In GitHub actions it's possible to download the binary and cache it if needed (making it available for other test runs in a determined period of time). I'm not sure if that's easy to do with cloudbuild. One option would be for us to limit the cloudbuild tests to not run the crdb test cases and only run those in GH actions. That way we'd keep the test images small.

@AlCutter
Copy link
Copy Markdown
Member Author

LGTM but we have to keep the tech debt (repo complexity) in mind.

Good call, what do you think would be a good way to help toward that here?

I'll submit this for now so PRs are unblocked, but more than happy to change approach in a follow-up.

@AlCutter AlCutter merged commit 988f4e1 into google:master Nov 29, 2022
@AlCutter AlCutter deleted the mo_roach branch November 29, 2022 15:52
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.

3 participants