Skip to content

Commit

Permalink
Merge pull request #1378 from adriengentil/api-platform-external
Browse files Browse the repository at this point in the history
OCPCLOUD-2011: Update implementation history of External platform type EP
  • Loading branch information
openshift-merge-robot authored Jun 27, 2023
2 parents 3b55ed3 + f61fe20 commit cb52a1a
Showing 1 changed file with 53 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ reviewers:
- "@mhrivnak"
- "@rvanderp3"
- "@mtulio"
- "@deads2k, to review library-go, KCMO related parts andopenshift/api changes"
- "@deads2k, to review library-go, KCMO related parts andopenshift/api changes"
- "@JoelSpeed, to review CCM, MAPO related parts and openshift/api changes"
- "@sinnykumari, to review MCO related parts"
- "@danwinship, to review CNO related parts"
Expand Down Expand Up @@ -287,7 +287,7 @@ Machine Api Operator is responsible for deploying and maintaining the set of mac
- machine controller

From the list above, only the "machine controller" is cloud-provider dependent, however, for now, Machine Api Operator
won't deploy anything if it encounters "None" or an unrecognized platform type.
won't deploy anything if it encounters "None" or an unrecognized platform type.

In the future, "External" platform type, in conjunction with an enabled capability,
would serve as a signal for Machine Api Operator to deploy only provider-agnostic
Expand All @@ -299,7 +299,7 @@ replicate everything that MAO does.
Cluster Storage Operator will go to no-op state if it encounters PlatformType "None" or an unknown PlatformType.

At this point, nothing requires storage to be there during cluster installation, and storage (CSI) drivers might be supplemented
later via OLM or some other way as day two operation.
later via OLM or some other way as day two operation.

No particular changes in regards to the "External" platform type introduction are expected there.

Expand Down Expand Up @@ -385,41 +385,13 @@ const (
Additionally, the respective external platform spec and status should be added to the infrastructure resource.

```go
type CloudControllerManagerMode string

const (
// Cloud Controller Manager is enabled and expected to be supplied.
// Signaling that kubelets and other CCM consumers should use --cloud-provider=external.
CloudControllerManagerExternal CloudControllerManagerMode = "External"
// Cloud Controller Manager is enabled and expected to be supplied.
// Signaling that kubelets and other CCM consumers should not set --cloud-provider flag.
CloudControllerManagerNone CloudControllerManagerMode = "None"
)

type CloudControllerManagerSettings struct {
// State determines whether or not an external Cloud Controller Manager is expected to
// be presented in the cluster.
// For engaging an external Cloud Controller Manager, certain flags are expected to be set to the kubelets.
// https://kubernetes.io/docs/tasks/administer-cluster/running-cloud-controller/#running-cloud-controller-manager
//
// When set to "External", the respective operator (machine config operator) should set `--cloud-provider=external` flag to the kubelet.
// When omitted or disabled, no `cloud-provider` flag should be set.
// +kubebuilder:validation:Enum=External;None
// +optional
State CloudControllerManagerMode `json:"state,omitempty"`
}


// ExternalPlatformSpec holds the desired state for the generic External infrastructure provider.
type ExternalPlatformSpec struct{
// PlatformName holds the arbitrary string represented cloud provider name, expected to be set at the installation time.
// Intended to serve only for informational purposes and not expected to be used for decision-making.
// +kubebuilder:default:="Unknown"
// +optional
PlatformName string `json:"platformName,omitempty"`
// CloudControllerManager contains settings specific to the external Cloud Controller Manager (a.k.a. CCM or CPI)
// +optional
CloudControllerManager CloudControllerManagerSettings `json:"cloudControllerManager"`
}

type PlatformSpec struct {
Expand All @@ -430,12 +402,47 @@ type PlatformSpec struct {
}
```

For the sake of consistency, status should be introduced as well. For now, this will be empty.
For the sake of consistency, status should be introduced as well, and will
define the settings set at installation time:

```go
type CloudControllerManagerMode string

const (
// Cloud Controller Manager is enabled and expected to be supplied.
// Signaling that kubelets and other CCM consumers should use --cloud-provider=external.
CloudControllerManagerExternal CloudControllerManagerMode = "External"
// Cloud Controller Manager is enabled and expected to be supplied.
// Signaling that kubelets and other CCM consumers should not set --cloud-provider flag.
CloudControllerManagerNone CloudControllerManagerMode = "None"
)

// CloudControllerManagerStatus holds the state of Cloud Controller Manager (a.k.a. CCM or CPI) related settings
// +kubebuilder:validation:XValidation:rule="(has(self.state) == has(oldSelf.state)) || (!has(oldSelf.state) && self.state != \"External\")",message="state may not be added or removed once set"
type CloudControllerManagerStatus struct {
// state determines whether or not an external Cloud Controller Manager is expected to
// be installed within the cluster.
// https://kubernetes.io/docs/tasks/administer-cluster/running-cloud-controller/#running-cloud-controller-manager
//
// When set to "External", new nodes will be tainted as uninitialized when created,
// preventing them from running workloads until they are initialized by the cloud controller manager.
// When omitted or set to "None", new nodes will be not tainted
// and no extra initialization from the cloud controller manager is expected.
// +kubebuilder:validation:Enum="";External;None
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="state is immutable once set"
// +optional
State CloudControllerManagerState `json:"state"`
}

// ExternalPlatformStatus holds the current status of the generic External infrastructure provider.
type ExternalPlatformStatus struct{}
// +kubebuilder:validation:XValidation:rule="has(self.cloudControllerManager) == has(oldSelf.cloudControllerManager)",message="cloudControllerManager added or removed once set"
type ExternalPlatformStatus struct {
// CloudControllerManager contains settings specific to the external Cloud Controller Manager (a.k.a. CCM or CPI)
// When omitted or set to "None", new nodes will be not tainted
// and no extra initialization from the cloud controller manager is expected.
// +optional
CloudControllerManager CloudControllerManagerStatus `json:"cloudControllerManager"`
}

type PlatformStatus struct {
...
Expand Down Expand Up @@ -573,6 +580,19 @@ OCP 4.13 will not contain the `CloudControllerManagerSpec` part.

- [PR for removing CloudControllerManagerSpec](https://github.com/openshift/api/pull/1409)

### Add CloudControllerManagerStatus into the infrastructure resource

- Remove CloudControllerManagerSpec from the ExternalPlatformSpec because
this setting is not meant to be updated after cluster installation
- Introduce CloudControllerManagerStatus to hold the setting which will be
defined at installation time
- CloudControllerManagerStatus is available behind the feature set
`TechPreviewNoUpgrade`
- Because of the feature set, the CEL on `ExternalPlatformStatus` couldn't
be defined, we must set it once the feature set is lifted

- [related PR](https://github.com/openshift/api/pull/1434)

## Alternatives

### Status Quo
Expand Down

0 comments on commit cb52a1a

Please sign in to comment.