From 57eb7f68df2ea4a85a90f20328a16b3e510c728b Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Tue, 11 Apr 2023 22:43:32 -0400 Subject: [PATCH 1/7] add a >63 character pvc name to the integration test --- testing/yaml/datacreation.yaml | 28 +++++++++++++++++++++++ testing/yaml/datavalidation.yaml | 39 ++++++++++++++++++++++++++++++++ testing/yaml/pvcs.yaml | 13 ++++++++++- 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/testing/yaml/datacreation.yaml b/testing/yaml/datacreation.yaml index da64aae..9151090 100644 --- a/testing/yaml/datacreation.yaml +++ b/testing/yaml/datacreation.yaml @@ -89,4 +89,32 @@ spec: - name: config-volume configMap: name: createfile +--- +apiVersion: batch/v1 +kind: Job +metadata: + name: prometheus-kube-prometheus-stack-prometheus-db-prometheus-kube-prometheus-stack-prometheus-0 +spec: + template: + spec: + containers: + - name: training-container + image: registry.k8s.io/nginx-slim:0.8 + command: + - bash + - /app/create.sh + volumeMounts: + - mountPath: /pvmigrate/ + name: data-volume + - name: config-volume + mountPath: /app/create.sh + subPath: create.sh + restartPolicy: Never + volumes: + - name: data-volume + persistentVolumeClaim: + claimName: prometheus-kube-prometheus-stack-prometheus-db-prometheus-kube-prometheus-stack-prometheus-0 + - name: config-volume + configMap: + name: createfile --- \ No newline at end of file diff --git a/testing/yaml/datavalidation.yaml b/testing/yaml/datavalidation.yaml index 8c2930b..ee84323 100644 --- a/testing/yaml/datavalidation.yaml +++ b/testing/yaml/datavalidation.yaml @@ -90,3 +90,42 @@ spec: configMap: name: filecontents --- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: prometheus-kube-prometheus-stack-prometheus-db-prometheus-kube-prometheus-stack-prometheus-0 +spec: + replicas: 1 + selector: + matchLabels: + app: very-long-pvc-name + template: + metadata: + labels: + app: very-long-pvc-name + spec: + containers: + - name: nginx + image: registry.k8s.io/nginx-slim:0.8 + ports: + - containerPort: 80 + name: web + volumeMounts: + - name: data-volume + mountPath: /pvmigrate + - name: config-volume + mountPath: /app/contents.sh + subPath: contents.sh + readinessProbe: + exec: + command: + - bash + - /app/contents.sh + volumes: + - name: data-volume + persistentVolumeClaim: + claimName: prometheus-kube-prometheus-stack-prometheus-db-prometheus-kube-prometheus-stack-prometheus-0 + - name: config-volume + configMap: + name: filecontents +--- diff --git a/testing/yaml/pvcs.yaml b/testing/yaml/pvcs.yaml index 6ded8ed..fecd6c9 100644 --- a/testing/yaml/pvcs.yaml +++ b/testing/yaml/pvcs.yaml @@ -38,4 +38,15 @@ spec: storage: 1Gi storageClassName: int-source --- -# TODO: add PVC name with >63 characters \ No newline at end of file +# this PVC name is more than 63 characters and thus will not fit in a label/annotation +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: prometheus-kube-prometheus-stack-prometheus-db-prometheus-kube-prometheus-stack-prometheus-0 +spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + storageClassName: int-source \ No newline at end of file From 45c822ce3c5eb8bfa60de9e4e0922c43e0acc376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Nussbaumer?= Date: Thu, 21 Mar 2024 10:46:41 +0100 Subject: [PATCH 2/7] fix: modify newPvcName function to accept names up to 253 chars long MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Clément Nussbaumer --- pkg/migrate/migrate.go | 4 ++-- pkg/migrate/migrate_test.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/migrate/migrate.go b/pkg/migrate/migrate.go index 73072d3..346732a 100644 --- a/pkg/migrate/migrate.go +++ b/pkg/migrate/migrate.go @@ -585,12 +585,12 @@ const nameSuffix = "-pvcmigrate" // TODO: refactor to k8sutil package func newPvcName(originalName string) string { candidate := originalName + nameSuffix - if len(candidate) <= 63 { + if len(candidate) <= 253 { return candidate } // remove characters from the middle of the string to reduce the total length to 63 characters - newCandidate := candidate[0:31] + candidate[len(candidate)-32:] + newCandidate := candidate[0:100] + candidate[len(candidate)-153:] return newCandidate } diff --git a/pkg/migrate/migrate_test.go b/pkg/migrate/migrate_test.go index 0e94205..dc20009 100644 --- a/pkg/migrate/migrate_test.go +++ b/pkg/migrate/migrate_test.go @@ -3230,16 +3230,16 @@ func Test_newPvcName(t *testing.T) { want: "abc-pvcmigrate", }, { - originalName: "very very very long name test with a suffix that might be the only unique part of it 0", - want: "very very very long name test wy unique part of it 0-pvcmigrate", + originalName: "very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a suffix that might be the only unique part of it 0", + want: "very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglongloonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a suffix that might be the only unique part of it 0-pvcmigrate", }, { - originalName: "0 very very very long name test with a prefix that might be the only unique part of it", - want: "0 very very very long name testnly unique part of it-pvcmigrate", + originalName: "0 very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a prefix that might be the only unique part of it", + want: "0 very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglongglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a prefix that might be the only unique part of it-pvcmigrate", }, { - originalName: "63 character (after suffix) name is untouched paddin", - want: "63 character (after suffix) name is untouched paddin-pvcmigrate", + originalName: "253 character (after suffix) 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name is untouched paddin", + want: "253 character (after suffix) 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name is untouched paddin-pvcmigrate", }, } for _, tt := range tests { From 39dfe4e92f5f5255285a5827482c6cfddfc6db9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Nussbaumer?= Date: Thu, 21 Mar 2024 15:26:23 +0100 Subject: [PATCH 3/7] refactor: move NewPvcName func to k8sutil package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Clément Nussbaumer --- pkg/k8sutil/label.go | 13 ------- pkg/k8sutil/label_test.go | 36 ------------------- pkg/k8sutil/truncate.go | 30 ++++++++++++++++ pkg/k8sutil/truncate_test.go | 67 ++++++++++++++++++++++++++++++++++++ pkg/migrate/migrate.go | 39 ++++++--------------- pkg/migrate/migrate_test.go | 31 ----------------- 6 files changed, 108 insertions(+), 108 deletions(-) delete mode 100644 pkg/k8sutil/label.go delete mode 100644 pkg/k8sutil/label_test.go create mode 100644 pkg/k8sutil/truncate.go create mode 100644 pkg/k8sutil/truncate_test.go diff --git a/pkg/k8sutil/label.go b/pkg/k8sutil/label.go deleted file mode 100644 index 00ea693..0000000 --- a/pkg/k8sutil/label.go +++ /dev/null @@ -1,13 +0,0 @@ -package k8sutil - -import "fmt" - -// NewPrefixedName returns a name prefixed by prefix and with length that is no longer than 63 -// chars -func NewPrefixedName(prefix, original string) string { - newName := fmt.Sprintf("%s-%s", prefix, original) - if len(newName) > 63 { - newName = newName[0:31] + newName[len(newName)-32:] - } - return newName -} diff --git a/pkg/k8sutil/label_test.go b/pkg/k8sutil/label_test.go deleted file mode 100644 index a00bb94..0000000 --- a/pkg/k8sutil/label_test.go +++ /dev/null @@ -1,36 +0,0 @@ -package k8sutil - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestNewPrefixedName(t *testing.T) { - tests := []struct { - name string - originalName string - prefix string - want string - }{ - { - name: "when name is < 63 chars expect new name to be prefixed", - originalName: "abc", - prefix: "pvcmigrate", - want: "pvcmigrate-abc", - }, - { - name: "when name is > 63 chars expect new name to be prefixed and 63 chars long", - originalName: "this label will exceed its allowed length and than be truncated", - prefix: "pvcmigrate", - want: "pvcmigrate-this label will excewed length and than be truncated", - }, - } - for _, tt := range tests { - t.Run(tt.originalName, func(t *testing.T) { - req := require.New(t) - got := NewPrefixedName(tt.prefix, tt.originalName) - req.Equal(tt.want, got) - }) - } -} diff --git a/pkg/k8sutil/truncate.go b/pkg/k8sutil/truncate.go new file mode 100644 index 0000000..adb4fa1 --- /dev/null +++ b/pkg/k8sutil/truncate.go @@ -0,0 +1,30 @@ +package k8sutil + +import "fmt" + +const nameSuffix = "-pvcmigrate" + +// if the length after adding the suffix is more than 63 characters, we need to reduce that to fit within k8s limits +// pruning from the end runs the risk of dropping the '0'/'1'/etc of a statefulset's PVC name +// pruning from the front runs the risk of making a-replica-... and b-replica-... collide +// so this removes characters from the middle of the string +func NewPvcName(originalName string) string { + candidate := originalName + nameSuffix + if len(candidate) <= 253 { + return candidate + } + + // remove characters from the middle of the string to reduce the total length to 63 characters + newCandidate := candidate[0:100] + candidate[len(candidate)-153:] + return newCandidate +} + +// NewPrefixedName returns a name prefixed by prefix and with length that is no longer than 63 +// chars +func NewPrefixedName(prefix, original string) string { + newName := fmt.Sprintf("%s-%s", prefix, original) + if len(newName) > 63 { + newName = newName[0:31] + newName[len(newName)-32:] + } + return newName +} diff --git a/pkg/k8sutil/truncate_test.go b/pkg/k8sutil/truncate_test.go new file mode 100644 index 0000000..1a7e175 --- /dev/null +++ b/pkg/k8sutil/truncate_test.go @@ -0,0 +1,67 @@ +package k8sutil + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_newPvcName(t *testing.T) { + tests := []struct { + originalName string + want string + }{ + { + originalName: "abc", + want: "abc-pvcmigrate", + }, + { + originalName: "very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a suffix that might be the only unique part of it 0", + want: "very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglongloonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a suffix that might be the only unique part of it 0-pvcmigrate", + }, + { + originalName: "0 very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a prefix that might be the only unique part of it", + want: "0 very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglongglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a prefix that might be the only unique part of it-pvcmigrate", + }, + { + originalName: "253 character (after suffix) 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name is untouched paddin", + want: "253 character (after suffix) 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name is untouched paddin-pvcmigrate", + }, + } + for _, tt := range tests { + t.Run(tt.originalName, func(t *testing.T) { + req := require.New(t) + got := NewPvcName(tt.originalName) + req.Equal(tt.want, got) + }) + } +} + +func TestNewPrefixedName(t *testing.T) { + tests := []struct { + name string + originalName string + prefix string + want string + }{ + { + name: "when name is < 63 chars expect new name to be prefixed", + originalName: "abc", + prefix: "pvcmigrate", + want: "pvcmigrate-abc", + }, + { + name: "when name is > 63 chars expect new name to be prefixed and 63 chars long", + originalName: "this label will exceed its allowed length and than be truncated", + prefix: "pvcmigrate", + want: "pvcmigrate-this label will excewed length and than be truncated", + }, + } + for _, tt := range tests { + t.Run(tt.originalName, func(t *testing.T) { + req := require.New(t) + got := NewPrefixedName(tt.prefix, tt.originalName) + req.Equal(tt.want, got) + }) + } +} diff --git a/pkg/migrate/migrate.go b/pkg/migrate/migrate.go index 346732a..a242026 100644 --- a/pkg/migrate/migrate.go +++ b/pkg/migrate/migrate.go @@ -11,6 +11,7 @@ import ( "text/tabwriter" "time" + "github.com/replicatedhq/pvmigrate/pkg/k8sutil" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -173,7 +174,7 @@ func copyAllPVCs(ctx context.Context, w *log.Logger, clientset k8sclient.Interfa w.Printf("\nCopying data from %s PVCs to %s PVCs\n", sourceSCName, destSCName) for ns, nsPvcs := range matchingPVCs { for _, nsPvc := range nsPvcs { - sourcePvcName, destPvcName := nsPvc.Name, newPvcName(nsPvc.Name) + sourcePvcName, destPvcName := nsPvc.Name, k8sutil.NewPvcName(nsPvc.Name) w.Printf("Copying data from %s (%s) to %s in %s\n", sourcePvcName, nsPvc.Spec.VolumeName, destPvcName, ns) err := copyOnePVC(ctx, w, clientset, ns, sourcePvcName, destPvcName, rsyncImage, verboseCopy, waitTime, rsyncFlags) @@ -471,7 +472,7 @@ func getPVCs(ctx context.Context, w *log.Logger, clientset k8sclient.Interface, w.Printf("\nCreating new PVCs to migrate data to using the %s StorageClass\n", destSCName) for ns, nsPvcs := range matchingPVCs { for _, nsPvc := range nsPvcs { - newName := newPvcName(nsPvc.Name) + newName := k8sutil.NewPvcName(nsPvc.Name) desiredPV, ok := pvsByName[nsPvc.Spec.VolumeName] if !ok { @@ -576,24 +577,6 @@ func validateStorageClasses(ctx context.Context, w *log.Logger, clientset k8scli return nil } -const nameSuffix = "-pvcmigrate" - -// if the length after adding the suffix is more than 63 characters, we need to reduce that to fit within k8s limits -// pruning from the end runs the risk of dropping the '0'/'1'/etc of a statefulset's PVC name -// pruning from the front runs the risk of making a-replica-... and b-replica-... collide -// so this removes characters from the middle of the string -// TODO: refactor to k8sutil package -func newPvcName(originalName string) string { - candidate := originalName + nameSuffix - if len(candidate) <= 253 { - return candidate - } - - // remove characters from the middle of the string to reduce the total length to 63 characters - newCandidate := candidate[0:100] + candidate[len(candidate)-153:] - return newCandidate -} - // get a PV, apply the selected mutator to the PV, update the PV, use the supplied validator to wait for the update to show up func mutatePV(ctx context.Context, w *log.Logger, clientset k8sclient.Interface, pvName string, mutator func(volume *corev1.PersistentVolume) (*corev1.PersistentVolume, error), checker func(volume *corev1.PersistentVolume) bool) error { tries := 0 @@ -975,9 +958,9 @@ func swapPVs(ctx context.Context, w *log.Logger, clientset k8sclient.Interface, if err != nil { return fmt.Errorf("failed to get original PVC %s in %s: %w", pvcName, ns, err) } - migratedPVC, err := clientset.CoreV1().PersistentVolumeClaims(ns).Get(ctx, newPvcName(pvcName), metav1.GetOptions{}) + migratedPVC, err := clientset.CoreV1().PersistentVolumeClaims(ns).Get(ctx, k8sutil.NewPvcName(pvcName), metav1.GetOptions{}) if err != nil { - return fmt.Errorf("failed to get migrated PVC %s in %s: %w", newPvcName(pvcName), ns, err) + return fmt.Errorf("failed to get migrated PVC %s in %s: %w", k8sutil.NewPvcName(pvcName), ns, err) } // mark PVs used by both originalPVC and migratedPVC as to-be-retained @@ -1018,10 +1001,10 @@ func swapPVs(ctx context.Context, w *log.Logger, clientset k8sclient.Interface, if err != nil { return fmt.Errorf("failed to delete original PVC %s in %s: %w", pvcName, ns, err) } - w.Printf("Deleting migrated-to PVC %s in %s to release the PV\n", newPvcName(pvcName), ns) - err = clientset.CoreV1().PersistentVolumeClaims(ns).Delete(ctx, newPvcName(pvcName), metav1.DeleteOptions{}) + w.Printf("Deleting migrated-to PVC %s in %s to release the PV\n", k8sutil.NewPvcName(pvcName), ns) + err = clientset.CoreV1().PersistentVolumeClaims(ns).Delete(ctx, k8sutil.NewPvcName(pvcName), metav1.DeleteOptions{}) if err != nil { - return fmt.Errorf("failed to delete migrated-to PVC %s in %s: %w", newPvcName(pvcName), ns, err) + return fmt.Errorf("failed to delete migrated-to PVC %s in %s: %w", k8sutil.NewPvcName(pvcName), ns, err) } // wait for the deleted PVCs to actually no longer exist @@ -1030,10 +1013,10 @@ func swapPVs(ctx context.Context, w *log.Logger, clientset k8sclient.Interface, if err != nil { return fmt.Errorf("failed to ensure deletion of original PVC %s in %s: %w", pvcName, ns, err) } - w.Printf("Waiting for migrated-to PVC %s in %s to finish deleting\n", newPvcName(pvcName), ns) - err = waitForDeletion(ctx, clientset, newPvcName(pvcName), ns) + w.Printf("Waiting for migrated-to PVC %s in %s to finish deleting\n", k8sutil.NewPvcName(pvcName), ns) + err = waitForDeletion(ctx, clientset, k8sutil.NewPvcName(pvcName), ns) if err != nil { - return fmt.Errorf("failed to ensure deletion of migrated-to PVC %s in %s: %w", newPvcName(pvcName), ns, err) + return fmt.Errorf("failed to ensure deletion of migrated-to PVC %s in %s: %w", k8sutil.NewPvcName(pvcName), ns, err) } // remove claimrefs from original and migrated-to PVs diff --git a/pkg/migrate/migrate_test.go b/pkg/migrate/migrate_test.go index dc20009..7ef5d88 100644 --- a/pkg/migrate/migrate_test.go +++ b/pkg/migrate/migrate_test.go @@ -3220,37 +3220,6 @@ func Test_waitForDeletion(t *testing.T) { } } -func Test_newPvcName(t *testing.T) { - tests := []struct { - originalName string - want string - }{ - { - originalName: "abc", - want: "abc-pvcmigrate", - }, - { - originalName: "very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a suffix that might be the only unique part of it 0", - want: "very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglongloonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a suffix that might be the only unique part of it 0-pvcmigrate", - }, - { - originalName: "0 very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a prefix that might be the only unique part of it", - want: "0 very very very 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglongglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name test with a prefix that might be the only unique part of it-pvcmigrate", - }, - { - originalName: "253 character (after suffix) 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name is untouched paddin", - want: "253 character (after suffix) 253longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong name is untouched paddin-pvcmigrate", - }, - } - for _, tt := range tests { - t.Run(tt.originalName, func(t *testing.T) { - req := require.New(t) - got := newPvcName(tt.originalName) - req.Equal(tt.want, got) - }) - } -} - func Test_copyAllPVCs(t *testing.T) { type podEvent struct { podAge time.Duration From 1a5809a63dd13394f9c662bc861edbfda06edef1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Nussbaumer?= Date: Fri, 22 Mar 2024 15:18:07 +0100 Subject: [PATCH 4/7] fix: put origPvc name in annotation and handle up to 253 chars PVC names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Clément Nussbaumer --- pkg/migrate/migrate.go | 10 ++++++---- pkg/migrate/migrate_test.go | 10 +++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/migrate/migrate.go b/pkg/migrate/migrate.go index a242026..2576db3 100644 --- a/pkg/migrate/migrate.go +++ b/pkg/migrate/migrate.go @@ -347,15 +347,17 @@ func createMigrationPod(ctx context.Context, clientset k8sclient.Interface, ns s podArgs = append(podArgs, rsyncFlags...) podArgs = append(podArgs, "/source/", "/dest") + podName := k8sutil.NewPrefixedName("migrate", sourcePvcName) + createdPod, err := clientset.CoreV1().Pods(ns).Create(ctx, &corev1.Pod{ TypeMeta: metav1.TypeMeta{ Kind: "Pod", APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Name: "migrate-" + sourcePvcName, + Name: podName, Namespace: ns, - Labels: map[string]string{ + Annotations: map[string]string{ baseAnnotation: sourcePvcName, kindAnnotation: "migrate", }, @@ -517,7 +519,7 @@ func getPVCs(ctx context.Context, w *log.Logger, clientset k8sclient.Interface, ObjectMeta: metav1.ObjectMeta{ Name: newName, Namespace: ns, - Labels: map[string]string{ + Annotations: map[string]string{ baseAnnotation: nsPvc.Name, kindAnnotation: "dest", }, @@ -765,7 +767,7 @@ func scaleDownPods(ctx context.Context, w *log.Logger, clientset k8sclient.Inter if len(nsPod.OwnerReferences) == 0 { // if this was a migrate job that wasn't cleaned up properly, delete it // (if it's still running, rsync will happily resume when we get back to it) - if _, ok := nsPod.Labels[baseAnnotation]; ok { + if _, ok := nsPod.Annotations[baseAnnotation]; ok { // this pod was created by pvmigrate, so it can be deleted by pvmigrate err := clientset.CoreV1().Pods(ns).Delete(ctx, nsPod.Name, metav1.DeleteOptions{}) if err != nil { diff --git a/pkg/migrate/migrate_test.go b/pkg/migrate/migrate_test.go index 7ef5d88..3563972 100644 --- a/pkg/migrate/migrate_test.go +++ b/pkg/migrate/migrate_test.go @@ -945,7 +945,7 @@ func Test_createMigrationPod(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "migrate-sourcepvc", Namespace: "testns", - Labels: map[string]string{ + Annotations: map[string]string{ baseAnnotation: "sourcepvc", kindAnnotation: "migrate", }, @@ -1019,7 +1019,7 @@ func Test_createMigrationPod(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "migrate-sourcepvc", Namespace: "testns", - Labels: map[string]string{ + Annotations: map[string]string{ baseAnnotation: "sourcepvc", kindAnnotation: "migrate", }, @@ -1115,7 +1115,7 @@ func Test_createMigrationPod(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "migrate-sourcepvc", Namespace: "testns", - Labels: map[string]string{ + Annotations: map[string]string{ baseAnnotation: "sourcepvc", kindAnnotation: "migrate", }, @@ -1196,7 +1196,7 @@ func Test_createMigrationPod(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "migrate-sourcepvc", Namespace: "testns", - Labels: map[string]string{ + Annotations: map[string]string{ baseAnnotation: "sourcepvc", kindAnnotation: "migrate", }, @@ -2078,7 +2078,7 @@ func Test_scaleDownPods(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "migrationpod", Namespace: "ns1", - Labels: map[string]string{ + Annotations: map[string]string{ baseAnnotation: "test", }, }, From 1571ce7e1578d6e3bc717b34c89868cde47451a5 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Mon, 22 Apr 2024 17:19:34 +0900 Subject: [PATCH 5/7] Fix comment to refer to 253 chars, not 63 --- pkg/k8sutil/truncate.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/k8sutil/truncate.go b/pkg/k8sutil/truncate.go index adb4fa1..9f4cfca 100644 --- a/pkg/k8sutil/truncate.go +++ b/pkg/k8sutil/truncate.go @@ -4,7 +4,7 @@ import "fmt" const nameSuffix = "-pvcmigrate" -// if the length after adding the suffix is more than 63 characters, we need to reduce that to fit within k8s limits +// if the length after adding the suffix is more than 253 characters, we need to reduce that to fit within k8s limits // pruning from the end runs the risk of dropping the '0'/'1'/etc of a statefulset's PVC name // pruning from the front runs the risk of making a-replica-... and b-replica-... collide // so this removes characters from the middle of the string @@ -14,7 +14,7 @@ func NewPvcName(originalName string) string { return candidate } - // remove characters from the middle of the string to reduce the total length to 63 characters + // remove characters from the middle of the string to reduce the total length to 253 characters newCandidate := candidate[0:100] + candidate[len(candidate)-153:] return newCandidate } From 638bb42351e373a8a83a05e81fe081ca2e190c21 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Mon, 22 Apr 2024 17:45:02 +0900 Subject: [PATCH 6/7] fix integrationt test case --- testing/validate.sh | 5 +++++ testing/yaml/datacreation.yaml | 4 ++-- testing/yaml/datavalidation.yaml | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/testing/validate.sh b/testing/validate.sh index 4dce243..73964ac 100755 --- a/testing/validate.sh +++ b/testing/validate.sh @@ -116,6 +116,11 @@ echo "" spinner_until 120 deployment_fully_updated default short-pvc-name echo "" echo "'short-pvc-name' deployment healthy" +echo "waiting for the 'very-long-prometheus-pvc-name' deployment" +echo "" +spinner_until 120 deployment_fully_updated default very-long-prometheus-pvc-name +echo "" +echo "'very-long-prometheus-pvc-name' deployment healthy" kubectl get statefulsets kubectl get deployments diff --git a/testing/yaml/datacreation.yaml b/testing/yaml/datacreation.yaml index 9151090..13ba3f9 100644 --- a/testing/yaml/datacreation.yaml +++ b/testing/yaml/datacreation.yaml @@ -93,7 +93,7 @@ spec: apiVersion: batch/v1 kind: Job metadata: - name: prometheus-kube-prometheus-stack-prometheus-db-prometheus-kube-prometheus-stack-prometheus-0 + name: very-long-prometheus-pvc-name spec: template: spec: @@ -117,4 +117,4 @@ spec: - name: config-volume configMap: name: createfile ---- \ No newline at end of file +--- diff --git a/testing/yaml/datavalidation.yaml b/testing/yaml/datavalidation.yaml index ee84323..7a180eb 100644 --- a/testing/yaml/datavalidation.yaml +++ b/testing/yaml/datavalidation.yaml @@ -93,7 +93,7 @@ spec: apiVersion: apps/v1 kind: Deployment metadata: - name: prometheus-kube-prometheus-stack-prometheus-db-prometheus-kube-prometheus-stack-prometheus-0 + name: very-long-prometheus-pvc-name spec: replicas: 1 selector: From c37e88976d6270df97f3e660d1a7537d147e858a Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Mon, 22 Apr 2024 17:50:32 +0900 Subject: [PATCH 7/7] update testdata creation script --- testing/init.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/testing/init.sh b/testing/init.sh index b0b2481..0ddc703 100755 --- a/testing/init.sh +++ b/testing/init.sh @@ -104,6 +104,15 @@ echo "" spinner_until 120 deployment_fully_updated default short-pvc-name echo "" echo "'short-pvc-name' deployment healthy" +echo "waiting for the 'very-long-prometheus-pvc-name' deployment" +echo "" +spinner_until 120 deployment_fully_updated default very-long-prometheus-pvc-name +echo "" +echo "'very-long-prometheus-pvc-name' deployment healthy" + +kubectl get statefulsets +kubectl get deployments +kubectl get pvc echo "" echo "setting up rbac for the testing service account"