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

CLOUDP-167782: Clusters Migration #1967

Merged
merged 37 commits into from
Jun 2, 2023
Merged

CLOUDP-167782: Clusters Migration #1967

merged 37 commits into from
Jun 2, 2023

Conversation

swattyT
Copy link
Contributor

@swattyT swattyT commented May 29, 2023 •
edited
Loading

Proposed changes

Jira ticket: CLOUDP-167782
Migrate "Clusters" CLI stores to v2 sdk

EVG: https://spruce.mongodb.com/version/64766c9fe3c331d96adfb38d/tasks?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC

Closes #[issue number]

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation in document requirements section listed in CONTRIBUTING.md (if appropriate)
  • I have addressed the @mongodb/docs-cloud-team comments (if appropriate)
  • I have updated test/README.md (if an e2e test has been added)
  • I have run make fmt and formatted my code

Further comments

@matt-condon matt-condon self-assigned this May 30, 2023
@matt-condon matt-condon force-pushed the temp/CLOUDP-167782 branch from b32f3c8 to ce3cfe6 Compare May 31, 2023 11:37
@matt-condon matt-condon force-pushed the temp/CLOUDP-167782 branch from ce3cfe6 to d6b32a4 Compare May 31, 2023 12:30
@swattyT swattyT requested review from wtrocki and fmenezes June 1, 2023 17:39
@wtrocki
Copy link
Member

wtrocki commented Jun 1, 2023

Changes look good.
Do we have all variations of setup commands working?

matt-condon
matt-condon previously approved these changes Jun 2, 2023
View reviewed changes
Copy link
Collaborator

@matt-condon matt-condon left a comment

Choose a reason for hiding this comment

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

Changes LGTM, nice work

@swattyT
Copy link
Contributor Author

swattyT commented Jun 2, 2023

Changes look good. Do we have all variations of setup commands working?

Ran both - setup and quickstart commands without skip and the cluster is created successfully, sample data is loaded Mongosh connected to the cluster.

wtrocki
wtrocki reviewed Jun 2, 2023
View reviewed changes
Copy link
Member

@wtrocki wtrocki left a comment

Choose a reason for hiding this comment

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

Verified using m0. We need dedicated tests - GCP and Azure - can be post merge of PR

wtrocki
wtrocki approved these changes Jun 2, 2023
View reviewed changes
Copy link
Member

@wtrocki wtrocki left a comment

Choose a reason for hiding this comment

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

True masterpiece! Thank you for all fixes and improvements

wtrocki
wtrocki approved these changes Jun 2, 2023
View reviewed changes
Copy link
Member

@wtrocki wtrocki left a comment

Choose a reason for hiding this comment

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

True masterpiece! Thank you for all fixes and improvements

@swattyT swattyT merged commit b7b2a6e into master Jun 2, 2023
@swattyT swattyT deleted the temp/CLOUDP-167782 branch June 2, 2023 17:54
@wtrocki
Copy link
Member

wtrocki commented Jun 5, 2023 •
edited
Loading

@swattyT I noticed some issue with the pointer being presented in the atlas setup:

Creating your cluster... [It's safe to 'Ctrl + C']
Cluster created.
Your connection string: 0x140009692a0

Working on the fix.

@wtrocki
Copy link
Member

wtrocki commented Jun 5, 2023

Verified cluster creation for Azure and GCP (dedicated)

@wtrocki
Copy link
Member

wtrocki commented Jun 5, 2023

Not an issue with PR but we might need to improve error handling.
Name duplication is being logged now as:

Error: https://cloud-dev.mongodb.com/api/atlas/v2/groups/63b6dd17362d4278259cfa25/clusters POST: HTTP 400 (Error code: "DUPLICATE_CLUSTER_NAME") Detail: A cluster or serverless instance named azure is already present in group 63b6dd17362d4278259cfa25. Reason: Bad Request. Params: [cluster or serverless instance azure 63b6dd17362d4278259cfa25]

Where it could be just:

Name of that cluster already exist

igor-karpukhin added a commit that referenced this pull request Mar 26, 2024
@igor-karpukhin
Revert changes in PR #1967 for DiskIOPS for Kubernetes deployment
90d756a
igor-karpukhin added a commit that referenced this pull request Mar 28, 2024
@igor-karpukhin
Revert changes in PR #1967 for DiskIOPS for Kubernetes deployment
4ef85a5
igor-karpukhin added a commit that referenced this pull request Mar 28, 2024
@igor-karpukhin
Revert changes in PR #1967 for DiskIOPS for Kubernetes deployment
1f67c53
igor-karpukhin added a commit that referenced this pull request Mar 28, 2024
@igor-karpukhin
HELP-56667: Revert changes in PR #1967 for DiskIOPS for Kubernetes de…
c79e7f7 
…ployment (#2812)
@fmenezes fmenezes mentioned this pull request Apr 5, 2024
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matt-condon matt-condon matt-condon approved these changes

@wtrocki wtrocki wtrocki approved these changes

@fmenezes fmenezes Awaiting requested review from fmenezes

Assignees

@matt-condon matt-condon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@swattyT @wtrocki @fmenezes @matt-condon