Skip to content
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

Jump to bottom

ceph: added support for multus for csi #5740

Merged
merged 1 commit into from
Aug 20, 2020
Merged

ceph: added support for multus for csi #5740

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Documentation/ceph-cluster-crd.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ To use host networking, set `provider: host`.
#### Multus (EXPERIMENTAL)

Rook has experimental support for Multus.
Currently there is an [open issue](https://github.com/ceph/ceph-csi/issues/1323) in ceph-csi which explains the csi-rbdPlugin issue while using multus network.

The selector keys are required to be `public` and `cluster` where each represent:

Expand Down
5 changes: 3 additions & 2 deletions pkg/operator/ceph/csi/csi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ import (
"strconv"

"github.com/pkg/errors"
rookclient "github.com/rook/rook/pkg/client/clientset/versioned"
controllerutil "github.com/rook/rook/pkg/operator/ceph/controller"
"github.com/rook/rook/pkg/operator/k8sutil"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/version"
"k8s.io/client-go/kubernetes"
)

func ValidateAndConfigureDrivers(clientset kubernetes.Interface, namespace, rookImage, securityAccount string, serverVersion *version.Info, ownerRef *metav1.OwnerReference) {
func ValidateAndConfigureDrivers(clientset kubernetes.Interface, rookclientset rookclient.Interface, namespace, rookImage, securityAccount string, serverVersion *version.Info, ownerRef *metav1.OwnerReference) {
if !AllowUnsupported {
if err := validateCSIVersion(clientset, namespace, rookImage, securityAccount, ownerRef); err != nil {
logger.Errorf("invalid csi version. %+v", err)
Expand All @@ -37,7 +38,7 @@ func ValidateAndConfigureDrivers(clientset kubernetes.Interface, namespace, rook
logger.Info("Skipping csi version check, since unsupported versions are allowed")
}

if err := startDrivers(clientset, namespace, serverVersion, ownerRef); err != nil {
if err := startDrivers(clientset, rookclientset, namespace, serverVersion, ownerRef); err != nil {
logger.Errorf("failed to start Ceph csi drivers. %v", err)
return
}
Expand Down
43 changes: 42 additions & 1 deletion pkg/operator/ceph/csi/spec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"
"time"

rookclient "github.com/rook/rook/pkg/client/clientset/versioned"
controllerutil "github.com/rook/rook/pkg/operator/ceph/controller"
"github.com/rook/rook/pkg/operator/k8sutil"
"github.com/rook/rook/pkg/operator/k8sutil/cmdreporter"
Expand Down Expand Up @@ -198,7 +199,7 @@ func ValidateCSIParam() error {
return nil
}

func startDrivers(clientset kubernetes.Interface, namespace string, ver *version.Info, ownerRef *metav1.OwnerReference) error {
func startDrivers(clientset kubernetes.Interface, rookclientset rookclient.Interface, namespace string, ver *version.Info, ownerRef *metav1.OwnerReference) error {
var (
err error
rbdPlugin, cephfsPlugin *apps.DaemonSet
Expand Down Expand Up @@ -361,6 +362,13 @@ func startDrivers(clientset kubernetes.Interface, namespace string, ver *version
// apply resource request and limit to rbdplugin containers
applyResourcesToContainers(clientset, rbdPluginResource, &rbdPlugin.Spec.Template.Spec)
k8sutil.SetOwnerRef(&rbdPlugin.ObjectMeta, ownerRef)
multusApplied, err := applyCephClusterNetworkConfig(&rbdPlugin.Spec.Template.ObjectMeta, rookclientset)
if err != nil {
return errors.Wrapf(err, "failed to apply network config to rbd plugin daemonset: %+v", rbdPlugin)
}
if multusApplied {
rbdPlugin.Spec.Template.Spec.HostNetwork = false
}
err = k8sutil.CreateDaemonSet(csiRBDPlugin, namespace, clientset, rbdPlugin)
if err != nil {
return errors.Wrapf(err, "failed to start rbdplugin daemonset: %+v", rbdPlugin)
Expand All @@ -379,6 +387,10 @@ func startDrivers(clientset kubernetes.Interface, namespace string, ver *version
Type: apps.RecreateDeploymentStrategyType,
}

_, err = applyCephClusterNetworkConfig(&rbdProvisionerDeployment.Spec.Template.ObjectMeta, rookclientset)
if err != nil {
return errors.Wrapf(err, "failed to apply network config to rbd plugin provisioner deployment: %+v", rbdProvisionerDeployment)
}
err = k8sutil.CreateDeployment(clientset, csiRBDProvisioner, namespace, rbdProvisionerDeployment)
if err != nil {
return errors.Wrapf(err, "failed to start rbd provisioner deployment: %+v", rbdProvisionerDeployment)
Expand All @@ -399,6 +411,13 @@ func startDrivers(clientset kubernetes.Interface, namespace string, ver *version
// apply resource request and limit to cephfs plugin containers
applyResourcesToContainers(clientset, cephFSPluginResource, &cephfsPlugin.Spec.Template.Spec)
k8sutil.SetOwnerRef(&cephfsPlugin.ObjectMeta, ownerRef)
multusApplied, err := applyCephClusterNetworkConfig(&cephfsPlugin.Spec.Template.ObjectMeta, rookclientset)
if err != nil {
return errors.Wrapf(err, "failed to apply network config to cephfs plugin daemonset: %+v", cephfsPlugin)
}
if multusApplied {
cephfsPlugin.Spec.Template.Spec.HostNetwork = false
}
err = k8sutil.CreateDaemonSet(csiCephFSPlugin, namespace, clientset, cephfsPlugin)
if err != nil {
return errors.Wrapf(err, "failed to start cephfs plugin daemonset: %+v", cephfsPlugin)
Expand All @@ -417,6 +436,11 @@ func startDrivers(clientset kubernetes.Interface, namespace string, ver *version
cephfsProvisionerDeployment.Spec.Strategy = apps.DeploymentStrategy{
Type: apps.RecreateDeploymentStrategyType,
}

_, err = applyCephClusterNetworkConfig(&cephfsProvisionerDeployment.Spec.Template.ObjectMeta, rookclientset)
if err != nil {
return errors.Wrapf(err, "failed to apply network config to cephfs plugin provisioner deployment: %+v", cephfsProvisionerDeployment)
}
err = k8sutil.CreateDeployment(clientset, csiCephFSProvisioner, namespace, cephfsProvisionerDeployment)
if err != nil {
return errors.Wrapf(err, "failed to start cephfs provisioner deployment: %+v", cephfsProvisionerDeployment)
Expand Down Expand Up @@ -498,6 +522,23 @@ func deleteCSIDriverResources(
return succeeded
}

func applyCephClusterNetworkConfig(objectMeta *metav1.ObjectMeta, rookclientset rookclient.Interface) (bool, error) {
cephClusters, err := rookclientset.CephV1().CephClusters(objectMeta.Namespace).List(metav1.ListOptions{})
Copy link
Member

Choose a reason for hiding this comment

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

question: won't this fail if the ceph cluster and CSI is in a different namespace?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.
The namespace we have here is actually the Rook Ceph operators namespace, right?

Copy link
Member

Choose a reason for hiding this comment

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

Yes its rook ceph operator namespace

if err != nil {
return false, errors.Errorf("failed to find CephClusters in namespace %q", objectMeta.Namespace)
}
rohan47 marked this conversation as resolved.
Show resolved Hide resolved
for _, cephCluster := range cephClusters.Items {
if cephCluster.Spec.Network.IsMultus() {
err = k8sutil.ApplyMultus(cephCluster.Spec.Network.NetworkSpec, objectMeta)
if err != nil {
return false, errors.Wrapf(err, "failed to apply multus configuration to CephCluster %q", cephCluster.Name)
}
}
}

return true, nil
Copy link
Member

Choose a reason for hiding this comment

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

even if the ceph cluster is not using multus it's returning true?
don't we need to return false if the ceph cluster network is not multus?

Copy link
Member

Choose a reason for hiding this comment

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

Good point, we should only return if multus got configured, I'm sending a PR to fix this.

Copy link
Member

Choose a reason for hiding this comment

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

#6152

}

// createCSIDriverInfo Registers CSI driver by creating a CSIDriver object
func createCSIDriverInfo(clientset kubernetes.Interface, name string, ownerRef *metav1.OwnerReference) error {
attach := true
Expand Down
4 changes: 3 additions & 1 deletion pkg/operator/ceph/csi/spec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package csi
import (
"testing"

rookclient "github.com/rook/rook/pkg/client/clientset/versioned"
"github.com/rook/rook/pkg/operator/test"

"github.com/stretchr/testify/assert"
Expand All @@ -38,10 +39,11 @@ func TestStartCSI(t *testing.T) {
SnapshotterImage: "image",
}
clientset := test.New(t, 3)
var rookclientset rookclient.Interface
serverVersion, err := clientset.Discovery().ServerVersion()
if err != nil {
assert.Nil(t, err)
}
err = startDrivers(clientset, "ns", serverVersion, nil)
err = startDrivers(clientset, rookclientset, "ns", serverVersion, nil)
assert.Nil(t, err)
}
2 changes: 1 addition & 1 deletion pkg/operator/ceph/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func (o *Operator) updateDrivers() error {
return errors.Wrap(err, "invalid csi params")
}

go csi.ValidateAndConfigureDrivers(o.context.Clientset, o.operatorNamespace, o.rookImage, o.securityAccount, serverVersion, ownerRef)
go csi.ValidateAndConfigureDrivers(o.context.Clientset, o.context.RookClientset, o.operatorNamespace, o.rookImage, o.securityAccount, serverVersion, ownerRef)
return nil
}

Expand Down
16 changes: 14 additions & 2 deletions pkg/operator/k8sutil/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

const (
// publicNetworkSelectorKeyName is the network selector key for the ceph public network
publicNetworkSelectorKeyName = "public"
)

// NetworkAttachmentConfig represents the configuration of the NetworkAttachmentDefinitions object
type NetworkAttachmentConfig struct {
CniVersion string `json:"cniVersion"`
Expand Down Expand Up @@ -110,7 +115,7 @@ func ApplyMultus(net rookv1.NetworkSpec, objectMeta *metav1.ObjectMeta) error {
shortSyntax := false
jsonSyntax := false

for _, ns := range net.Selectors {
for k, ns := range net.Selectors {
var multusMap map[string]string
err := json.Unmarshal([]byte(ns), &multusMap)

Expand All @@ -120,7 +125,14 @@ func ApplyMultus(net rookv1.NetworkSpec, objectMeta *metav1.ObjectMeta) error {
shortSyntax = true
}

v = append(v, string(ns))
isCsi := strings.Contains(objectMeta.Labels["app"], "csi-")
if isCsi {
if k == publicNetworkSelectorKeyName {
v = append(v, string(ns))
}
} else {
v = append(v, string(ns))
}
}

if shortSyntax && jsonSyntax {
Expand Down