🌱 Deprecate NoCloudProvider and add CloudProviderEnabled #2108
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
metal3-io-bot
added
the
size/M
Signed-off-by: Muhammad Adil Ghaffar <[email protected]>
adilGhaffarDev
force-pushed
the
deprecate-noCloudProvider/adil
branch
from
7f7a168
to
18178b7
Compare
|
cc @lentzi90 |
There was a problem hiding this comment.
Thanks for working on this!
I think we need to add some validation or other logic to make sure we don't end up in confusing situations where these two contradict each other. Do you think it is possible to do this and also add a testcase for it?
I'm not sure if it is best to do this in the validating webhook or if we can somehow detect later which one of them were set explicitly.
| // If set to false, CAPM3 will use node labels to set providerID on the kubernetes nodes. | ||
| // If set to true, providerID is set on nodes by other entities and CAPM3 uses the value of the providerID on the m3m resource. | ||
| // +optional | ||
| CloudProviderEnabled bool `json:"cloudProviderEnabled,omitempty"` |
There was a problem hiding this comment.
Can you add to the description that the default value is false, and that this takes precedence over NoCloudProvider?
What this PR does / why we need it:
Deprecate NoCloudProvider and add CloudProviderEnabled
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #