diff --git a/docs/usage/usage.md b/docs/usage/usage.md
index bc5564870..bc35217a6 100644
--- a/docs/usage/usage.md
+++ b/docs/usage/usage.md
@@ -155,6 +155,7 @@ The worker configuration contains:
If you attach the disk with `SCRATCH` type, either an `NVMe` interface or a `SCSI` interface must be specified.
It is only meaningful to provide this volume interface if only `SCRATCH` data volumes are used.
+
* Volume Encryption config that specifies values for `kmsKeyName` and `kmsKeyServiceAccountName`.
* The `kmsKeyName` is the
key name of the cloud kms disk encryption key and must be specified if CMEK disk encryption is needed.
@@ -166,6 +167,12 @@ The worker configuration contains:
gcloud projects add-iam-policy-binding projectId --member
serviceAccount:name@projectIdgserviceaccount.com --role roles/cloudkms.cryptoKeyEncrypterDecrypter
```
+
+* Setting a volume image with `dataVolume.sourceImage`.
+ However, this parameter should only be used with particular caution.
+ For example Gardenlinux works with filesystem LABELs only and creating another disk form the very same image causes the LABELs to be duplicated.
+ See: https://github.com/gardener/gardener-extension-provider-gcp/issues/323
+
* Service Account with their specified scopes, authorized for this worker.
Service accounts created in advance that generate access tokens that can be accessed through the metadata server and used to authenticate applications on the instance.
diff --git a/example/30-worker.yaml b/example/30-worker.yaml
index e35a4c042..3923ffbf7 100644
--- a/example/30-worker.yaml
+++ b/example/30-worker.yaml
@@ -95,6 +95,8 @@ spec:
# encryption: # Encryption Details
# kmsKeyName: "projects/projectId/locations/ DataVolumes contains configuration for the additional disks attached to VMs.
+(Appears on:
+WorkerConfig)
+
+ DataVolume contains configuration for data volumes attached to VMs.
+
+
+dataVolumes
+
+
+[]DataVolume
+
+
+
+(Optional)
+
+
+
+
minCpuPlatform
string
@@ -487,6 +501,51 @@ string
DataVolume
+
+
Field | +Description | +
---|---|
+name
+
+string
+
+ |
+
+ Name is the name of the data volume this configuration applies to. + |
+
+sourceImage
+
+string
+
+ |
+
+ SourceImage is the image to create this disk +However, this parameter should only be used with particular caution. +For example GardenLinux works with filesystem LABELs only and creating +another disk form the very same image causes the LABELs to be duplicated. +See: https://github.com/gardener/gardener-extension-provider-gcp/issues/323 + |
+
diff --git a/pkg/apis/gcp/types_worker.go b/pkg/apis/gcp/types_worker.go index ab43096c5..18571012d 100644 --- a/pkg/apis/gcp/types_worker.go +++ b/pkg/apis/gcp/types_worker.go @@ -22,6 +22,10 @@ type WorkerConfig struct { // Volume contains configuration for the root disks attached to VMs. Volume *Volume + // DataVolumes contains configuration for the additional disks attached to VMs. + // +optional + DataVolumes []DataVolume + // MinCpuPlatform is the name of the minimum CPU platform that is to be // requested for the VM. MinCpuPlatform *string @@ -46,6 +50,19 @@ type Volume struct { Encryption *DiskEncryption } +// DataVolume contains configuration for data volumes attached to VMs. +type DataVolume struct { + // Name is the name of the data volume this configuration applies to. + Name string + + // SourceImage is the image to create this disk + // However, this parameter should only be used with particular caution. + // For example GardenLinux works with filesystem LABELs only and creating + // another disk form the very same image causes the LABELs to be duplicated. + // See: https://github.com/gardener/gardener-extension-provider-gcp/issues/323 + SourceImage *string +} + // DiskEncryption encapsulates the encryption configuration for a disk. type DiskEncryption struct { // KmsKeyName specifies the customer-managed encryption key (CMEK) used for encryption of the volume. diff --git a/pkg/apis/gcp/v1alpha1/types_worker.go b/pkg/apis/gcp/v1alpha1/types_worker.go index c6b1b295d..caa20411c 100644 --- a/pkg/apis/gcp/v1alpha1/types_worker.go +++ b/pkg/apis/gcp/v1alpha1/types_worker.go @@ -24,6 +24,10 @@ type WorkerConfig struct { // +optional Volume *Volume `json:"volume,omitempty"` + // DataVolumes contains configuration for the additional disks attached to VMs. + // +optional + DataVolumes []DataVolume `json:"dataVolumes,omitempty"` + // MinCpuPlatform is the name of the minimum CPU platform that is to be // requested for the VM. MinCpuPlatform *string `json:"minCpuPlatform,omitempty"` @@ -52,6 +56,19 @@ type Volume struct { Encryption *DiskEncryption `json:"encryption,omitempty"` } +// DataVolume contains configuration for data volumes attached to VMs. +type DataVolume struct { + // Name is the name of the data volume this configuration applies to. + Name string `json:"name"` + + // SourceImage is the image to create this disk + // However, this parameter should only be used with particular caution. + // For example GardenLinux works with filesystem LABELs only and creating + // another disk form the very same image causes the LABELs to be duplicated. + // See: https://github.com/gardener/gardener-extension-provider-gcp/issues/323 + SourceImage *string `json:"sourceImage,omitempty"` +} + // DiskEncryption encapsulates the encryption configuration for a disk. type DiskEncryption struct { // KmsKeyName specifies the customer-managed encryption key (CMEK) used for encryption of the volume. diff --git a/pkg/apis/gcp/v1alpha1/zz_generated.conversion.go b/pkg/apis/gcp/v1alpha1/zz_generated.conversion.go index c38b925cd..2a385f32b 100644 --- a/pkg/apis/gcp/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/gcp/v1alpha1/zz_generated.conversion.go @@ -75,6 +75,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddGeneratedConversionFunc((*DataVolume)(nil), (*gcp.DataVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_DataVolume_To_gcp_DataVolume(a.(*DataVolume), b.(*gcp.DataVolume), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*gcp.DataVolume)(nil), (*DataVolume)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_gcp_DataVolume_To_v1alpha1_DataVolume(a.(*gcp.DataVolume), b.(*DataVolume), scope) + }); err != nil { + return err + } if err := s.AddGeneratedConversionFunc((*DiskEncryption)(nil), (*gcp.DiskEncryption)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha1_DiskEncryption_To_gcp_DiskEncryption(a.(*DiskEncryption), b.(*gcp.DiskEncryption), scope) }); err != nil { @@ -410,6 +420,28 @@ func Convert_gcp_ControlPlaneConfig_To_v1alpha1_ControlPlaneConfig(in *gcp.Contr return autoConvert_gcp_ControlPlaneConfig_To_v1alpha1_ControlPlaneConfig(in, out, s) } +func autoConvert_v1alpha1_DataVolume_To_gcp_DataVolume(in *DataVolume, out *gcp.DataVolume, s conversion.Scope) error { + out.Name = in.Name + out.SourceImage = (*string)(unsafe.Pointer(in.SourceImage)) + return nil +} + +// Convert_v1alpha1_DataVolume_To_gcp_DataVolume is an autogenerated conversion function. +func Convert_v1alpha1_DataVolume_To_gcp_DataVolume(in *DataVolume, out *gcp.DataVolume, s conversion.Scope) error { + return autoConvert_v1alpha1_DataVolume_To_gcp_DataVolume(in, out, s) +} + +func autoConvert_gcp_DataVolume_To_v1alpha1_DataVolume(in *gcp.DataVolume, out *DataVolume, s conversion.Scope) error { + out.Name = in.Name + out.SourceImage = (*string)(unsafe.Pointer(in.SourceImage)) + return nil +} + +// Convert_gcp_DataVolume_To_v1alpha1_DataVolume is an autogenerated conversion function. +func Convert_gcp_DataVolume_To_v1alpha1_DataVolume(in *gcp.DataVolume, out *DataVolume, s conversion.Scope) error { + return autoConvert_gcp_DataVolume_To_v1alpha1_DataVolume(in, out, s) +} + func autoConvert_v1alpha1_DiskEncryption_To_gcp_DiskEncryption(in *DiskEncryption, out *gcp.DiskEncryption, s conversion.Scope) error { out.KmsKeyName = (*string)(unsafe.Pointer(in.KmsKeyName)) out.KmsKeyServiceAccount = (*string)(unsafe.Pointer(in.KmsKeyServiceAccount)) @@ -879,6 +911,7 @@ func Convert_gcp_Volume_To_v1alpha1_Volume(in *gcp.Volume, out *Volume, s conver func autoConvert_v1alpha1_WorkerConfig_To_gcp_WorkerConfig(in *WorkerConfig, out *gcp.WorkerConfig, s conversion.Scope) error { out.GPU = (*gcp.GPU)(unsafe.Pointer(in.GPU)) out.Volume = (*gcp.Volume)(unsafe.Pointer(in.Volume)) + out.DataVolumes = *(*[]gcp.DataVolume)(unsafe.Pointer(&in.DataVolumes)) out.MinCpuPlatform = (*string)(unsafe.Pointer(in.MinCpuPlatform)) out.ServiceAccount = (*gcp.ServiceAccount)(unsafe.Pointer(in.ServiceAccount)) out.NodeTemplate = (*extensionsv1alpha1.NodeTemplate)(unsafe.Pointer(in.NodeTemplate)) @@ -893,6 +926,7 @@ func Convert_v1alpha1_WorkerConfig_To_gcp_WorkerConfig(in *WorkerConfig, out *gc func autoConvert_gcp_WorkerConfig_To_v1alpha1_WorkerConfig(in *gcp.WorkerConfig, out *WorkerConfig, s conversion.Scope) error { out.GPU = (*GPU)(unsafe.Pointer(in.GPU)) out.Volume = (*Volume)(unsafe.Pointer(in.Volume)) + out.DataVolumes = *(*[]DataVolume)(unsafe.Pointer(&in.DataVolumes)) out.MinCpuPlatform = (*string)(unsafe.Pointer(in.MinCpuPlatform)) out.ServiceAccount = (*ServiceAccount)(unsafe.Pointer(in.ServiceAccount)) out.NodeTemplate = (*extensionsv1alpha1.NodeTemplate)(unsafe.Pointer(in.NodeTemplate)) diff --git a/pkg/apis/gcp/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/gcp/v1alpha1/zz_generated.deepcopy.go index 3ed9cf393..257e5a67d 100644 --- a/pkg/apis/gcp/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/gcp/v1alpha1/zz_generated.deepcopy.go @@ -181,6 +181,27 @@ func (in *ControlPlaneConfig) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DataVolume) DeepCopyInto(out *DataVolume) { + *out = *in + if in.SourceImage != nil { + in, out := &in.SourceImage, &out.SourceImage + *out = new(string) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataVolume. +func (in *DataVolume) DeepCopy() *DataVolume { + if in == nil { + return nil + } + out := new(DataVolume) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DiskEncryption) DeepCopyInto(out *DiskEncryption) { *out = *in @@ -638,6 +659,13 @@ func (in *WorkerConfig) DeepCopyInto(out *WorkerConfig) { *out = new(Volume) (*in).DeepCopyInto(*out) } + if in.DataVolumes != nil { + in, out := &in.DataVolumes, &out.DataVolumes + *out = make([]DataVolume, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.MinCpuPlatform != nil { in, out := &in.MinCpuPlatform, &out.MinCpuPlatform *out = new(string) diff --git a/pkg/apis/gcp/validation/worker.go b/pkg/apis/gcp/validation/worker.go index 1b38b1762..2a9e774e9 100644 --- a/pkg/apis/gcp/validation/worker.go +++ b/pkg/apis/gcp/validation/worker.go @@ -10,6 +10,7 @@ import ( "github.com/gardener/gardener/pkg/apis/core" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + "golang.org/x/exp/slices" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" @@ -22,8 +23,9 @@ import ( var ( validVolumeLocalSSDInterfacesTypes = sets.New("NVME", "SCSI") - providerFldPath = field.NewPath("providerConfig") - volumeFldPath = providerFldPath.Child("volume") + providerFldPath = field.NewPath("providerConfig") + volumeFldPath = providerFldPath.Child("volume") + dataVolumeFldPath = providerFldPath.Child("dataVolume") ) // ValidateWorkerConfig validates a WorkerConfig object. @@ -42,6 +44,9 @@ func ValidateWorkerConfig(workerConfig *gcp.WorkerConfig, dataVolumes []core.Dat allErrs = append(allErrs, validateDiskEncryption(workerConfig.Volume.Encryption, volumeFldPath.Child("encryption"))...) } allErrs = append(allErrs, validateNodeTemplate(workerConfig.NodeTemplate, providerFldPath.Child("nodeTemplate"))...) + if workerConfig.DataVolumes != nil { + allErrs = append(allErrs, validateDataVolumeNames(workerConfig.DataVolumes, dataVolumes)...) + } } return allErrs @@ -120,23 +125,52 @@ func validateDataVolume(workerConfig *gcp.WorkerConfig, volume core.DataVolume, allErrs = append(allErrs, field.Required(fldPath.Child("type"), "must not be empty")) return allErrs } - if *volume.Type == worker.VolumeTypeScratch { + + allErrs = append(allErrs, validateScratchDisk(*volume.Type, workerConfig)...) + + return allErrs +} + +func validateScratchDisk(volumeType string, workerConfig *gcp.WorkerConfig) field.ErrorList { + allErrs := field.ErrorList{} + + interfacePath := volumeFldPath.Child("interface") + encryptionPath := volumeFldPath.Child("encryption") + + if volumeType == worker.VolumeTypeScratch { if workerConfig == nil || workerConfig.Volume == nil || workerConfig.Volume.LocalSSDInterface == nil { - allErrs = append(allErrs, field.Required(volumeFldPath.Child("interface"), fmt.Sprintf("must be set when using %s volumes", worker.VolumeTypeScratch))) + allErrs = append(allErrs, field.Required(interfacePath, fmt.Sprintf("must be set when using %s volumes", worker.VolumeTypeScratch))) } else { if !validVolumeLocalSSDInterfacesTypes.Has(*workerConfig.Volume.LocalSSDInterface) { - allErrs = append(allErrs, field.NotSupported(volumeFldPath.Child("interface"), *workerConfig.Volume.LocalSSDInterface, validVolumeLocalSSDInterfacesTypes.UnsortedList())) + allErrs = append(allErrs, field.NotSupported(interfacePath, *workerConfig.Volume.LocalSSDInterface, validVolumeLocalSSDInterfacesTypes.UnsortedList())) } } // DiskEncryption not allowed for type SCRATCH if workerConfig != nil && workerConfig.Volume != nil && workerConfig.Volume.Encryption != nil { - allErrs = append(allErrs, field.Invalid(volumeFldPath.Child("encryption"), *workerConfig.Volume.Encryption, fmt.Sprintf("must not be set in combination with %s volumes", worker.VolumeTypeScratch))) + allErrs = append(allErrs, field.Invalid(encryptionPath, *workerConfig.Volume.Encryption, fmt.Sprintf("must not be set in combination with %s volumes", worker.VolumeTypeScratch))) } } else { // LocalSSDInterface only allowed for type SCRATCH if workerConfig != nil && workerConfig.Volume != nil && workerConfig.Volume.LocalSSDInterface != nil { - allErrs = append(allErrs, field.Invalid(volumeFldPath.Child("interface"), *workerConfig.Volume.LocalSSDInterface, fmt.Sprintf("is only allowed for type %s", worker.VolumeTypeScratch))) + allErrs = append(allErrs, field.Invalid(encryptionPath, *workerConfig.Volume.LocalSSDInterface, fmt.Sprintf("is only allowed for type %s", worker.VolumeTypeScratch))) + } + } + return allErrs +} + +func validateDataVolumeNames(workerConfigDataVolumes []gcp.DataVolume, dataVolumes []core.DataVolume) field.ErrorList { + allErrs := field.ErrorList{} + + for _, configDataVolume := range workerConfigDataVolumes { + if slices.ContainsFunc(dataVolumes, func(dataVolume core.DataVolume) bool { + return configDataVolume.Name == dataVolume.Name + }) { + continue } + allErrs = append(allErrs, field.Invalid( + dataVolumeFldPath, + configDataVolume.Name, + fmt.Sprintf("could not find dataVolume with name %s", configDataVolume.Name))) } return allErrs diff --git a/pkg/apis/gcp/validation/worker_test.go b/pkg/apis/gcp/validation/worker_test.go index d2686a785..9f9adc453 100644 --- a/pkg/apis/gcp/validation/worker_test.go +++ b/pkg/apis/gcp/validation/worker_test.go @@ -37,6 +37,7 @@ var _ = Describe("#ValidateWorkers", func() { }, DataVolumes: []core.DataVolume{ { + Name: "foo", Type: ptr.To("Volume"), VolumeSize: "30G", }, @@ -280,6 +281,29 @@ var _ = Describe("#ValidateWorkers", func() { Expect(errorList).To(BeEmpty()) }) + It("should allow valid dataVolume name", func() { + errorList := validateWorkerConfig([]core.Worker{workers[0]}, &gcp.WorkerConfig{ + DataVolumes: []gcp.DataVolume{{ + Name: "foo", + }}, + }) + Expect(errorList).To(BeEmpty()) + }) + + It("should forbid invalid dataVolume name", func() { + errorList := validateWorkerConfig([]core.Worker{workers[0]}, &gcp.WorkerConfig{ + DataVolumes: []gcp.DataVolume{{ + Name: "foo-invalid", + }}, + }) + Expect(errorList).To(ConsistOf( + PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("providerConfig.dataVolume"), + })), + )) + }) + Describe("#Volume type SCRATCH", func() { It("should pass because worker config is configured correctly", func() { workers[0].DataVolumes[0].Type = ptr.To(worker.VolumeTypeScratch) diff --git a/pkg/apis/gcp/zz_generated.deepcopy.go b/pkg/apis/gcp/zz_generated.deepcopy.go index 2bd4d2715..1ed03df3a 100644 --- a/pkg/apis/gcp/zz_generated.deepcopy.go +++ b/pkg/apis/gcp/zz_generated.deepcopy.go @@ -181,6 +181,27 @@ func (in *ControlPlaneConfig) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DataVolume) DeepCopyInto(out *DataVolume) { + *out = *in + if in.SourceImage != nil { + in, out := &in.SourceImage, &out.SourceImage + *out = new(string) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataVolume. +func (in *DataVolume) DeepCopy() *DataVolume { + if in == nil { + return nil + } + out := new(DataVolume) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DiskEncryption) DeepCopyInto(out *DiskEncryption) { *out = *in @@ -638,6 +659,13 @@ func (in *WorkerConfig) DeepCopyInto(out *WorkerConfig) { *out = new(Volume) (*in).DeepCopyInto(*out) } + if in.DataVolumes != nil { + in, out := &in.DataVolumes, &out.DataVolumes + *out = make([]DataVolume, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.MinCpuPlatform != nil { in, out := &in.MinCpuPlatform, &out.MinCpuPlatform *out = new(string) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index 670da761c..9313d409e 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -127,7 +127,7 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error { disks := make([]map[string]interface{}, 0) // root volume if pool.Volume != nil { - disk, err := createDiskSpec(pool.Volume.Size, true, &machineImage, pool.Volume.Type, workerConfig, poolLabels) + disk, err := createDiskSpecForVolume(pool.Volume, machineImage, workerConfig, poolLabels) if err != nil { return err } @@ -136,9 +136,7 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error { // additional volumes for _, volume := range pool.DataVolumes { - // Don't set machine image for data volumes. Any pre-existing data on the disk can interfere with the boot disk. - // See https://github.com/gardener/gardener-extension-provider-gcp/issues/323 - disk, err := createDiskSpec(volume.Size, false, nil, volume.Type, workerConfig, poolLabels) + disk, err := createDiskSpecForDataVolume(volume, workerConfig, poolLabels) if err != nil { return err } @@ -280,7 +278,18 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error { return nil } -func createDiskSpec(size string, boot bool, machineImage, volumeType *string, workerConfig *apisgcp.WorkerConfig, labels map[string]interface{}) (map[string]interface{}, error) { +func createDiskSpecForVolume(volume *v1alpha1.Volume, image string, workerConfig *apisgcp.WorkerConfig, labels map[string]interface{}) (map[string]interface{}, error) { + return createDiskSpec(volume.Size, true, &image, volume.Type, workerConfig, labels) +} + +func createDiskSpecForDataVolume(volume v1alpha1.DataVolume, workerConfig *apisgcp.WorkerConfig, labels map[string]interface{}) (map[string]interface{}, error) { + // Careful when setting machine image for data volumes. Any pre-existing data on the disk can interfere with the boot disk. + // See https://github.com/gardener/gardener-extension-provider-gcp/issues/323 + dataVolumeImage := getDataVolumeImage(volume.Name, workerConfig.DataVolumes) + return createDiskSpec(volume.Size, false, dataVolumeImage, volume.Type, workerConfig, labels) +} + +func createDiskSpec(size string, boot bool, image, volumeType *string, workerConfig *apisgcp.WorkerConfig, labels map[string]interface{}) (map[string]interface{}, error) { volumeSize, err := worker.DiskSize(size) if err != nil { return nil, err @@ -296,8 +305,8 @@ func createDiskSpec(size string, boot bool, machineImage, volumeType *string, wo disk["labels"] = labels } - if machineImage != nil { - disk["image"] = *machineImage + if image != nil { + disk["image"] = *image } if volumeType != nil { @@ -330,6 +339,15 @@ func addDiskEncryptionDetails(disk map[string]interface{}, encryption *apisgcp.D disk["encryption"] = encryptionMap } +func getDataVolumeImage(volumeName string, dataVolumes []apisgcp.DataVolume) *string { + for _, dv := range dataVolumes { + if dv.Name == volumeName { + return dv.SourceImage + } + } + return nil +} + func getGcePoolLabels(worker *v1alpha1.Worker, pool v1alpha1.WorkerPool) map[string]interface{} { gceInstanceLabels := map[string]interface{}{ "name": SanitizeGcpLabelValue(worker.Name),