Skip to content

Commit 846d359

Browse files
AndrewChubatiukAndrewChubatiuk
committed
changed order of volume and mounts for backward compatibility
1 parent 80f8959 commit 846d359

File tree

9 files changed

+139
-101
lines changed

9 files changed

+139
-101
lines changed

‎internal/controller/operator/factory/build/container.go‎

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package build
22

33
import (
44
"fmt"
5+
"path/filepath"
56
"strings"
67

78
corev1 "k8s.io/api/core/v1"
@@ -536,46 +537,41 @@ func AddSyslogTLSConfigToVolumes(dstVolumes []corev1.Volume, dstMounts []corev1.
536537
return dstVolumes, dstMounts
537538
}
538539

539-
func StorageVolumeMountsTo(volumes []corev1.Volume, mounts []corev1.VolumeMount, pvcSrc *corev1.PersistentVolumeClaimVolumeSource, volumeName, storagePath string) ([]corev1.Volume, []corev1.VolumeMount) {
540-
var alreadyMounted bool
540+
func StorageVolumeMountsTo(volumes []corev1.Volume, mounts []corev1.VolumeMount, pvcSrc *corev1.PersistentVolumeClaimVolumeSource, storagePath string) ([]corev1.Volume, []corev1.VolumeMount) {
541+
volumeName := "data"
542+
foundMount := false
541543
for _, volumeMount := range mounts {
542-
if volumeMount.Name == volumeName {
543-
alreadyMounted = true
544+
if _, err := filepath.Rel(volumeMount.MountPath, storagePath); err == nil {
545+
volumeName = volumeMount.Name
546+
foundMount = true
544547
break
545548
}
546549
}
547-
if !alreadyMounted {
548-
mounts = append(mounts, corev1.VolumeMount{
550+
if !foundMount {
551+
mounts = append([]corev1.VolumeMount{{
549552
Name: volumeName,
550553
MountPath: storagePath,
551-
})
552-
}
553-
if pvcSrc != nil {
554-
volumes = append(volumes, corev1.Volume{
555-
Name: volumeName,
556-
VolumeSource: corev1.VolumeSource{
557-
PersistentVolumeClaim: pvcSrc,
558-
},
559-
})
560-
return volumes, mounts
554+
}}, mounts...)
561555
}
562-
563-
var volumePresent bool
556+
foundVolume := false
564557
for _, volume := range volumes {
565558
if volume.Name == volumeName {
566-
volumePresent = true
559+
foundVolume = true
567560
break
568561
}
569562
}
570-
if volumePresent {
563+
if foundVolume {
571564
return volumes, mounts
572565
}
573-
574-
volumes = append(volumes, corev1.Volume{
575-
Name: volumeName,
576-
VolumeSource: corev1.VolumeSource{
577-
EmptyDir: &corev1.EmptyDirVolumeSource{},
578-
},
579-
})
566+
var source corev1.VolumeSource
567+
if pvcSrc != nil {
568+
source.PersistentVolumeClaim = pvcSrc
569+
} else {
570+
source.EmptyDir = &corev1.EmptyDirVolumeSource{}
571+
}
572+
volumes = append([]corev1.Volume{{
573+
Name: defaultVolumeName,
574+
VolumeSource: source,
575+
}}, volumes...)
580576
return volumes, mounts
581577
}

‎internal/controller/operator/factory/build/container_test.go‎

Lines changed: 93 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,6 @@ func TestAddSyslogArgsTo(t *testing.T) {
353353
func TestStorageVolumeMountsTo(t *testing.T) {
354354
type opts struct {
355355
pvcSrc *corev1.PersistentVolumeClaimVolumeSource
356-
volumeName string
357356
storagePath string
358357
volumes []corev1.Volume
359358
expectedVolumes []corev1.Volume
@@ -362,88 +361,115 @@ func TestStorageVolumeMountsTo(t *testing.T) {
362361
}
363362
f := func(o opts) {
364363
t.Helper()
365-
gotVolumes, gotMounts := StorageVolumeMountsTo(o.volumes, o.mounts, o.pvcSrc, o.volumeName, o.storagePath)
364+
gotVolumes, gotMounts := StorageVolumeMountsTo(o.volumes, o.mounts, o.pvcSrc, o.storagePath)
366365
assert.Equal(t, o.expectedMounts, gotMounts)
367366
assert.Equal(t, o.expectedVolumes, gotVolumes)
368367
}
369368

370369
// no PVC spec and no volumes and mounts
371370
f(opts{
372-
volumeName: "test",
373371
storagePath: "/test",
374372
expectedVolumes: []corev1.Volume{{
375-
Name: "test",
373+
Name: "data",
376374
VolumeSource: corev1.VolumeSource{
377375
EmptyDir: &corev1.EmptyDirVolumeSource{},
378376
},
379377
}},
380378
expectedMounts: []corev1.VolumeMount{{
381-
Name: "test",
379+
Name: "data",
382380
MountPath: "/test",
383381
}},
384382
})
385383

386384
// with PVC spec and no volumes and mounts
387385
f(opts{
388-
volumeName: "test",
389386
storagePath: "/test",
390387
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
391388
ClaimName: "test-claim",
392389
},
393390
expectedVolumes: []corev1.Volume{{
394-
Name: "test",
391+
Name: "data",
395392
VolumeSource: corev1.VolumeSource{
396393
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
397394
ClaimName: "test-claim",
398395
},
399396
},
400397
}},
401398
expectedMounts: []corev1.VolumeMount{{
402-
Name: "test",
399+
Name: "data",
403400
MountPath: "/test",
404401
}},
405402
})
406403

407404
// with PVC spec and matching data volume
408405
f(opts{
409406
volumes: []corev1.Volume{{
410-
Name: "test",
407+
Name: "data",
411408
VolumeSource: corev1.VolumeSource{
412409
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
413410
VolumeID: "aws-volume",
414411
},
415412
},
416413
}},
417-
volumeName: "test",
418414
storagePath: "/test",
419415
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
420416
ClaimName: "test-claim",
421417
},
422418
expectedVolumes: []corev1.Volume{
423419
{
424-
Name: "test",
420+
Name: "data",
425421
VolumeSource: corev1.VolumeSource{
426422
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
427423
VolumeID: "aws-volume",
428424
},
429425
},
430426
},
427+
},
428+
expectedMounts: []corev1.VolumeMount{{
429+
Name: "data",
430+
MountPath: "/test",
431+
}},
432+
})
433+
434+
// with PVC spec and not matching data volume
435+
f(opts{
436+
volumes: []corev1.Volume{{
437+
Name: "extra",
438+
VolumeSource: corev1.VolumeSource{
439+
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
440+
VolumeID: "aws-volume",
441+
},
442+
},
443+
}},
444+
storagePath: "/test",
445+
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
446+
ClaimName: "test-claim",
447+
},
448+
expectedVolumes: []corev1.Volume{
431449
{
432-
Name: "test",
450+
Name: "data",
433451
VolumeSource: corev1.VolumeSource{
434452
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
435453
ClaimName: "test-claim",
436454
},
437455
},
438456
},
457+
{
458+
Name: "extra",
459+
VolumeSource: corev1.VolumeSource{
460+
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
461+
VolumeID: "aws-volume",
462+
},
463+
},
464+
},
439465
},
440466
expectedMounts: []corev1.VolumeMount{{
441-
Name: "test",
467+
Name: "data",
442468
MountPath: "/test",
443469
}},
444470
})
445471

446-
// with PVC spec and not matching data volume
472+
// with PVC spec and existing data volume mount
447473
f(opts{
448474
volumes: []corev1.Volume{{
449475
Name: "extra",
@@ -453,36 +479,39 @@ func TestStorageVolumeMountsTo(t *testing.T) {
453479
},
454480
},
455481
}},
456-
volumeName: "test",
482+
mounts: []corev1.VolumeMount{{
483+
Name: "data",
484+
MountPath: "/other-path",
485+
}},
457486
storagePath: "/test",
458487
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
459488
ClaimName: "test-claim",
460489
},
461490
expectedVolumes: []corev1.Volume{
462491
{
463-
Name: "extra",
492+
Name: "data",
464493
VolumeSource: corev1.VolumeSource{
465-
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
466-
VolumeID: "aws-volume",
494+
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
495+
ClaimName: "test-claim",
467496
},
468497
},
469498
},
470499
{
471-
Name: "test",
500+
Name: "extra",
472501
VolumeSource: corev1.VolumeSource{
473-
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
474-
ClaimName: "test-claim",
502+
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
503+
VolumeID: "aws-volume",
475504
},
476505
},
477506
},
478507
},
479508
expectedMounts: []corev1.VolumeMount{{
480-
Name: "test",
481-
MountPath: "/test",
509+
Name: "data",
510+
MountPath: "/other-path",
482511
}},
483512
})
484513

485-
// with PVC spec and existing data volume mount
514+
// with PVC spec and intersecting data volume mount
486515
f(opts{
487516
volumes: []corev1.Volume{{
488517
Name: "extra",
@@ -493,15 +522,22 @@ func TestStorageVolumeMountsTo(t *testing.T) {
493522
},
494523
}},
495524
mounts: []corev1.VolumeMount{{
496-
Name: "test",
497-
MountPath: "/other-path",
525+
Name: "data",
526+
MountPath: "/test",
498527
}},
499-
volumeName: "test",
500-
storagePath: "/test",
528+
storagePath: "/test/data",
501529
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
502530
ClaimName: "test-claim",
503531
},
504532
expectedVolumes: []corev1.Volume{
533+
{
534+
Name: "data",
535+
VolumeSource: corev1.VolumeSource{
536+
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
537+
ClaimName: "test-claim",
538+
},
539+
},
540+
},
505541
{
506542
Name: "extra",
507543
VolumeSource: corev1.VolumeSource{
@@ -510,18 +546,44 @@ func TestStorageVolumeMountsTo(t *testing.T) {
510546
},
511547
},
512548
},
549+
},
550+
expectedMounts: []corev1.VolumeMount{{
551+
Name: "data",
552+
MountPath: "/test",
553+
}},
554+
})
555+
556+
// with PVC spec and intersecting volume mount and absent volume
557+
f(opts{
558+
volumes: []corev1.Volume{{
559+
Name: "test",
560+
VolumeSource: corev1.VolumeSource{
561+
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
562+
VolumeID: "aws-volume",
563+
},
564+
},
565+
}},
566+
mounts: []corev1.VolumeMount{{
567+
Name: "test",
568+
MountPath: "/test",
569+
}},
570+
storagePath: "/test/data",
571+
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
572+
ClaimName: "test-claim",
573+
},
574+
expectedVolumes: []corev1.Volume{
513575
{
514576
Name: "test",
515577
VolumeSource: corev1.VolumeSource{
516-
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
517-
ClaimName: "test-claim",
578+
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
579+
VolumeID: "aws-volume",
518580
},
519581
},
520582
},
521583
},
522584
expectedMounts: []corev1.VolumeMount{{
523585
Name: "test",
524-
MountPath: "/other-path",
586+
MountPath: "/test",
525587
}},
526588
})
527589
}

‎internal/controller/operator/factory/vlsingle/vlogs.go‎

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,6 @@ func makeVLogsPodSpec(r *vmv1beta1.VLogs) (*corev1.PodTemplateSpec, error) {
147147
fmt.Sprintf("-retentionPeriod=%s", r.Spec.RetentionPeriod),
148148
}
149149

150-
// if customStorageDataPath is not empty, do not add pvc.
151-
shouldAddPVC := r.Spec.StorageDataPath == ""
152-
153150
storagePath := dataDataDir
154151
if r.Spec.StorageDataPath != "" {
155152
storagePath = r.Spec.StorageDataPath
@@ -180,37 +177,19 @@ func makeVLogsPodSpec(r *vmv1beta1.VLogs) (*corev1.PodTemplateSpec, error) {
180177

181178
var ports []corev1.ContainerPort
182179
ports = append(ports, corev1.ContainerPort{Name: "http", Protocol: "TCP", ContainerPort: intstr.Parse(r.Spec.Port).IntVal})
183-
volumes := []corev1.Volume{}
184-
185-
storageSpec := r.Spec.Storage
186-
187-
if storageSpec == nil {
188-
volumes = append(volumes, corev1.Volume{
189-
Name: dataVolumeName,
190-
VolumeSource: corev1.VolumeSource{
191-
EmptyDir: &corev1.EmptyDirVolumeSource{},
192-
},
193-
})
194-
} else if shouldAddPVC {
195-
volumes = append(volumes, corev1.Volume{
196-
Name: dataVolumeName,
197-
VolumeSource: corev1.VolumeSource{
198-
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
199-
ClaimName: r.PrefixedName(),
200-
},
201-
},
202-
})
203-
}
180+
var volumes []corev1.Volume
181+
var vmMounts []corev1.VolumeMount
204182
volumes = append(volumes, r.Spec.Volumes...)
205-
vmMounts := []corev1.VolumeMount{
206-
{
207-
Name: dataVolumeName,
208-
MountPath: storagePath,
209-
},
210-
}
211-
212183
vmMounts = append(vmMounts, r.Spec.VolumeMounts...)
213184

185+
var pvcSrc *corev1.PersistentVolumeClaimVolumeSource
186+
if r.Spec.Storage != nil {
187+
pvcSrc = &corev1.PersistentVolumeClaimVolumeSource{
188+
ClaimName: r.PrefixedName(),
189+
}
190+
}
191+
volumes, vmMounts = build.StorageVolumeMountsTo(volumes, vmMounts, pvcSrc, storagePath)
192+
214193
for _, s := range r.Spec.Secrets {
215194
volumes = append(volumes, corev1.Volume{
216195
Name: k8stools.SanitizeVolumeName("secret-" + s),

0 commit comments

Comments
 (0)