diff --git a/Documentation/CRDs/Cluster/ceph-cluster-crd.md b/Documentation/CRDs/Cluster/ceph-cluster-crd.md index 2f464f270..294575223 100755 --- a/Documentation/CRDs/Cluster/ceph-cluster-crd.md +++ b/Documentation/CRDs/Cluster/ceph-cluster-crd.md @@ -187,7 +187,12 @@ If not specified, the default SDN will be used. Configure the network that will be enabled for the cluster and services. * `provider`: Specifies the network provider that will be used to connect the network interface. You can choose between `host`, and `multus`. -* `selectors`: List the network selector(s) that will be used associated by a key. +* `selectors`: Used for `multus` provider only. Select NetworkAttachmentDefinitions to use for Ceph networks. + * `public`: Select the NetworkAttachmentDefinition to use for the public network. + * `cluster`: Select the NetworkAttachmentDefinition to use for the cluster network. +* `addressRanges`: Used for `host` or `multus` providers only. Allows overriding the address ranges (CIDRs) that Ceph will listen on. + * `public`: A list of individual network ranges in CIDR format to use for Ceph's public network. + * `cluster`: A list of individual network ranges in CIDR format to use for Ceph's cluster network. * `ipFamily`: Specifies the network stack Ceph daemons should listen on. * `dualStack`: Specifies that Ceph daemon should listen on both IPv4 and IPv6 network stacks. * `connections`: Settings for network connections using Ceph's msgr2 protocol @@ -211,10 +216,52 @@ Configure the network that will be enabled for the cluster and services. Changing networking configuration after a Ceph cluster has been deployed is NOT supported and will result in a non-functioning cluster. +#### Ceph public and cluster networks + +Ceph daemons can operate on up to two distinct networks: public, and cluster. + +Ceph daemons always use the public network, which is the Kubernetes pod network by default. The +public network is used for client communications with the Ceph cluster (reads/writes). + +If specified, the cluster network is used to isolate internal Ceph replication traffic. This includes +additional copies of data replicated between OSDs during client reads/writes. This also includes OSD +data recovery (re-replication) when OSDs or nodes go offline. If the cluster network is unspecified, +the public network is used for this traffic instead. + +Some Rook network providers allow manually specifying the public and network interfaces that Ceph +will use for data traffic. Use `addressRanges` to specify this. For example: + +```yaml + network: + provider: host + addressRanges: + public: + - "192.168.100.0/24" + - "192.168.101.0/24" + cluster: + - "192.168.200.0/24" +``` + +This spec translates directly to Ceph's `public_network` and `host_network` configurations. +Refer to [Ceph networking documentation](https://docs.ceph.com/docs/master/rados/configuration/network-config-ref/) +for more details. + +The default, unspecified network provider cannot make use of these configurations. + +Ceph public and cluster network configurations are allowed to change, but this should be done with +great care. When updating underlying networks or Ceph network settings, Rook assumes that the +current network configuration used by Ceph daemons will continue to operate as intended. Network +changes are not applied to Ceph daemon pods (like OSDs and MDSes) until the pod is restarted. When +making network changes, ensure that restarted pods will not lose connectivity to existing pods, and +vice versa. + #### Host Networking To use host networking, set `provider: host`. +To instruct Ceph to operate on specific host interfaces or networks, use `addressRanges` to select +the network CIDRs Ceph will bind to on the host. + If the host networking setting is changed in a cluster where mons are already running, the existing mons will remain running with the same network settings with which they were created. To complete the conversion to or from host networking after you update this setting, you will need to @@ -223,93 +270,73 @@ in order to have mons on the desired network configuration. #### Multus -Rook supports addition of public and cluster network for ceph using Multus +Rook supports using Multus NetworkAttachmentDefinitions for Ceph public and cluster networks. -The selector keys are required to be `public` and `cluster` where each represent: +Refer to [Multus documentation](https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/how-to-use.md) +for details about how to set up and select Multus networks. -* `public`: client communications with the cluster (reads/writes) -* `cluster`: internal Ceph replication network +Rook will attempt to auto-discover the network CIDRs for selected public and/or cluster networks. +This process is not guaranteed to succeed. Furthermore, this process will get a new network lease +for each CephCluster reconcile. Specify `addressRanges` manually if the auto-detection process +fails or if the selected network configuration cannot automatically recycle released network leases. -If you want to learn more, please read: +Only OSD pods will have both public and cluster networks attached (if specified). The rest of the +Ceph component pods and CSI pods will only have the public network attached. The Rook operator will +not have any networks attached; it proxies Ceph commands via a sidecar container in the mgr pod. -* [Ceph Networking reference](https://docs.ceph.com/docs/master/rados/configuration/network-config-ref/). -* [Multus documentation](https://intel.github.io/multus-cni/doc/how-to-use.html) - -Based on the configuration, the operator will do the following: - -1. If only the `public` selector is specified, all communication will happen on that network - - ```yaml - network: - provider: multus - selectors: - public: rook-ceph/rook-public-nw - ``` - -2. If only the `cluster` selector is specified, the internal cluster traffic\* will happen on that network. All other traffic to mons, OSDs, and other daemons will be on the default network. - - ```yaml - network: - provider: multus - selectors: - cluster: rook-ceph/rook-cluster-nw - ``` - -3. If both `public` and `cluster` selectors are specified the first one will run all the communication network and the second the internal cluster network\* - - ```yaml - network: - provider: multus - selectors: - public: rook-ceph/rook-public-nw - cluster: rook-ceph/rook-cluster-nw - ``` - -\* Internal cluster traffic includes OSD heartbeats, data replication, and data recovery - -Only OSD pods will have both Public and Cluster networks attached. The rest of the Ceph component pods and CSI pods will only have the Public network attached. -Rook Ceph operator will not have any networks attached as it proxies the required commands via a sidecar container in the mgr pod. - -In order to work, each selector value must match a `NetworkAttachmentDefinition` object name in Multus. - -For `multus` network provider, an already working cluster with Multus networking is required. Network attachment definition that later will be attached to the cluster needs to be created before the Cluster CRD. -The Network attachment definitions should be using whereabouts cni. -If Rook cannot find the provided Network attachment definition it will fail running the Ceph OSD pods. -You can add the Multus network attachment selection annotation selecting the created network attachment definition on `selectors`. - -A valid NetworkAttachmentDefinition will look like following: +A NetworkAttachmentDefinition must exist before it can be used by Multus for a Ceph network. A +recommended definition will look like the following: ```yaml apiVersion: "k8s.cni.cncf.io/v1" kind: NetworkAttachmentDefinition metadata: - name: rook-public-nw + name: ceph-multus-net + namespace: rook-ceph spec: config: '{ "cniVersion": "0.3.0", - "name": "public-nad", "type": "macvlan", - "master": "ens5", + "master": "eth0", "mode": "bridge", "ipam": { "type": "whereabouts", - "range": "192.168.1.0/24" + "range": "192.168.200.0/24" } }' ``` -* Ensure that `master` matches the network interface of the host that you want to use. -* IPAM type `whereabouts` is required because it makes sure that all the pods get a unique IP address from the multus network. -* The NetworkAttachmentDefinition should be referenced along with the namespace in which it is present like `public: /`. - e.g., the network attachment definition are in `default` namespace: +* Ensure that `master` matches the network interface on hosts that you want to use. + It must be the same across all hosts. +* CNI type `macvlan` is highly recommended. + It has less CPU and memory overhead compared to traditional Linux `bridge` configurations. +* IPAM type `whereabouts` is recommended because it ensures each pod gets an IP address unique + within the Kubernetes cluster. No DHCP server is required. If a DHCP server is present on the + network, ensure the IP range does not overlap with the DHCP server's range. - ```yaml - public: default/rook-public-nw - cluster: default/rook-cluster-nw - ``` +NetworkAttachmentDefinitions are selected for the desired Ceph network using `selectors`. Selector +values should include the namespace in which the NAD is present. `public` and `cluster` may be +selected independently. If `public` is left unspecified, Rook will configure Ceph to use the +Kubernetes pod network for Ceph client traffic. - * This format is required in order to use the NetworkAttachmentDefinition across namespaces. - * In Openshift, to use a NetworkAttachmentDefinition (NAD) across namespaces, the NAD must be deployed in the `default` namespace. The NAD is then referenced with the namespace: `default/rook-public-nw` +Consider the example below which selects a hypothetical Kubernetes-wide Multus network in the +default namespace for Ceph's public network and selects a Ceph-specific network in the `rook-ceph` +namespace for Ceph's cluster network. The commented-out portion shows an example of how address +ranges could be manually specified for the networks if needed. + +```yaml + network: + provider: multus + selectors: + public: default/kube-multus-net + cluster: rook-ceph/ceph-multus-net + # addressRanges: + # public: + # - "192.168.100.0/24" + # - "192.168.101.0/24" + # cluster: + # - "192.168.200.0/24" +``` ##### Validating Multus configuration diff --git a/Documentation/CRDs/specification.md b/Documentation/CRDs/specification.md index aeafb2beb..f9b8cd385 100644 --- a/Documentation/CRDs/specification.md +++ b/Documentation/CRDs/specification.md @@ -2505,6 +2505,51 @@ string +

AddressRangesSpec +

+

+(Appears on:NetworkSpec) +

+
+
+ + + + + + + + + + + + + + + + + +
FieldDescription
+public
+ + +CIDRList + + +		 +(Optional) +

Public defines a list of CIDRs to use for Ceph public network communication.

+
+cluster
+ + +CIDRList + + +		 +(Optional) +

Cluster defines a list of CIDRs to use for Ceph cluster network communication.

+

Annotations (map[string]string alias)

@@ -2716,6 +2761,13 @@ int64 +

CIDR +(string alias)

+
+

An IPv4 or IPv6 network CIDR.

+

This naive kubebuilder regex provides immediate feedback for some typos and for a common problem +case where the range spec is forgotten (e.g., /24). Rook does in-depth validation in code.

+

COSIDeploymentStrategy (string alias)

@@ -3508,6 +3560,25 @@ string +

CephNetworkType +(string alias)

+
+

CephNetworkType should be “public” or “cluster”. +Allow any string so that over-specified legacy clusters do not break on CRD update.

+
+ + + + + + + + + + + + +
ValueDescription

"cluster"

"public"

CephStatus

@@ -7979,6 +8050,29 @@ PoolSpec +

NetworkProviderType +(string alias)

+

+(Appears on:NetworkSpec) +

+
+

NetworkProviderType defines valid network providers for Rook.

+
+ + + + + + + + + + + + + + +
ValueDescription

""

"host"

"multus"

NetworkSpec

@@ -7999,7 +8093,9 @@ PoolSpec provider
-string + +NetworkProviderType + @@ -8011,14 +8107,45 @@ string selectors
-map[string]string +map[github.com/rook/rook/pkg/apis/ceph.rook.io/v1.CephNetworkType]string (Optional) -

Selectors string values describe what networks will be used to connect the cluster. -Meanwhile the keys describe each network respective responsibilities or any metadata -storage provider decide.

+

Selectors define NetworkAttachmentDefinitions to be used for Ceph public and/or cluster +networks when the “multus” network provider is used. This config section is not used for +other network providers.

+

Valid keys are “public” and “cluster”. Refer to Ceph networking documentation for more: +https://docs.ceph.com/en/reef/rados/configuration/network-config-ref/

+

Refer to Multus network annotation documentation for help selecting values: +https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/how-to-use.md#run-pod-with-network-annotation

+

Rook will make a best-effort attempt to automatically detect CIDR address ranges for given +network attachment definitions. Rook’s methods are robust but may be imprecise for +sufficiently complicated networks. Rook’s auto-detection process obtains a new IP address +lease for each CephCluster reconcile. If Rook fails to detect, incorrectly detects, only +partially detects, or if underlying networks do not support reusing old IP addresses, it is +best to use the ‘addressRanges’ config section to specify CIDR ranges for the Ceph cluster.

+

As a contrived example, one can use a theoretical Kubernetes-wide network for Ceph client +traffic and a theoretical Rook-only network for Ceph replication traffic as shown: +selectors: +public: “default/cluster-fast-net” +cluster: “rook-ceph/ceph-backend-net”

+ + + + +addressRanges
+ + +AddressRangesSpec + + + + +(Optional) +

AddressRanges specify a list of CIDRs that Rook will apply to Ceph’s ‘public_network’ and/or +‘cluster_network’ configurations. This config section may be used for the “host” or “multus” +network providers.

diff --git a/Documentation/Storage-Configuration/Advanced/ceph-configuration.md b/Documentation/Storage-Configuration/Advanced/ceph-configuration.md index ef9536b7d..5b9ff44a4 100644 --- a/Documentation/Storage-Configuration/Advanced/ceph-configuration.md +++ b/Documentation/Storage-Configuration/Advanced/ceph-configuration.md @@ -363,12 +363,17 @@ ceph osd primary-affinity osd.0 0 ## OSD Dedicated Network +!!! tip + This documentation is left for historical purposes. It is still valid, but Rook offers native + support for this feature via the + [CephCluster network configuration](../../CRDs/Cluster/ceph-cluster-crd.md#ceph-public-and-cluster-networks). + It is possible to configure ceph to leverage a dedicated network for the OSDs to -communicate across. A useful overview is the [CEPH Networks](http://docs.ceph.com/docs/master/rados/configuration/network-config-ref/#ceph-networks) +communicate across. A useful overview is the [Ceph Networks](http://docs.ceph.com/docs/master/rados/configuration/network-config-ref/#ceph-networks) section of the Ceph documentation. If you declare a cluster network, OSDs will -route heartbeat, object replication and recovery traffic over the cluster +route heartbeat, object replication, and recovery traffic over the cluster network. This may improve performance compared to using a single network, -especially when slower network technologies are used, with the tradeoff of +especially when slower network technologies are used. The tradeoff is additional expense and subtle failure modes. Two changes are necessary to the configuration to enable this capability: @@ -404,7 +409,7 @@ apiVersion: v1 data: config: | [global] - public network = 10.0.7.0/24 + public network = 10.0.7.0/24 cluster network = 10.0.10.0/24 public addr = "" cluster addr = "" diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index 75c8ad415..5ed351b3d 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -2125,6 +2125,25 @@ spec: description: Network related configuration nullable: true properties: + addressRanges: + description: AddressRanges specify a list of CIDRs that Rook will apply to Ceph's 'public_network' and/or 'cluster_network' configurations. This config section may be used for the "host" or "multus" network providers. + nullable: true + properties: + cluster: + description: Cluster defines a list of CIDRs to use for Ceph cluster network communication. + items: + description: "An IPv4 or IPv6 network CIDR. \n This naive kubebuilder regex provides immediate feedback for some typos and for a common problem case where the range spec is forgotten (e.g., /24). Rook does in-depth validation in code." + pattern: ^[0-9a-fA-F:.]{2,}\/[0-9]{1,3}$ + type: string + type: array + public: + description: Public defines a list of CIDRs to use for Ceph public network communication. + items: + description: "An IPv4 or IPv6 network CIDR. \n This naive kubebuilder regex provides immediate feedback for some typos and for a common problem case where the range spec is forgotten (e.g., /24). Rook does in-depth validation in code." + pattern: ^[0-9a-fA-F:.]{2,}\/[0-9]{1,3}$ + type: string + type: array + type: object connections: description: Settings for network connections such as compression and encryption across the wire. nullable: true @@ -2174,12 +2193,16 @@ spec: type: object provider: description: Provider is what provides network connectivity to the cluster e.g. "host" or "multus" + enum: + - "" + - host + - multus nullable: true type: string selectors: additionalProperties: type: string - description: Selectors string values describe what networks will be used to connect the cluster. Meanwhile the keys describe each network respective responsibilities or any metadata storage provider decide. + description: "Selectors define NetworkAttachmentDefinitions to be used for Ceph public and/or cluster networks when the \"multus\" network provider is used. This config section is not used for other network providers. \n Valid keys are \"public\" and \"cluster\". Refer to Ceph networking documentation for more: https://docs.ceph.com/en/reef/rados/configuration/network-config-ref/ \n Refer to Multus network annotation documentation for help selecting values: https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/how-to-use.md#run-pod-with-network-annotation \n Rook will make a best-effort attempt to automatically detect CIDR address ranges for given network attachment definitions. Rook's methods are robust but may be imprecise for sufficiently complicated networks. Rook's auto-detection process obtains a new IP address lease for each CephCluster reconcile. If Rook fails to detect, incorrectly detects, only partially detects, or if underlying networks do not support reusing old IP addresses, it is best to use the 'addressRanges' config section to specify CIDR ranges for the Ceph cluster. \n As a contrived example, one can use a theoretical Kubernetes-wide network for Ceph client traffic and a theoretical Rook-only network for Ceph replication traffic as shown: selectors: public: \"default/cluster-fast-net\" cluster: \"rook-ceph/ceph-backend-net\"" nullable: true type: object type: object diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index 213f53db4..731992302 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -2123,6 +2123,25 @@ spec: description: Network related configuration nullable: true properties: + addressRanges: + description: AddressRanges specify a list of CIDRs that Rook will apply to Ceph's 'public_network' and/or 'cluster_network' configurations. This config section may be used for the "host" or "multus" network providers. + nullable: true + properties: + cluster: + description: Cluster defines a list of CIDRs to use for Ceph cluster network communication. + items: + description: "An IPv4 or IPv6 network CIDR. \n This naive kubebuilder regex provides immediate feedback for some typos and for a common problem case where the range spec is forgotten (e.g., /24). Rook does in-depth validation in code." + pattern: ^[0-9a-fA-F:.]{2,}\/[0-9]{1,3}$ + type: string + type: array + public: + description: Public defines a list of CIDRs to use for Ceph public network communication. + items: + description: "An IPv4 or IPv6 network CIDR. \n This naive kubebuilder regex provides immediate feedback for some typos and for a common problem case where the range spec is forgotten (e.g., /24). Rook does in-depth validation in code." + pattern: ^[0-9a-fA-F:.]{2,}\/[0-9]{1,3}$ + type: string + type: array + type: object connections: description: Settings for network connections such as compression and encryption across the wire. nullable: true @@ -2172,12 +2191,16 @@ spec: type: object provider: description: Provider is what provides network connectivity to the cluster e.g. "host" or "multus" + enum: + - "" + - host + - multus nullable: true type: string selectors: additionalProperties: type: string - description: Selectors string values describe what networks will be used to connect the cluster. Meanwhile the keys describe each network respective responsibilities or any metadata storage provider decide. + description: "Selectors define NetworkAttachmentDefinitions to be used for Ceph public and/or cluster networks when the \"multus\" network provider is used. This config section is not used for other network providers. \n Valid keys are \"public\" and \"cluster\". Refer to Ceph networking documentation for more: https://docs.ceph.com/en/reef/rados/configuration/network-config-ref/ \n Refer to Multus network annotation documentation for help selecting values: https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/how-to-use.md#run-pod-with-network-annotation \n Rook will make a best-effort attempt to automatically detect CIDR address ranges for given network attachment definitions. Rook's methods are robust but may be imprecise for sufficiently complicated networks. Rook's auto-detection process obtains a new IP address lease for each CephCluster reconcile. If Rook fails to detect, incorrectly detects, only partially detects, or if underlying networks do not support reusing old IP addresses, it is best to use the 'addressRanges' config section to specify CIDR ranges for the Ceph cluster. \n As a contrived example, one can use a theoretical Kubernetes-wide network for Ceph client traffic and a theoretical Rook-only network for Ceph replication traffic as shown: selectors: public: \"default/cluster-fast-net\" cluster: \"rook-ceph/ceph-backend-net\"" nullable: true type: object type: object diff --git a/go.mod b/go.mod index 963643f3f..e6a15c2e8 100644 --- a/go.mod +++ b/go.mod @@ -117,6 +117,7 @@ require ( github.com/prometheus/procfs v0.11.1 // indirect github.com/ryanuber/go-glob v1.0.0 // indirect github.com/sirupsen/logrus v1.9.3 // indirect + github.com/stretchr/objx v0.5.0 // indirect github.com/xlab/treeprint v1.2.0 // indirect go.starlark.net v0.0.0-20230814145427-12f4cb8177e4 // indirect go.uber.org/multierr v1.11.0 // indirect diff --git a/pkg/apis/ceph.rook.io/v1/cluster.go b/pkg/apis/ceph.rook.io/v1/cluster.go index 05b4d7515..f839c2d4f 100644 --- a/pkg/apis/ceph.rook.io/v1/cluster.go +++ b/pkg/apis/ceph.rook.io/v1/cluster.go @@ -67,6 +67,11 @@ func (c *CephCluster) ValidateCreate() (admission.Warnings, error) { return nil, errors.New("invalid create : external mode enabled cannot have mon,dashboard,monitoring,network,disruptionManagement,storage fields in CR") } } + + if err := ValidateNetworkSpec(c.GetNamespace(), c.Spec.Network); err != nil { + return nil, errors.Errorf("invalid create: %s", err) + } + return nil, nil } @@ -85,11 +90,8 @@ func validateUpdatedCephCluster(updatedCephCluster *CephCluster, found *CephClus return nil, errors.Errorf("invalid update: DataDirHostPath change from %q to %q is not allowed", found.Spec.DataDirHostPath, updatedCephCluster.Spec.DataDirHostPath) } - // Allow an attempt to enable or disable host networking, but not other provider changes - oldProvider := updatedCephCluster.Spec.Network.Provider - newProvider := found.Spec.Network.Provider - if oldProvider != newProvider && oldProvider != "host" && newProvider != "host" { - return nil, errors.Errorf("invalid update: Provider change from %q to %q is not allowed", found.Spec.Network.Provider, updatedCephCluster.Spec.Network.Provider) + if err := ValidateNetworkSpecUpdate(found.GetNamespace(), found.Spec.Network, updatedCephCluster.Spec.Network); err != nil { + return nil, errors.Errorf("invalid update: %s", err) } if len(updatedCephCluster.Spec.Storage.StorageClassDeviceSets) == len(found.Spec.Storage.StorageClassDeviceSets) { diff --git a/pkg/apis/ceph.rook.io/v1/network.go b/pkg/apis/ceph.rook.io/v1/network.go index 68c0296b3..6abd842ee 100644 --- a/pkg/apis/ceph.rook.io/v1/network.go +++ b/pkg/apis/ceph.rook.io/v1/network.go @@ -16,13 +16,160 @@ limitations under the License. package v1 +import ( + "encoding/json" + "fmt" + "net" + "strings" + + nadv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + nadutils "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/utils" + "github.com/pkg/errors" +) + // IsMultus get whether to use multus network provider func (n *NetworkSpec) IsMultus() bool { - return n.Provider == "multus" + return n.Provider == NetworkProviderMultus } // IsHost get whether to use host network provider. This method also preserve // compatibility with the old HostNetwork field. func (n *NetworkSpec) IsHost() bool { - return (n.HostNetwork && n.Provider == "") || n.Provider == "host" + return (n.HostNetwork && n.Provider == NetworkProviderDefault) || n.Provider == NetworkProviderHost +} + +func ValidateNetworkSpec(clusterNamespace string, spec NetworkSpec) error { + if spec.IsMultus() { + if len(spec.Selectors) == 0 { + return errors.Errorf("at least one network selector must be specified when using the %q network provider", NetworkProviderMultus) + } + + if _, err := spec.GetNetworkSelection(clusterNamespace, CephNetworkPublic); err != nil { + return errors.Wrap(err, "ceph public network selector provided for multus is invalid") + } + if _, err := spec.GetNetworkSelection(clusterNamespace, CephNetworkCluster); err != nil { + return errors.Wrap(err, "ceph cluster network selector provided for multus is invalid") + } + } + + if !spec.AddressRanges.IsEmpty() { + if !spec.IsMultus() && !spec.IsHost() { + // TODO: be sure to update docs that AddressRanges can be specified for host networking as + // well as multus so that the override configmap doesn't need to be set + return errors.Errorf("network ranges can only be specified for %q and %q network providers", NetworkProviderHost, NetworkProviderMultus) + } + if spec.IsMultus() { + if len(spec.AddressRanges.Public) > 0 && !spec.NetworkHasSelection(CephNetworkPublic) { + return errors.Errorf("public address range can only be specified for multus if there is a public network selection") + } + if len(spec.AddressRanges.Cluster) > 0 && !spec.NetworkHasSelection(CephNetworkCluster) { + return errors.Errorf("cluster address range can only be specified for multus if there is a cluster network selection") + } + } + } + + if err := spec.AddressRanges.Validate(); err != nil { + return err + } + + return nil +} + +func ValidateNetworkSpecUpdate(clusterNamespace string, oldSpec, newSpec NetworkSpec) error { + // Allow an attempt to enable or disable host networking, but not other provider changes + oldProvider := oldSpec.Provider + newProvider := newSpec.Provider + if oldProvider != newProvider && oldProvider != "host" && newProvider != "host" { + return errors.Errorf("invalid update: network provider change from %q to %q is not allowed", oldProvider, newProvider) + } + + return ValidateNetworkSpec(clusterNamespace, newSpec) +} + +// NetworkHasSelection returns true if the given Ceph network has a selection. +func (n *NetworkSpec) NetworkHasSelection(network CephNetworkType) bool { + s, ok := n.Selectors[network] + if !ok || s == "" { + return false + } + return true +} + +// GetNetworkSelection gets the network selection for a given Ceph network, or nil if the network +// doesn't have a selection. +func (n *NetworkSpec) GetNetworkSelection(clusterNamespace string, network CephNetworkType) (*nadv1.NetworkSelectionElement, error) { + if !n.NetworkHasSelection(network) { + return nil, nil // no selection for network + } + s := n.Selectors[network] + // From documentation of the "k8s.v1.cni.cncf.io/network-status" annotation, valid JSON inputs + // must be in list form, surrounded with brackets. The NAD utility library will only parse + // list-format JSON input. However, old versions of Rook code allowed non-list JSON objects. + // In order to support legacy users, make an attempt to turn single-JSON-object inputs into + // len(1) lists so that they parse correctly by the util library. Do not advertise this + // "feature" in documentation since it is not technically the correct format. + if strings.HasPrefix(s, "{") && strings.HasSuffix(s, "}") { + s = "[" + s + "]" + } + selection, err := nadutils.ParseNetworkAnnotation(s, clusterNamespace) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse %q network selector %q", network, s) + } + if len(selection) != 1 { + return nil, errors.Errorf("%q network selector %q has multiple (%d) selections, which is not supported", network, s, len(selection)) + } + return selection[0], nil +} + +// NetworkSelectionsToAnnotationValue converts NetworkAttachmentDefinition network selection +// elements to an annotation value for the "k8s.v1.cni.cncf.io/networks" annotation key. +func NetworkSelectionsToAnnotationValue(selections ...*nadv1.NetworkSelectionElement) (string, error) { + reduced := []*nadv1.NetworkSelectionElement{} + for _, s := range selections { + if s != nil { + reduced = append(reduced, s) + } + } + if len(reduced) == 0 { + return "", nil + } + b, err := json.Marshal(reduced) + if err != nil { + return "", errors.Wrap(err, "failed to convert network selections to annotation value") + } + return string(b), nil +} + +func (n *AddressRangesSpec) IsEmpty() bool { + return n == nil || len(n.Public) == 0 && len(n.Cluster) == 0 +} + +func (n *AddressRangesSpec) Validate() error { + if n.IsEmpty() { + return nil + } + + allRanges := append(n.Public, n.Cluster...) + invalid := []string{} + for _, cidr := range allRanges { + _, _, err := net.ParseCIDR(string(cidr)) + if err != nil { + // returned err is "invalid CIDR: " & not more useful than invalid list below + invalid = append(invalid, string(cidr)) + } + } + if len(invalid) == 0 { + return nil + } + + return fmt.Errorf("%d network ranges are invalid: %v", len(invalid), invalid) +} + +// String turns a CIDR list into a comma-delimited string of CIDRs +func (l *CIDRList) String() string { + sl := []string{} + for _, c := range *l { + sl = append(sl, string(c)) + } + return strings.Join(sl, ", ") } diff --git a/pkg/apis/ceph.rook.io/v1/network_test.go b/pkg/apis/ceph.rook.io/v1/network_test.go index 7866c4796..38d4be111 100644 --- a/pkg/apis/ceph.rook.io/v1/network_test.go +++ b/pkg/apis/ceph.rook.io/v1/network_test.go @@ -18,8 +18,10 @@ package v1 import ( "encoding/json" + "fmt" "testing" + nadv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/yaml" ) @@ -63,7 +65,7 @@ selectors: expected := NetworkSpec{ Provider: "host", - Selectors: map[string]string{ + Selectors: map[CephNetworkType]string{ "server": "enp2s0f0", "broker": "enp2s0f0", }, @@ -71,3 +73,292 @@ selectors: assert.Equal(t, expected, net) } + +func TestAddressRangesSpec_IsEmpty(t *testing.T) { + var specNil *AddressRangesSpec + assert.True(t, specNil.IsEmpty()) + + empty := &AddressRangesSpec{} + assert.True(t, empty.IsEmpty()) + + someCIDR := CIDR("1.1.1.1/16") + nonEmptyTests := []AddressRangesSpec{ + {Public: []CIDR{someCIDR}}, + {Public: []CIDR{someCIDR, someCIDR}}, + {Cluster: []CIDR{someCIDR}}, + {Cluster: []CIDR{someCIDR, someCIDR}}, + {Public: []CIDR{someCIDR}, Cluster: []CIDR{someCIDR}}, + {Public: []CIDR{someCIDR, someCIDR}, Cluster: []CIDR{someCIDR, someCIDR}}, + } + for _, spec := range nonEmptyTests { + assert.False(t, spec.IsEmpty()) + } + +} + +func TestAddressRangesSpec_Validate(t *testing.T) { + // only test a small subset of CIDRs since Rook should definitely use the Go stdlib underneath + v1 := CIDR("123.123.123.123/24") + v2 := CIDR("1.0.0.1/24") + v3 := CIDR("2000::/64") + v4 := CIDR("2000:2000:2000:2000:2000:2000:2000:2000/64") + v5 := CIDR("2000::128.128.128.128/96") // ipv4 expressed as subnet of ipv6 is valid + + // invalid CIDRs + i1 := CIDR("123.123.123/24") + i2 := CIDR("123.123.123.123/33") + i4 := CIDR("2000/64") + i3 := CIDR("2000:/64") + i5 := CIDR("2000::128.128.128.128/129") + + tests := []struct { + name string + spec AddressRangesSpec + numErrs int + }{ + {"empty", AddressRangesSpec{}, 0}, + {"all valid", AddressRangesSpec{ + Public: []CIDR{v1}, + Cluster: []CIDR{v2, v3, v4, v5}, + }, 0}, + {"all invalid", AddressRangesSpec{ + Public: []CIDR{i1}, + Cluster: []CIDR{i2, i3, i4, i5}, + }, 5}, + {"public only, valid", AddressRangesSpec{Public: []CIDR{v1}}, 0}, + {"public only, invalid", AddressRangesSpec{Public: []CIDR{i1}}, 1}, + {"cluster only, valid", AddressRangesSpec{Cluster: []CIDR{v2}}, 0}, + {"cluster only, invalid", AddressRangesSpec{Cluster: []CIDR{i2}}, 1}, + {"public valid, cluster valid", AddressRangesSpec{ + Public: []CIDR{v1}, + Cluster: []CIDR{v2}, + }, 0}, + {"public valid, cluster invalid", AddressRangesSpec{ + Public: []CIDR{v2}, + Cluster: []CIDR{i2}, + }, 1}, + {"public invalid, cluster valid", AddressRangesSpec{ + Public: []CIDR{i3}, + Cluster: []CIDR{v2}, + }, 1}, + {"public invalid, cluster invalid", AddressRangesSpec{ + Public: []CIDR{i3}, + Cluster: []CIDR{i4}, + }, 2}, + {"both, valid and invalid", AddressRangesSpec{ + Public: []CIDR{v1, i2}, + Cluster: []CIDR{v3, i4}, + }, 2}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.spec.Validate() + if tt.numErrs > 0 { + assert.Error(t, err) + t.Log(err) + assert.ErrorContains(t, err, fmt.Sprintf("%d network ranges are invalid", tt.numErrs)) + } else { + assert.NoError(t, err) + } + }) + } +} + +// these two functions are should almost always used together and can be unit tested together more +// easily than apart +func TestNetworkSpec_GetNetworkSelection_NetworkSelectionsToAnnotationValue(t *testing.T) { + // inputs are the same definition expressed in json format or non-json format + input1 := func(json bool) string { + if json { + return `[{"name": "macvlan", "interface": "net1"}]` + } + return "macvlan@net1" + } + input2 := func(json bool) string { + if json { + return `[{"name": "macvlan", "interface": "net2"}]` + } + return "macvlan@net2" + } + + // allow running the test suite with json-format or non-json-format inputs + testGetNetworkAnnotationValue := func(t *testing.T, json bool) { + t.Helper() + + tests := []struct { + name string + specSelectors map[CephNetworkType]string + cephNets []CephNetworkType + want string + wantErr bool + }{ + { + name: "public want public", + specSelectors: map[CephNetworkType]string{ + "public": input1(json), + }, + cephNets: []CephNetworkType{CephNetworkPublic}, + want: `[{"name":"macvlan","namespace":"ns","interface":"net1"}]`, + wantErr: false, + }, + { + name: "cluster want cluster", + specSelectors: map[CephNetworkType]string{ + "cluster": input1(json), + }, + cephNets: []CephNetworkType{CephNetworkCluster}, + want: `[{"name":"macvlan","namespace":"ns","interface":"net1"}]`, + wantErr: false, + }, + { + name: "public want cluster", + specSelectors: map[CephNetworkType]string{ + "public": input1(json), + }, + cephNets: []CephNetworkType{CephNetworkCluster}, + want: ``, + wantErr: false, + }, + { + name: "cluster want public", + specSelectors: map[CephNetworkType]string{ + "cluster": input1(json), + }, + cephNets: []CephNetworkType{CephNetworkPublic}, + want: ``, + wantErr: false, + }, + { + name: "nothing want public", + specSelectors: map[CephNetworkType]string{}, + cephNets: []CephNetworkType{CephNetworkPublic}, + want: ``, + wantErr: false, + }, + { + name: "nothing want cluster", + specSelectors: map[CephNetworkType]string{}, + cephNets: []CephNetworkType{CephNetworkCluster}, + want: ``, + wantErr: false, + }, + { + name: "unknown want public", + specSelectors: map[CephNetworkType]string{ + "uncleKnown": input1(json), + }, + cephNets: []CephNetworkType{CephNetworkPublic}, + want: ``, + wantErr: false, + }, + { + name: "unknown want cluster", + specSelectors: map[CephNetworkType]string{ + "uncleKnown": input1(json), + }, + cephNets: []CephNetworkType{CephNetworkCluster}, + want: ``, + wantErr: false, + }, + { + name: "public want public and cluster", + specSelectors: map[CephNetworkType]string{ + "public": input1(json), + }, + cephNets: []CephNetworkType{CephNetworkPublic, CephNetworkCluster}, + want: `[{"name":"macvlan","namespace":"ns","interface":"net1"}]`, + wantErr: false, + }, + { + name: "cluster want public and cluster", + specSelectors: map[CephNetworkType]string{ + "cluster": input1(json), + }, + cephNets: []CephNetworkType{CephNetworkPublic, CephNetworkCluster}, + want: `[{"name":"macvlan","namespace":"ns","interface":"net1"}]`, + wantErr: false, + }, + { + name: "public and cluster want public and cluster", + specSelectors: map[CephNetworkType]string{ + "public": input1(json), + "cluster": input2(json), + }, + cephNets: []CephNetworkType{CephNetworkPublic, CephNetworkCluster}, + want: `[{"name":"macvlan","namespace":"ns","interface":"net1"},{"name":"macvlan","namespace":"ns","interface":"net2"}]`, + wantErr: false, + }, + { + name: "support mixed json-non-json spec", + specSelectors: map[CephNetworkType]string{ + "public": input1(json), + "cluster": input2(!json), // invert json-ness of this one + }, + cephNets: []CephNetworkType{CephNetworkPublic, CephNetworkCluster}, + want: `[{"name":"macvlan","namespace":"ns","interface":"net1"},{"name":"macvlan","namespace":"ns","interface":"net2"}]`, + wantErr: false, + }, + { + name: "public and cluster want nothing", + specSelectors: map[CephNetworkType]string{ + "public": input1(json), + "cluster": input2(json), + }, + cephNets: []CephNetworkType{}, + want: ``, + wantErr: false, + }, + { + name: "legacy single json object support", + specSelectors: map[CephNetworkType]string{ + "public": `{"name": "legacyJsonObject"}`, + }, + cephNets: []CephNetworkType{CephNetworkPublic, CephNetworkCluster}, + want: `[{"name":"legacyJsonObject","namespace":"ns"}]`, + wantErr: false, + }, + { + name: "invalid network selections", + specSelectors: map[CephNetworkType]string{ + "public": `[{"name": "jsonWithNoClosingBracket"}`, + "cluster": "multus%net", + }, + cephNets: []CephNetworkType{CephNetworkPublic, CephNetworkCluster}, + want: ``, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + n := &NetworkSpec{ + Selectors: tt.specSelectors, + } + + selections := []*nadv1.NetworkSelectionElement{} + errs := []error{} + for _, net := range tt.cephNets { + s, err := n.GetNetworkSelection("ns", net) + if err != nil { + errs = append(errs, err) + } + selections = append(selections, s) + } + got, err := NetworkSelectionsToAnnotationValue(selections...) + if err != nil { + errs = append(errs, err) + } + + assert.Equal(t, tt.wantErr, len(errs) > 0, "wantErr %v but got errs %v", tt.wantErr, errs) + assert.Equal(t, tt.want, got) + }) + } + } + + // Actual subtests + t.Run("non-JSON input", func(t *testing.T) { + testGetNetworkAnnotationValue(t, false) + }) + t.Run("JSON input", func(t *testing.T) { + testGetNetworkAnnotationValue(t, true) + }) +} diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index 132eb89a2..c4c347e66 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -2267,14 +2267,41 @@ type NetworkSpec struct { // Provider is what provides network connectivity to the cluster e.g. "host" or "multus" // +nullable // +optional - Provider string `json:"provider,omitempty"` + Provider NetworkProviderType `json:"provider,omitempty"` - // Selectors string values describe what networks will be used to connect the cluster. - // Meanwhile the keys describe each network respective responsibilities or any metadata - // storage provider decide. + // Selectors define NetworkAttachmentDefinitions to be used for Ceph public and/or cluster + // networks when the "multus" network provider is used. This config section is not used for + // other network providers. + // + // Valid keys are "public" and "cluster". Refer to Ceph networking documentation for more: + // https://docs.ceph.com/en/reef/rados/configuration/network-config-ref/ + // + // Refer to Multus network annotation documentation for help selecting values: + // https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/how-to-use.md#run-pod-with-network-annotation + // + // Rook will make a best-effort attempt to automatically detect CIDR address ranges for given + // network attachment definitions. Rook's methods are robust but may be imprecise for + // sufficiently complicated networks. Rook's auto-detection process obtains a new IP address + // lease for each CephCluster reconcile. If Rook fails to detect, incorrectly detects, only + // partially detects, or if underlying networks do not support reusing old IP addresses, it is + // best to use the 'addressRanges' config section to specify CIDR ranges for the Ceph cluster. + // + // As a contrived example, one can use a theoretical Kubernetes-wide network for Ceph client + // traffic and a theoretical Rook-only network for Ceph replication traffic as shown: + // selectors: + // public: "default/cluster-fast-net" + // cluster: "rook-ceph/ceph-backend-net" + // // +nullable // +optional - Selectors map[string]string `json:"selectors,omitempty"` + Selectors map[CephNetworkType]string `json:"selectors,omitempty"` + + // AddressRanges specify a list of CIDRs that Rook will apply to Ceph's 'public_network' and/or + // 'cluster_network' configurations. This config section may be used for the "host" or "multus" + // network providers. + // +nullable + // +optional + AddressRanges *AddressRangesSpec `json:"addressRanges,omitempty"` // Settings for network connections such as compression and encryption across the // wire. @@ -2301,6 +2328,45 @@ type NetworkSpec struct { MultiClusterService MultiClusterServiceSpec `json:"multiClusterService,omitempty"` } +// NetworkProviderType defines valid network providers for Rook. +// +kubebuilder:validation:Enum="";host;multus +type NetworkProviderType string + +const ( + NetworkProviderDefault = NetworkProviderType("") + NetworkProviderHost = NetworkProviderType("host") + NetworkProviderMultus = NetworkProviderType("multus") +) + +// CephNetworkType should be "public" or "cluster". +// Allow any string so that over-specified legacy clusters do not break on CRD update. +type CephNetworkType string + +const ( + CephNetworkPublic = CephNetworkType("public") + CephNetworkCluster = CephNetworkType("cluster") +) + +type AddressRangesSpec struct { + // Public defines a list of CIDRs to use for Ceph public network communication. + // +optional + Public CIDRList `json:"public"` + + // Cluster defines a list of CIDRs to use for Ceph cluster network communication. + // +optional + Cluster CIDRList `json:"cluster"` +} + +// An IPv4 or IPv6 network CIDR. +// +// This naive kubebuilder regex provides immediate feedback for some typos and for a common problem +// case where the range spec is forgotten (e.g., /24). Rook does in-depth validation in code. +// +kubebuilder:validation:Pattern=`^[0-9a-fA-F:.]{2,}\/[0-9]{1,3}$` +type CIDR string + +// A list of CIDRs. +type CIDRList []CIDR + type MultiClusterServiceSpec struct { // Enable multiClusterService to export the mon and OSD services to peer cluster. // Ensure that peer clusters are connected using an MCS API compatible application, diff --git a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go b/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go index f8bc4ed97..9731f39b2 100644 --- a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go +++ b/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go @@ -43,6 +43,32 @@ func (in *AMQPEndpointSpec) DeepCopy() *AMQPEndpointSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AddressRangesSpec) DeepCopyInto(out *AddressRangesSpec) { + *out = *in + if in.Public != nil { + in, out := &in.Public, &out.Public + *out = make(CIDRList, len(*in)) + copy(*out, *in) + } + if in.Cluster != nil { + in, out := &in.Cluster, &out.Cluster + *out = make(CIDRList, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AddressRangesSpec. +func (in *AddressRangesSpec) DeepCopy() *AddressRangesSpec { + if in == nil { + return nil + } + out := new(AddressRangesSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in Annotations) DeepCopyInto(out *Annotations) { { @@ -161,6 +187,26 @@ func (in *BucketTopicStatus) DeepCopy() *BucketTopicStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in CIDRList) DeepCopyInto(out *CIDRList) { + { + in := &in + *out = make(CIDRList, len(*in)) + copy(*out, *in) + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CIDRList. +func (in CIDRList) DeepCopy() CIDRList { + if in == nil { + return nil + } + out := new(CIDRList) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Capacity) DeepCopyInto(out *Capacity) { *out = *in @@ -2909,6 +2955,13 @@ func (in *Module) DeepCopy() *Module { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MonSpec) DeepCopyInto(out *MonSpec) { *out = *in + if in.Zones != nil { + in, out := &in.Zones, &out.Zones + *out = make([]MonZoneSpec, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.StretchCluster != nil { in, out := &in.StretchCluster, &out.StretchCluster *out = new(StretchClusterSpec) @@ -2932,6 +2985,27 @@ func (in *MonSpec) DeepCopy() *MonSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MonZoneSpec) DeepCopyInto(out *MonZoneSpec) { + *out = *in + if in.VolumeClaimTemplate != nil { + in, out := &in.VolumeClaimTemplate, &out.VolumeClaimTemplate + *out = new(corev1.PersistentVolumeClaim) + (*in).DeepCopyInto(*out) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MonZoneSpec. +func (in *MonZoneSpec) DeepCopy() *MonZoneSpec { + if in == nil { + return nil + } + out := new(MonZoneSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MonitoringSpec) DeepCopyInto(out *MonitoringSpec) { *out = *in @@ -3064,11 +3138,16 @@ func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { *out = *in if in.Selectors != nil { in, out := &in.Selectors, &out.Selectors - *out = make(map[string]string, len(*in)) + *out = make(map[CephNetworkType]string, len(*in)) for key, val := range *in { (*out)[key] = val } } + if in.AddressRanges != nil { + in, out := &in.AddressRanges, &out.AddressRanges + *out = new(AddressRangesSpec) + (*in).DeepCopyInto(*out) + } if in.Connections != nil { in, out := &in.Connections, &out.Connections *out = new(ConnectionsSpec) @@ -4294,27 +4373,6 @@ func (in *StretchClusterSpec) DeepCopy() *StretchClusterSpec { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MonZoneSpec) DeepCopyInto(out *MonZoneSpec) { - *out = *in - if in.VolumeClaimTemplate != nil { - in, out := &in.VolumeClaimTemplate, &out.VolumeClaimTemplate - *out = new(corev1.PersistentVolumeClaim) - (*in).DeepCopyInto(*out) - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MonZoneSpec. -func (in *MonZoneSpec) DeepCopy() *MonZoneSpec { - if in == nil { - return nil - } - out := new(MonZoneSpec) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TopicEndpointSpec) DeepCopyInto(out *TopicEndpointSpec) { *out = *in diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index 5df5caaf3..9ba83f9f6 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/pkg/operator/ceph/cluster/cluster.go @@ -44,7 +44,6 @@ import ( "github.com/rook/rook/pkg/operator/k8sutil" rookversion "github.com/rook/rook/pkg/version" v1 "k8s.io/api/core/v1" - kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) @@ -290,34 +289,9 @@ func preClusterStartValidation(cluster *cluster) error { if err := validateStretchCluster(cluster); err != nil { return err } - if cluster.Spec.Network.IsMultus() { - _, isPublic := cluster.Spec.Network.Selectors[config.PublicNetworkSelectorKeyName] - _, isCluster := cluster.Spec.Network.Selectors[config.ClusterNetworkSelectorKeyName] - if !isPublic && !isCluster { - return errors.New("both network selector values for public and cluster selector cannot be empty for multus provider") - } - for _, selector := range config.NetworkSelectors { - // If one selector is empty, we continue - // This means a single interface is used both public and cluster network - if _, ok := cluster.Spec.Network.Selectors[selector]; !ok { - continue - } - - multusNamespace, nad := config.GetMultusNamespace(cluster.Spec.Network.Selectors[selector]) - if multusNamespace == "" { - multusNamespace = cluster.Namespace - } - - // Get network attachment definition - _, err := cluster.context.NetworkClient.NetworkAttachmentDefinitions(multusNamespace).Get(cluster.ClusterInfo.Context, nad, metav1.GetOptions{}) - if err != nil { - if kerrors.IsNotFound(err) { - return errors.Wrapf(err, "specified network attachment definition for selector %q does not exist", selector) - } - return errors.Wrapf(err, "failed to fetch network attachment definition for selector %q", selector) - } - } + if err := cephv1.ValidateNetworkSpec(cluster.Namespace, cluster.Spec.Network); err != nil { + return errors.Wrapf(err, "failed to validate network spec for cluster in namespace %q", cluster.Namespace) } // Validate on-PVC cluster encryption KMS settings @@ -597,7 +571,7 @@ func (c *cluster) reportTelemetry() { telemetry.ReportKeyValue(c.context, c.ClusterInfo, telemetry.DeviceSetNonPortableKey, strconv.Itoa(nonportableDeviceSets)) // Set the telemetry for network settings - telemetry.ReportKeyValue(c.context, c.ClusterInfo, telemetry.NetworkProviderKey, c.Spec.Network.Provider) + telemetry.ReportKeyValue(c.context, c.ClusterInfo, telemetry.NetworkProviderKey, string(c.Spec.Network.Provider)) // Set the telemetry for external cluster settings telemetry.ReportKeyValue(c.context, c.ClusterInfo, telemetry.ExternalModeEnabledKey, strconv.FormatBool(c.Spec.External.Enable)) diff --git a/pkg/operator/ceph/cluster/mgr/spec.go b/pkg/operator/ceph/cluster/mgr/spec.go index bf616e5ee..48d3b8d34 100644 --- a/pkg/operator/ceph/cluster/mgr/spec.go +++ b/pkg/operator/ceph/cluster/mgr/spec.go @@ -97,7 +97,7 @@ func (c *Cluster) makeDeployment(mgrConfig *mgrConfig) (*apps.Deployment, error) if c.spec.Network.IsHost() { podSpec.Spec.DNSPolicy = v1.DNSClusterFirstWithHostNet } else if c.spec.Network.IsMultus() { - if err := k8sutil.ApplyMultus(c.spec.Network, &podSpec.ObjectMeta); err != nil { + if err := k8sutil.ApplyMultus(c.clusterInfo.Namespace, &c.spec.Network, &podSpec.ObjectMeta); err != nil { return nil, err } podSpec.Spec.Containers = append(podSpec.Spec.Containers, c.makeCmdProxySidecarContainer(mgrConfig)) diff --git a/pkg/operator/ceph/cluster/mon/mon.go b/pkg/operator/ceph/cluster/mon/mon.go index 5b6539132..c08d04e14 100644 --- a/pkg/operator/ceph/cluster/mon/mon.go +++ b/pkg/operator/ceph/cluster/mon/mon.go @@ -105,7 +105,7 @@ type Cluster struct { spec cephv1.ClusterSpec Namespace string Keyring string - rookVersion string + rookImage string orchestrationMutex sync.Mutex Port int32 maxMonID int @@ -170,7 +170,7 @@ func (c *Cluster) MaxMonID() int { } // Start begins the process of running a cluster of Ceph mons. -func (c *Cluster) Start(clusterInfo *cephclient.ClusterInfo, rookVersion string, cephVersion cephver.CephVersion, spec cephv1.ClusterSpec) (*cephclient.ClusterInfo, error) { +func (c *Cluster) Start(clusterInfo *cephclient.ClusterInfo, rookImage string, cephVersion cephver.CephVersion, spec cephv1.ClusterSpec) (*cephclient.ClusterInfo, error) { // Only one goroutine can orchestrate the mons at a time c.acquireOrchestrationLock() defer c.releaseOrchestrationLock() @@ -180,7 +180,7 @@ func (c *Cluster) Start(clusterInfo *cephclient.ClusterInfo, rookVersion string, if c.ClusterInfo.Context == nil { panic("nil context") } - c.rookVersion = rookVersion + c.rookImage = rookImage c.spec = spec // fail if we were instructed to deploy more than one mon on the same machine with host networking @@ -287,6 +287,11 @@ func (c *Cluster) startMons(targetCount int) error { } } + // apply network settings after mons have been created b/c they are set in the mon k-v store + if err := controller.ApplyCephNetworkSettings(c.ClusterInfo.Context, c.rookImage, c.context, &c.spec, c.ClusterInfo); err != nil { + return errors.Wrap(err, "failed to apply ceph network settings") + } + if c.spec.IsStretchCluster() { if err := c.configureStretchCluster(mons); err != nil { return errors.Wrap(err, "failed to configure stretch mons") @@ -658,7 +663,7 @@ func scheduleMonitor(c *Cluster, mon *monConfig) (*apps.Deployment, error) { // avoid issues with the real deployment, the canary should be careful not // to modify the storage by instead running an innocuous command. d.Spec.Template.Spec.InitContainers = []v1.Container{} - d.Spec.Template.Spec.Containers[0].Image = c.rookVersion + d.Spec.Template.Spec.Containers[0].Image = c.rookImage d.Spec.Template.Spec.Containers[0].Command = []string{"sleep"} // sleep responds to signals so we don't need to wrap it d.Spec.Template.Spec.Containers[0].Args = []string{"3600"} // remove the startup and liveness probes on the canary pod diff --git a/pkg/operator/ceph/cluster/mon/mon_test.go b/pkg/operator/ceph/cluster/mon/mon_test.go index 0d4c13c0a..045cf8e8e 100644 --- a/pkg/operator/ceph/cluster/mon/mon_test.go +++ b/pkg/operator/ceph/cluster/mon/mon_test.go @@ -104,7 +104,7 @@ func newCluster(context *clusterd.Context, namespace string, allowMultiplePerNod ClusterInfo: nil, context: context, Namespace: namespace, - rookVersion: "myversion", + rookImage: "myversion", spec: cephv1.ClusterSpec{ Mon: cephv1.MonSpec{ Count: 3, @@ -123,11 +123,11 @@ func newCluster(context *clusterd.Context, namespace string, allowMultiplePerNod } // setCommonMonProperties is a convenience helper for setting common test properties -func setCommonMonProperties(c *Cluster, currentMons int, mon cephv1.MonSpec, rookVersion string) { +func setCommonMonProperties(c *Cluster, currentMons int, mon cephv1.MonSpec, rookImage string) { c.ClusterInfo = clienttest.CreateTestClusterInfo(currentMons) c.spec.Mon.Count = mon.Count c.spec.Mon.AllowMultiplePerNode = mon.AllowMultiplePerNode - c.rookVersion = rookVersion + c.rookImage = rookImage } func TestResourceName(t *testing.T) { @@ -186,7 +186,7 @@ func TestStartMonPods(t *testing.T) { } // start a basic cluster - _, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + _, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) // test annotations @@ -197,7 +197,7 @@ func TestStartMonPods(t *testing.T) { validateStart(t, c) // starting again should be a no-op - _, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + _, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) validateStart(t, c) @@ -211,7 +211,7 @@ func TestOperatorRestart(t *testing.T) { c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // start a basic cluster - info, err := c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + info, err := c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) assert.NoError(t, info.IsInitialized()) @@ -221,7 +221,7 @@ func TestOperatorRestart(t *testing.T) { c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // starting again should be a no-op, but will not result in an error - info, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + info, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) assert.NoError(t, info.IsInitialized()) @@ -239,7 +239,7 @@ func TestOperatorRestartHostNetwork(t *testing.T) { c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // start a basic cluster - info, err := c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + info, err := c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) assert.NoError(t, info.IsInitialized()) @@ -251,7 +251,7 @@ func TestOperatorRestartHostNetwork(t *testing.T) { c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // starting again should be a no-op, but still results in an error - info, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + info, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) assert.NoError(t, info.IsInitialized(), info) diff --git a/pkg/operator/ceph/cluster/mon/node_test.go b/pkg/operator/ceph/cluster/mon/node_test.go index 67549859d..9c555b837 100644 --- a/pkg/operator/ceph/cluster/mon/node_test.go +++ b/pkg/operator/ceph/cluster/mon/node_test.go @@ -86,7 +86,7 @@ func TestHostNetworkSameNode(t *testing.T) { c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // start a basic cluster - _, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + _, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.Error(t, err) } @@ -104,7 +104,7 @@ func TestPodMemory(t *testing.T) { c := newCluster(context, namespace, true, r) c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // start a basic cluster - _, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + _, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) // Test REQUEST == LIMIT @@ -120,7 +120,7 @@ func TestPodMemory(t *testing.T) { c = newCluster(context, namespace, true, r) c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // start a basic cluster - _, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + _, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) // Test LIMIT != REQUEST but obviously LIMIT > REQUEST @@ -136,7 +136,7 @@ func TestPodMemory(t *testing.T) { c = newCluster(context, namespace, true, r) c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // start a basic cluster - _, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + _, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) // Test valid case where pod resource is set appropriately @@ -152,7 +152,7 @@ func TestPodMemory(t *testing.T) { c = newCluster(context, namespace, true, r) c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // start a basic cluster - _, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + _, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) // Test no resources were specified on the pod @@ -160,7 +160,7 @@ func TestPodMemory(t *testing.T) { c = newCluster(context, namespace, true, r) c.ClusterInfo = clienttest.CreateTestClusterInfo(1) // start a basic cluster - _, err = c.Start(c.ClusterInfo, c.rookVersion, cephver.Quincy, c.spec) + _, err = c.Start(c.ClusterInfo, c.rookImage, cephver.Quincy, c.spec) assert.NoError(t, err) } diff --git a/pkg/operator/ceph/cluster/mon/spec.go b/pkg/operator/ceph/cluster/mon/spec.go index 8d4c8ab81..a3a6f2f64 100644 --- a/pkg/operator/ceph/cluster/mon/spec.go +++ b/pkg/operator/ceph/cluster/mon/spec.go @@ -217,7 +217,13 @@ func (c *Cluster) makeMonPod(monConfig *monConfig, canary bool) (*corev1.Pod, er if monConfig.UseHostNetwork { pod.Spec.DNSPolicy = corev1.DNSClusterFirstWithHostNet } else if c.spec.Network.IsMultus() { - if err := k8sutil.ApplyMultus(c.spec.Network, &pod.ObjectMeta); err != nil { + cluster := cephv1.CephCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: c.Namespace, + }, + Spec: c.spec, + } + if err := k8sutil.ApplyMultus(cluster.GetNamespace(), &cluster.Spec.Network, &pod.ObjectMeta); err != nil { return nil, err } } diff --git a/pkg/operator/ceph/cluster/osd/key_rotation.go b/pkg/operator/ceph/cluster/osd/key_rotation.go index f5c2e4f31..b1af021af 100644 --- a/pkg/operator/ceph/cluster/osd/key_rotation.go +++ b/pkg/operator/ceph/cluster/osd/key_rotation.go @@ -166,7 +166,7 @@ func (c *Cluster) getKeyRotationPodTemplateSpec(osdProps osdProperties, osd OSDI if c.spec.Network.IsHost() { podTemplateSpec.Spec.DNSPolicy = v1.DNSClusterFirstWithHostNet } else if c.spec.Network.IsMultus() { - if err := k8sutil.ApplyMultus(c.spec.Network, &podTemplateSpec.ObjectMeta); err != nil { + if err := k8sutil.ApplyMultus(c.clusterInfo.Namespace, &c.spec.Network, &podTemplateSpec.ObjectMeta); err != nil { return nil, err } } diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index 3f3f9fbe0..789514ef4 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -638,7 +638,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC if c.spec.Network.IsHost() { podTemplateSpec.Spec.DNSPolicy = v1.DNSClusterFirstWithHostNet } else if c.spec.Network.IsMultus() { - if err := k8sutil.ApplyMultus(c.spec.Network, &podTemplateSpec.ObjectMeta); err != nil { + if err := k8sutil.ApplyMultus(c.clusterInfo.Namespace, &c.spec.Network, &podTemplateSpec.ObjectMeta); err != nil { return nil, err } } diff --git a/pkg/operator/ceph/cluster/rbd/spec.go b/pkg/operator/ceph/cluster/rbd/spec.go index 72d1eaf59..2fff871fa 100644 --- a/pkg/operator/ceph/cluster/rbd/spec.go +++ b/pkg/operator/ceph/cluster/rbd/spec.go @@ -66,7 +66,7 @@ func (r *ReconcileCephRBDMirror) makeDeployment(daemonConfig *daemonConfig, rbdM if r.cephClusterSpec.Network.IsHost() { podSpec.Spec.DNSPolicy = v1.DNSClusterFirstWithHostNet } else if r.cephClusterSpec.Network.IsMultus() { - if err := k8sutil.ApplyMultus(r.cephClusterSpec.Network, &podSpec.ObjectMeta); err != nil { + if err := k8sutil.ApplyMultus(r.clusterInfo.Namespace, &r.cephClusterSpec.Network, &podSpec.ObjectMeta); err != nil { return nil, err } } diff --git a/pkg/operator/ceph/config/config.go b/pkg/operator/ceph/config/config.go index f61b2e384..5e11bd6a0 100644 --- a/pkg/operator/ceph/config/config.go +++ b/pkg/operator/ceph/config/config.go @@ -137,20 +137,6 @@ func SetOrRemoveDefaultConfigs( } } - // Apply Multus if needed - if clusterSpec.Network.IsMultus() { - logger.Info("configuring ceph network(s) with multus") - cephNetworks, err := generateNetworkSettings(clusterInfo.Context, context, clusterInfo.Namespace, clusterSpec.Network.Selectors) - if err != nil { - return errors.Wrap(err, "failed to generate network settings") - } - - // Apply ceph network settings to the mon config store - if err := monStore.SetAll("global", cephNetworks); err != nil { - return errors.Wrap(err, "failed to network config overrides") - } - } - // This section will remove any previously configured option(s) from the mon centralized store // This is useful for scenarios where options are not needed anymore and we just want to reset to internal's default // On upgrade, the flag will be removed diff --git a/pkg/operator/ceph/config/monstore.go b/pkg/operator/ceph/config/monstore.go index 854ef7437..5abc9d5cd 100644 --- a/pkg/operator/ceph/config/monstore.go +++ b/pkg/operator/ceph/config/monstore.go @@ -56,8 +56,22 @@ type Option struct { Value string } +// SetIfChanged sets a config in the centralized mon configuration database if the config has +// changed value. +// https://docs.ceph.com/docs/master/rados/configuration/ceph-conf/#monitor-configuration-database +// +// There is a bug through at least Ceph v18 where `ceph config get global