Skip to content
This repository was archived by the owner on Mar 22, 2018. It is now read-only.

Commit b5eb673

Browse files
author
FengyunPan
committed
Fix the version detection of OpenStack Cinder
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.
1 parent dfbbd7a commit b5eb673

3 files changed

Lines changed: 17 additions & 114 deletions

File tree

‎pkg/cloudprovider/providers/openstack/BUILD‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ go_library(
2828
"//vendor/github.com/golang/glog:go_default_library",
2929
"//vendor/github.com/gophercloud/gophercloud:go_default_library",
3030
"//vendor/github.com/gophercloud/gophercloud/openstack:go_default_library",
31-
"//vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v1/apiversions:go_default_library",
3231
"//vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v1/volumes:go_default_library",
3332
"//vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v2/volumes:go_default_library",
3433
"//vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces:go_default_library",
@@ -74,7 +73,6 @@ go_test(
7473
deps = [
7574
"//pkg/cloudprovider:go_default_library",
7675
"//vendor/github.com/gophercloud/gophercloud:go_default_library",
77-
"//vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v1/apiversions:go_default_library",
7876
"//vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/servers:go_default_library",
7977
"//vendor/k8s.io/api/core/v1:go_default_library",
8078
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

‎pkg/cloudprovider/providers/openstack/openstack.go‎

Lines changed: 17 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,11 @@ import (
2424
"io/ioutil"
2525
"net/http"
2626
"regexp"
27-
"sort"
2827
"strings"
2928
"time"
3029

3130
"github.com/gophercloud/gophercloud"
3231
"github.com/gophercloud/gophercloud/openstack"
33-
apiversions_v1 "github.com/gophercloud/gophercloud/openstack/blockstorage/v1/apiversions"
3432
"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces"
3533
"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
3634
"github.com/gophercloud/gophercloud/openstack/identity/v3/extensions/trusts"
@@ -644,49 +642,6 @@ func (os *OpenStack) Routes() (cloudprovider.Routes, bool) {
644642
return r, true
645643
}
646644

647-
// Implementation of sort interface for blockstorage version probing
648-
type APIVersionsByID []apiversions_v1.APIVersion
649-
650-
func (apiVersions APIVersionsByID) Len() int {
651-
return len(apiVersions)
652-
}
653-
654-
func (apiVersions APIVersionsByID) Swap(i, j int) {
655-
apiVersions[i], apiVersions[j] = apiVersions[j], apiVersions[i]
656-
}
657-
658-
func (apiVersions APIVersionsByID) Less(i, j int) bool {
659-
return apiVersions[i].ID > apiVersions[j].ID
660-
}
661-
662-
func autoVersionSelector(apiVersion *apiversions_v1.APIVersion) string {
663-
switch strings.ToLower(apiVersion.ID) {
664-
case "v2.0":
665-
return "v2"
666-
case "v1.0":
667-
return "v1"
668-
default:
669-
return ""
670-
}
671-
}
672-
673-
func doBsApiVersionAutodetect(availableApiVersions []apiversions_v1.APIVersion) string {
674-
sort.Sort(APIVersionsByID(availableApiVersions))
675-
for _, status := range []string{"CURRENT", "SUPPORTED"} {
676-
for _, version := range availableApiVersions {
677-
if strings.ToUpper(version.Status) == status {
678-
if detectedApiVersion := autoVersionSelector(&version); detectedApiVersion != "" {
679-
glog.V(3).Infof("Blockstorage API version probing has found a suitable %s api version: %s", status, detectedApiVersion)
680-
return detectedApiVersion
681-
}
682-
}
683-
}
684-
}
685-
686-
return ""
687-
688-
}
689-
690645
func (os *OpenStack) volumeService(forceVersion string) (volumeService, error) {
691646
bsVersion := ""
692647
if forceVersion == "" {
@@ -701,43 +656,36 @@ func (os *OpenStack) volumeService(forceVersion string) (volumeService, error) {
701656
if err != nil {
702657
return nil, err
703658
}
659+
glog.V(3).Infof("Using Blockstorage API V1")
704660
return &VolumesV1{sClient, os.bsOpts}, nil
705661
case "v2":
706662
sClient, err := os.NewBlockStorageV2()
707663
if err != nil {
708664
return nil, err
709665
}
666+
glog.V(3).Infof("Using Blockstorage API V2")
710667
return &VolumesV2{sClient, os.bsOpts}, nil
711668
case "auto":
712-
sClient, err := os.NewBlockStorageV1()
669+
// Currently kubernetes just support Cinder v1 and Cinder v2.
670+
// Choose Cinder v2 firstly, if kubernetes can't initialize cinder v2 client, try to initialize cinder v1 client.
671+
// Return appropriate message when kubernetes can't initialize them.
672+
// TODO(FengyunPan): revisit 'auto' after supporting Cinder v3.
673+
sClient, err := os.NewBlockStorageV2()
713674
if err != nil {
714-
return nil, err
715-
}
716-
availableApiVersions := []apiversions_v1.APIVersion{}
717-
err = apiversions_v1.List(sClient).EachPage(func(page pagination.Page) (bool, error) {
718-
// returning false from this handler stops page iteration, error is propagated to the upper function
719-
apiversions, err := apiversions_v1.ExtractAPIVersions(page)
675+
sClient, err = os.NewBlockStorageV1()
720676
if err != nil {
721-
glog.Errorf("Unable to extract api versions from page: %v", err)
722-
return false, err
677+
// Nothing suitable found, failed autodetection, just exit with appropriate message
678+
err_txt := "BlockStorage API version autodetection failed. " +
679+
"Please set it explicitly in cloud.conf in section [BlockStorage] with key `bs-version`"
680+
return nil, errors.New(err_txt)
681+
} else {
682+
glog.V(3).Infof("Using Blockstorage API V1")
683+
return &VolumesV1{sClient, os.bsOpts}, nil
723684
}
724-
availableApiVersions = append(availableApiVersions, apiversions...)
725-
return true, nil
726-
})
727-
728-
if err != nil {
729-
glog.Errorf("Error when retrieving list of supported blockstorage api versions: %v", err)
730-
return nil, err
731-
}
732-
if autodetectedVersion := doBsApiVersionAutodetect(availableApiVersions); autodetectedVersion != "" {
733-
return os.volumeService(autodetectedVersion)
734685
} else {
735-
// Nothing suitable found, failed autodetection, just exit with appropriate message
736-
err_txt := "BlockStorage API version autodetection failed. " +
737-
"Please set it explicitly in cloud.conf in section [BlockStorage] with key `bs-version`"
738-
return nil, errors.New(err_txt)
686+
glog.V(3).Infof("Using Blockstorage API V2")
687+
return &VolumesV2{sClient, os.bsOpts}, nil
739688
}
740-
741689
default:
742690
err_txt := fmt.Sprintf("Config error: unrecognised bs-version \"%v\"", os.bsOpts.BSVersion)
743691
glog.Warningf(err_txt)

‎pkg/cloudprovider/providers/openstack/openstack_test.go‎

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"time"
2727

2828
"github.com/gophercloud/gophercloud"
29-
"github.com/gophercloud/gophercloud/openstack/blockstorage/v1/apiversions"
3029
"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
3130
"k8s.io/api/core/v1"
3231

@@ -508,48 +507,6 @@ func TestVolumes(t *testing.T) {
508507

509508
}
510509

511-
func TestCinderAutoDetectApiVersion(t *testing.T) {
512-
updated := "" // not relevant to this test, can be set to any value
513-
status_current := "CURRENT"
514-
status_supported := "SUPpORTED" // lowercase to test regression resitance if api returns different case
515-
status_deprecated := "DEPRECATED"
516-
517-
var result_version, api_version [4]string
518-
519-
for ver := 0; ver <= 3; ver++ {
520-
api_version[ver] = fmt.Sprintf("v%d.0", ver)
521-
result_version[ver] = fmt.Sprintf("v%d", ver)
522-
}
523-
result_version[0] = ""
524-
api_current_v1 := apiversions.APIVersion{ID: api_version[1], Status: status_current, Updated: updated}
525-
api_current_v2 := apiversions.APIVersion{ID: api_version[2], Status: status_current, Updated: updated}
526-
api_current_v3 := apiversions.APIVersion{ID: api_version[3], Status: status_current, Updated: updated}
527-
528-
api_supported_v1 := apiversions.APIVersion{ID: api_version[1], Status: status_supported, Updated: updated}
529-
api_supported_v2 := apiversions.APIVersion{ID: api_version[2], Status: status_supported, Updated: updated}
530-
531-
api_deprecated_v1 := apiversions.APIVersion{ID: api_version[1], Status: status_deprecated, Updated: updated}
532-
api_deprecated_v2 := apiversions.APIVersion{ID: api_version[2], Status: status_deprecated, Updated: updated}
533-
534-
var testCases = []struct {
535-
test_case []apiversions.APIVersion
536-
wanted_result string
537-
}{
538-
{[]apiversions.APIVersion{api_current_v1}, result_version[1]},
539-
{[]apiversions.APIVersion{api_current_v2}, result_version[2]},
540-
{[]apiversions.APIVersion{api_supported_v1, api_current_v2}, result_version[2]}, // current always selected
541-
{[]apiversions.APIVersion{api_current_v1, api_supported_v2}, result_version[1]}, // current always selected
542-
{[]apiversions.APIVersion{api_current_v3, api_supported_v2, api_deprecated_v1}, result_version[2]}, // with current v3, but should fall back to v2
543-
{[]apiversions.APIVersion{api_current_v3, api_deprecated_v2, api_deprecated_v1}, result_version[0]}, // v3 is not supported
544-
}
545-
546-
for _, suite := range testCases {
547-
if autodetectedVersion := doBsApiVersionAutodetect(suite.test_case); autodetectedVersion != suite.wanted_result {
548-
t.Fatalf("Autodetect for suite: %s, failed with result: '%s', wanted '%s'", suite.test_case, autodetectedVersion, suite.wanted_result)
549-
}
550-
}
551-
}
552-
553510
func TestInstanceIDFromProviderID(t *testing.T) {
554511
testCases := []struct {
555512
providerID string

0 commit comments

Comments
 (0)