Add external topology mode #940
Conversation
csrwng
force-pushed
the
external_cp_topology
branch
2 times, most recently
from
c053f01 to
00b1146
Compare
Implementation of https://github.com/openshift/enhancements/blob/master/enhancements/external-control-plane-topology.md Adds External value to the ToplogyMode type.
csrwng
force-pushed
the
external_cp_topology
branch
from
00b1146 to
9958693
Compare
|
/approve |
openshift-ci
bot
added
the
approved
| enum: | ||
| - HighlyAvailable | ||
| - SingleReplica | ||
| - External |
There was a problem hiding this comment.
are we actually consistent about infra?
in hypershift we have some infra components that run external (olm) and some that run on cluster (registry) right?
There was a problem hiding this comment.
That is correct. From an operator point of view, I don't think the External value makes sense for infra topology (Unless we really externalize all of infra). It is meant to apply only to control plane topology for now. However, not sure that we can express that through the API as it is.
There was a problem hiding this comment.
So "external" will not be a valid value for status.infrastructureTopology ?
I guess can understand not creating another Type to split them, so i guess we can live with this but it is somewhat unfortunate that it confusingly implies it is valid, even if practice we'd never actually set it to that value.
|
/approve cancel |
openshift-ci
bot
removed
the
approved
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, csrwng The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
|
||
| // TopologyMode defines the topology mode of the control/infra nodes. | ||
| // +kubebuilder:validation:Enum=HighlyAvailable;SingleReplica | ||
| // +kubebuilder:validation:Enum=HighlyAvailable;SingleReplica;External |
There was a problem hiding this comment.
If we move this declaration up to the fields above, we can provide different enum values and prevent the infra topology from being set to external. We would want a comment here to explain why we're doing it that way, too.
There was a problem hiding this comment.
@dhellmann I like this suggestion. We can then make sure we don't set an invalid value for infra topology. Will submit a follow up PR
4 participants
Implementation of https://github.com/openshift/enhancements/blob/master/enhancements/external-control-plane-topology.md
Adds External value to the ToplogyMode type.