Merge pull request #56437 from dims/fix-unit-tests-that-need-openstack

Automatic merge from submit-queue (batch tested with PRs 56446, 56437). 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 unit tests that need openstack

**What this PR does / why we need it**:
Currently the unit tests that depend that they be on running inside an openstack vm fail as no one seem to have run them for a while.

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

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

Author: Kubernetes Submit Queue

pkg/cloudprovider/providers/openstack/BUILD

@@ -76,6 +76,7 @@ go_test(
         "//pkg/cloudprovider:go_default_library",
         "//vendor/github.com/gophercloud/gophercloud:go_default_library",
         "//vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/servers:go_default_library",
+        "//vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/routers:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",

pkg/cloudprovider/providers/openstack/openstack_routes_test.go

@@ -20,6 +20,8 @@ import (
 	"net"
 	"testing"
 
+	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/routers"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kubernetes/pkg/cloudprovider"
 )
@@ -37,14 +39,18 @@ func TestRoutes(t *testing.T) {
 		t.Fatalf("Failed to construct/authenticate OpenStack: %s", err)
 	}
 
+	// Pick the first router and server to try a test with
+	os.routeOpts.RouterId = getRouters(os)[0].ID
+	servername := getServers(os)[0].Name
+
 	r, ok := os.Routes()
 	if !ok {
-		t.Fatalf("Routes() returned false - perhaps your stack doens't support Neutron?")
+		t.Skip("Routes() returned false - perhaps your stack does not support Neutron extraroute extension?")
 	}
 
 	newroute := cloudprovider.Route{
 		DestinationCIDR: "10.164.2.0/24",
-		TargetNode:      types.NodeName("testinstance"),
+		TargetNode:      types.NodeName(servername),
 	}
 	err = r.CreateRoute(clusterName, "myhint", &newroute)
 	if err != nil {
@@ -69,3 +75,39 @@ func TestRoutes(t *testing.T) {
 		t.Fatalf("DeleteRoute error: %v", err)
 	}
 }
+
+func getServers(os *OpenStack) []servers.Server {
+	c, err := os.NewComputeV2()
+	allPages, err := servers.List(c, servers.ListOpts{}).AllPages()
+	if err != nil {
+		panic(err)
+	}
+	allServers, err := servers.ExtractServers(allPages)
+	if err != nil {
+		panic(err)
+	}
+	if len(allServers) == 0 {
+		panic("No servers to test with")
+	}
+	return allServers
+}
+
+func getRouters(os *OpenStack) []routers.Router {
+	listOpts := routers.ListOpts{}
+	n, err := os.NewNetworkV2()
+	if err != nil {
+		panic(err)
+	}
+	allPages, err := routers.List(n, listOpts).AllPages()
+	if err != nil {
+		panic(err)
+	}
+	allRouters, err := routers.ExtractRouters(allPages)
+	if err != nil {
+		panic(err)
+	}
+	if len(allRouters) == 0 {
+		panic("No routers to test with")
+	}
+	return allRouters
+}

pkg/cloudprovider/providers/openstack/openstack_test.go

@@ -421,6 +421,7 @@ func configFromEnv() (cfg Config, ok bool) {
 			cfg.Global.DomainId != "" || cfg.Global.DomainName != ""))
 
 	cfg.Metadata.SearchOrder = fmt.Sprintf("%s,%s", configDriveID, metadataID)
+	cfg.BlockStorage.BSVersion = "auto"
 
 	return
 }
@@ -443,7 +444,7 @@ func TestLoadBalancer(t *testing.T) {
 		t.Skipf("No config found in environment")
 	}
 
-	versions := []string{"v1", "v2", ""}
+	versions := []string{"v2", ""}
 
 	for _, v := range versions {
 		t.Logf("Trying LBVersion = '%s'\n", v)
@@ -525,30 +526,30 @@ func TestVolumes(t *testing.T) {
 
 	id, err := os.InstanceID()
 	if err != nil {
-		t.Fatalf("Cannot find instance id: %v", err)
-	}
+		t.Logf("Cannot find instance id: %v - perhaps you are running this test outside a VM launched by OpenStack", err)
+	} else {
+		diskId, err := os.AttachDisk(id, vol)
+		if err != nil {
+			t.Fatalf("Cannot AttachDisk Cinder volume %s: %v", vol, err)
+		}
+		t.Logf("Volume (%s) attached, disk ID: %s\n", vol, diskId)
 
-	diskId, err := os.AttachDisk(id, vol)
-	if err != nil {
-		t.Fatalf("Cannot AttachDisk Cinder volume %s: %v", vol, err)
-	}
-	t.Logf("Volume (%s) attached, disk ID: %s\n", vol, diskId)
+		WaitForVolumeStatus(t, os, vol, volumeInUseStatus)
 
-	WaitForVolumeStatus(t, os, vol, volumeInUseStatus)
+		devicePath := os.GetDevicePath(diskId)
+		if diskPathRegexp.FindString(devicePath) == "" {
+			t.Fatalf("GetDevicePath returned and unexpected path for Cinder volume %s, returned %s", vol, devicePath)
+		}
+		t.Logf("Volume (%s) found at path: %s\n", vol, devicePath)
 
-	devicePath := os.GetDevicePath(diskId)
-	if diskPathRegexp.FindString(devicePath) == "" {
-		t.Fatalf("GetDevicePath returned and unexpected path for Cinder volume %s, returned %s", vol, devicePath)
-	}
-	t.Logf("Volume (%s) found at path: %s\n", vol, devicePath)
+		err = os.DetachDisk(id, vol)
+		if err != nil {
+			t.Fatalf("Cannot DetachDisk Cinder volume %s: %v", vol, err)
+		}
+		t.Logf("Volume (%s) detached\n", vol)
 
-	err = os.DetachDisk(id, vol)
-	if err != nil {
-		t.Fatalf("Cannot DetachDisk Cinder volume %s: %v", vol, err)
+		WaitForVolumeStatus(t, os, vol, volumeAvailableStatus)
 	}
-	t.Logf("Volume (%s) detached\n", vol)
-
-	WaitForVolumeStatus(t, os, vol, volumeAvailableStatus)
 
 	err = os.DeleteVolume(vol)
 	if err != nil {