🌱 Add failureDomains in MachineDeploymentTopology #5850
Conversation
k8s-ci-robot
added
cncf-cla: yes
| @@ -143,6 +143,11 @@ type MachineDeploymentTopology struct { | |||
| // the values are hashed together. | |||
| Name string `json:"name"` | |||
|
|
|||
| // FailureDomain is the failure domain the machines will be created in. | |||
| // Must match a key in the FailureDomains map stored on the cluster object. | |||
There was a problem hiding this comment.
When we go create/update a MachineDeployment, could we validate the input? We can't do it beforehand (e.g. on cluster creation) because the FailureDomains wouldn't be available yet, but we could inform users if the failure domain isn't available?
We can also open an issue and do it later
There was a problem hiding this comment.
I'll open an issue.
This is another use case where we are putting changes on hold (like for upgrades), so we should make sure that the TopologyReconciled condition is updated accordingly
@ykakarap PTAL
There was a problem hiding this comment.
Noted. Looks like that work will come in a follow up PR.
To help with my understanding, we are looking to update the FailureDomain on the MD only after the cluster object is reconciled and cluster.status.failureDomains is available?
OR
Are we planing to hold creating the MD till cluster.status.failureDomains is available so that we can validate the provided failureDomain and use that to create the MD.?
There was a problem hiding this comment.
@ykakarap The second option sounds better to me. I think it would be surprising to the user if the Machines first show up in the wrong failure domain and then get re-created in the right one. (not entirely sure if that can happen because possibly the cluster reconcile will block everything else long enough anyway)
|
LGTM pending squash |
fabriziopandini
force-pushed
the
failureDomains-in-MachineDeploymentTopology
branch
from
fbab740 to
1483dcc
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vincepri The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
fabriziopandini
deleted the
failureDomains-in-MachineDeploymentTopology
branch
What this PR does / why we need it:
This PR adds support for defining the
failureDomainwhere a MachineDeploymentTopology should deploy machines toWhich issue(s) this PR fixes:
Fixes #5636
/cc @yastij