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

CLOUDP-224541: Added terminationProtection flag #1356

Merged
merged 4 commits into from
Feb 9, 2024

Conversation

igor-karpukhin
Copy link
Collaborator

@igor-karpukhin igor-karpukhin commented Feb 5, 2024

All Submissions:

Added terminationProtection flag

  • Have you signed our CLA?
  • Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if there is one).
  • Update docs/release-notes/release-notes.md if your changes should be included in the release notes for the next release.

@igor-karpukhin igor-karpukhin added the cloud-tests Run expensive Cloud Tests: Integration & E2E label Feb 5, 2024
@igor-karpukhin igor-karpukhin marked this pull request as ready for review February 5, 2024 14:31
Copy link
Contributor

github-actions bot commented Feb 5, 2024

Copy link
Collaborator

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

Please add e2e tests as a follow-up, preferably before we start the release tomorrow. If not, that's ok as well.

@s-urbaniak
Copy link
Collaborator

@igor-karpukhin ^

@igor-karpukhin
Copy link
Collaborator Author

I added tests. PTAL @s-urbaniak @josvazg

@s-urbaniak
Copy link
Collaborator

@igor-karpukhin instead of commenting I took the freedom to put a commit on top to untangle the nested ifs, PTAL

@s-urbaniak s-urbaniak force-pushed the termination-protection-advanced-clusters branch from fbd3d17 to 715aa6c Compare February 8, 2024 09:09
@@ -33,7 +33,7 @@ func init() {
excludedClusterFieldsTheirs["replicationFactor"] = true

// Termination protection
excludedClusterFieldsTheirs["terminationProtectionEnabled"] = true
// excludedClusterFieldsTheirs["terminationProtectionEnabled"] = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like these are leftovers from ToAtlas methods tests. I can remove this in a separate PR

@igor-karpukhin igor-karpukhin merged commit 351ff88 into main Feb 9, 2024
44 checks passed
@igor-karpukhin igor-karpukhin deleted the termination-protection-advanced-clusters branch February 9, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cloud-tests Run expensive Cloud Tests: Integration & E2E
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants