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

Make "active-active" a valid production_type value #249

Merged

Conversation

miguelhrocha
Copy link
Contributor

Background

The active-active mode used to be identified by the vm_node_count parameter. This breaks possible workflows when someone wants to modify the node_count to a value lower than 2 for maintenance or other reasons.

This adds a non-breaking change to decouple the active-active mode from the vm_node_count parameter.

@miguelhrocha miguelhrocha requested a review from a team as a code owner May 31, 2024 10:29
Copy link
Contributor

@nikolasrieble nikolasrieble left a comment

Choose a reason for hiding this comment

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

Great idea, thank you.

I would like to go even further and simply require the operational mode to be set and remove any magic behind the curtains.

var.production_type == "disk" ||
var.production_type == null
)
condition = contains(["external", "disk", "active-active", null], var.production_type)
Copy link
Contributor

Choose a reason for hiding this comment

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

💭 Would it not be even cleaner if we were to start requiring the operational mode and remove the logic with the node_count influencing the operational mode?

Also, while we are here, please do rename it to operational_mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to avoid introducing breaking changes with this PR. Renaming it to operational_mode will break all of the existing deployments relying on this module.

Part of the cons of having the module public, and non-versioned 🤷🏼

@miguelhrocha miguelhrocha merged commit 051a9fb into main May 31, 2024
4 checks passed
@miguelhrocha miguelhrocha deleted the TF-15070-make-active-active-a-valid-production-type branch May 31, 2024 10:38
miguelhrocha pushed a commit that referenced this pull request May 31, 2024
…e-active simple (#250)

Follow up from my previous PR #249. It removes all the complicated logic around active-active and renames the property to operational_mode to be consistent with Google and AWS modules.
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.

3 participants