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.

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

Add CSI migration of OpenStack Cinder volumes #113826

Merged
merged 2 commits into from Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions pkg/apis/core/validation/validation_test.go
Expand Up @@ -4097,6 +4097,20 @@ func TestValidateVolumes(t *testing.T) {
field: "rbd.image",
}},
},
// Cinder
{
name: "valid Cinder",
vol: core.Volume{
Name: "cinder",
VolumeSource: core.VolumeSource{
Cinder: &core.CinderVolumeSource{
VolumeID: "29ea5088-4f60-4757-962e-dba678767887",
FSType: "ext4",
ReadOnly: false,
},
},
},
},
// CephFS
{
name: "valid CephFS",
Expand Down
8 changes: 8 additions & 0 deletions pkg/features/kube_features.go
Expand Up @@ -408,6 +408,12 @@ const (
// Disables the GCE PD in-tree driver.
InTreePluginGCEUnregister featuregate.Feature = "InTreePluginGCEUnregister"

// owner: @adisky
// alpha: v1.21
//
// Disables the OpenStack Cinder in-tree driver.
InTreePluginOpenStackUnregister featuregate.Feature = "InTreePluginOpenStackUnregister"

// owner: @trierra
// alpha: v1.23
//
Expand Down Expand Up @@ -993,6 +999,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS

InTreePluginGCEUnregister: {Default: false, PreRelease: featuregate.Alpha},

InTreePluginOpenStackUnregister: {Default: false, PreRelease: featuregate.Alpha},

InTreePluginPortworxUnregister: {Default: false, PreRelease: featuregate.Alpha},

InTreePluginRBDUnregister: {Default: false, PreRelease: featuregate.Alpha},
Expand Down
1 change: 1 addition & 0 deletions pkg/scheduler/framework/plugins/names/names.go
Expand Up @@ -30,6 +30,7 @@ const (
NodeUnschedulable = "NodeUnschedulable"
NodeVolumeLimits = "NodeVolumeLimits"
AzureDiskLimits = "AzureDiskLimits"
CinderLimits = "CinderLimits"
EBSLimits = "EBSLimits"
GCEPDLimits = "GCEPDLimits"
PodTopologySpread = "PodTopologySpread"
Expand Down
2 changes: 2 additions & 0 deletions pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go
Expand Up @@ -58,6 +58,8 @@ func getVolumeLimitKey(filterType string) v1.ResourceName {
return v1.ResourceName(volumeutil.GCEVolumeLimitKey)
case azureDiskVolumeFilterType:
return v1.ResourceName(volumeutil.AzureVolumeLimitKey)
case cinderVolumeFilterType:
return v1.ResourceName(volumeutil.CinderVolumeLimitKey)
default:
return v1.ResourceName(volumeutil.GetCSIAttachLimitKey(filterType))
}
Expand Down
43 changes: 43 additions & 0 deletions pkg/scheduler/framework/plugins/nodevolumelimits/non_csi.go
Expand Up @@ -56,6 +56,8 @@ const (
gcePDVolumeFilterType = "GCE"
// azureDiskVolumeFilterType defines the filter name for azureDiskVolumeFilter.
azureDiskVolumeFilterType = "AzureDisk"
// cinderVolumeFilterType defines the filter name for cinderVolumeFilter.
cinderVolumeFilterType = "Cinder"

// ErrReasonMaxVolumeCountExceeded is used for MaxVolumeCount predicate error.
ErrReasonMaxVolumeCountExceeded = "node(s) exceed max volume count"
Expand All @@ -73,6 +75,15 @@ func NewAzureDisk(_ runtime.Object, handle framework.Handle, fts feature.Feature
return newNonCSILimitsWithInformerFactory(azureDiskVolumeFilterType, informerFactory, fts), nil
}

// CinderName is the name of the plugin used in the plugin registry and configurations.
const CinderName = names.CinderLimits

// NewCinder returns function that initializes a new plugin and returns it.
func NewCinder(_ runtime.Object, handle framework.Handle, fts feature.Features) (framework.Plugin, error) {
informerFactory := handle.SharedInformerFactory()
return newNonCSILimitsWithInformerFactory(cinderVolumeFilterType, informerFactory, fts), nil
}

// EBSName is the name of the plugin used in the plugin registry and configurations.
const EBSName = names.EBSLimits

Expand Down Expand Up @@ -160,6 +171,10 @@ func newNonCSILimits(
name = AzureDiskName
filter = azureDiskVolumeFilter
volumeLimitKey = v1.ResourceName(volumeutil.AzureVolumeLimitKey)
case cinderVolumeFilterType:
name = CinderName
filter = cinderVolumeFilter
volumeLimitKey = v1.ResourceName(volumeutil.CinderVolumeLimitKey)
default:
klog.ErrorS(errors.New("wrong filterName"), "Cannot create nonCSILimits plugin")
return nil
Expand Down Expand Up @@ -460,6 +475,32 @@ var azureDiskVolumeFilter = VolumeFilter{
},
}

// cinderVolumeFilter is a VolumeFilter for filtering cinder Volumes.
// It will be deprecated once Openstack cloudprovider has been removed from in-tree.
Copy link
Member

Choose a reason for hiding this comment

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

Hm so we may not be able to remove this logic after all

var cinderVolumeFilter = VolumeFilter{
FilterVolume: func(vol *v1.Volume) (string, bool) {
if vol.Cinder != nil {
return vol.Cinder.VolumeID, true
}
return "", false
},

FilterPersistentVolume: func(pv *v1.PersistentVolume) (string, bool) {
if pv.Spec.Cinder != nil {
return pv.Spec.Cinder.VolumeID, true
}
return "", false
},

MatchProvisioner: func(sc *storage.StorageClass) bool {
return sc.Provisioner == csilibplugins.CinderInTreePluginName
},

IsMigrated: func(csiNode *storage.CSINode) bool {
return isCSIMigrationOn(csiNode, csilibplugins.CinderInTreePluginName)
},
}

func getMaxVolumeFunc(filterName string) func(node *v1.Node) int {
return func(node *v1.Node) int {
maxVolumesFromEnv := getMaxVolLimitFromEnv()
Expand All @@ -481,6 +522,8 @@ func getMaxVolumeFunc(filterName string) func(node *v1.Node) int {
return defaultMaxGCEPDVolumes
case azureDiskVolumeFilterType:
return defaultMaxAzureDiskVolumes
case cinderVolumeFilterType:
Copy link
Member

Choose a reason for hiding this comment

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

Can you open up an issue to investigate if this logic can be removed? I believe we should be always taking the limit from CSINode now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened #113885

return volumeutil.DefaultMaxCinderVolumes
default:
return -1
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/scheduler/framework/plugins/nodevolumelimits/utils.go
Expand Up @@ -55,6 +55,8 @@ func isCSIMigrationOn(csiNode *storagev1.CSINode, pluginName string) bool {
if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAzureDisk) {
return false
}
case csilibplugins.CinderInTreePluginName:
return true
case csilibplugins.RBDVolumePluginName:
if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationRBD) {
return false
Expand Down
1 change: 1 addition & 0 deletions pkg/scheduler/framework/plugins/registry.go
Expand Up @@ -72,6 +72,7 @@ func NewInTreeRegistry() runtime.Registry {
nodevolumelimits.EBSName: runtime.FactoryAdapter(fts, nodevolumelimits.NewEBS),
nodevolumelimits.GCEPDName: runtime.FactoryAdapter(fts, nodevolumelimits.NewGCEPD),
nodevolumelimits.AzureDiskName: runtime.FactoryAdapter(fts, nodevolumelimits.NewAzureDisk),
nodevolumelimits.CinderName: runtime.FactoryAdapter(fts, nodevolumelimits.NewCinder),
interpodaffinity.Name: interpodaffinity.New,
queuesort.Name: queuesort.New,
defaultbinder.Name: defaultbinder.New,
Expand Down
2 changes: 2 additions & 0 deletions pkg/scheduler/framework/plugins/volumebinding/binder.go
Expand Up @@ -1010,6 +1010,8 @@ func isCSIMigrationOnForPlugin(pluginName string) bool {
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationGCE)
case csiplugins.AzureDiskInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAzureDisk)
case csiplugins.CinderInTreePluginName:
return true
case csiplugins.PortworxVolumePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationPortworx)
case csiplugins.RBDVolumePluginName:
Expand Down
3 changes: 3 additions & 0 deletions pkg/volume/csi/csi_plugin.go
Expand Up @@ -222,6 +222,9 @@ func (p *csiPlugin) Init(host volume.VolumeHost) error {
csitranslationplugins.AWSEBSInTreePluginName: func() bool {
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAWS)
},
csitranslationplugins.CinderInTreePluginName: func() bool {
return true
},
csitranslationplugins.AzureDiskInTreePluginName: func() bool {
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAzureDisk)
},
Expand Down
4 changes: 4 additions & 0 deletions pkg/volume/csimigration/plugin_manager.go
Expand Up @@ -68,6 +68,8 @@ func (pm PluginManager) IsMigrationCompleteForPlugin(pluginName string) bool {
return pm.featureGate.Enabled(features.InTreePluginAzureFileUnregister)
case csilibplugins.AzureDiskInTreePluginName:
return pm.featureGate.Enabled(features.InTreePluginAzureDiskUnregister)
case csilibplugins.CinderInTreePluginName:
return pm.featureGate.Enabled(features.InTreePluginOpenStackUnregister)
case csilibplugins.VSphereInTreePluginName:
return pm.featureGate.Enabled(features.InTreePluginvSphereUnregister)
case csilibplugins.PortworxVolumePluginName:
Expand All @@ -94,6 +96,8 @@ func (pm PluginManager) IsMigrationEnabledForPlugin(pluginName string) bool {
return pm.featureGate.Enabled(features.CSIMigrationAzureFile)
case csilibplugins.AzureDiskInTreePluginName:
return pm.featureGate.Enabled(features.CSIMigrationAzureDisk)
case csilibplugins.CinderInTreePluginName:
return true
case csilibplugins.VSphereInTreePluginName:
return pm.featureGate.Enabled(features.CSIMigrationvSphere)
case csilibplugins.PortworxVolumePluginName:
Expand Down
7 changes: 7 additions & 0 deletions pkg/volume/util/attach_limit.go
Expand Up @@ -40,6 +40,13 @@ const (
// GCEVolumeLimitKey stores resource name that will store volume limits for GCE node
GCEVolumeLimitKey = "attachable-volumes-gce-pd"

// CinderVolumeLimitKey contains Volume limit key for Cinder
CinderVolumeLimitKey = "attachable-volumes-cinder"
// DefaultMaxCinderVolumes defines the maximum number of PD Volumes for Cinder
// For Openstack we are keeping this to a high enough value so as depending on backend
// cluster admins can configure it.
DefaultMaxCinderVolumes = 256

// CSIAttachLimitPrefix defines prefix used for CSI volumes
CSIAttachLimitPrefix = "attachable-volumes-csi-"

Expand Down
25 changes: 25 additions & 0 deletions pkg/volume/util/util_test.go
Expand Up @@ -21,6 +21,7 @@ import (
"os"
"reflect"
"runtime"
"strings"
"testing"

v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -260,6 +261,30 @@ func TestFsUserFrom(t *testing.T) {
}
}

func TestGenerateVolumeName(t *testing.T) {

// Normal operation, no truncate
v1 := GenerateVolumeName("kubernetes", "pv-cinder-abcde", 255)
if v1 != "kubernetes-dynamic-pv-cinder-abcde" {
t.Errorf("Expected kubernetes-dynamic-pv-cinder-abcde, got %s", v1)
}

// Truncate trailing "6789-dynamic"
prefix := strings.Repeat("0123456789", 9) // 90 characters prefix + 8 chars. of "-dynamic"
v2 := GenerateVolumeName(prefix, "pv-cinder-abcde", 100)
expect := prefix[:84] + "-pv-cinder-abcde"
if v2 != expect {
t.Errorf("Expected %s, got %s", expect, v2)
}

// Truncate really long cluster name
prefix = strings.Repeat("0123456789", 1000) // 10000 characters prefix
v3 := GenerateVolumeName(prefix, "pv-cinder-abcde", 100)
if v3 != expect {
t.Errorf("Expected %s, got %s", expect, v3)
}
}

func TestHasMountRefs(t *testing.T) {
testCases := map[string]struct {
mountPath string
Expand Down
Expand Up @@ -72,7 +72,7 @@ var _ kubeapiserveradmission.WantsCloudConfig = &persistentVolumeLabel{}
// As a side effect, the cloud provider may block invalid or non-existent volumes.
func newPersistentVolumeLabel() *persistentVolumeLabel {
// DEPRECATED: in a future release, we will use mutating admission webhooks to apply PV labels.
// Once the mutating admission webhook is used for AWS, Azure and GCE,
// Once the mutating admission webhook is used for AWS, Azure, and GCE,
// this admission controller will be removed.
klog.Warning("PersistentVolumeLabel admission controller is deprecated. " +
"Please remove this controller from your configuration files and scripts.")
Expand Down
Expand Up @@ -912,7 +912,7 @@ func Test_PVLAdmission(t *testing.T) {
// setPVLabler applies the given mock pvlabeler to implement PV labeling for all cloud providers.
// Given we mock out the values of the labels anyways, assigning the same mock labeler for every
// provider does not reduce test coverage but it does simplify/clean up the tests here because
// the provider is then decided based on the type of PV (EBS, Cinder, GCEPD, Azure Disk, etc)
// the provider is then decided based on the type of PV (EBS, GCEPD, Azure Disk, etc)
func setPVLabeler(handler *persistentVolumeLabel, pvlabeler cloudprovider.PVLabeler) {
handler.awsPVLabeler = pvlabeler
handler.gcePVLabeler = pvlabeler
Expand Down
Expand Up @@ -421,6 +421,48 @@ func TestTranslateTopologyFromCSIToInTree(t *testing.T) {
v1.LabelTopologyRegion: "us-east1",
},
},
{
name: "cinder translation",
key: CinderTopologyKey,
expErr: false,
regionParser: nil,
pv: &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "cinder", Namespace: "myns",
},
Spec: v1.PersistentVolumeSpec{
NodeAffinity: &v1.VolumeNodeAffinity{
Required: &v1.NodeSelector{
NodeSelectorTerms: []v1.NodeSelectorTerm{
{
MatchExpressions: []v1.NodeSelectorRequirement{
{
Key: CinderTopologyKey,
Operator: v1.NodeSelectorOpIn,
Values: []string{"nova"},
},
},
},
},
},
},
},
},
expectedNodeSelectorTerms: []v1.NodeSelectorTerm{
{
MatchExpressions: []v1.NodeSelectorRequirement{
{
Key: v1.LabelTopologyZone,
Operator: v1.NodeSelectorOpIn,
Values: []string{"nova"},
},
},
},
},
expectedLabels: map[string]string{
v1.LabelTopologyZone: "nova",
},
},
}

for _, tc := range testCases {
Expand Down