Repository: kubermatic/machine-controller
Pull request #786 (2020-06-24 10:05:31)

Fix creation of servers with custom disk size on OpenStack
**What this PR does / why we need it**:
This PR:

- Fix an bug related to machine creation in OpenStack creation
- Add log support for requests addressed to OpenStack API to ease investigation

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>` format, will close the issue(s) when PR gets merged)*:
Fixes # #785

**Special notes for your reviewer**:

**Optional Release Note**:
<!--
Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note
NONE
```


kubermatic-bot: @irozzo-1A: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our [release note process](https://git.k8s.io/community/contributors/guide/release-notes.md) to remove it.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
kubermatic-bot: @irozzo-1A: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our [release note process](https://git.k8s.io/community/contributors/guide/release-notes.md) to remove it.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/786#" title="Author self-approved">irozzo-1A</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/master/OWNERS)~~ [irozzo-1A]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
kubermatic-bot: @irozzo-1A: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-machine-controller-lint | 22162a568839a9308c4cc6cd801b1ebf59c084fe | [link](https://public-prow.loodse.com/view/gcs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/786/pull-machine-controller-lint/1275731845568270337) | `/test pull-machine-controller-lint`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=786)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kubermatic-bot: @irozzo-1A: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-machine-controller-e2e-openstack | 501281704c17d4765c8df1bac14e4312bd09aa4e | [link](https://public-prow.loodse.com/view/gcs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/786/pull-machine-controller-e2e-openstack/1275851382166917120) | `/test pull-machine-controller-e2e-openstack`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=786)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
chrkl: Thanks for the PR. Looks good for me so far. Since I'm not involved in the machine-controller development at all, I would prefer someone else to give a LGTM (also think I lack permissions).
irozzo-1A: > Thanks for the PR. Looks good for me so far. Since I'm not involved in the machine-controller development at all, I would prefer someone else to give a LGTM (also think I lack permissions).

Yep no pb with that, it was just FYI. Thx for taking a look ;-)
kubermatic-bot: LGTM label has been added.  <details>Git tree hash: def6d3f22242e073c72bc494d4e5564ff0279fad</details>

Patch:
diff --git a/go.mod b/go.mod
index fc783fb9..dbdf713f 100644
--- a/go.mod
+++ b/go.mod
@@ -33,6 +33,7 @@ require (
 	github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect
 	github.com/google/go-querystring v1.0.0 // indirect
 	github.com/google/gofuzz v1.1.0 // indirect
+	github.com/google/uuid v1.1.1
 	github.com/googleapis/gnostic v0.4.0 // indirect
 	github.com/gophercloud/gophercloud v0.2.1-0.20190626201551-2949719e8258
 	github.com/hashicorp/golang-lru v0.5.4 // indirect
diff --git a/pkg/cloudprovider/provider/openstack/provider.go b/pkg/cloudprovider/provider/openstack/provider.go
index 745f8b7a..d27e89a2 100644
--- a/pkg/cloudprovider/provider/openstack/provider.go
+++ b/pkg/cloudprovider/provider/openstack/provider.go
@@ -39,6 +39,7 @@ import (
 	"github.com/kubermatic/machine-controller/pkg/cloudprovider/instance"
 	openstacktypes "github.com/kubermatic/machine-controller/pkg/cloudprovider/provider/openstack/types"
 	cloudprovidertypes "github.com/kubermatic/machine-controller/pkg/cloudprovider/types"
+	cloudproviderutil "github.com/kubermatic/machine-controller/pkg/cloudprovider/util"
 	"github.com/kubermatic/machine-controller/pkg/providerconfig"
 	providerconfigtypes "github.com/kubermatic/machine-controller/pkg/providerconfig/types"
 
@@ -56,13 +57,26 @@ const (
 	floatingIPIDAnnotationKey  = "kubermatic.io/release-openstack-floating-ip"
 )
 
+// clientGetterFunc returns an OpenStack client.
+type clientGetterFunc func(c *Config) (*gophercloud.ProviderClient, error)
+
+// serverReadinessWaiterFunc waits for the server with the given ID to be
+// ACTIVE.
+type serverReadinessWaiterFunc func(computeClient *gophercloud.ServiceClient, serverID string) error
+
 type provider struct {
-	configVarResolver *providerconfig.ConfigVarResolver
+	configVarResolver     *providerconfig.ConfigVarResolver
+	clientGetter          clientGetterFunc
+	serverReadinessWaiter serverReadinessWaiterFunc
 }
 
 // New returns a openstack provider
 func New(configVarResolver *providerconfig.ConfigVarResolver) cloudprovidertypes.Provider {
-	return &provider{configVarResolver: configVarResolver}
+	return &provider{
+		configVarResolver:     configVarResolver,
+		clientGetter:          getClient,
+		serverReadinessWaiter: waitUntilInstanceIsActive,
+	}
 }
 
 type Config struct {
@@ -226,7 +240,12 @@ func getClient(c *Config) (*gophercloud.ProviderClient, error) {
 		TokenID:          c.TokenID,
 	}
 
-	return goopenstack.AuthenticatedClient(opts)
+	pc, err := goopenstack.AuthenticatedClient(opts)
+	if pc != nil {
+		pc.HTTPClient = cloudproviderutil.HTTPClientConfig{LogPrefix: "[OpenStack API]"}.New()
+	}
+
+	return pc, err
 }
 
 func (p *provider) AddDefaults(spec v1alpha1.MachineSpec) (v1alpha1.MachineSpec, error) {
@@ -238,7 +257,7 @@ func (p *provider) AddDefaults(spec v1alpha1.MachineSpec) (v1alpha1.MachineSpec,
 		}
 	}
 
-	client, err := getClient(c)
+	client, err := p.clientGetter(c)
 	if err != nil {
 		return spec, osErrorToTerminalError(err, "failed to get a openstack client")
 	}
@@ -339,7 +358,7 @@ func (p *provider) Validate(spec v1alpha1.MachineSpec) error {
 		return errors.New("flavor must be configured")
 	}
 
-	client, err := getClient(c)
+	client, err := p.clientGetter(c)
 	if err != nil {
 		return fmt.Errorf("failed to get a openstack client: %v", err)
 	}
@@ -410,7 +429,7 @@ func (p *provider) Create(machine *v1alpha1.Machine, data *cloudprovidertypes.Pr
 		}
 	}
 
-	client, err := getClient(c)
+	client, err := p.clientGetter(c)
 	if err != nil {
 		return nil, osErrorToTerminalError(err, "failed to get a openstack client")
 	}
@@ -435,7 +454,7 @@ func (p *provider) Create(machine *v1alpha1.Machine, data *cloudprovidertypes.Pr
 		klog.V(2).Infof("creating security group %s for worker nodes", securityGroupName)
 		err = ensureKubernetesSecurityGroupExist(client, c.Region, securityGroupName)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("Error occurred creating security groups: %v", err)
 		}
 		securityGroups = append(securityGroups, securityGroupName)
 	}
@@ -452,7 +471,6 @@ func (p *provider) Create(machine *v1alpha1.Machine, data *cloudprovidertypes.Pr
 	serverOpts := osservers.CreateOpts{
 		Name:             machine.Spec.Name,
 		FlavorRef:        flavor.ID,
-		ImageRef:         image.ID,
 		UserData:         []byte(userdata),
 		SecurityGroups:   securityGroups,
 		AvailabilityZone: c.AvailabilityZone,
@@ -464,6 +482,7 @@ func (p *provider) Create(machine *v1alpha1.Machine, data *cloudprovidertypes.Pr
 	if c.RootDiskSizeGB != nil {
 		blockDevices := []bootfromvolume.BlockDevice{
 			{
+				BootIndex:           0,
 				DeleteOnTermination: true,
 				DestinationType:     bootfromvolume.DestinationVolume,
 				SourceType:          bootfromvolume.SourceImage,
@@ -479,12 +498,16 @@ func (p *provider) Create(machine *v1alpha1.Machine, data *cloudprovidertypes.Pr
 			return nil, osErrorToTerminalError(err, "failed to create server with volume")
 		}
 	} else {
+		// Image ID should only be set in server options when block device
+		// mapping is not used. Otherwish an error may occur with some
+		// OpenStack providers/versions .e.g. OpenTelekom Cloud
+		serverOpts.ImageRef = image.ID
 		if err := osservers.Create(computeClient, serverOpts).ExtractInto(&server); err != nil {
 			return nil, osErrorToTerminalError(err, "failed to create server")
 		}
 	}
 
-	if err := waitUntilInstanceIsActive(computeClient, server.ID); err != nil {
+	if err := p.serverReadinessWaiter(computeClient, server.ID); err != nil {
 		defer deleteInstanceDueToFatalLogged(computeClient, server.ID)
 		return nil, fmt.Errorf("instance %s became not active: %v", server.ID, err)
 	}
@@ -835,7 +858,7 @@ func (p *provider) cleanupFloatingIP(machine *v1alpha1.Machine, updater cloudpro
 		}
 	}
 
-	client, err := getClient(c)
+	client, err := p.clientGetter(c)
 	if err != nil {
 		return osErrorToTerminalError(err, "failed to get a openstack client")
 	}
diff --git a/pkg/cloudprovider/util/http.go b/pkg/cloudprovider/util/http.go
new file mode 100644
index 00000000..ec4a8416
--- /dev/null
+++ b/pkg/cloudprovider/util/http.go
@@ -0,0 +1,121 @@
+/*
+Copyright 2019 The Machine Controller Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"bytes"
+	"fmt"
+	"net/http"
+	"net/http/httputil"
+	"time"
+
+	"github.com/google/uuid"
+	"k8s.io/klog"
+)
+
+const defaultClientTimeout = 15 * time.Second
+
+type HTTPClientConfig struct {
+	// LogPrefix is pre-pended to request/response logs
+	LogPrefix string
+	// Global timeout used by the client
+	Timeout time.Duration
+}
+
+// New return a custom HTTP client that allows for logging
+// HTTP request and response information.
+func (c HTTPClientConfig) New() http.Client {
+	timeout := c.Timeout
+	// Enforce a global timeout
+	if timeout <= 0 {
+		timeout = defaultClientTimeout
+	}
+	return http.Client{
+		Transport: &LogRoundTripper{
+			logPrefix: c.LogPrefix,
+			rt:        http.DefaultTransport,
+		},
+		Timeout: timeout,
+	}
+}
+
+// LogRoundTripper is used to log information about requests and responses that
+// may be useful for debugging purposes.
+// Note that setting log level >5 results in full dumps of requests and
+// responses, including sensitive invormation (e.g. Authorization header).
+type LogRoundTripper struct {
+	logPrefix string
+	rt        http.RoundTripper
+}
+
+func (lrt *LogRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {
+	var log []byte
+	var err error
+	// Generate unique ID to correlate requests and responses
+	id := uuid.New()
+	switch {
+	case bool(klog.V(6)):
+		log, err = httputil.DumpRequest(request, true)
+		if err != nil {
+			klog.Warningf("Error occurred while dumping request: %v", err)
+		}
+	case bool(klog.V(5)):
+		log, err = httputil.DumpRequest(request, false)
+		if err != nil {
+			klog.Warningf("Error occurred while dumping request: %v", err)
+		}
+	default:
+		var b bytes.Buffer
+		fmt.Fprintf(&b, "%s %s HTTP/%d.%d", valueOrDefault(request.Method, "GET"),
+			request.URL.RequestURI(), request.ProtoMajor, request.ProtoMinor)
+		log = b.Bytes()
+	}
+	klog.V(1).Infof("%s request sent [%s]: %s\n", lrt.logPrefix, id.String(), string(log))
+
+	response, err := lrt.rt.RoundTrip(request)
+	if response == nil {
+		return nil, err
+	}
+
+	switch {
+	case bool(klog.V(6)):
+		log, err = httputil.DumpResponse(response, true)
+		if err != nil {
+			klog.Warningf("Error occurred while dumping response: %v", err)
+		}
+	case bool(klog.V(5)):
+		log, err = httputil.DumpResponse(response, false)
+		if err != nil {
+			klog.Warningf("Error occurred while dumping response: %v", err)
+		}
+	default:
+		var b bytes.Buffer
+		fmt.Fprintf(&b, "HTTP/%d.%d %03d", response.ProtoMajor, response.ProtoMinor, response.StatusCode)
+		log = b.Bytes()
+	}
+	klog.V(1).Infof("%s request received [%s]: %s\n", lrt.logPrefix, id.String(), string(log))
+
+	return response, nil
+}
+
+// Return value if nonempty, def otherwise.
+func valueOrDefault(value, def string) string {
+	if value != "" {
+		return value
+	}
+	return def
+}


Test patch:
diff --git a/pkg/cloudprovider/provider/openstack/provider_test.go b/pkg/cloudprovider/provider/openstack/provider_test.go
new file mode 100644
index 00000000..94fd7002
--- /dev/null
+++ b/pkg/cloudprovider/provider/openstack/provider_test.go
@@ -0,0 +1,379 @@
+/*
+Copyright 2019 The Machine Controller Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package openstack
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"testing"
+	"text/template"
+
+	cloudprovidertesting "github.com/kubermatic/machine-controller/pkg/cloudprovider/testing"
+	cloudprovidertypes "github.com/kubermatic/machine-controller/pkg/cloudprovider/types"
+	"github.com/kubermatic/machine-controller/pkg/providerconfig"
+
+	"k8s.io/utils/pointer"
+	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/gophercloud/gophercloud"
+	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
+	th "github.com/gophercloud/gophercloud/testhelper"
+	"github.com/gophercloud/gophercloud/testhelper/client"
+)
+
+const expectedServerRequest = `{
+  "server": {
+	  "availability_zone": "eu-de-01",
+	  "flavorRef": "1",
+	  "imageRef": "f3e4a95d-1f4f-4989-97ce-f3a1fb8c04d7",
+	  "metadata": {
+		"kubernetes-cluster": "xyz",
+		"machine-uid": "",
+		"system-cluster": "zyx",
+		"system-project": "xxx"
+	  },
+	  "name": "test",
+	  "networks": [
+		{
+		  "uuid": "d32019d3-bc6e-4319-9c1d-6722fc136a22"
+		}
+	  ],
+	  "security_groups": [
+		{
+		  "name": "kubernetes-xyz"
+		}
+	  ],
+	  "user_data": "ZmFrZS11c2VyZGF0YQ=="
+  }
+}
+`
+
+const expectedBlockDeviceBootRequest = `{
+  "server": {
+	"availability_zone": "eu-de-01",
+	"block_device_mapping_v2": [
+	  {
+		"boot_index": 0,
+		"delete_on_termination": true,
+		"destination_type": "volume",
+		"source_type": "image",
+		"uuid": "f3e4a95d-1f4f-4989-97ce-f3a1fb8c04d7",
+		"volume_size": 10
+	  }
+	],
+	"flavorRef": "1",
+	"imageRef": "",
+	"metadata": {
+	  "kubernetes-cluster": "xyz",
+	  "machine-uid": "",
+	  "system-cluster": "zyx",
+	  "system-project": "xxx"
+	},
+	"name": "test",
+	"networks": [
+	  {
+		"uuid": "d32019d3-bc6e-4319-9c1d-6722fc136a22"
+	  }
+	],
+	"security_groups": [
+	  {
+		"name": "kubernetes-xyz"
+	  }
+	],
+	"user_data": "ZmFrZS11c2VyZGF0YQ=="
+  }
+}`
+
+type openstackProviderSpecConf struct {
+	IdentityEndpointURL string
+	RootDiskSizeGB      *int32
+}
+
+func (o openstackProviderSpecConf) rawProviderSpec(t *testing.T) []byte {
+	var out bytes.Buffer
+	tmpl, err := template.New("test").Parse(`{
+	"cloudProvider": "openstack",
+	"cloudProviderSpec": {
+		"availabilityZone": "eu-de-01",
+		"domainName": "openstack_domain_name",
+		"flavor": "m1.tiny",
+		"identityEndpoint": "{{ .IdentityEndpointURL }}",
+		"image": "Standard_Ubuntu_18.04_latest",
+		"network": "public",
+		"nodeVolumeAttachLimit": null,
+		"password": "this_is_a_password",
+		"region": "eu-de",
+		{{- if .RootDiskSizeGB }}
+		"rootDiskSizeGB": {{ .RootDiskSizeGB }},
+		{{- end }}
+		"securityGroups": [
+			"kubernetes-xyz"
+		],
+		"subnet": "subnetid",
+		"tags": {
+			"kubernetes-cluster": "xyz",
+			"system-cluster": "zyx",
+			"system-project": "xxx"
+		},
+		"tenantID": "",
+		"tenantName": "eu-de",
+		"tokenId": "",
+		"trustDevicePath": false,
+		"username": "dummy"
+	},
+	"operatingSystem": "coreos",
+	"operatingSystemSpec": {
+		"disableAutoUpdate": false,
+		"disableLocksmithD": true,
+		"disableUpdateEngine": false
+	}
+}`)
+	if err != nil {
+		t.Fatalf("Error occurred while parsing openstack provider spec template: %v", err)
+	}
+	err = tmpl.Execute(&out, o)
+	if err != nil {
+		t.Fatalf("Error occurred while executing openstack provider spec template: %v", err)
+	}
+	t.Logf("Generated providerSpec: %s", out.String())
+	return out.Bytes()
+}
+
+func TestCreateServer(t *testing.T) {
+	tests := []struct {
+		name          string
+		specConf      openstackProviderSpecConf
+		data          *cloudprovidertypes.ProviderData
+		userdata      string
+		wantServerReq string
+		wantErr       bool
+	}{
+		{
+			name:          "Nominal case",
+			specConf:      openstackProviderSpecConf{},
+			userdata:      "fake-userdata",
+			wantServerReq: expectedServerRequest,
+		},
+		{
+			name:          "Custom disk size",
+			specConf:      openstackProviderSpecConf{RootDiskSizeGB: pointer.Int32Ptr(10)},
+			userdata:      "fake-userdata",
+			wantServerReq: expectedBlockDeviceBootRequest,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			th.SetupHTTP()
+			defer th.TeardownHTTP()
+			ExpectServerCreated(t, tt.wantServerReq)
+			p := &provider{
+				// Note that configVarResolver is not used in this test as the getConfigFunc is mocked.
+				configVarResolver: providerconfig.NewConfigVarResolver(context.Background(), fakeclient.NewFakeClient()),
+				// mock client config getter
+				clientGetter: func(c *Config) (*gophercloud.ProviderClient, error) {
+					pc := client.ServiceClient()
+					// endpoint locator used to redirect to local test endpoint
+					pc.ProviderClient.EndpointLocator = func(_ gophercloud.EndpointOpts) (string, error) {
+						return pc.Endpoint, nil
+					}
+					return pc.ProviderClient, nil
+				},
+				// mock server readiness checker
+				serverReadinessWaiter: func(computeClient *gophercloud.ServiceClient, serverID string) error {
+					return nil
+				},
+			}
+			// Use the endpoint of the local server simulating OpenStack API
+			tt.specConf.IdentityEndpointURL = th.Endpoint()
+			m := cloudprovidertesting.Creator{
+				Name:               "test",
+				Namespace:          "openstack",
+				ProviderSpecGetter: tt.specConf.rawProviderSpec,
+			}.CreateMachine(t)
+			// It only verifies that the content of the create request matches
+			// the expectation
+			// TODO(irozzo) check the returned instance too
+			_, err := p.Create(m, tt.data, tt.userdata)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("provider.Create() or = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+		})
+	}
+}
+
+type ServerResponse struct {
+	Server servers.Server `json:"server"`
+}
+
+// ExpectServerCreated is used to verify that the manifest used to create the
+// server matches the expectation.
+// It also provides all the the handlers required to mock the Openstack APIs
+// used during the server creation.
+func ExpectServerCreated(t *testing.T, expectedServer string) {
+	// Prepare the server manifest
+	var res ServerResponse
+	// This is just a hacky way of getting the values matching from the
+	// expectedServer copied into the response (e.g. name).
+	err := json.Unmarshal([]byte(expectedServer), &res)
+	if err != nil {
+		t.Fatalf("Error occurred while unmarshaling the expected server manifest.")
+	}
+	res.Server.ID = "f3e4a95d-1f4f-4989-97ce-f3a1fb8c04d7"
+	srvRes, err := json.Marshal(res)
+	if err != nil {
+		t.Fatalf("Error occurred while marshaling the server response manifest.")
+	}
+	t.Logf("Server response: %s", srvRes)
+	// Handle server creation requests.
+	th.Mux.HandleFunc("/servers", func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "POST")
+		th.TestHeader(t, r, "X-Auth-Token", client.TokenID)
+		th.TestJSONRequest(t, r, expectedServer)
+
+		w.WriteHeader(http.StatusAccepted)
+		w.Header().Add("Content-Type", "application/json")
+		fmt.Fprintf(w, string(srvRes))
+	})
+	th.Mux.HandleFunc("/os-volumes_boot", func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "POST")
+		th.TestHeader(t, r, "X-Auth-Token", client.TokenID)
+		th.TestJSONRequest(t, r, expectedServer)
+
+		w.WriteHeader(http.StatusAccepted)
+		w.Header().Add("Content-Type", "application/json")
+		fmt.Fprintf(w, string(srvRes))
+	})
+
+	// Handle listing images.
+	th.Mux.HandleFunc("/images/detail", func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "GET")
+		th.TestHeader(t, r, "X-Auth-Token", client.TokenID)
+
+		w.Header().Add("Content-Type", "application/json")
+		fmt.Fprintf(w, `
+			{
+				"images": [
+					{
+						"status": "ACTIVE",
+						"updated": "2014-09-23T12:54:56Z",
+						"id": "f3e4a95d-1f4f-4989-97ce-f3a1fb8c04d7",
+						"OS-EXT-IMG-SIZE:size": 476704768,
+						"name": "F17-x86_64-cfntools",
+						"created": "2014-09-23T12:54:52Z",
+						"minDisk": 0,
+						"progress": 100,
+						"minRam": 0
+					},
+					{
+						"status": "ACTIVE",
+						"updated": "2014-09-23T12:51:43Z",
+						"id": "f90f6034-2570-4974-8351-6b49732ef2eb",
+						"OS-EXT-IMG-SIZE:size": 13167616,
+						"name": "cirros-0.3.2-x86_64-disk",
+						"created": "2014-09-23T12:51:42Z",
+						"minDisk": 0,
+						"progress": 100,
+						"minRam": 0
+					}
+				]
+			}
+		`)
+	})
+	// Handle listing flavours.
+	th.Mux.HandleFunc("/flavors/detail", func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "GET")
+		th.TestHeader(t, r, "X-Auth-Token", client.TokenID)
+
+		w.Header().Add("Content-Type", "application/json")
+		err := r.ParseForm()
+		if err != nil {
+			t.Fatalf("Error occurred while parsing form: %v", err)
+		}
+		marker := r.Form.Get("marker")
+		switch marker {
+		case "":
+			fmt.Fprintf(w, `
+				{
+					"flavors": [
+						{
+							"id": "1",
+							"name": "m1.tiny",
+							"disk": 1,
+							"ram": 512,
+							"vcpus": 1,
+							"swap":""
+						},
+						{
+							"id": "2",
+							"name": "m2.small",
+							"disk": 10,
+							"ram": 1024,
+							"vcpus": 2,
+							"swap": 1000
+						}
+					],
+					"flavors_links": [
+						{
+							"href": "%s/flavors/detail?marker=2",
+							"rel": "next"
+						}
+					]
+				}
+			`, th.Server.URL)
+		case "2":
+			fmt.Fprintf(w, `{ "flavors": [] }`)
+		default:
+			t.Fatalf("Unexpected marker: [%s]", marker)
+		}
+	})
+	// Handle listing networks.
+	th.Mux.HandleFunc("/v2.0/networks", func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "GET")
+		th.TestHeader(t, r, "X-Auth-Token", client.TokenID)
+
+		w.Header().Add("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+
+		fmt.Fprintf(w, `
+			{
+				"networks": [
+					{
+						"status": "ACTIVE",
+						"subnets": [
+							"54d6f61d-db07-451c-9ab3-b9609b6b6f0b"
+						],
+						"name": "public",
+						"admin_state_up": true,
+						"tenant_id": "4fd44f30292945e481c7b8a0c8908869",
+						"shared": true,
+						"id": "d32019d3-bc6e-4319-9c1d-6722fc136a22",
+						"provider:segmentation_id": 9876543210,
+						"provider:physical_network": null,
+						"provider:network_type": "local",
+						"router:external": true,
+						"port_security_enabled": true,
+						"dns_domain": "local.",
+						"mtu": 1500
+					}
+				]
+			}`)
+	})
+}
diff --git a/pkg/cloudprovider/provider/vsphere/provider_test.go b/pkg/cloudprovider/provider/vsphere/provider_test.go
index f7fec207..eff5d284 100644
--- a/pkg/cloudprovider/provider/vsphere/provider_test.go
+++ b/pkg/cloudprovider/provider/vsphere/provider_test.go
@@ -17,127 +17,131 @@ limitations under the License.
 package vsphere
 
 import (
+	"bytes"
 	"context"
-	"fmt"
 	"log"
 	"strings"
 	"testing"
+	"text/template"
 
-	"github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
+	cloudprovidertesting "github.com/kubermatic/machine-controller/pkg/cloudprovider/testing"
 	"github.com/kubermatic/machine-controller/pkg/providerconfig"
 	"github.com/vmware/govmomi/simulator"
-	"k8s.io/apimachinery/pkg/runtime"
 
+	"k8s.io/utils/pointer"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
-type machineSpecArgs struct {
-	datastore        *string
-	datastoreCluster *string
+type vsphereProviderSpecConf struct {
+	Datastore        *string
+	DatastoreCluster *string
+	User             string
+	Password         string
+	URL              string
 }
 
-func (m machineSpecArgs) generateMachineSpec(user, password, url string) v1alpha1.MachineSpec {
-	dsSpec := ""
-	if m.datastore != nil {
-		dsSpec = fmt.Sprintf(`"datastore": "%s",`, *m.datastore)
+func (v vsphereProviderSpecConf) rawProviderSpec(t *testing.T) []byte {
+	var out bytes.Buffer
+	tmpl, err := template.New("test").Parse(`{
+	"cloudProvider": "vsphere",
+	"cloudProviderSpec": {
+		"allowInsecure": false,
+		"cluster": "DC0_C0",
+		"cpus": 1,
+		"datacenter": "DC0",
+		{{- if .Datastore }}
+		"datastore": "{{ .Datastore }}",
+		{{- end }}
+		{{- if .DatastoreCluster }}
+		"datastoreCluster": "{{ .DatastoreCluster }}",
+		{{- end }}
+		"folder": "/",
+		"resourcePool": "/DC0/host/DC0_C0/Resources",
+		"memoryMB": 2000,
+		"password": "{{ .Password }}",
+		"templateVMName": "DC0_H0_VM0",
+		"username": "{{ .User }}",
+		"vmNetName": "",
+		"vsphereURL": "{{ .URL }}"
+	},
+	"operatingSystem": "coreos",
+	"operatingSystemSpec": {
+		"disableAutoUpdate": false,
+		"disableLocksmithD": true,
+		"disableUpdateEngine": false
 	}
-	if m.datastoreCluster != nil {
-		dsSpec = dsSpec + fmt.Sprintf(`"datastoreCluster": "%s",`, *m.datastoreCluster)
+}`)
+	if err != nil {
+		t.Fatalf("Error occurred while parsing openstack provider spec template: %v", err)
 	}
-	return v1alpha1.MachineSpec{
-		ProviderSpec: v1alpha1.ProviderSpec{
-			Value: &runtime.RawExtension{
-				Raw: []byte(fmt.Sprintf(`{
-					"cloudProvider": "vsphere",
-					"cloudProviderSpec": {
-						"allowInsecure": false,
-						"cluster": "DC0_C0",
-						"cpus": 1,
-						"datacenter": "DC0",
-						%s
-						"folder": "/",
-						"resourcePool": "/DC0/host/DC0_C0/Resources",
-						"memoryMB": 2000,
-						"password": "%s",
-						"templateVMName": "DC0_H0_VM0",
-						"username": "%s",
-						"vmNetName": "",
-						"vsphereURL": "%s"
-					},
-					"operatingSystem": "coreos",
-					"operatingSystemSpec": {
-						"disableAutoUpdate": false,
-						"disableLocksmithD": true,
-						"disableUpdateEngine": false
-					}
-				}`, dsSpec, password, user, url)),
-			},
-		},
+	err = tmpl.Execute(&out, v)
+	if err != nil {
+		t.Fatalf("Error occurred while executing openstack provider spec template: %v", err)
 	}
+	t.Logf("Generated providerSpec: %s", out.String())
+	return out.Bytes()
 }
-func Test_provider_Validate(t *testing.T) {
-	toPointer := func(s string) *string {
-		return &s
-	}
+
+func TestValidate(t *testing.T) {
 	tests := []struct {
 		name         string
-		args         machineSpecArgs
+		args         vsphereProviderSpecConf
 		getConfigErr error
 		wantErr      bool
 	}{
 		{
 			name: "Valid Datastore",
-			args: machineSpecArgs{
-				datastore: toPointer("LocalDS_0"),
+			args: vsphereProviderSpecConf{
+				Datastore: pointer.StringPtr("LocalDS_0"),
 			},
 			getConfigErr: nil,
 			wantErr:      false,
 		},
 		{
 			name: "Valid Datastore end empty DatastoreCluster",
-			args: machineSpecArgs{
-				datastore:        toPointer("LocalDS_0"),
-				datastoreCluster: toPointer(""),
+			args: vsphereProviderSpecConf{
+				Datastore:        pointer.StringPtr("LocalDS_0"),
+				DatastoreCluster: pointer.StringPtr(""),
 			},
 			getConfigErr: nil,
 			wantErr:      false,
 		},
 		{
 			name: "Valid DatastoreCluster",
-			args: machineSpecArgs{
-				datastoreCluster: toPointer("DC0_POD0"),
+			args: vsphereProviderSpecConf{
+				DatastoreCluster: pointer.StringPtr("DC0_POD0"),
 			},
 			getConfigErr: nil,
 			wantErr:      false,
 		},
 		{
 			name: "Invalid Datastore",
-			args: machineSpecArgs{
-				datastore: toPointer("LocalDS_10"),
+			args: vsphereProviderSpecConf{
+				Datastore: pointer.StringPtr("LocalDS_10"),
 			},
 			getConfigErr: nil,
 			wantErr:      true,
 		},
 		{
 			name: "Invalid DatastoreCluster",
-			args: machineSpecArgs{
-				datastore: toPointer("DC0_POD10"),
+			args: vsphereProviderSpecConf{
+				Datastore: pointer.StringPtr("DC0_POD10"),
 			},
 			getConfigErr: nil,
 			wantErr:      true,
 		},
 		{
 			name: "Both Datastore and DatastoreCluster specified",
-			args: machineSpecArgs{
-				datastore:        toPointer("DC0_POD10"),
-				datastoreCluster: toPointer("DC0_POD0"),
+			args: vsphereProviderSpecConf{
+				Datastore:        pointer.StringPtr("DC0_POD10"),
+				DatastoreCluster: pointer.StringPtr("DC0_POD0"),
 			},
 			getConfigErr: nil,
 			wantErr:      true,
 		},
 		{
 			name:         "Neither Datastore nor DatastoreCluster specified",
-			args:         machineSpecArgs{},
+			args:         vsphereProviderSpecConf{},
 			getConfigErr: nil,
 			wantErr:      true,
 		},
@@ -167,7 +171,12 @@ func Test_provider_Validate(t *testing.T) {
 				// Note that configVarResolver is not used in this test as the getConfigFunc is mocked.
 				configVarResolver: providerconfig.NewConfigVarResolver(context.Background(), fakeclient.NewFakeClient()),
 			}
-			if err := p.Validate(tt.args.generateMachineSpec(username, password, vSphereURL)); (err != nil) != tt.wantErr {
+			tt.args.User = username
+			tt.args.Password = password
+			tt.args.URL = vSphereURL
+			m := cloudprovidertesting.Creator{Name: "test", Namespace: "vsphere", ProviderSpecGetter: tt.args.rawProviderSpec}.
+				CreateMachine(t)
+			if err := p.Validate(m.Spec); (err != nil) != tt.wantErr {
 				t.Errorf("provider.Validate() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})
diff --git a/pkg/cloudprovider/testing/testing.go b/pkg/cloudprovider/testing/testing.go
new file mode 100644
index 00000000..aae583cf
--- /dev/null
+++ b/pkg/cloudprovider/testing/testing.go
@@ -0,0 +1,56 @@
+/*
+Copyright 2019 The Machine Controller Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package testing
+
+import (
+	"testing"
+
+	"github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+// ProvProviderSpecGetter generates provider spec for testing purposes.
+type ProviderSpecGetter func(t *testing.T) []byte
+
+// Creator is used to generate test resources.
+type Creator struct {
+	Name               string
+	Namespace          string
+	ProviderSpecGetter ProviderSpecGetter
+}
+
+func (c Creator) CreateMachine(t *testing.T) *v1alpha1.Machine {
+	return &v1alpha1.Machine{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      c.Name,
+			Namespace: c.Namespace,
+		},
+		Spec: v1alpha1.MachineSpec{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      c.Name,
+				Namespace: c.Namespace,
+			},
+			ProviderSpec: v1alpha1.ProviderSpec{
+				Value: &runtime.RawExtension{
+					Raw: c.ProviderSpecGetter(t),
+				},
+			},
+		},
+	}
+}
diff --git a/test/e2e/provisioning/testdata/machinedeployment-openstack.yaml b/test/e2e/provisioning/testdata/machinedeployment-openstack.yaml
index d7789603..4a46aec3 100644
--- a/test/e2e/provisioning/testdata/machinedeployment-openstack.yaml
+++ b/test/e2e/provisioning/testdata/machinedeployment-openstack.yaml
@@ -34,6 +34,7 @@ spec:
             domainName: "<< DOMAIN_NAME >>"
             region: "<< REGION >>"
             network: "<< NETWORK_NAME >>"
+            rootDiskSizeGB: 20
           operatingSystem: "<< OS_NAME >>"
           operatingSystemSpec:
             distUpgradeOnBoot: false