Skip to content

Add HighAvailabilityMode field to 'infrastructure' CRD #827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 8, 2021
Merged

Add HighAvailabilityMode field to 'infrastructure' CRD #827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions config/v1/0000_10_config-operator_01_infrastructure.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,18 @@ spec:
used by components like the web console to tell users where to find
the Kubernetes API.
type: string
controlPlaneTopology:
description: controlPlaneTopology expresses the expectations for operands
that normally run on control nodes. The default is 'HighlyAvailable',
which represents the behavior operators have in a "normal" cluster.
The 'SingleReplica' mode will be used in single-node deployments
and the operators should not configure the operand for highly-available
operation
type: string
default: HighlyAvailable
enum:
- HighlyAvailable
- SingleReplica
etcdDiscoveryDomain:
description: 'etcdDiscoveryDomain is the domain used to fetch the
SRV records for discovering etcd servers and clients. For more info:
Expand All @@ -185,6 +197,19 @@ spec:
a human friendly name. Once set it should not be changed. Must be
of max length 27 and must have only alphanumeric or hyphen characters.
type: string
infrastructureTopology:
description: infrastructureTopology expresses the expectations for
infrastructure services that do not run on control plane nodes,
usually indicated by a node selector for a `role` value other than
`master`. The default is 'HighlyAvailable', which represents the
behavior operators have in a "normal" cluster. The 'SingleReplica'
mode will be used in single-node deployments and the operators should
not configure the operand for highly-available operation
type: string
default: HighlyAvailable
enum:
- HighlyAvailable
- SingleReplica
platform:
description: "platform is the underlying infrastructure provider for
the cluster. \n Deprecated: Use platformStatus.type instead."
Expand Down
28 changes: 28 additions & 0 deletions config/v1/types_infrastructure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,36 @@ type InfrastructureStatus struct {
// like kubelets, to contact the Kubernetes API server using the
// infrastructure provider rather than Kubernetes networking.
APIServerInternalURL string `json:"apiServerInternalURI"`

// controlPlaneTopology expresses the expectations for operands that normally run on control nodes.
// The default is 'HighlyAvailable', which represents the behavior operators have in a "normal" cluster.
// The 'SingleReplica' mode will be used in single-node deployments
// and the operators should not configure the operand for highly-available operation
// +kubebuilder:default=HighlyAvailable
ControlPlaneTopology TopologyMode `json:"controlPlaneTopology"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// +default=HighlyAvailable

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this field is only under the 'status' and not under the 'spec'.
does it need to have a default in the definition here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not have to. But a default value shows up automatically if not set otherwise. What should operators do if this field is not set?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point, I will add it

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the enhancement we said that if the field wasn't set operators should assume HighlyAvailable as a default. If setting the default in the CRD definition ensures that a value is provided for clusters upgraded from 4.7 to 4.8 (when the field is added), then maybe we didn't need to worry about that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked it, and I can't use a default value here since this CRD is apiextensions.k8s.io/v1beta1, and default values aren't supported yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked it, and I can't use a default value here since this CRD is apiextensions.k8s.io/v1beta1, and default values aren't supported yet.

Switch it to v1. We have done that with many CRDs in openshift/api already. No reason not to, just time.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sttts not sure we have extra time to invest into it at this point. do we understand how much effort is it?

Copy link
Contributor

@sttts sttts Jan 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#834 merged. It's v1 CRD now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the default here and tested the upgrade from an old version. The fields were added to the existing 'cluster' CR with the default value.


// infrastructureTopology expresses the expectations for infrastructure services that do not run on control
// plane nodes, usually indicated by a node selector for a `role` value
// other than `master`.
// The default is 'HighlyAvailable', which represents the behavior operators have in a "normal" cluster.
// The 'SingleReplica' mode will be used in single-node deployments
// and the operators should not configure the operand for highly-available operation
// +kubebuilder:default=HighlyAvailable
InfrastructureTopology TopologyMode `json:"infrastructureTopology"`
}

// TopologyMode defines the topology mode of the control/infra nodes.
// +kubebuilder:validation:Enum=HighlyAvailable;SingleReplica
type TopologyMode string

const (
// "HighlyAvailable" is for operators to configure high-availability as much as possible.
HighlyAvailableTopologyMode TopologyMode = "HighlyAvailable"

// "SingleReplica" is for operators to avoid spending resources for high-availability purpose.
SingleReplicaTopologyMode TopologyMode = "SingleReplica"
)

// PlatformType is a specific supported infrastructure provider.
// +kubebuilder:validation:Enum="";AWS;Azure;BareMetal;GCP;Libvirt;OpenStack;None;VSphere;oVirt;IBMCloud;KubeVirt
type PlatformType string
Expand Down
16 changes: 9 additions & 7 deletions config/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.