Address issues in TeamCity configuration code #8560

SarahFrench · 2023-08-04T18:22:52Z

(Apologies for the size of this PR/refactor 🙏 )

To see the config changes from this PR in action, see this test project in TeamCity: https://hashicorp.teamcity.com/project/TerraformProviders_Google_SarahTestMainBranchPr8560applied

Description

This PR updates code in .teammcity/ to change:

General refactoring; lots of code in these files was copied from the Azure provider and either could be refactored or removed.
Fixed sweeper behaviour:
- Pre-sweeper runs once, before any acc
- Post-sweeper runs once, after all acc tests finish
- The sweeper builds run all sweepers for all resources in one build
All builds will fail if running for more than 12h (this is per individual build, not across the whole build chain)
Builds will run even if there are no code changes since the last run
- this will allow tests to run at the scheduled time (4am) on Sunday and Monday, despite no new commits since the last run at 4am Saturday
Builds will have ids reflecting the commit hash they're using
Add ability to run tests from 'root' directory as well as service packages. This means we do not need tests to be migrated at the same time as transitioning to the new TeamCity projects
Update strategy when triggering builds with a cron trigger:
- A build chain is used to set up dependencies between build configurations within the Project instance
- Only the post-sweeper has a cron trigger. Once it's triggered all builds 'upstream' in the build chain are triggered too.

Release Note Template for Downstream PRs (will be copied)

…figs are made

…toring

…nt in Google service packages

Sweepers running after each build config will clash with other running tests

…out need for refactoring code

…between both

… update how maps are generated, update tools/teamcity-generator to match

SarahFrench

Added some notes to parts of the code to help with review.

mmv1/third_party/terraform/.teamcity/components/sweepers.kt

SarahFrench · 2023-08-09T10:19:03Z

mmv1/third_party/terraform/.teamcity/components/generated/settings.erb

@@ -10,7 +10,7 @@
 var defaultStartHour = 4

 // specifies the default level of parallelism per-service-package
-var defaultParallelism = 12
+var defaultParallelism = 20


Parallelism set to 20, matching the current test project.

I have previously mentioned how this could need reducing once tests are split into multiple packages.

mmv1/third_party/terraform/.teamcity/components/build_config_package.kt

SarahFrench · 2023-08-09T12:15:44Z

After getting some input from @alexsomesan I realised that the dependencies can be made simpler following the recent refactor I did. Instead of defining dependencies on the build configurations themselves, I'll use code similar to this pseudocode below when I register the build configurations in the project:

sequential {
        buildType(preSweeperConfig)
        parallel {
                <all service packages + other packages>.forEach { buildConfiguration ->
                        buildType(buildConfiguration)
                }
        }
        buildType(postSweeperConfig)
}

See commit d41e116 below for this

…er needed as a result

modular-magician · 2023-08-09T14:37:33Z

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.

Terraform GA: Diff ( 11 files changed, 882 insertions(+), 238 deletions(-))
Terraform Beta: Diff ( 11 files changed, 923 insertions(+), 247 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

SarahFrench · 2023-08-09T14:39:16Z

mmv1/third_party/terraform/.teamcity/components/generated/project.erb

+        // Register build configs in the project
+        buildType(preSweeperConfig)
+        (packageConfigs + servicePackageConfigs).forEach { buildConfiguration ->
            buildType(buildConfiguration)
        }
+        buildType(postSweeperConfig)
+
+        // Set up dependencies between builds using `sequential` block
+        // Acc test builds run in parallel
+        sequential {
+            buildType(preSweeperConfig)
+
+            parallel{
+                (packageConfigs + servicePackageConfigs).forEach { buildConfiguration ->
+                    buildType(buildConfiguration)
+                }
+            }
+
+            buildType(postSweeperConfig)
+        }


This code is where all the build configurations are put in the Project being created, and the sequential and nested parallel block are what create dependencies between builds.

SarahFrench · 2023-08-09T14:39:48Z

mmv1/third_party/terraform/.teamcity/components/generated/project.erb

+    // Add trigger to last step of build chain (post-sweeper)
+    val triggerConfig = NightlyTriggerConfiguration(environment, branchRef)
+    postSweeperConfig.addTrigger(triggerConfig)


This is the only place where CRON triggers are defined and attached to a build.

mmv1/third_party/terraform/.teamcity/components/sweepers.kt

mmv1/third_party/terraform/.teamcity/components/generated/project.erb

zli82016

LGTM. Thanks for working hard on this. Just left a couple of refactor comments.

shuyama1

Thanks for the work, @SarahFrench!

modular-magician · 2023-08-09T18:32:57Z

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.

Terraform GA: Diff ( 11 files changed, 859 insertions(+), 238 deletions(-))
Terraform Beta: Diff ( 11 files changed, 900 insertions(+), 247 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

* Refactor: Pass `packageName` into packageDetails constructor instead * Refactor how nightly trigger information is passed to where build configs are made * Remove lingering references to `"public"` environment * Remove unnecessary assignments and func parameters * Fix inconsistent use of `var`/`val` * Refactor test steps into separate functions, to help with later refactoring * Remove `DetermineWorkingDirectory` as /tests directories aren't present in Google service packages * Remove sweeper steps from per-service build configurations Sweepers running after each build config will clash with other running tests * Stop setting `SWEEPER_REGIONS` when sweepers aren't in a build config * Add ability to set build ID as git commit hash * Refactor other script to use multiline string * Refactor how build config ids are built * Add pre-sweeper build and make acc tests dependent on it finishing * Add post-sweeper and ability to make it dependent on all other builds * Make tests run even if there are no new commits * Add timeout failure conditon to all builds * Increase parallelism from 12 -> 20 to match old TC project * Make sure sweepers have parameters to configure tests * Add ability to run tests from `google/-beta` package, and comments about need for refactoring code * Add properties to cron trigger present in old project * Make build step names more informative, update comment * Fix error where two parameters called `timeout`; clarify distinction between both * Initial refactor of how package lists made, and how used in Kotlin code * WIP: separate addition of build configs to project * Refactor TeamCity config to use different structure of maps as input, update how maps are generated, update tools/teamcity-generator to match * Change how builds are triggered Avoid duplicate builds being queued due to cron and dependency requirements. Instead trigger fewer things and let dependency relationships trigger the rest. * Make packages.kt provide pre-migration path for sweepers * Remove excess argument left from refactoring * Update ignored folders in generation of packages.kt * Simplify how dependencies are created, and remove code that's no longer needed as a result * Combine `failureConditions` blocks * Fix test prefix * Refactor to reduce code repetition around sweepers

SarahFrench added 16 commits August 2, 2023 11:43

Refactor: Pass packageName into packageDetails constructor instead

9104be2

Refactor how nightly trigger information is passed to where build con…

f9f631f

…figs are made

Remove lingering references to "public" environment

13bb0d3

Remove unnecessary assignments and func parameters

6ca35ac

Fix inconsistent use of var/val

2e6658a

Refactor test steps into separate functions, to help with later refac…

27e8133

…toring

Remove DetermineWorkingDirectory as /tests directories aren't prese…

984ee6f

…nt in Google service packages

Remove sweeper steps from per-service build configurations

f06a691

Sweepers running after each build config will clash with other running tests

Stop setting SWEEPER_REGIONS when sweepers aren't in a build config

47c92c7

Add ability to set build ID as git commit hash

9a2d05e

Refactor other script to use multiline string

29f8e5f

Refactor how build config ids are built

d5d9be7

Add pre-sweeper build and make acc tests dependent on it finishing

2d3e9c3

Add post-sweeper and ability to make it dependent on all other builds

d011bfa

Make tests run even if there are no new commits

9a580ed

Add timeout failure conditon to all builds

f78dda6

This comment was marked as outdated.

Sign in to view

Increase parallelism from 12 -> 20 to match old TC project

ebaca6a

This comment was marked as outdated.

Sign in to view

SarahFrench added 2 commits August 5, 2023 15:51

Make sure sweepers have parameters to configure tests

3adaad7

Add ability to run tests from google/-beta package, and comments ab…

470fdc9

…out need for refactoring code

This comment was marked as outdated.

Sign in to view

SarahFrench added 2 commits August 7, 2023 12:55

Add properties to cron trigger present in old project

8757fca

Make build step names more informative, update comment

b82f120

This comment was marked as outdated.

Sign in to view

Fix error where two parameters called timeout; clarify distinction …

00b6e2a

…between both

This comment was marked as outdated.

Sign in to view

SarahFrench added 3 commits August 7, 2023 17:26

Initial refactor of how package lists made, and how used in Kotlin code

324ccfb

WIP: separate addition of build configs to project

841dfbc

Refactor TeamCity config to use different structure of maps as input,…

333d01b

… update how maps are generated, update tools/teamcity-generator to match

SarahFrench commented Aug 9, 2023

View reviewed changes

SarahFrench added 3 commits August 9, 2023 15:21

Simplify how dependencies are created, and remove code that's no long…

d41e116

…er needed as a result

Combine failureConditions blocks

32e9eb1

Fix test prefix

438fac3

SarahFrench commented Aug 9, 2023

View reviewed changes

zli82016 reviewed Aug 9, 2023

View reviewed changes

mmv1/third_party/terraform/.teamcity/components/sweepers.kt Outdated Show resolved Hide resolved

zli82016 reviewed Aug 9, 2023

View reviewed changes

mmv1/third_party/terraform/.teamcity/components/generated/project.erb Outdated Show resolved Hide resolved

zli82016 approved these changes Aug 9, 2023

View reviewed changes

shuyama1 approved these changes Aug 9, 2023

View reviewed changes

Refactor to reduce code repetition around sweepers

f091fa9

SarahFrench merged commit 7687d4d into main Aug 9, 2023

SarahFrench mentioned this pull request Aug 9, 2023

TeamCity: Address edge case of google-beta package containing a hyphen #8594

Merged

This comment was marked as outdated.

Sign in to view

This was referenced Aug 9, 2023

TeamCity: Address edge case of google-beta package containing a hyphen hashicorp/terraform-provider-google-beta#6039

Merged

TeamCity: Address edge case of google-beta package containing a hyphen hashicorp/terraform-provider-google#15449

Merged

SarahFrench mentioned this pull request Aug 9, 2023

TeamCity : Re-add templating of alternate cron days of week for feature branchs #8597

Merged

modular-magician mentioned this pull request Aug 9, 2023

TeamCity : Re-add templating of alternate cron days of week for feature branchs hashicorp/terraform-provider-google-beta#6040

Merged

SarahFrench mentioned this pull request Aug 10, 2023

TeamCity: Update cron to use Sun=1...Sat=7 days of week #8601

Merged

SarahFrench deleted the clean-up-teamcity-config branch March 19, 2024 21:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address issues in TeamCity configuration code #8560

Address issues in TeamCity configuration code #8560

SarahFrench commented Aug 4, 2023 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

SarahFrench left a comment

SarahFrench Aug 9, 2023

SarahFrench commented Aug 9, 2023 •

edited

Loading

modular-magician commented Aug 9, 2023

SarahFrench Aug 9, 2023

SarahFrench Aug 9, 2023

zli82016 left a comment

shuyama1 left a comment

modular-magician commented Aug 9, 2023

This comment was marked as outdated.

Address issues in TeamCity configuration code #8560

Address issues in TeamCity configuration code #8560

Conversation

SarahFrench commented Aug 4, 2023 • edited Loading

Description

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

SarahFrench left a comment

Choose a reason for hiding this comment

SarahFrench Aug 9, 2023

Choose a reason for hiding this comment

SarahFrench commented Aug 9, 2023 • edited Loading

modular-magician commented Aug 9, 2023

Diff report

SarahFrench Aug 9, 2023

Choose a reason for hiding this comment

SarahFrench Aug 9, 2023

Choose a reason for hiding this comment

zli82016 left a comment

Choose a reason for hiding this comment

shuyama1 left a comment

Choose a reason for hiding this comment

modular-magician commented Aug 9, 2023

Diff report

This comment was marked as outdated.

SarahFrench commented Aug 4, 2023 •

edited

Loading

SarahFrench commented Aug 9, 2023 •

edited

Loading