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

TeamCity: Ensure nightly tests use same git commit #10785

Merged
merged 14 commits into from
Sep 9, 2024

Conversation

BBBmau
Copy link
Collaborator

@BBBmau BBBmau commented May 24, 2024

PR insures that nightly tests will run on a separate branch from main. The branch nightly-test will be created with a GHA job set in terraform-google-provider repo and each night the branch will be recreated from the tip of main. Effectively this tags a single commit for TeamCity to use in a given night's tests

(Related PR: hashicorp/terraform-provider-google#18241 which must be merged first)

New related PRs that need to be merged first:

Release Note Template for Downstream PRs (will be copied)


@BBBmau BBBmau changed the title initial teamcity nightly config update TeamCity: Ensure nightly tests use same git commit May 24, 2024
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 7 insertions(+), 6 deletions(-))

@BBBmau BBBmau marked this pull request as ready for review May 24, 2024 09:41
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 11 insertions(+), 6 deletions(-))

@BBBmau
Copy link
Collaborator Author

BBBmau commented May 24, 2024

PR is pretty much ready for review, the only concern I have is that it could be confusing for future users due to the date. the date is set based on UTC time zone. I added UTC into the branch name to prevent any confusion. Other then that it should be good to go.

The next thing would be to add a sweeper that removes branches after two weeks.

@SarahFrench SarahFrench self-requested a review May 24, 2024 14:07
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 11 insertions(+), 6 deletions(-))

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 11 insertions(+), 6 deletions(-))

Copy link

This PR has been waiting for review for 2 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

@BBBmau
Copy link
Collaborator Author

BBBmau commented Jun 11, 2024

Linking this build which has been using the teamcity-nightly-test workflow

it also prints out a hello world! along with the branch that it just ran the build on.

https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_MauricioSTestProjectSameCommitNightlyTestsWork_GOOGLE_NIGHTLYTESTS_GOOGLE_PACKAGE_ACCESSAPPROVAL/163757?buildTab=log&logFilter=debug&focusLine=32&logView=linear

A previous build can be seen here as well:

https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_MauricioSTestProjectSameCommitNightlyTestsWork_GOOGLE_NIGHTLYTESTS_GOOGLE_PACKAGE_ACCESSAPPROVAL/153512?buildTab=log&focusLine=525&logView=flowAware&linesState=520

We can see that only one branch is present, which makes sense since it would have deleted the branches from friday since the threshold had passed (this is something we should keep in mind actually. A good enough reason to increase it to 5 days)

Looks like it's triggering the nightly tests on weekends also. Another PR will be made to update the cronjob to only run on weekdays 0 4 * * 1-5 It's intended to run every night.

@SarahFrench
Copy link
Collaborator

@BBBmau our nightly tests run every night of the week- the automation to add branches and the TeamCity CRON schedule should run every night

@BBBmau
Copy link
Collaborator Author

BBBmau commented Jun 11, 2024

Refer to these nightly tests that print the nightly-test branch name and commit hash from main, this simulates the behavior that we shall see when this PR gets merged into magic modules:

https://hashicorp.teamcity.com/project/TerraformProviders_MauricioSTestProjectSameCommitNightlyTestsWork_GOOGLE_NIGHTLYTESTS?branch=%3Cdefault%3E&buildTypeTab=overview&mode=builds

The teamcity config that was applied can be found here: BBBmau/terraform-provider-google@4acef83

Apart from the changes made in order to output hello world for my forked branch, the change is exact to what's found in this PR.

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 10 insertions(+), 6 deletions(-))

@BBBmau
Copy link
Collaborator Author

BBBmau commented Jun 14, 2024

new linked PR allows static branch filtering: we now only build if we detect a branch that matches both rules:
+:refs/heads/UTC-*
-:refs/heads/UTC-nightly-*

image

Once a period of time has passed (12 hours) the newly used branch for nightly tests will be renamed to match -:refs/heads/UTC-nightly-*. Allowing a new branch to be created which will be the only branch matching both rules. (As seen in the image above)

An explanation of the new gha workflow for this can be seen: hashicorp/terraform-provider-google#18447

Link to the TeamCity builds can be seen: https://hashicorp.teamcity.com/project/TerraformProviders_MauricioSTestProjectSameCommitNightlyTestsWork_GOOGLE_NIGHTLYTESTS?branch=refs%2Fheads%2FUTC-2024-06-14&mode=builds

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 6 insertions(+), 7 deletions(-))

1 similar comment
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 6 insertions(+), 7 deletions(-))

@SarahFrench
Copy link
Collaborator

SarahFrench commented Jun 18, 2024

I took a look at builds from your test project and from this page I can see that a build running on the 18th picked up refs/heads/UTC-2024-06-18 but prior to that refs/heads/UTC-2024-06-14 was used for multiple nights:

Screenshot 2024-06-18 at 14 13 50

I believe that's because that's because the config in TeamCity was updated on the 17th to use the new branch names (I can see that the config in your TeamCity test project was updated on the 17th), so I think we should wait another day or two to confirm that refs/heads/UTC-2024-06-19 and refs/heads/UTC-2024-06-20 get used as expected.

@BBBmau
Copy link
Collaborator Author

BBBmau commented Jun 18, 2024

I took a look at builds from your test project and from this page I can see that a build running on the 18th picked up refs/heads/UTC-2024-06-18 but prior to that refs/heads/UTC-2024-06-14 was used for multiple nights:

Screenshot 2024-06-18 at 14 13 50

I believe that's because that's because the config in TeamCity was updated on the 17th to use the new branch names (I can see that the config in your TeamCity test project was updated on the 17th), so I think we should wait another day or two to confirm that refs/heads/UTC-2024-06-19 and refs/heads/UTC-2024-06-20 get used as expected.

Agreed! The first successful branch creation and nightly build was UTC-2024-06-18. an extra two days will be enough to confirm the behavior for running nightly tests.

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 10 files changed, 18 insertions(+), 7 deletions(-))

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 10 files changed, 22 insertions(+), 7 deletions(-))

@BBBmau
Copy link
Collaborator Author

BBBmau commented Jun 21, 2024

Looks to be good now, we'll let it run tests over the weekend and determine whether it's ready based off of the weekend results.

@BBBmau
Copy link
Collaborator Author

BBBmau commented Jun 24, 2024

@SarahFrench update:

Saw this morning that inactive builds were not being kept after removing a branch. I found this odd since the activeBuildBranch setting is set to 10 days:
hiddenVariable("teamcity.activeBuildBranch.age.days", "10")

after looking into this more I realized that my assumption of activeBuildBranch having the parameter age.days that is used by activeVCSbranches was wrong. activeBuildBranch works when setting the age by hours. This was confirmed after applying age.days == 0 and not seeing the inactive branch instantly disappear. This did happen when setting age.hours == 0, resulting in the deleted branch disappearing from teamcity ui instantly.

The latest commit now uses age.hours with the following activebranch setting:
hiddenVariable("teamcity.activeBuildBranch.age.hours", "240") this allows inactive branches (aka deleted branches) to still be present and will disappear after 10 days of inactivity.

We'll revisit this PR wednesday morning and consider merging once we see that inactive builds are still kept after the default age of 24 hours.

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 10 files changed, 22 insertions(+), 7 deletions(-))

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 10 files changed, 25 insertions(+), 7 deletions(-))

@BBBmau
Copy link
Collaborator Author

BBBmau commented Jun 24, 2024

@BBBmau BBBmau requested a review from SarahFrench June 24, 2024 21:49
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 6 insertions(+), 6 deletions(-))

1 similar comment
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 6 insertions(+), 6 deletions(-))

@BBBmau
Copy link
Collaborator Author

BBBmau commented Aug 7, 2024

recent commit is based off of wanting to use a static branch filter naming for teamcity to use such as nightly-test. This allows us to not have to set activeBranchSettings which were removed in the recent commits. Through this can keep all nightly-builds on one branch without needing to worry about test history. and example as well as an update to the github workflows can be found here: hashicorp/terraform-provider-google#19023

TeamCity UI with nightly-test branch:
image

Copy link
Collaborator

@SarahFrench SarahFrench left a comment

Choose a reason for hiding this comment

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

This PR looks good to me, but I think we need a final round of testing before merging.

I think it'd be good for us to force a scenario where the test project has multiple builds in the queue (from the cron trigger) and we merge new commits to main in the provider repo that's being tested by those builds. If the later builds (that run after the new commit was added) use the same commit as the prior builds we'll know that we've solved the problem, and there'll be less risk of disrupting the nightly tests by merging this PR.

Lets pair on doing that in our next 1:1 and take it from there

Copy link
Collaborator

@SarahFrench SarahFrench left a comment

Choose a reason for hiding this comment

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

Found a small problem that's an easy fix!

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 7 insertions(+), 7 deletions(-))

Copy link

@BBBmau, this PR is waiting for action from you. Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

Image showing the re-request review button

If no action is taken, this PR will be closed in 28 days.

This notification can be disabled with the disable-automatic-closure label.

Copy link
Collaborator

@SarahFrench SarahFrench left a comment

Choose a reason for hiding this comment

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

OK to merge and release!

@BBBmau BBBmau merged commit cc6f95c into GoogleCloudPlatform:main Sep 9, 2024
10 checks passed
@SarahFrench
Copy link
Collaborator

image

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