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.

Sign up for GitHub

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

Jump to bottom

Dispatch MM unit test workflows based on modular-magician downstream SHAs instead of branches #17924

Open
melinath opened this issue Apr 19, 2024 · 1 comment · May be fixed by GoogleCloudPlatform/magic-modules#10671
Open

Dispatch MM unit test workflows based on modular-magician downstream SHAs instead of branches #17924

melinath opened this issue Apr 19, 2024 · 1 comment · May be fixed by GoogleCloudPlatform/magic-modules#10671
Assignees
@shuyama1
Labels
magician-pr-ux size/s technical-debt
Milestone
Near-Term Goals

Comments

@melinath
Copy link
Collaborator

What kind of contribution is this issue about?

Other (specify in details)

Details

We currently dispatch workflows like https://github.com/GoogleCloudPlatform/magic-modules/blob/main/.github/workflows/test-tgc.yml based on a branch, which can lead to issues with TGC specifically where it can end up installing an older version of TPG (and then failing to compile) - for example, https://github.com/GoogleCloudPlatform/magic-modules/actions/runs/8759519595/job/24042712410.

This could also hypothetically lead to race conditions if multiple commits are pushed close to each other, resulting in unit tests running for a newer SHA than intended.

We should pass specific SHAs to GHA workflows - and for TGC we should pass a SHA for TPGB in addition to one for TGC.

Ideally we would pass the downstream SHAs from the cloud build step that commits them to the steps that consume them, but I'm not sure offhand whether cloud build supports that.

References

No response
@melinath melinath added this to the Near-Term Goals milestone Apr 22, 2024
@melinath melinath assigned melinath and shuyama1 and unassigned ScottSuarez and melinath Apr 30, 2024
@shuyama1 shuyama1 linked a pull request May 13, 2024 that will close this issue
Draft
@shuyama1
Copy link
Collaborator

This could also hypothetically lead to race conditions if multiple commits are pushed close to each other, resulting in unit tests running for a newer SHA than intended.

I don't think this is possible as we use concurrency to group the same job runs for each PR (with different commits), so as soon as a new commit is pushed, it will kick off a new job and the old one will be cancelled immediately. So I think the issue for this is always that the TGC is built on an older version of TPGB due to some caching issues, as we use upstream branch @auto-pr-xxxx to patch the version. Removing cacheing should potentially resolve this issue easily, I think. But I don't think it's a desirable approach that we'd like to move forward with. And yes, passing the exact upstream commit SHA is definitely more "correct" in this case. I followed the Cloud Build Guide to pass upstream commit SHA between build steps, but I just wish there's a more "straightforward" or "elegant" way for GCB to do the job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shuyama1 shuyama1

Labels
magician-pr-ux size/s technical-debt
Projects
None yet
Milestone
Near-Term Goals
Development

Successfully merging a pull request may close this issue.

Dispatch MM unit test workflows based on modular-magician downstream SHAs shuyama1/magic-modules
3 participants
@melinath @ScottSuarez @shuyama1