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: Runs tests for latest minor versions of each major TF version #1769

Merged
merged 10 commits into from
Dec 19, 2023

Conversation

lantoli
Copy link
Member

@lantoli lantoli commented Dec 18, 2023

Description

Runs tests for latest minor versions of each major TF version.

  • Allows to run manually Test Suite for a matrix of versions, by default it has the latest minor versions of each major TF version.
  • Concurrency is limited to one Test Suite run at a time. You can always stop any run you don't need any more.
  • However concurrency for Acceptance or Migration workflows (run manually or from a PR) is not limited.

Link to any related issue(s): https://jira.mongodb.org/browse/CLOUDP-218618

Type of change:

  • Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
  • New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR.
  • Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR.
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the contribution guidelines
  • I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
  • I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code
  • If changes include deprecations or removals, I defined an isolated PR with a relevant title as it will be used in the auto-generated changelog.

Further comments

@lantoli lantoli force-pushed the CLOUDP-218618_test_suite branch from 74e1df0 to 71c50d4 Compare December 18, 2023 22:09
@lantoli lantoli marked this pull request as ready for review December 18, 2023 23:11
@lantoli lantoli requested a review from a team as a code owner December 18, 2023 23:11
inputs:
terraform_matrix:
description: 'Terraform matrix version (JSON array)'
default: '["1.6.x", "1.5.x", "1.4.x", "1.3.x", "1.2.x"]'
Copy link
Member Author

Choose a reason for hiding this comment

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

I checked that this notation works, e.g. 1.4.x will run version 1.4.7.
TF version list is here: https://releases.hashicorp.com/terraform/

@@ -1 +1,2 @@
golang 1.21.3
terraform 1.6.6
Copy link
Member Author

@lantoli lantoli Dec 18, 2023

Choose a reason for hiding this comment

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

opportunistic change not related to the main goal of the PR

@lantoli lantoli changed the title ci: Runs tests for latest minor versions of each major TF version chore: Runs tests for latest minor versions of each major TF version Dec 18, 2023

concurrency:
group: '${{ github.workflow }}'
cancel-in-progress: false
Copy link
Member Author

Choose a reason for hiding this comment

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

only one Test Suite run at a time to avoid interference. new runs will wait until previous ones finish

versions:
env:
schedule_terraform_matrix: '["1.6.x", "1.0.8"]'
schedule_provider_matrix: '["", "1.12.3]' # "" for latest version
Copy link
Member Author

@lantoli lantoli Dec 18, 2023

Choose a reason for hiding this comment

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

schedule triggers can't have input parameters so we define the values we want here

default: '["1.6.x", "1.5.x", "1.4.x", "1.3.x", "1.2.x"]'
provider_matrix:
description: 'MongoDB Atlas Provider version matrix (JSON array)'
default: '["", "1.12.3"]' # "" for latest version
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we still have 1.12.3 hardcoded here? I think what we want ultimately is that our latest version is compatible... right @Zuhairahmed ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@marcosuma Zuhair told me to check our latest 2 minor patch versions, "" is the latest one (currently 1.13.1).
for example once we release 1.14.0, we'll have to change this to: "", "1.13.1", where "" will be 1.14.0

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand why 😅 and also this is something we'll have to manually update too often, every time we make a release (yet one more thing to do manually). Unless there is a strong reason (let's wait for Zuhair) that I am missing let's please remove it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Feedback is to test last 2 Majors of Terraform Atlas Provider versions (BigMajor.Major.Minor format) so today the latest minor patch on each would be v1.13.1 and v1.12.3. This is to get ahead of any unintended breaking changes before our customers were to identify them so we can do this by ensuring the same tests pass last 2 Major versions. Would however explore ways to prevent hardcoding here if possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

is it just me or I am really not seeing the value in this?

Once you've gone over 1.12.3, what is the point in keep testing the second last version? That will never change because we will never touch it.

To me it's more the extra manual effort we are introducing in having to edit the last version every time than the benefit of this

@lantoli @Zuhairahmed

Copy link
Collaborator

Choose a reason for hiding this comment

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

would consider the HashiCorp Terraform testing scenarios where:

  1. acceptance tests fail on Terraform Atlas Provider v1.13.1 and pass on v1.12.3
  2. acceptance tests fail on both Terraform Atlas Provider v1.13.1 and v1.12.3

if we only tested most recent version all we would know for certain is that tests have failed on v1.13.1 and we need to investigate why this occurred which can be from multiple sources. however by running testing on most 2 recent recent version of Terraform Atlas Provider we help accelerate triage of finding a bug by narrowing scope of issue. if scenario 1 one occurs, then we know that error is something that we introduced with most recent changes catching breaking changes ahead of customers. if it's scenario 2, then we know that issue most likely has to do with change made on HashiCorp Terraform side (for example they released bug) so we can then escalate to HashiCorp thereby saving time with bug resolution.

Does this help? Agree again that we should be automating/avoiding hardcodes as much of this as possible.

Copy link
Collaborator

@marcosuma marcosuma Dec 20, 2023

Choose a reason for hiding this comment

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

removing previous comment

Ok, I got some updates from Leo offline, so I think the main misunderstanding here is that these are migration tests, hence 1.12.3 is used to first create the resource and then "latest" is used to check if there are no plan changes.

Let us discuss this offline.

provider_matrix:
description: 'MongoDB Atlas Provider version matrix (JSON array)'
default: '["", "1.12.3"]' # "" for latest version

schedule:
- cron: "0 0 * * *" # workflow runs every day at midnight UTC
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we do longer frequency? every month?

Copy link
Member Author

@lantoli lantoli Dec 19, 2023

Choose a reason for hiding this comment

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

this frequency is for:
- schedule_terraform_matrix: '["1.6.x", "1.0.8"]'
- schedule_provider_matrix: '["", "1.12.3]' # "" for latest version

the longer version test has to be run manually, for instance we can do it monthly or before a release or a major release.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we look at the requirement we discussed yesterday, we want to test our latest version against the terraform versions that we declare we support. This was also stated in one of the document Zuhair has recently proposed.

So my ask is: can we please have a monthly-based testing that checks our latest provider version against '["1.6.x", "1.5.x", "1.4.x", "1.3.x", "1.2.x"]'. I'd prefer not to do this manually as we have already lots of things we do manually.

Let me know if I am missing something

Copy link
Member Author

@lantoli lantoli Dec 19, 2023

Choose a reason for hiding this comment

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

we could but the problem I see is that somebody has to look at the results, and if it's done once per month I see it very easy to miss that execution result. Also I find it interesting to run it at other occasions like before a release.

this is also happening with the daily Test Suite. We run them automatically every night, but you have to go to the GH UI to see the results.

If we run something monthly I think that execution will probably be missed.
I don't see the benefit of running it monthly automatically. I think it's better something like I did today: I run them manually and then I look at the results.

Copy link
Member Author

Choose a reason for hiding this comment

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

that said, I you still think it's a good idea please confirm and I'll add the monthly schedule

Copy link
Collaborator

Choose a reason for hiding this comment

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

On this, I don't have a strong opinion, I am ok to run it manually or before a release. @gssbzn how do we do it for AtlasCLI?

Copy link
Collaborator

Choose a reason for hiding this comment

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

how do we do it for AtlasCLI?

We always run packaging on master (we call it the waterfall on evergreen), this means after every merge, and then all the OS compatibility depends on packaging succeeding. The compatibility matrix exploded on the CLI at some point and evergreen let's us generate test programmatically so we write the yaml via code here

Copy link
Member

@AgustinBettati AgustinBettati left a comment

Choose a reason for hiding this comment

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

LGTM, one detail

.github/workflows/test-suite.yml Outdated Show resolved Hide resolved
Copy link
Contributor

Code Coverage

Package Line Rate Health
github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion 31%
github.com/mongodb/terraform-provider-mongodbatlas/internal/common/dsschema 100%
github.com/mongodb/terraform-provider-mongodbatlas/internal/common/validate 68%
github.com/mongodb/terraform-provider-mongodbatlas/internal/provider 7%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/accesslistapikey 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/advancedcluster 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/alertconfiguration 33%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/apikey 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/atlasuser 0%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/auditing 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/backupcompliancepolicy 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/cloudbackupschedule 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/cloudbackupsnapshot 5%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/cloudbackupsnapshotexportbucket 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/cloudbackupsnapshotexportjob 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/cloudbackupsnapshotrestorejob 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/cloudprovideraccess 4%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/cluster 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/clusteroutagesimulation 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/customdbrole 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/customdnsconfigurationclusteraws 5%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/databaseuser 30%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/datalakepipeline 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/encryptionatrest 31%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/eventtrigger 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/federateddatabaseinstance 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/federatedquerylimit 4%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/federatedsettingsidentityprovider 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/federatedsettingsorgconfig 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/federatedsettingsorgrolemapping 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/globalclusterconfig 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/ldapconfiguration 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/ldapverify 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/maintenancewindow 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/networkcontainer 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/networkpeering 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/onlinearchive 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/organization 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/orginvitation 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/privateendpointregionalmode 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/privatelinkendpoint 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/privatelinkendpointserverless 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/privatelinkendpointservice 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/privatelinkendpointservicedatafederationonlinearchive 3%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/privatelinkendpointserviceserverless 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/project 30%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/projectapikey 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/projectinvitation 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/projectipaccesslist 10%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/rolesorgid 7%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/searchdeployment 23%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/searchindex 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/serverlessinstance 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/streamconnection 26%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/streaminstance 14%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/teams 1%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/thirdpartyintegration 4%
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/x509authenticationdatabaseuser 2%
github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc 4%
Summary 7% (765 / 11018)

@lantoli lantoli merged commit 480a64f into master Dec 19, 2023
30 checks passed
@lantoli lantoli deleted the CLOUDP-218618_test_suite branch December 19, 2023 16:13
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.

6 participants