Skip to content

Commit

Permalink
refactor: move NewPvcName func to k8sutil package
Browse files Browse the repository at this point in the history
Signed-off-by: Clément Nussbaumer <clement.nussbaumer@postfinance.ch>
  • Loading branch information
clementnuss committed Mar 21, 2024
1 parent 45c822c commit 39dfe4e
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 108 deletions.
13 changes: 0 additions & 13 deletions pkg/k8sutil/label.go

This file was deleted.

36 changes: 0 additions & 36 deletions pkg/k8sutil/label_test.go

This file was deleted.

30 changes: 30 additions & 0 deletions pkg/k8sutil/truncate.go
Original file line number Diff line number Diff line change
@@ -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
}
67 changes: 67 additions & 0 deletions pkg/k8sutil/truncate_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
39 changes: 11 additions & 28 deletions pkg/migrate/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
31 changes: 0 additions & 31 deletions pkg/migrate/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 39dfe4e

Please sign in to comment.