Repository: kubermatic/machine-controller
Pull request #947 (2021-04-15 15:03:57)

Cloud init refactoring
**What this PR does / why we need it**:
Remove the cloud-init bootstraping template from the machine controller and refactor tinkerbell code.
 
**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>` format, will close the issue(s) when PR gets merged)*:
Fixes #

**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
Support Tinkerbell as a bare-metal provider 
```


kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/947#" title="Author self-approved">moadqassem</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)~~ [moadqassem]

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: @moadqassem: PR needs rebase.

<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: @moadqassem: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-kubermatic-machine-controller-validate-prow-yaml | 233695917d38d1fd1c1415be124495356777fab5 | [link](https://public-prow.loodse.com/view/gs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/947/pull-kubermatic-machine-controller-validate-prow-yaml/1382711360982355968) | `/test pull-kubermatic-machine-controller-validate-prow-yaml`

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

<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: LGTM label has been added.  <details>Git tree hash: b7a0a410a358a73cef58277796c0aadcf72c087a</details>

Patch:
diff --git a/pkg/cloudprovider/provider/baremetal/plugins/driver.go b/pkg/cloudprovider/provider/baremetal/plugins/driver.go
index 37ba88ba..165f990e 100644
--- a/pkg/cloudprovider/provider/baremetal/plugins/driver.go
+++ b/pkg/cloudprovider/provider/baremetal/plugins/driver.go
@@ -27,11 +27,18 @@ type Driver string
 
 const Tinkerbell Driver = "tinkerbell"
 
+type CloudConfigSettings struct {
+	Token       string
+	Namespace   string
+	SecretName  string
+	ClusterHost string
+}
+
 // PluginDriver manages the communications between the machine controller cloud provider and the bare metal env.
 type PluginDriver interface {
 	GetServer(context.Context, types.UID, runtime.RawExtension) (Server, error)
 	Validate(runtime.RawExtension) error
-	ProvisionServer(context.Context, types.UID, runtime.RawExtension) (Server, error)
+	ProvisionServer(context.Context, types.UID, *CloudConfigSettings, runtime.RawExtension) (Server, error)
 	DeprovisionServer(types.UID, runtime.RawExtension) (string, error)
 }
 
diff --git a/pkg/cloudprovider/provider/baremetal/plugins/tinkerbell/driver.go b/pkg/cloudprovider/provider/baremetal/plugins/tinkerbell/driver.go
index ef2d5842..74c4bc3e 100644
--- a/pkg/cloudprovider/provider/baremetal/plugins/tinkerbell/driver.go
+++ b/pkg/cloudprovider/provider/baremetal/plugins/tinkerbell/driver.go
@@ -87,7 +87,7 @@ func (d *driver) GetServer(ctx context.Context, uid types.UID, hwSpec runtime.Ra
 	}, nil
 }
 
-func (d *driver) ProvisionServer(ctx context.Context, uid types.UID, hwSpec runtime.RawExtension) (plugins.Server, error) {
+func (d *driver) ProvisionServer(ctx context.Context, uid types.UID, cfg *plugins.CloudConfigSettings, hwSpec runtime.RawExtension) (plugins.Server, error) {
 	hw := HardwareSpec{}
 	if err := json.Unmarshal(hwSpec.Raw, &hw); err != nil {
 		return nil, fmt.Errorf("failed to unmarshal tinkerbell hardware spec: %v", err)
@@ -98,19 +98,25 @@ func (d *driver) ProvisionServer(ctx context.Context, uid types.UID, hwSpec runt
 		return nil, fmt.Errorf("failed to register hardware to tink-server: %v", err)
 	}
 
-	tmpl := createTemplate(hw.GetMACAddress(), d.TinkServerAddress, d.ImageRepoAddress)
-	payload, err := yaml.Marshal(tmpl)
+	workflowTemplate, err := d.templateClient.Get(ctx, "", provisioningTemplate)
 	if err != nil {
-		return nil, fmt.Errorf("failed marshalling workflow template: %v", err)
-	}
-
-	workflowTemplate := &tinktmpl.WorkflowTemplate{
-		Name: tmpl.Name,
-		Data: string(payload),
-	}
-
-	if err := d.templateClient.Create(ctx, workflowTemplate); err != nil {
-		return nil, fmt.Errorf("failed to create workflow template: %v", err)
+		if resourceNotFoundErr(err) {
+			tmpl := createTemplate(hw.GetMACAddress(), d.TinkServerAddress, d.ImageRepoAddress, cfg)
+			payload, err := yaml.Marshal(tmpl)
+			if err != nil {
+				return nil, fmt.Errorf("failed marshalling workflow template: %v", err)
+			}
+
+			workflowTemplate := &tinktmpl.WorkflowTemplate{
+				Name: tmpl.Name,
+				Id:   tmpl.ID,
+				Data: string(payload),
+			}
+
+			if err := d.templateClient.Create(ctx, workflowTemplate); err != nil {
+				return nil, fmt.Errorf("failed to create workflow template: %v", err)
+			}
+		}
 	}
 
 	if _, err := d.workflowClient.Create(ctx, workflowTemplate.Id, hw.GetID()); err != nil {
diff --git a/pkg/cloudprovider/provider/baremetal/plugins/tinkerbell/template.go b/pkg/cloudprovider/provider/baremetal/plugins/tinkerbell/template.go
index 53acc2e5..a9db67df 100644
--- a/pkg/cloudprovider/provider/baremetal/plugins/tinkerbell/template.go
+++ b/pkg/cloudprovider/provider/baremetal/plugins/tinkerbell/template.go
@@ -16,12 +16,21 @@ limitations under the License.
 
 package tinkerbell
 
-import "github.com/tinkerbell/tink/workflow"
+import (
+	"github.com/tinkerbell/tink/workflow"
 
-func createTemplate(worker, tinkServerAddress, imageRepoAddress string) *workflow.Workflow {
+	"github.com/kubermatic/machine-controller/pkg/cloudprovider/provider/baremetal/plugins"
+)
+
+const (
+	provisioningTemplate = "ubuntu_os_provisioning"
+)
+
+func createTemplate(worker, tinkServerAddress, imageRepoAddress string, cfg *plugins.CloudConfigSettings) *workflow.Workflow {
 	return &workflow.Workflow{
 		Version:       "0.1",
-		Name:          "ubuntu_provisioning",
+		Name:          provisioningTemplate,
+		ID:            "",
 		GlobalTimeout: 6000,
 		Tasks: []workflow.Task{
 			{
@@ -69,6 +78,18 @@ func createTemplate(worker, tinkServerAddress, imageRepoAddress string) *workflo
 							"/statedir:/statedir",
 						},
 					},
+					{
+						Name:    "cloud-init",
+						Image:   "cloud-init:v1",
+						Timeout: 600,
+						Environment: map[string]string{
+							"MIRROR_HOST":                   imageRepoAddress,
+							"CLOUD_INIT_TOKEN":              cfg.Token,
+							"CLOUD_INIT_SETTINGS_NAMESPACE": cfg.Namespace,
+							"SECRET_NAME":                   cfg.SecretName,
+							"CLUSTER_HOST":                  cfg.ClusterHost,
+						},
+					},
 				},
 			},
 		},
diff --git a/pkg/cloudprovider/provider/baremetal/provider.go b/pkg/cloudprovider/provider/baremetal/provider.go
index fb61dc4b..a81ced33 100644
--- a/pkg/cloudprovider/provider/baremetal/provider.go
+++ b/pkg/cloudprovider/provider/baremetal/provider.go
@@ -30,6 +30,7 @@ import (
 	"github.com/kubermatic/machine-controller/pkg/cloudprovider/provider/baremetal/plugins/tinkerbell"
 	baremetaltypes "github.com/kubermatic/machine-controller/pkg/cloudprovider/provider/baremetal/types"
 	cloudprovidertypes "github.com/kubermatic/machine-controller/pkg/cloudprovider/types"
+	"github.com/kubermatic/machine-controller/pkg/cloudprovider/util"
 	"github.com/kubermatic/machine-controller/pkg/providerconfig"
 	providerconfigtypes "github.com/kubermatic/machine-controller/pkg/providerconfig/types"
 
@@ -162,7 +163,7 @@ func (p provider) Get(machine *v1alpha1.Machine, _ *cloudprovidertypes.ProviderD
 			return nil, cloudprovidererrors.ErrInstanceNotFound
 		}
 
-		return nil, fmt.Errorf("failed to fetch server with the id %s: %v", server.GetID(), err)
+		return nil, fmt.Errorf("failed to fetch server with the id %s: %v", machine.Name, err)
 	}
 
 	return &bareMetalServer{
@@ -183,7 +184,24 @@ func (p provider) Create(machine *v1alpha1.Machine, data *cloudprovidertypes.Pro
 		}
 	}
 
-	server, err := c.driver.ProvisionServer(context.Background(), machine.UID, c.driverSpec)
+	ctx := context.Background()
+	if err := util.CreateMachineCloudInitSecret(ctx, userdata, machine.Name, data.Client); err != nil {
+		return nil, fmt.Errorf("failed to create cloud-init secret for machine %s: %v", machine.Name, err)
+	}
+
+	token, apiServer, err := util.ExtractTokenAndAPIServer(ctx, userdata, data.Client)
+	if err != nil {
+		return nil, fmt.Errorf("failed to extarct token and api server address: %v", err)
+	}
+
+	cfg := &plugins.CloudConfigSettings{
+		Token:       token,
+		Namespace:   util.CloudInitNamespace,
+		SecretName:  machine.Name,
+		ClusterHost: apiServer,
+	}
+
+	server, err := c.driver.ProvisionServer(ctx, machine.UID, cfg, c.driverSpec)
 	if err != nil {
 		return nil, fmt.Errorf("failed to provisioner server: %v", err)
 	}
diff --git a/pkg/cloudprovider/util/cloud_init_settings.go b/pkg/cloudprovider/util/cloud_init_settings.go
index cc21472c..4967e8e2 100644
--- a/pkg/cloudprovider/util/cloud_init_settings.go
+++ b/pkg/cloudprovider/util/cloud_init_settings.go
@@ -17,14 +17,11 @@ limitations under the License.
 package util
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
-	"strings"
-	"text/template"
-
 	"gopkg.in/yaml.v3"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
@@ -37,50 +34,17 @@ import (
 const (
 	CloudInitNamespace = "cloud-init-settings"
 	jwtTokenNamePrefix = "cloud-init-getter-token"
-
-	bootstrapCloudConfig = `#cloud-config
-
-write_files:
-- path: "/opt/bin/bootstrap"
-  permissions: "0755"
-  content: |
-    #!/bin/bash
-    set -xeuo pipefail
-    apt-get update
-    apt-get install jq -y
-    wget --no-check-certificate --quiet \
-      --directory-prefix /etc/cloud/cloud.cfg.d/ \
-      --method GET \
-      --timeout=60 \
-      --header 'Authorization: Bearer {{ .Token }}' \
-      '{{ .ClusterHost }}/api/v1/namespaces/{{ .CloudInitSettingsNamespace }}/secrets/{{ .SecretName }}'
-    cat /etc/cloud/cloud.cfg.d/{{ .SecretName }} | jq '.data.cloud_init' -r | base64 -d > /etc/cloud/cloud.cfg.d/99-provisioning-config.cfg
-    rm /etc/cloud/cloud.cfg.d/{{ .SecretName }}
-    cloud-init clean
-    cloud-init --file /etc/cloud/cloud.cfg.d/99-provisioning-config.cfg init
-    systemctl daemon-reload
-    systemctl start setup.service 
-
-- path: /etc/systemd/system/bootstrap.service
-  permissions: "0644"
-  content: |
-    [Install]
-    WantedBy=multi-user.target
-    [Unit]
-    Requires=network-online.target
-    After=network-online.target
-    [Service]
-    Type=oneshot
-    RemainAfterExit=true
-    ExecStart=/opt/bin/bootstrap
-runcmd:
-- systemctl restart bootstrap.service`
 )
 
-func ExtractCloudInitSettingsToken(ctx context.Context, client ctrlruntimeclient.Client) (string, error) {
+func ExtractTokenAndAPIServer(ctx context.Context, userdata string, client ctrlruntimeclient.Client) (token string, apiServer string, err error) {
 	secretList := corev1.SecretList{}
 	if err := client.List(ctx, &secretList, &ctrlruntimeclient.ListOptions{Namespace: CloudInitNamespace}); err != nil {
-		return "", fmt.Errorf("failed to list secrets in namespace %s: %v", CloudInitNamespace, err)
+		return "", "", fmt.Errorf("failed to list secrets in namespace %s: %v", CloudInitNamespace, err)
+	}
+
+	apiServer, err = extractAPIServer(userdata)
+	if err != nil {
+		return "", "", fmt.Errorf("failed to extract api server address: %v", err)
 	}
 
 	for _, secret := range secretList.Items {
@@ -88,42 +52,14 @@ func ExtractCloudInitSettingsToken(ctx context.Context, client ctrlruntimeclient
 			if secret.Data != nil {
 				jwtToken := secret.Data["token"]
 				if jwtToken != nil {
-					return string(jwtToken), nil
+					token = string(jwtToken)
+					return token, apiServer, nil
 				}
 			}
 		}
 	}
 
-	return "", errors.New("failed to find cloud-init secret")
-}
-
-func GenerateCloudInitGetterScript(token, secretName, userdata string) (string, error) {
-	tmpl, err := template.New("bootstrap").Parse(bootstrapCloudConfig)
-	if err != nil {
-		return "", fmt.Errorf("failed to parse template: %v", err)
-	}
-
-	clusterHost, err := extractAPIServer(userdata)
-	if err != nil {
-		return "", fmt.Errorf("failed to extract api-server url: %v", err)
-	}
-
-	var buf bytes.Buffer
-	if err := tmpl.Execute(&buf, &struct {
-		Token                      string
-		SecretName                 string
-		ClusterHost                string
-		CloudInitSettingsNamespace string
-	}{
-		Token:                      token,
-		SecretName:                 secretName,
-		ClusterHost:                clusterHost,
-		CloudInitSettingsNamespace: CloudInitNamespace,
-	}); err != nil {
-		return "", fmt.Errorf("failed to execute bootstrap template: %v", err)
-	}
-
-	return buf.String(), nil
+	return "", "", errors.New("failed to find cloud-init secret")
 }
 
 func CreateMachineCloudInitSecret(ctx context.Context, userdata, machineName string, client ctrlruntimeclient.Client) error {


Test patch:
diff --git a/pkg/cloudprovider/util/cloud_init_settings_test.go b/pkg/cloudprovider/util/cloud_init_settings_test.go
index e09b8d1e..e1cf26a0 100644
--- a/pkg/cloudprovider/util/cloud_init_settings_test.go
+++ b/pkg/cloudprovider/util/cloud_init_settings_test.go
@@ -31,7 +31,7 @@ var testData = []struct {
 	userdata          string
 	secret            *corev1.Secret
 	expectedToken     string
-	expectedCloudInit string
+	expectedAPIServer string
 }{
 	{
 		name:     "bootstrap_cloud_init_generating",
@@ -46,7 +46,7 @@ var testData = []struct {
 			},
 		},
 		expectedToken:     "eyTestToken",
-		expectedCloudInit: "./testdata/expected_userdata.yaml",
+		expectedAPIServer: "https://88.99.224.97:6443",
 	},
 }
 
@@ -55,32 +55,20 @@ func TestCloudInitGeneration(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			fakeClient := fake.NewFakeClient(test.secret)
 
-			token, err := ExtractCloudInitSettingsToken(context.Background(), fakeClient)
-			if err != nil {
-				t.Fatalf("failed to extarct token: %v", err)
-			}
-			if token != test.expectedToken {
-				t.Fatalf("unexpected cloud-init token: wants %s got %s", test.expectedToken, token)
-			}
-
 			userdata, err := ioutil.ReadFile(test.userdata)
 			if err != nil {
 				t.Fatalf("failed to read userdata testing file: %v", err)
 			}
 
-			cloudInit, err := GenerateCloudInitGetterScript(token, test.secret.Name, string(userdata))
+			token, apiServer, err := ExtractTokenAndAPIServer(context.Background(), string(userdata), fakeClient)
 			if err != nil {
-				t.Fatalf("failed to generate bootstrap cloud-init: %v", err)
+				t.Fatalf("failed to extarct token: %v", err)
 			}
-
-			expectedCloudInit, err := ioutil.ReadFile(test.expectedCloudInit)
-			if err != nil {
-				t.Fatalf("failed to read expected cloud-init testing file: %v", err)
+			if token != test.expectedToken {
+				t.Fatalf("unexpected cloud-init token: wants %s got %s", test.expectedToken, token)
 			}
-
-			if cloudInit == string(expectedCloudInit) {
-				t.Fatalf("unexpected cloud-init: wants %s got %s", test.expectedCloudInit, cloudInit)
-
+			if apiServer != test.expectedAPIServer {
+				t.Fatalf("unexpected cloud-init api-server: wants %s got %s", test.expectedAPIServer, apiServer)
 			}
 		})
 	}
diff --git a/pkg/cloudprovider/util/testdata/expected_userdata.yaml b/pkg/cloudprovider/util/testdata/expected_userdata.yaml
deleted file mode 100644
index c5503d6c..00000000
--- a/pkg/cloudprovider/util/testdata/expected_userdata.yaml
+++ /dev/null
@@ -1,36 +0,0 @@
-#cloud-config
-
-write_files:
-  - path: "/opt/bin/bootstrap"
-    permissions: "0755"
-    content: |
-      #!/bin/bash
-      set -xeuo pipefail
-      apt-get update
-      apt-get install jq -y
-      wget --no-check-certificate --quiet \
-        --directory-prefix /etc/cloud/cloud.cfg.d/ \
-        --method GET \
-        --timeout=60 \
-        --header 'Authorization: Bearer eyTestToken' \
-        'https://88.99.224.97:6443/api/v1/namespaces/cloud-init-settings/secrets/cloud-init-getter-token'
-      cat /etc/cloud/cloud.cfg.d/cloud-init-getter-token | jq '.data.cloud_init' -r | base64 -d > /etc/cloud/cloud.cfg.d/99-provisioning-config.cfg
-      cloud-init clean
-      cloud-init --file /etc/cloud/cloud.cfg.d/99-provisioning-config.cfg init
-      systemctl daemon-reload
-      systemctl start setup.service
-
-  - path: /etc/systemd/system/bootstrap.service
-    permissions: "0644"
-    content: |
-      [Install]
-      WantedBy=multi-user.target
-      [Unit]
-      Requires=network-online.target
-      After=network-online.target
-      [Service]
-      Type=oneshot
-      RemainAfterExit=true
-      ExecStart=/opt/bin/bootstrap
-runcmd:
-  - systemctl restart bootstrap.service