Add Option to control the injection of provider-id to metadata (#347)

mcbenjemaa · web-flow · commit 3f6355d0fdab · 2024-12-05T12:52:24.000+01:00
* Add Option to control the injection of provider-id to metadata

* typo and more comment in the field
diff --git a/api/v1alpha1/proxmoxmachine_types.go b/api/v1alpha1/proxmoxmachine_types.go
@@ -106,7 +106,13 @@ type ProxmoxMachineSpec struct {
 	// +kubebuilder:validation:XValidation:rule="self.end >= self.start",message="end should be greater than or equal to start"
 	VMIDRange *VMIDRange `json:"vmIDRange,omitempty"`
 
+	// Checks defines possibles checks to skip.
+	// +optional
 	Checks *ProxmoxMachineChecks `json:"checks,omitempty"`
+
+	// MetadataSettings defines the metadata settings for this machine's VM.
+	// +optional
+	MetadataSettings *MetadataSettings `json:"metadataSettings,omitempty"`
 }
 
 // Storage is the physical storage on the node.
@@ -497,6 +503,14 @@ type VMIDRange struct {
 	End int64 `json:"end"`
 }
 
+// MetadataSettings defines the metadata settings for the machine.
+type MetadataSettings struct {
+	// ProviderIDInjection enables the injection of the `providerID` into the cloudinit metadata.
+	// this will basically set the `provider-id` field in the metadata to `proxmox://<instanceID>`.
+	// +optional
+	ProviderIDInjection bool `json:"providerIDInjection,omitempty"`
+}
+
 // +kubebuilder:object:root=true
 // +kubebuilder:storageversion
 // +kubebuilder:subresource:status
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml
@@ -73,8 +73,7 @@ spec:
                         a ProxmoxMachine.
                       properties:
                         checks:
-                          description: ProxmoxMachineChecks defines possibibles checks
-                            to skip.
+                          description: Checks defines possibles checks to skip.
                           properties:
                             skipCloudInitStatus:
                               description: Skip checking CloudInit which can be very
@@ -147,6 +146,16 @@ spec:
                           format: int32
                           multipleOf: 8
                           type: integer
+                        metadataSettings:
+                          description: MetadataSettings defines the metadata settings
+                            for this machine's VM.
+                          properties:
+                            providerIDInjection:
+                              description: |-
+                                ProviderIDInjection enables the injection of the `providerID` into the cloudinit metadata.
+                                this will basically set the `provider-id` field in the metadata to `proxmox://<instanceID>`.
+                              type: boolean
+                          type: object
                         network:
                           description: Network is the network configuration for this
                             machine's VM.
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml
@@ -95,8 +95,8 @@ spec:
                                 state of a ProxmoxMachine.
                               properties:
                                 checks:
-                                  description: ProxmoxMachineChecks defines possibibles
-                                    checks to skip.
+                                  description: Checks defines possibles checks to
+                                    skip.
                                   properties:
                                     skipCloudInitStatus:
                                       description: Skip checking CloudInit which can
@@ -170,6 +170,16 @@ spec:
                                   format: int32
                                   multipleOf: 8
                                   type: integer
+                                metadataSettings:
+                                  description: MetadataSettings defines the metadata
+                                    settings for this machine's VM.
+                                  properties:
+                                    providerIDInjection:
+                                      description: |-
+                                        ProviderIDInjection enables the injection of the `providerID` into the cloudinit metadata.
+                                        this will basically set the `provider-id` field in the metadata to `proxmox://<instanceID>`.
+                                      type: boolean
+                                  type: object
                                 network:
                                   description: Network is the network configuration
                                     for this machine's VM.
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml
@@ -66,7 +66,7 @@ spec:
             description: ProxmoxMachineSpec defines the desired state of a ProxmoxMachine.
             properties:
               checks:
-                description: ProxmoxMachineChecks defines possibibles checks to skip.
+                description: Checks defines possibles checks to skip.
                 properties:
                   skipCloudInitStatus:
                     description: Skip checking CloudInit which can be very useful
@@ -137,6 +137,16 @@ spec:
                 format: int32
                 multipleOf: 8
                 type: integer
+              metadataSettings:
+                description: MetadataSettings defines the metadata settings for this
+                  machine's VM.
+                properties:
+                  providerIDInjection:
+                    description: |-
+                      ProviderIDInjection enables the injection of the `providerID` into the cloudinit metadata.
+                      this will basically set the `provider-id` field in the metadata to `proxmox://<instanceID>`.
+                    type: boolean
+                type: object
               network:
                 description: Network is the network configuration for this machine's
                   VM.
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml
@@ -77,8 +77,7 @@ spec:
                       ProxmoxMachine.
                     properties:
                       checks:
-                        description: ProxmoxMachineChecks defines possibibles checks
-                          to skip.
+                        description: Checks defines possibles checks to skip.
                         properties:
                           skipCloudInitStatus:
                             description: Skip checking CloudInit which can be very
@@ -151,6 +150,16 @@ spec:
                         format: int32
                         multipleOf: 8
                         type: integer
+                      metadataSettings:
+                        description: MetadataSettings defines the metadata settings
+                          for this machine's VM.
+                        properties:
+                          providerIDInjection:
+                            description: |-
+                              ProviderIDInjection enables the injection of the `providerID` into the cloudinit metadata.
+                              this will basically set the `provider-id` field in the metadata to `proxmox://<instanceID>`.
+                            type: boolean
+                        type: object
                       network:
                         description: Network is the network configuration for this
                           machine's VM.
diff --git a/internal/inject/inject_test.go b/internal/inject/inject_test.go
@@ -91,7 +91,7 @@ func TestISOInjectorInjectCloudInit(t *testing.T) {
 	injector := &ISOInjector{
 		VirtualMachine: vm,
 		BootstrapData:  []byte(""),
-		MetaRenderer:   cloudinit.NewMetadata("xxx-xxxx", "my-custom-vm"),
+		MetaRenderer:   cloudinit.NewMetadata("xxx-xxxx", "my-custom-vm", true),
 		NetworkRenderer: cloudinit.NewNetworkConfig([]cloudinit.NetworkConfigData{
 			{
 				Name:       "eth0",
@@ -135,7 +135,7 @@ func TestISOInjectorInjectCloudInit_Errors(t *testing.T) {
 	injector := &ISOInjector{
 		VirtualMachine: vm,
 		BootstrapData:  []byte(""),
-		MetaRenderer:   cloudinit.NewMetadata("xxx-xxxx", ""),
+		MetaRenderer:   cloudinit.NewMetadata("xxx-xxxx", "", true),
 		NetworkRenderer: cloudinit.NewNetworkConfig([]cloudinit.NetworkConfigData{
 			{
 				Name:       "eth0",
@@ -151,7 +151,7 @@ func TestISOInjectorInjectCloudInit_Errors(t *testing.T) {
 	require.Error(t, err)
 
 	// missing network
-	injector.MetaRenderer = cloudinit.NewMetadata("xxx-xxxx", "my-custom-vm")
+	injector.MetaRenderer = cloudinit.NewMetadata("xxx-xxxx", "my-custom-vm", false)
 	injector.NetworkRenderer = cloudinit.NewNetworkConfig(nil)
 	err = injector.Inject(context.Background(), "cloudinit")
 	require.Error(t, err)
@@ -200,7 +200,7 @@ func TestISOInjectorInjectIgnition(t *testing.T) {
 	injector := &ISOInjector{
 		VirtualMachine:   vm,
 		BootstrapData:    []byte(bootstrapData),
-		MetaRenderer:     cloudinit.NewMetadata("xxx-xxxx", "my-custom-vm"),
+		MetaRenderer:     cloudinit.NewMetadata("xxx-xxxx", "my-custom-vm", false),
 		IgnitionEnricher: enricher,
 	}
 
@@ -260,14 +260,14 @@ func TestISOInjectorInjectIgnition_Errors(t *testing.T) {
 	require.Error(t, err)
 
 	// missing hostname
-	injector.MetaRenderer = cloudinit.NewMetadata("xxxx-xxxxx", "")
+	injector.MetaRenderer = cloudinit.NewMetadata("xxxx-xxxxx", "", false)
 	e.BootstrapData = []byte(bootstrapData)
 	err = injector.Inject(context.Background(), "ignition")
 	require.Error(t, err)
 
 	// no bootstrapdata
 	e.BootstrapData = nil
-	injector.MetaRenderer = cloudinit.NewMetadata("xxxx-xxxxx", "my-custom-vm")
+	injector.MetaRenderer = cloudinit.NewMetadata("xxxx-xxxxx", "my-custom-vm", true)
 	injector.BootstrapData = []byte("invalid")
 	err = injector.Inject(context.Background(), "ignition")
 	require.Error(t, err)
@@ -292,7 +292,7 @@ func TestISOInjectorInject_Unsupported(t *testing.T) {
 	injector := &ISOInjector{
 		VirtualMachine: vm,
 		BootstrapData:  []byte(""),
-		MetaRenderer:   cloudinit.NewMetadata("xxx-xxxx", ""),
+		MetaRenderer:   cloudinit.NewMetadata("xxx-xxxx", "", false),
 		NetworkRenderer: cloudinit.NewNetworkConfig([]cloudinit.NetworkConfigData{
 			{
 				Name:       "eth0",
diff --git a/internal/service/vmservice/bootstrap.go b/internal/service/vmservice/bootstrap.go
@@ -91,7 +91,7 @@ func injectCloudInit(ctx context.Context, machineScope *scope.MachineScope, boot
 	network := cloudinit.NewNetworkConfig(nicData)
 
 	// create metadata renderer
-	metadata := cloudinit.NewMetadata(biosUUID, machineScope.Name())
+	metadata := cloudinit.NewMetadata(biosUUID, machineScope.Name(), ptr.Deref(machineScope.ProxmoxMachine.Spec.MetadataSettings, infrav1alpha1.MetadataSettings{ProviderIDInjection: false}).ProviderIDInjection)
 
 	injector := getISOInjector(machineScope.VirtualMachine, bootstrapData, metadata, network)
 	if err := injector.Inject(ctx, inject.CloudConfigFormat); err != nil {
@@ -103,7 +103,7 @@ func injectCloudInit(ctx context.Context, machineScope *scope.MachineScope, boot
 
 func injectIgnition(ctx context.Context, machineScope *scope.MachineScope, bootstrapData []byte, biosUUID string, nicData []cloudinit.NetworkConfigData) error {
 	// create metadata renderer
-	metadata := cloudinit.NewMetadata(biosUUID, machineScope.Name())
+	metadata := cloudinit.NewMetadata(biosUUID, machineScope.Name(), ptr.Deref(machineScope.ProxmoxMachine.Spec.MetadataSettings, infrav1alpha1.MetadataSettings{ProviderIDInjection: false}).ProviderIDInjection)
 
 	// create an enricher
 	enricher := &ignition.Enricher{
diff --git a/internal/service/vmservice/bootstrap_test.go b/internal/service/vmservice/bootstrap_test.go
@@ -492,14 +492,14 @@ func TestReconcileBootstrapDataMissingNetworkConfig(t *testing.T) {
 }
 
 func TestDefaultISOInjector(t *testing.T) {
-	injector := defaultISOInjector(newRunningVM(), []byte("data"), cloudinit.NewMetadata(biosUUID, "test"), cloudinit.NewNetworkConfig(nil))
+	injector := defaultISOInjector(newRunningVM(), []byte("data"), cloudinit.NewMetadata(biosUUID, "test", true), cloudinit.NewNetworkConfig(nil))
 
 	require.NotEmpty(t, injector)
 	require.Equal(t, []byte("data"), injector.(*inject.ISOInjector).BootstrapData)
 }
 
 func TestIgnitionISOInjector(t *testing.T) {
-	injector := ignitionISOInjector(newRunningVM(), cloudinit.NewMetadata(biosUUID, "test"), &ignition.Enricher{
+	injector := ignitionISOInjector(newRunningVM(), cloudinit.NewMetadata(biosUUID, "test", true), &ignition.Enricher{
 		BootstrapData: []byte("data"),
 		Hostname:      "test",
 	})
diff --git a/pkg/cloudinit/metadata.go b/pkg/cloudinit/metadata.go
@@ -20,7 +20,9 @@ const (
 	metadataTPl = `instance-id: {{ .InstanceID }}
 local-hostname: {{ .Hostname }}
 hostname: {{ .Hostname }}
+{{- if .ProviderIDInjection }}
 provider-id: proxmox://{{ .InstanceID }}
+{{- end }}
 `
 )
 
@@ -30,11 +32,12 @@ type Metadata struct {
 }
 
 // NewMetadata returns a new Metadata object.
-func NewMetadata(instanceID, hostname string) *Metadata {
+func NewMetadata(instanceID, hostname string, injectProviderID bool) *Metadata {
 	ci := new(Metadata)
 	ci.data = BaseCloudInitData{
-		Hostname:   hostname,
-		InstanceID: instanceID,
+		Hostname:            hostname,
+		InstanceID:          instanceID,
+		ProviderIDInjection: injectProviderID,
 	}
 	return ci
 }
diff --git a/pkg/cloudinit/metadata_test.go b/pkg/cloudinit/metadata_test.go
@@ -27,13 +27,18 @@ const (
 local-hostname: proxmox-control-plane
 hostname: proxmox-control-plane
 provider-id: proxmox://9a82e2ca-4294-11ee-be56-0242ac120002
+`
+	expectedValidMetadataWithoutProviderID = `instance-id: 9a82e2ca-4294-11ee-be56-0242ac120002
+local-hostname: proxmox-control-plane
+hostname: proxmox-control-plane
 `
 )
 
 func TestMetadata_Render(t *testing.T) {
 	type args struct {
-		instanceID string
-		hostname   string
+		instanceID       string
+		hostname         string
+		injectProviderID bool
 	}
 
 	type want struct {
@@ -47,10 +52,11 @@ func TestMetadata_Render(t *testing.T) {
 		want   want
 	}{
 		"ValidCloudinit": {
-			reason: "rendering metadata, network-config",
+			reason: "rendering metadata",
 			args: args{
-				instanceID: "9a82e2ca-4294-11ee-be56-0242ac120002",
-				hostname:   "proxmox-control-plane",
+				instanceID:       "9a82e2ca-4294-11ee-be56-0242ac120002",
+				hostname:         "proxmox-control-plane",
+				injectProviderID: true,
 			},
 			want: want{
 				metadata: expectedValidMetadata,
@@ -75,11 +81,23 @@ func TestMetadata_Render(t *testing.T) {
 				err:      ErrMissingHostname,
 			},
 		},
+		"ValidCloudinitwithoutProviderID": {
+			reason: "rendering metadata if providerID is not injected",
+			args: args{
+				instanceID:       "9a82e2ca-4294-11ee-be56-0242ac120002",
+				hostname:         "proxmox-control-plane",
+				injectProviderID: false,
+			},
+			want: want{
+				metadata: expectedValidMetadataWithoutProviderID,
+				err:      nil,
+			},
+		},
 	}
 
 	for n, tc := range cases {
 		t.Run(n, func(t *testing.T) {
-			ci := NewMetadata(tc.args.instanceID, tc.args.hostname)
+			ci := NewMetadata(tc.args.instanceID, tc.args.hostname, tc.args.injectProviderID)
 			metadata, err := ci.Render()
 			require.ErrorIs(t, err, tc.want.err)
 			require.Equal(t, tc.want.metadata, string(metadata))
diff --git a/pkg/cloudinit/types.go b/pkg/cloudinit/types.go
@@ -24,9 +24,10 @@ const (
 // BaseCloudInitData is shared across all the various types of files written to disk.
 // used to render cloudinit.
 type BaseCloudInitData struct {
-	Hostname          string
-	InstanceID        string
-	NetworkConfigData []NetworkConfigData
+	Hostname            string
+	InstanceID          string
+	ProviderIDInjection bool
+	NetworkConfigData   []NetworkConfigData
 }
 
 // NetworkConfigData is used to render network-config.

Original file line number	Diff line number	Diff line change
`@@ -492,14 +492,14 @@ func TestReconcileBootstrapDataMissingNetworkConfig(t *testing.T) {`
`492`	`492`	`}`
`493`	`493`
`494`	`494`	`func TestDefaultISOInjector(t *testing.T) {`
`495`		`- injector := defaultISOInjector(newRunningVM(), []byte("data"), cloudinit.NewMetadata(biosUUID, "test"), cloudinit.NewNetworkConfig(nil))`
	`495`	`+ injector := defaultISOInjector(newRunningVM(), []byte("data"), cloudinit.NewMetadata(biosUUID, "test", true), cloudinit.NewNetworkConfig(nil))`
`496`	`496`
`497`	`497`	`require.NotEmpty(t, injector)`
`498`	`498`	`require.Equal(t, []byte("data"), injector.(*inject.ISOInjector).BootstrapData)`
`499`	`499`	`}`
`500`	`500`
`501`	`501`	`func TestIgnitionISOInjector(t *testing.T) {`
`502`		`- injector := ignitionISOInjector(newRunningVM(), cloudinit.NewMetadata(biosUUID, "test"), &ignition.Enricher{`
	`502`	`+ injector := ignitionISOInjector(newRunningVM(), cloudinit.NewMetadata(biosUUID, "test", true), &ignition.Enricher{`
`503`	`503`	`BootstrapData: []byte("data"),`
`504`	`504`	`Hostname: "test",`
`505`	`505`	`})`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,9 @@ const (`
`20`	`20`	metadataTPl = `instance-id: {{ .InstanceID }}
`21`	`21`	`local-hostname: {{ .Hostname }}`
`22`	`22`	`hostname: {{ .Hostname }}`
	`23`	`+{{- if .ProviderIDInjection }}`
`23`	`24`	`provider-id: proxmox://{{ .InstanceID }}`
	`25`	`+{{- end }}`
`24`	`26`	`
`25`	`27`	`)`
`26`	`28`
`@@ -30,11 +32,12 @@ type Metadata struct {`
`30`	`32`	`}`
`31`	`33`
`32`	`34`	`// NewMetadata returns a new Metadata object.`
`33`		`-func NewMetadata(instanceID, hostname string) *Metadata {`
	`35`	`+func NewMetadata(instanceID, hostname string, injectProviderID bool) *Metadata {`
`34`	`36`	`ci := new(Metadata)`
`35`	`37`	`ci.data = BaseCloudInitData{`
`36`		`- Hostname: hostname,`
`37`		`- InstanceID: instanceID,`
	`38`	`+ Hostname: hostname,`
	`39`	`+ InstanceID: instanceID,`
	`40`	`+ ProviderIDInjection: injectProviderID,`
`38`	`41`	`}`
`39`	`42`	`return ci`
`40`	`43`	`}`