vmsingle: properly add volumes with non-empty storageDataPath

Previously, operator inconsistenetely added volumeMounts for vmsingle deployment,
if spec.storageDataPath is set. It added only volumeMount without volumes. And it produces runtime deployment creation error.

 Intentional behaviour and the main puprose of storageDataPath option is to allow to override storage settings
configured by operator.

 This commit skips adding volumeMounts for this case and fixes runtime error.
Also it adds additional check to the validation webhook, which validates if volumes and volumeMounts is set for non-mepty storageDataPath.

Related issue:
#1158

Signed-off-by: f41gh7 <[email protected]>

api/operator/v1beta1/vmsingle_types.go

@@ -38,10 +38,12 @@ type VMSingleSpec struct {
 	LogFormat string `json:"logFormat,omitempty"`
 	// StorageDataPath disables spec.storage option and overrides arg for victoria-metrics binary --storageDataPath,
 	// its users responsibility to mount proper device into given path.
+	// It requires to provide spec.volumes and spec.volumeMounts with at least 1 value
 	// +optional
 	StorageDataPath string `json:"storageDataPath,omitempty"`
 	// Storage is the definition of how storage will be used by the VMSingle
 	// by default it`s empty dir
+	// this option is ignored if storageDataPath is set
 	// +optional
 	Storage *v1.PersistentVolumeClaimSpec `json:"storage,omitempty"`
 

api/operator/v1beta1/vmsingle_webhook.go

@@ -46,6 +46,14 @@ func (r *VMSingle) sanityCheck() error {
 			return err
 		}
 	}
+	if r.Spec.StorageDataPath != "" {
+		if len(r.Spec.VolumeMounts) == 0 {
+			return fmt.Errorf("spec.volumeMounts must have at least 1 value for spec.storageDataPath=%q", r.Spec.StorageDataPath)
+		}
+		if len(r.Spec.Volumes) == 0 {
+			return fmt.Errorf("spec.volumes must have at least 1 value for spec.storageDataPath=%q", r.Spec.StorageDataPath)
+		}
+	}
 	return nil
 }
 

config/crd/overlay/crd.yaml

@@ -28248,6 +28248,7 @@ spec:
                 description: |-
                   Storage is the definition of how storage will be used by the VMSingle
                   by default it`s empty dir
+                  this option is ignored if storageDataPath is set
                 properties:
                   accessModes:
                     description: |-
@@ -28447,6 +28448,7 @@ spec:
                 description: |-
                   StorageDataPath disables spec.storage option and overrides arg for victoria-metrics binary --storageDataPath,
                   its users responsibility to mount proper device into given path.
+                  It requires to provide spec.volumes and spec.volumeMounts with at least 1 value
                 type: string
               storageMetadata:
                 description: StorageMeta defines annotations and labels attached to

docs/api.md

@@ -3897,8 +3897,8 @@ _Appears in:_
 | `serviceAccountName` | ServiceAccountName is the name of the ServiceAccount to use to run the pods | _string_ | false |
 | `serviceScrapeSpec` | ServiceScrapeSpec that will be added to vmsingle VMServiceScrape spec | _[VMServiceScrapeSpec](#vmservicescrapespec)_ | false |
 | `serviceSpec` | ServiceSpec that will be added to vmsingle service spec | _[AdditionalServiceSpec](#additionalservicespec)_ | false |
-| `storage` | Storage is the definition of how storage will be used by the VMSingle<br />by default it`s empty dir | _[PersistentVolumeClaimSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#persistentvolumeclaimspec-v1-core)_ | false |
-| `storageDataPath` | StorageDataPath disables spec.storage option and overrides arg for victoria-metrics binary --storageDataPath,<br />its users responsibility to mount proper device into given path. | _string_ | false |
+| `storage` | Storage is the definition of how storage will be used by the VMSingle<br />by default it`s empty dir<br />this option is ignored if storageDataPath is set | _[PersistentVolumeClaimSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#persistentvolumeclaimspec-v1-core)_ | false |
+| `storageDataPath` | StorageDataPath disables spec.storage option and overrides arg for victoria-metrics binary --storageDataPath,<br />its users responsibility to mount proper device into given path.<br />It requires to provide spec.volumes and spec.volumeMounts with at least 1 value | _string_ | false |
 | `storageMetadata` | StorageMeta defines annotations and labels attached to PVC for given vmsingle CR | _[EmbeddedObjectMetadata](#embeddedobjectmetadata)_ | false |
 | `streamAggrConfig` | StreamAggrConfig defines stream aggregation configuration for VMSingle | _[StreamAggrConfig](#streamaggrconfig)_ | true |
 | `terminationGracePeriodSeconds` | TerminationGracePeriodSeconds period for container graceful termination | _integer_ | false |

internal/controller/operator/factory/vmsingle/vmsingle.go

@@ -138,8 +138,10 @@ func makeSpecForVMSingle(ctx context.Context, cr *vmv1beta1.VMSingle) (*corev1.P
 		fmt.Sprintf("-retentionPeriod=%s", cr.Spec.RetentionPeriod),
 	}
 
-	// if customStorageDataPath is not empty, do not add pvc.
-	shouldAddPVC := cr.Spec.StorageDataPath == ""
+	// if customStorageDataPath is not empty, do not add volumes
+	// and volumeMounts
+	// it's user responsobility to provide correct values
+	addVolumeMounts := cr.Spec.StorageDataPath == ""
 
 	storagePath := vmSingleDataDir
 	if cr.Spec.StorageDataPath != "" {
@@ -165,27 +167,32 @@ func makeSpecForVMSingle(ctx context.Context, cr *vmv1beta1.VMSingle) (*corev1.P
 	var ports []corev1.ContainerPort
 	ports = append(ports, corev1.ContainerPort{Name: "http", Protocol: "TCP", ContainerPort: intstr.Parse(cr.Spec.Port).IntVal})
 	ports = build.AppendInsertPorts(ports, cr.Spec.InsertPorts)
-	volumes := []corev1.Volume{}
 
-	storageSpec := cr.Spec.Storage
+	var volumes []corev1.Volume
+	var vmMounts []corev1.VolumeMount
 
-	if storageSpec == nil {
-		volumes = append(volumes, corev1.Volume{
-			Name: vmDataVolumeName,
-			VolumeSource: corev1.VolumeSource{
-				EmptyDir: &corev1.EmptyDirVolumeSource{},
-			},
-		})
-	} else if shouldAddPVC {
-		volumes = append(volumes, corev1.Volume{
-			Name: vmDataVolumeName,
-			VolumeSource: corev1.VolumeSource{
+	// conditionally add volume mounts
+	if addVolumeMounts {
+		vmMounts = append(vmMounts, corev1.VolumeMount{
+			Name:      vmDataVolumeName,
+			MountPath: storagePath},
+		)
+
+		vlSource := corev1.VolumeSource{
+			EmptyDir: &corev1.EmptyDirVolumeSource{},
+		}
+		if cr.Spec.Storage != nil {
+			vlSource = corev1.VolumeSource{
 				PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
 					ClaimName: cr.PrefixedName(),
 				},
-			},
-		})
+			}
+		}
+		volumes = append(volumes, corev1.Volume{
+			Name:         vmDataVolumeName,
+			VolumeSource: vlSource})
 	}
+
 	if cr.Spec.VMBackup != nil && cr.Spec.VMBackup.CredentialsSecret != nil {
 		volumes = append(volumes, corev1.Volume{
 			Name: k8stools.SanitizeVolumeName("secret-" + cr.Spec.VMBackup.CredentialsSecret.Name),
@@ -196,14 +203,8 @@ func makeSpecForVMSingle(ctx context.Context, cr *vmv1beta1.VMSingle) (*corev1.P
 			},
 		})
 	}
-	volumes = append(volumes, cr.Spec.Volumes...)
-	vmMounts := []corev1.VolumeMount{
-		{
-			Name:      vmDataVolumeName,
-			MountPath: storagePath,
-		},
-	}
 
+	volumes = append(volumes, cr.Spec.Volumes...)
 	vmMounts = append(vmMounts, cr.Spec.VolumeMounts...)
 
 	for _, s := range cr.Spec.Secrets {

test/e2e/vmsingle_test.go

@@ -195,6 +195,39 @@ var _ = Describe("test  vmsingle Controller", func() {
 						Expect(*createdDeploy.Spec.Template.Spec.Containers[0].SecurityContext.RunAsNonRoot).To(BeTrue())
 
 					}),
+				Entry("with data emptyDir", "emptydir",
+					&vmv1beta1.VMSingle{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: namespace,
+						},
+						Spec: vmv1beta1.VMSingleSpec{
+							CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{
+								ReplicaCount: ptr.To[int32](1),
+							},
+							CommonDefaultableParams: vmv1beta1.CommonDefaultableParams{
+								UseStrictSecurity: ptr.To(false),
+							},
+							RetentionPeriod:      "1",
+							RemovePvcAfterDelete: true,
+							StorageDataPath:      "/tmp/",
+							Storage: &corev1.PersistentVolumeClaimSpec{
+								Resources: corev1.VolumeResourceRequirements{
+									Requests: corev1.ResourceList{
+										corev1.ResourceStorage: resource.MustParse("1Gi"),
+									},
+								},
+							},
+						},
+					},
+					func(cr *vmv1beta1.VMSingle) {
+						createdChildObjects := types.NamespacedName{Namespace: namespace, Name: cr.PrefixedName()}
+						var createdDeploy appsv1.Deployment
+						Expect(k8sClient.Get(ctx, createdChildObjects, &createdDeploy)).To(Succeed())
+						ts := createdDeploy.Spec.Template.Spec
+						Expect(ts.Containers).To(HaveLen(1))
+						Expect(ts.Volumes).To(HaveLen(0))
+						Expect(ts.Containers[0].VolumeMounts).To(HaveLen(0))
+					}),
 			)
 
 			existSingle := &vmv1beta1.VMSingle{