Merge pull request #53115 from FengyunPan/fix-auto

Automatic merge from submit-queue (batch tested with PRs 53418, 53366, 53115, 53402, 53130). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix the version detection of OpenStack Cinder

**What this PR does / why we need it**:
When running Kubernetes against an installation of DevStack which
deploys the Cinder service at a path rather than a port (ex:
http://foo.bar/volume rather than http://foo.bar:xxx), the version
detection fails. It is better to use the OpenStack service catalog.
OTOH, when initialize cinder client, kubernetes will check the
endpoint from the OpenStack service catalog, so we can do this
version detection by it.

There are two case should be fixed in other PR:
1. revisit the version detection after supporting Cinder V3 API.
2. add codes to support MicroVersion after gophercloud supports MicroVersion.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #50461

**Special notes for your reviewer**:
/assign @dims 
/assign @xsgordon 

**Release note**:
```release-note
Using OpenStack service catalog to do version detection
```

Author: Kubernetes Submit Queue

pkg/cloudprovider/providers/openstack/BUILD

@@ -28,7 +28,6 @@ go_library(
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/github.com/gophercloud/gophercloud:go_default_library",
         "//vendor/github.com/gophercloud/gophercloud/openstack:go_default_library",
-        "//vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v1/apiversions:go_default_library",
         "//vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v1/volumes:go_default_library",
         "//vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v2/volumes:go_default_library",
         "//vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces:go_default_library",
@@ -74,7 +73,6 @@ go_test(
     deps = [
         "//pkg/cloudprovider:go_default_library",
         "//vendor/github.com/gophercloud/gophercloud:go_default_library",
-        "//vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v1/apiversions:go_default_library",
         "//vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/servers:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

pkg/cloudprovider/providers/openstack/openstack.go

@@ -24,13 +24,11 @@ import (
 	"io/ioutil"
 	"net/http"
 	"regexp"
-	"sort"
 	"strings"
 	"time"
 
 	"github.com/gophercloud/gophercloud"
 	"github.com/gophercloud/gophercloud/openstack"
-	apiversions_v1 "github.com/gophercloud/gophercloud/openstack/blockstorage/v1/apiversions"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
 	"github.com/gophercloud/gophercloud/openstack/identity/v3/extensions/trusts"
@@ -664,49 +662,6 @@ func (os *OpenStack) Routes() (cloudprovider.Routes, bool) {
 	return r, true
 }
 
-// Implementation of sort interface for blockstorage version probing
-type APIVersionsByID []apiversions_v1.APIVersion
-
-func (apiVersions APIVersionsByID) Len() int {
-	return len(apiVersions)
-}
-
-func (apiVersions APIVersionsByID) Swap(i, j int) {
-	apiVersions[i], apiVersions[j] = apiVersions[j], apiVersions[i]
-}
-
-func (apiVersions APIVersionsByID) Less(i, j int) bool {
-	return apiVersions[i].ID > apiVersions[j].ID
-}
-
-func autoVersionSelector(apiVersion *apiversions_v1.APIVersion) string {
-	switch strings.ToLower(apiVersion.ID) {
-	case "v2.0":
-		return "v2"
-	case "v1.0":
-		return "v1"
-	default:
-		return ""
-	}
-}
-
-func doBsApiVersionAutodetect(availableApiVersions []apiversions_v1.APIVersion) string {
-	sort.Sort(APIVersionsByID(availableApiVersions))
-	for _, status := range []string{"CURRENT", "SUPPORTED"} {
-		for _, version := range availableApiVersions {
-			if strings.ToUpper(version.Status) == status {
-				if detectedApiVersion := autoVersionSelector(&version); detectedApiVersion != "" {
-					glog.V(3).Infof("Blockstorage API version probing has found a suitable %s api version: %s", status, detectedApiVersion)
-					return detectedApiVersion
-				}
-			}
-		}
-	}
-
-	return ""
-
-}
-
 func (os *OpenStack) volumeService(forceVersion string) (volumeService, error) {
 	bsVersion := ""
 	if forceVersion == "" {
@@ -721,43 +676,36 @@ func (os *OpenStack) volumeService(forceVersion string) (volumeService, error) {
 		if err != nil {
 			return nil, err
 		}
+		glog.V(3).Infof("Using Blockstorage API V1")
 		return &VolumesV1{sClient, os.bsOpts}, nil
 	case "v2":
 		sClient, err := os.NewBlockStorageV2()
 		if err != nil {
 			return nil, err
 		}
+		glog.V(3).Infof("Using Blockstorage API V2")
 		return &VolumesV2{sClient, os.bsOpts}, nil
 	case "auto":
-		sClient, err := os.NewBlockStorageV1()
+		// Currently kubernetes just support Cinder v1 and Cinder v2.
+		// Choose Cinder v2 firstly, if kubernetes can't initialize cinder v2 client, try to initialize cinder v1 client.
+		// Return appropriate message when kubernetes can't initialize them.
+		// TODO(FengyunPan): revisit 'auto' after supporting Cinder v3.
+		sClient, err := os.NewBlockStorageV2()
 		if err != nil {
-			return nil, err
-		}
-		availableApiVersions := []apiversions_v1.APIVersion{}
-		err = apiversions_v1.List(sClient).EachPage(func(page pagination.Page) (bool, error) {
-			// returning false from this handler stops page iteration, error is propagated to the upper function
-			apiversions, err := apiversions_v1.ExtractAPIVersions(page)
+			sClient, err = os.NewBlockStorageV1()
 			if err != nil {
-				glog.Errorf("Unable to extract api versions from page: %v", err)
-				return false, err
+				// Nothing suitable found, failed autodetection, just exit with appropriate message
+				err_txt := "BlockStorage API version autodetection failed. " +
+					"Please set it explicitly in cloud.conf in section [BlockStorage] with key `bs-version`"
+				return nil, errors.New(err_txt)
+			} else {
+				glog.V(3).Infof("Using Blockstorage API V1")
+				return &VolumesV1{sClient, os.bsOpts}, nil
 			}
-			availableApiVersions = append(availableApiVersions, apiversions...)
-			return true, nil
-		})
-
-		if err != nil {
-			glog.Errorf("Error when retrieving list of supported blockstorage api versions: %v", err)
-			return nil, err
-		}
-		if autodetectedVersion := doBsApiVersionAutodetect(availableApiVersions); autodetectedVersion != "" {
-			return os.volumeService(autodetectedVersion)
 		} else {
-			// Nothing suitable found, failed autodetection, just exit with appropriate message
-			err_txt := "BlockStorage API version autodetection failed. " +
-				"Please set it explicitly in cloud.conf in section [BlockStorage] with key `bs-version`"
-			return nil, errors.New(err_txt)
+			glog.V(3).Infof("Using Blockstorage API V2")
+			return &VolumesV2{sClient, os.bsOpts}, nil
 		}
-
 	default:
 		err_txt := fmt.Sprintf("Config error: unrecognised bs-version \"%v\"", os.bsOpts.BSVersion)
 		glog.Warningf(err_txt)

pkg/cloudprovider/providers/openstack/openstack_test.go

@@ -26,7 +26,6 @@ import (
 	"time"
 
 	"github.com/gophercloud/gophercloud"
-	"github.com/gophercloud/gophercloud/openstack/blockstorage/v1/apiversions"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
 	"k8s.io/api/core/v1"
 
@@ -557,48 +556,6 @@ func TestVolumes(t *testing.T) {
 
 }
 
-func TestCinderAutoDetectApiVersion(t *testing.T) {
-	updated := "" // not relevant to this test, can be set to any value
-	status_current := "CURRENT"
-	status_supported := "SUPpORTED" // lowercase to test regression resitance if api returns different case
-	status_deprecated := "DEPRECATED"
-
-	var result_version, api_version [4]string
-
-	for ver := 0; ver <= 3; ver++ {
-		api_version[ver] = fmt.Sprintf("v%d.0", ver)
-		result_version[ver] = fmt.Sprintf("v%d", ver)
-	}
-	result_version[0] = ""
-	api_current_v1 := apiversions.APIVersion{ID: api_version[1], Status: status_current, Updated: updated}
-	api_current_v2 := apiversions.APIVersion{ID: api_version[2], Status: status_current, Updated: updated}
-	api_current_v3 := apiversions.APIVersion{ID: api_version[3], Status: status_current, Updated: updated}
-
-	api_supported_v1 := apiversions.APIVersion{ID: api_version[1], Status: status_supported, Updated: updated}
-	api_supported_v2 := apiversions.APIVersion{ID: api_version[2], Status: status_supported, Updated: updated}
-
-	api_deprecated_v1 := apiversions.APIVersion{ID: api_version[1], Status: status_deprecated, Updated: updated}
-	api_deprecated_v2 := apiversions.APIVersion{ID: api_version[2], Status: status_deprecated, Updated: updated}
-
-	var testCases = []struct {
-		test_case     []apiversions.APIVersion
-		wanted_result string
-	}{
-		{[]apiversions.APIVersion{api_current_v1}, result_version[1]},
-		{[]apiversions.APIVersion{api_current_v2}, result_version[2]},
-		{[]apiversions.APIVersion{api_supported_v1, api_current_v2}, result_version[2]},                     // current always selected
-		{[]apiversions.APIVersion{api_current_v1, api_supported_v2}, result_version[1]},                     // current always selected
-		{[]apiversions.APIVersion{api_current_v3, api_supported_v2, api_deprecated_v1}, result_version[2]},  // with current v3, but should fall back to v2
-		{[]apiversions.APIVersion{api_current_v3, api_deprecated_v2, api_deprecated_v1}, result_version[0]}, // v3 is not supported
-	}
-
-	for _, suite := range testCases {
-		if autodetectedVersion := doBsApiVersionAutodetect(suite.test_case); autodetectedVersion != suite.wanted_result {
-			t.Fatalf("Autodetect for suite: %s, failed with result: '%s', wanted '%s'", suite.test_case, autodetectedVersion, suite.wanted_result)
-		}
-	}
-}
-
 func TestInstanceIDFromProviderID(t *testing.T) {
 	testCases := []struct {
 		providerID string