fix: placement group logic redone, including a compatibility mode in … #1217
Conversation
…order to keep existing clusters compatible with the new logic.
|
@valkenburg-prevue-ch will test this tonight. So grateful about this 🙏🙏 💙 |
There was a problem hiding this comment.
@valkenburg-prevue-ch Super elegant man! Well done.
Just one clarification I would like to make, and asking your opinion here, we have those limitations in Hetzner:
In Hetzner Cloud, you can have up to 100 servers per network, up to 50 subnets per project, and each Placement Group can contain up to 10 servers, with a maximum of 50 Placement Groups per project.
Hence, it's pretty fast to reach the limit of max 10 servers per group. Shouldn't we:
- Enforce a check on the count. I think it's not done now.
- Impose one uniquely named placement group per nodepool.
What do you think?
There was a problem hiding this comment.
Do we really need the compatibility mode? The code seems quite cluttered with it, and I don't see a real benefit.. upgrading the version of module would only result in a different arrangement of nodes via placement groups but nothing breaking would actually happen?
I'm fine with merging this and removing the compat_idx in v3 if we want to be proper semver.
|
Yeah, this is exactly what I was hoping, that we only keep the extra compatibility for a limited period, and certainly drop it for v3. I thought that not having it, would lead Terraform to replace nodes, because of the changing placement groups. But Perhaps I'm wrong there? |
I think they won't be replaced, I'll try it on my cluster now to see what really happens. |
|
In light of #1218 that will come with v3, I think we are good to go. The Hetzner system will give an error if more than 10 nodes are used, it's good enough for now, not a lot of folks use that many nodes. And will add it to the readme. This is an important fix for this major version. |
…order to keep existing clusters compatible with the new logic.
Fixes #1157 .