Merge pull request kubernetes#74542 from gnufied/make-cinder-limits-via-openshift-conf

Allow cinder volume limits to be configurable

Author: k8s-ci-robot

pkg/cloudprovider/providers/openstack/openstack.go

@@ -110,9 +110,10 @@ type LoadBalancerOpts struct {
 
 // BlockStorageOpts is used to talk to Cinder service
 type BlockStorageOpts struct {
-	BSVersion       string `gcfg:"bs-version"`        // overrides autodetection. v1 or v2. Defaults to auto
-	TrustDevicePath bool   `gcfg:"trust-device-path"` // See Issue #33128
-	IgnoreVolumeAZ  bool   `gcfg:"ignore-volume-az"`
+	BSVersion             string `gcfg:"bs-version"`        // overrides autodetection. v1 or v2. Defaults to auto
+	TrustDevicePath       bool   `gcfg:"trust-device-path"` // See Issue #33128
+	IgnoreVolumeAZ        bool   `gcfg:"ignore-volume-az"`
+	NodeVolumeAttachLimit int    `gcfg:"node-volume-attach-limit"` // override volume attach limit for Cinder. Default is : 256
 }
 
 // RouterOpts is used for Neutron routes
@@ -369,6 +370,32 @@ func newOpenStack(cfg Config) (*OpenStack, error) {
 	return &os, nil
 }
 
+// NewFakeOpenStackCloud creates and returns an instance of Openstack cloudprovider.
+// Mainly for use in tests that require instantiating Openstack without having
+// to go through cloudprovider interface.
+func NewFakeOpenStackCloud(cfg Config) (*OpenStack, error) {
+	provider, err := openstack.NewClient(cfg.Global.AuthURL)
+	if err != nil {
+		return nil, err
+	}
+	emptyDuration := MyDuration{}
+	if cfg.Metadata.RequestTimeout == emptyDuration {
+		cfg.Metadata.RequestTimeout.Duration = time.Duration(defaultTimeOut)
+	}
+	provider.HTTPClient.Timeout = cfg.Metadata.RequestTimeout.Duration
+
+	os := OpenStack{
+		provider:     provider,
+		region:       cfg.Global.Region,
+		lbOpts:       cfg.LoadBalancer,
+		bsOpts:       cfg.BlockStorage,
+		routeOpts:    cfg.Route,
+		metadataOpts: cfg.Metadata,
+	}
+
+	return &os, nil
+}
+
 // Initialize passes a Kubernetes clientBuilder interface to the cloud provider
 func (os *OpenStack) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
 }

pkg/cloudprovider/providers/openstack/openstack_volumes.go

@@ -695,6 +695,11 @@ func (os *OpenStack) ShouldTrustDevicePath() bool {
 	return os.bsOpts.TrustDevicePath
 }
 
+// NodeVolumeAttachLimit specifies number of cinder volumes that can be attached to this node.
+func (os *OpenStack) NodeVolumeAttachLimit() int {
+	return os.bsOpts.NodeVolumeAttachLimit
+}
+
 // GetLabelsForVolume implements PVLabeler.GetLabelsForVolume
 func (os *OpenStack) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) (map[string]string, error) {
 	// Ignore if not Cinder.

pkg/volume/cinder/BUILD

@@ -49,9 +49,11 @@ go_test(
     ],
     embed = [":go_default_library"],
     deps = [
+        "//pkg/cloudprovider/providers/openstack:go_default_library",
         "//pkg/util/mount:go_default_library",
         "//pkg/volume:go_default_library",
         "//pkg/volume/testing:go_default_library",
+        "//pkg/volume/util:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

pkg/volume/cinder/cinder.go

@@ -143,6 +143,12 @@ func (plugin *cinderPlugin) GetVolumeLimits() (map[string]int64, error) {
 	if cloud.ProviderName() != openstack.ProviderName {
 		return nil, fmt.Errorf("Expected Openstack cloud, found %s", cloud.ProviderName())
 	}
+
+	openstackCloud, ok := cloud.(*openstack.OpenStack)
+	if ok && openstackCloud.NodeVolumeAttachLimit() > 0 {
+		volumeLimits[util.CinderVolumeLimitKey] = int64(openstackCloud.NodeVolumeAttachLimit())
+	}
+
 	return volumeLimits, nil
 }
 

pkg/volume/cinder/cinder_test.go

@@ -26,9 +26,11 @@ import (
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	utiltesting "k8s.io/client-go/util/testing"
+	"k8s.io/kubernetes/pkg/cloudprovider/providers/openstack"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
 	volumetest "k8s.io/kubernetes/pkg/volume/testing"
+	"k8s.io/kubernetes/pkg/volume/util"
 )
 
 func TestCanSupport(t *testing.T) {
@@ -255,3 +257,76 @@ func TestPlugin(t *testing.T) {
 		t.Errorf("Deleter() failed: %v", err)
 	}
 }
+
+func TestGetVolumeLimit(t *testing.T) {
+	tmpDir, err := utiltesting.MkTmpdir("cinderTest")
+	if err != nil {
+		t.Fatalf("can't make a temp dir: %v", err)
+	}
+
+	cloud, err := getOpenstackCloudProvider()
+	if err != nil {
+		t.Fatalf("can not instantiate openstack cloudprovider : %v", err)
+	}
+
+	defer os.RemoveAll(tmpDir)
+	plugMgr := volume.VolumePluginMgr{}
+	volumeHost := volumetest.NewFakeVolumeHostWithCloudProvider(tmpDir, nil, nil, cloud)
+	plugMgr.InitPlugins(ProbeVolumePlugins(), nil /* prober */, volumeHost)
+
+	plug, err := plugMgr.FindPluginByName("kubernetes.io/cinder")
+	if err != nil {
+		t.Fatalf("Can't find the plugin by name")
+	}
+	attachablePlugin, ok := plug.(volume.VolumePluginWithAttachLimits)
+	if !ok {
+		t.Fatalf("plugin %s is not of attachable type", plug.GetPluginName())
+	}
+
+	limits, err := attachablePlugin.GetVolumeLimits()
+	if err != nil {
+		t.Errorf("error fetching limits : %v", err)
+	}
+	if len(limits) == 0 {
+		t.Fatalf("expecting limit from openstack got none")
+	}
+	limit, _ := limits[util.CinderVolumeLimitKey]
+	if limit != 10 {
+		t.Fatalf("expected volume limit to be 10 got %d", limit)
+	}
+}
+
+func getOpenstackCloudProvider() (*openstack.OpenStack, error) {
+	cfg := getOpenstackConfig()
+	return openstack.NewFakeOpenStackCloud(cfg)
+}
+
+func getOpenstackConfig() openstack.Config {
+	cfg := openstack.Config{
+		Global: struct {
+			AuthURL    string `gcfg:"auth-url"`
+			Username   string
+			UserID     string `gcfg:"user-id"`
+			Password   string
+			TenantID   string `gcfg:"tenant-id"`
+			TenantName string `gcfg:"tenant-name"`
+			TrustID    string `gcfg:"trust-id"`
+			DomainID   string `gcfg:"domain-id"`
+			DomainName string `gcfg:"domain-name"`
+			Region     string
+			CAFile     string `gcfg:"ca-file"`
+		}{
+			Username:   "user",
+			Password:   "pass",
+			TenantID:   "foobar",
+			DomainID:   "2a73b8f597c04551a0fdc8e95544be8a",
+			DomainName: "local",
+			AuthURL:    "http://auth.url",
+			UserID:     "user",
+		},
+		BlockStorage: openstack.BlockStorageOpts{
+			NodeVolumeAttachLimit: 10,
+		},
+	}
+	return cfg
+}