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

test: Introduces shared cluster resource for running search index tests #2086

Merged
merged 4 commits into from Mar 27, 2024

Conversation

AgustinBettati
Copy link
Collaborator

@AgustinBettati AgustinBettati commented Mar 26, 2024

Description

Link to any related issue(s): CLOUDP-239134

Defines mechanism for sharing a single cluster in a test execution. Applied new acc.ClusterNameExecution(t) for search index resource.

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.
  • If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

Comment on lines -282 to +287
func configBasic(projectIDStr, indexName, databaseName, clusterNameStr, clusterTerraformStr string, explicitType bool) string {
func configBasic(projectID, indexName, databaseName, clusterName string, explicitType bool) string {
var indexType string
if explicitType {
indexType = `type="search"`
}
return clusterTerraformStr + fmt.Sprintf(`
return fmt.Sprintf(`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cluster is now created outside of the terraform configuration so no need to receive the cluster config (clusterTerraformStr).

}
}

func clusterReq(name, projectID string) admin.AdvancedClusterDescription {
Copy link
Member

Choose a reason for hiding this comment

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

i like it, and later we can make it more configurable

_, _, err := ConnV2().ClustersApi.CreateCluster(context.Background(), projectID, &req).Execute()
require.NoError(tb, err, "Cluster creation failed: %s, err: %s", name, err)

stateConf := advancedcluster.CreateStateChangeConfig(context.Background(), ConnV2(), projectID, name, 1*time.Hour)
Copy link
Member

Choose a reason for hiding this comment

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

what is this for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is needed so that we wait for the cluster creation to complete. If this is not included tests will start running against a cluster that is not yet available.

Copy link
Member

@lantoli lantoli left a comment

Choose a reason for hiding this comment

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

I like it!

if err == nil && searchIndex != nil {
if err == nil && searchIndex != nil && searchIndex.Status != "IN_PROGRESS" { // index can be in progess for some seconds after delete is called
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change was needed because a cluster definition is no longer present in the config, so the check destroy is called instantly after acceptance tests deletes the search index. When the cluster config was defined the delete operation would take several minutes and leave room for the search index to fully delete.

@AgustinBettati AgustinBettati marked this pull request as ready for review March 27, 2024 13:17
@AgustinBettati AgustinBettati requested a review from a team as a code owner March 27, 2024 13:17
{
RegionConfigs: &[]admin.CloudRegionConfig{
{
ProviderName: admin.PtrString("AWS"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: use constants for the strings "AWS", "US_WEST_2" and "M10"

Copy link
Collaborator

@oarbusi oarbusi left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator

@EspenAlbert EspenAlbert left a comment

Choose a reason for hiding this comment

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

Nice 👍
Tested with MONGODB_ATLAS_CLUSTER_NAME locally, and it works 👏

@AgustinBettati
Copy link
Collaborator Author

merging the PR, failing test in cluster are cloud-dev related, had a successful pass before latest commit which only did a refactor.

@AgustinBettati AgustinBettati merged commit 01f585c into master Mar 27, 2024
44 of 46 checks passed
@AgustinBettati AgustinBettati deleted the CLOUDP-239134 branch March 27, 2024 16:06
lantoli added a commit that referenced this pull request Apr 1, 2024
* master:
  test: Introduces shared cluster resource for running search index tests (#2086)
  doc: Generates v1.15.3 (#2095)
  chore: Bump github.com/zclconf/go-cty from 1.14.3 to 1.14.4 (#2089)
  chore: Bump github.com/mongodb-forks/digest from 1.0.5 to 1.1.0 (#2087)
  chore: Bump github.com/hashicorp/hcl/v2 from 2.20.0 to 2.20.1 (#2091)
  chore: Bump tj-actions/verify-changed-files (#2092)
  chore: Bump github.com/aws/aws-sdk-go from 1.51.3 to 1.51.8 (#2088)
  test: Converting a test case to a migration test (#2081)
  chore: Fixes Slack notification button to GH action run text (#2093)
  doc: Specifies that upgrades From Replica Sets to Multi-Sharded Instances of cluster and advanced cluster might lead to error (#2080)
  doc: Fixes import command in third_party_integration doc (#2083)
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.

None yet

4 participants