Skip to content

Commit 088ef81

Browse files
AndrewChubatiukAndrewChubatiuk
committed
changed order of volume and mounts for backward compatibility
1 parent ef838b6 commit 088ef81

File tree

11 files changed

+147
-154
lines changed

11 files changed

+147
-154
lines changed

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

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ func VMBackupManager(
2121
ctx context.Context,
2222
cr *vmv1beta1.VMBackup,
2323
port string,
24-
storagePath, dataVolumeName string,
24+
storagePath string,
25+
mounts []corev1.VolumeMount,
2526
extraArgs map[string]string,
2627
isCluster bool,
2728
license *vmv1beta1.License,
@@ -85,16 +86,6 @@ func VMBackupManager(
8586

8687
var ports []corev1.ContainerPort
8788
ports = append(ports, corev1.ContainerPort{Name: "http", Protocol: "TCP", ContainerPort: intstr.Parse(cr.Port).IntVal})
88-
89-
mounts := []corev1.VolumeMount{
90-
{
91-
Name: dataVolumeName,
92-
MountPath: storagePath,
93-
ReadOnly: false,
94-
},
95-
}
96-
mounts = append(mounts, cr.VolumeMounts...)
97-
9889
if cr.CredentialsSecret != nil {
9990
mounts = append(mounts, corev1.VolumeMount{
10091
Name: k8stools.SanitizeVolumeName("secret-" + cr.CredentialsSecret.Name),
@@ -171,9 +162,9 @@ func VMBackupManager(
171162
// VMRestore conditionally creates vmrestore container
172163
func VMRestore(
173164
cr *vmv1beta1.VMBackup,
174-
storagePath, dataVolumeName string,
165+
storagePath string,
166+
mounts []corev1.VolumeMount,
175167
) (*corev1.Container, error) {
176-
177168
args := []string{
178169
fmt.Sprintf("-storageDataPath=%s", storagePath),
179170
"-eula",
@@ -198,15 +189,6 @@ func VMRestore(
198189
var ports []corev1.ContainerPort
199190
ports = append(ports, corev1.ContainerPort{Name: "http", Protocol: "TCP", ContainerPort: intstr.Parse(cr.Port).IntVal})
200191

201-
mounts := []corev1.VolumeMount{
202-
{
203-
Name: dataVolumeName,
204-
MountPath: storagePath,
205-
ReadOnly: false,
206-
},
207-
}
208-
mounts = append(mounts, cr.VolumeMounts...)
209-
210192
if cr.CredentialsSecret != nil {
211193
mounts = append(mounts, corev1.VolumeMount{
212194
Name: k8stools.SanitizeVolumeName("secret-" + cr.CredentialsSecret.Name),

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

Lines changed: 37 additions & 33 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"
@@ -15,6 +16,7 @@ import (
1516
)
1617

1718
const probeTimeoutSeconds int32 = 5
19+
const DataVolumeName = "data"
1820

1921
type probeCRD interface {
2022
Probe() *vmv1beta1.EmbeddedProbes
@@ -536,46 +538,48 @@ func AddSyslogTLSConfigToVolumes(dstVolumes []corev1.Volume, dstMounts []corev1.
536538
return dstVolumes, dstMounts
537539
}
538540

539-
func StorageVolumeMountsTo(volumes []corev1.Volume, mounts []corev1.VolumeMount, pvcSrc *corev1.PersistentVolumeClaimVolumeSource, volumeName, storagePath string) ([]corev1.Volume, []corev1.VolumeMount) {
540-
var alreadyMounted bool
541+
func StorageVolumeMountsTo(volumes []corev1.Volume, mounts []corev1.VolumeMount, pvcSrc *corev1.PersistentVolumeClaimVolumeSource, storagePath, dataVolumeName string) ([]corev1.Volume, []corev1.VolumeMount, error) {
542+
foundMount := false
541543
for _, volumeMount := range mounts {
542-
if volumeMount.Name == volumeName {
543-
alreadyMounted = true
544-
break
544+
rel, err := filepath.Rel(volumeMount.MountPath, storagePath)
545+
if err == nil && !strings.HasPrefix(rel, "..") {
546+
if volumeMount.Name == dataVolumeName {
547+
foundMount = true
548+
break
549+
}
550+
return nil, nil, fmt.Errorf(
551+
"unexpected volume=%q mounted to path=%q, which is reserved for volume=%q, path=%q",
552+
volumeMount.Name, volumeMount.MountPath, dataVolumeName, storagePath)
553+
} else {
554+
if volumeMount.Name != dataVolumeName {
555+
continue
556+
}
557+
return nil, nil, fmt.Errorf(
558+
"unexpected volume=%q mounted to path=%q, expected path=%q",
559+
volumeMount.Name, volumeMount.MountPath, dataVolumeName)
545560
}
546561
}
547-
if !alreadyMounted {
548-
mounts = append(mounts, corev1.VolumeMount{
549-
Name: volumeName,
562+
if !foundMount {
563+
mounts = append([]corev1.VolumeMount{{
564+
Name: dataVolumeName,
550565
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
566+
}}, mounts...)
561567
}
562568

563-
var volumePresent bool
564569
for _, volume := range volumes {
565-
if volume.Name == volumeName {
566-
volumePresent = true
567-
break
570+
if volume.Name == dataVolumeName {
571+
return volumes, mounts, nil
568572
}
569573
}
570-
if volumePresent {
571-
return volumes, mounts
572-
}
573-
574-
volumes = append(volumes, corev1.Volume{
575-
Name: volumeName,
576-
VolumeSource: corev1.VolumeSource{
577-
EmptyDir: &corev1.EmptyDirVolumeSource{},
578-
},
579-
})
580-
return volumes, mounts
574+
var source corev1.VolumeSource
575+
if pvcSrc != nil {
576+
source.PersistentVolumeClaim = pvcSrc
577+
} else {
578+
source.EmptyDir = &corev1.EmptyDirVolumeSource{}
579+
}
580+
volumes = append([]corev1.Volume{{
581+
Name: dataVolumeName,
582+
VolumeSource: source,
583+
}}, volumes...)
584+
return volumes, mounts, nil
581585
}

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

Lines changed: 71 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -353,92 +353,86 @@ 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
360359
mounts []corev1.VolumeMount
361360
expectedMounts []corev1.VolumeMount
361+
wantErr bool
362362
}
363363
f := func(o opts) {
364364
t.Helper()
365-
gotVolumes, gotMounts := StorageVolumeMountsTo(o.volumes, o.mounts, o.pvcSrc, o.volumeName, o.storagePath)
365+
gotVolumes, gotMounts, err := StorageVolumeMountsTo(o.volumes, o.mounts, o.pvcSrc, o.storagePath, DataVolumeName)
366366
assert.Equal(t, o.expectedMounts, gotMounts)
367367
assert.Equal(t, o.expectedVolumes, gotVolumes)
368+
if o.wantErr {
369+
assert.Error(t, err)
370+
} else {
371+
assert.NoError(t, err)
372+
}
368373
}
369374

370375
// no PVC spec and no volumes and mounts
371376
f(opts{
372-
volumeName: "test",
373377
storagePath: "/test",
374378
expectedVolumes: []corev1.Volume{{
375-
Name: "test",
379+
Name: DataVolumeName,
376380
VolumeSource: corev1.VolumeSource{
377381
EmptyDir: &corev1.EmptyDirVolumeSource{},
378382
},
379383
}},
380384
expectedMounts: []corev1.VolumeMount{{
381-
Name: "test",
385+
Name: DataVolumeName,
382386
MountPath: "/test",
383387
}},
384388
})
385389

386390
// with PVC spec and no volumes and mounts
387391
f(opts{
388-
volumeName: "test",
389392
storagePath: "/test",
390393
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
391394
ClaimName: "test-claim",
392395
},
393396
expectedVolumes: []corev1.Volume{{
394-
Name: "test",
397+
Name: DataVolumeName,
395398
VolumeSource: corev1.VolumeSource{
396399
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
397400
ClaimName: "test-claim",
398401
},
399402
},
400403
}},
401404
expectedMounts: []corev1.VolumeMount{{
402-
Name: "test",
405+
Name: DataVolumeName,
403406
MountPath: "/test",
404407
}},
405408
})
406409

407410
// with PVC spec and matching data volume
408411
f(opts{
409412
volumes: []corev1.Volume{{
410-
Name: "test",
413+
Name: DataVolumeName,
411414
VolumeSource: corev1.VolumeSource{
412415
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
413416
VolumeID: "aws-volume",
414417
},
415418
},
416419
}},
417-
volumeName: "test",
418420
storagePath: "/test",
419421
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
420422
ClaimName: "test-claim",
421423
},
422424
expectedVolumes: []corev1.Volume{
423425
{
424-
Name: "test",
426+
Name: DataVolumeName,
425427
VolumeSource: corev1.VolumeSource{
426428
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
427429
VolumeID: "aws-volume",
428430
},
429431
},
430432
},
431-
{
432-
Name: "test",
433-
VolumeSource: corev1.VolumeSource{
434-
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
435-
ClaimName: "test-claim",
436-
},
437-
},
438-
},
439433
},
440434
expectedMounts: []corev1.VolumeMount{{
441-
Name: "test",
435+
Name: DataVolumeName,
442436
MountPath: "/test",
443437
}},
444438
})
@@ -453,31 +447,30 @@ func TestStorageVolumeMountsTo(t *testing.T) {
453447
},
454448
},
455449
}},
456-
volumeName: "test",
457450
storagePath: "/test",
458451
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
459452
ClaimName: "test-claim",
460453
},
461454
expectedVolumes: []corev1.Volume{
462455
{
463-
Name: "extra",
456+
Name: DataVolumeName,
464457
VolumeSource: corev1.VolumeSource{
465-
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
466-
VolumeID: "aws-volume",
458+
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
459+
ClaimName: "test-claim",
467460
},
468461
},
469462
},
470463
{
471-
Name: "test",
464+
Name: "extra",
472465
VolumeSource: corev1.VolumeSource{
473-
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
474-
ClaimName: "test-claim",
466+
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
467+
VolumeID: "aws-volume",
475468
},
476469
},
477470
},
478471
},
479472
expectedMounts: []corev1.VolumeMount{{
480-
Name: "test",
473+
Name: DataVolumeName,
481474
MountPath: "/test",
482475
}},
483476
})
@@ -493,35 +486,76 @@ func TestStorageVolumeMountsTo(t *testing.T) {
493486
},
494487
}},
495488
mounts: []corev1.VolumeMount{{
496-
Name: "test",
489+
Name: DataVolumeName,
497490
MountPath: "/other-path",
498491
}},
499-
volumeName: "test",
492+
wantErr: true,
500493
storagePath: "/test",
501494
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
502495
ClaimName: "test-claim",
503496
},
497+
})
498+
499+
// with PVC spec and intersecting data volume mount
500+
f(opts{
501+
volumes: []corev1.Volume{{
502+
Name: "extra",
503+
VolumeSource: corev1.VolumeSource{
504+
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
505+
VolumeID: "aws-volume",
506+
},
507+
},
508+
}},
509+
mounts: []corev1.VolumeMount{{
510+
Name: DataVolumeName,
511+
MountPath: "/test",
512+
}},
513+
storagePath: "/test/data",
514+
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
515+
ClaimName: "test-claim",
516+
},
504517
expectedVolumes: []corev1.Volume{
505518
{
506-
Name: "extra",
519+
Name: DataVolumeName,
507520
VolumeSource: corev1.VolumeSource{
508-
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
509-
VolumeID: "aws-volume",
521+
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
522+
ClaimName: "test-claim",
510523
},
511524
},
512525
},
513526
{
514-
Name: "test",
527+
Name: "extra",
515528
VolumeSource: corev1.VolumeSource{
516-
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
517-
ClaimName: "test-claim",
529+
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
530+
VolumeID: "aws-volume",
518531
},
519532
},
520533
},
521534
},
522535
expectedMounts: []corev1.VolumeMount{{
536+
Name: DataVolumeName,
537+
MountPath: "/test",
538+
}},
539+
})
540+
541+
// with PVC spec and intersecting volume mount and absent volume
542+
f(opts{
543+
volumes: []corev1.Volume{{
544+
Name: "test",
545+
VolumeSource: corev1.VolumeSource{
546+
AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
547+
VolumeID: "aws-volume",
548+
},
549+
},
550+
}},
551+
mounts: []corev1.VolumeMount{{
523552
Name: "test",
524-
MountPath: "/other-path",
553+
MountPath: "/test",
525554
}},
555+
storagePath: "/test/data",
556+
pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{
557+
ClaimName: "test-claim",
558+
},
559+
wantErr: true,
526560
})
527561
}

0 commit comments

Comments
 (0)