Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if PV/PVC Access Modes are Supported by the Volume Provider #139

Merged
merged 41 commits into from Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
6eead06
WIP: Check PVC access mode
rrpolanco Nov 3, 2022
3ac07a8
Still WIP
rrpolanco Nov 4, 2022
3ef7e7d
More WIP
rrpolanco Nov 16, 2022
aa01da4
Adding tests
rrpolanco Nov 17, 2022
61bc196
Add more unit test
rrpolanco Nov 17, 2022
00a0a86
Passing UT
rrpolanco Nov 18, 2022
7f3aabb
Add option to skip validation
rrpolanco Nov 18, 2022
d02b37c
Export PVCError
rrpolanco Nov 18, 2022
304e3d8
correctly return map
rrpolanco Nov 18, 2022
5f1f4a8
print when there are validation errors
rrpolanco Nov 18, 2022
0a40686
don't overwrite status for non-pending pvcs
rrpolanco Nov 18, 2022
24a161e
revert previous change
rrpolanco Nov 18, 2022
834ee8a
fix events and tab writer output
rrpolanco Nov 19, 2022
8c6683d
Add more unit test
rrpolanco Nov 21, 2022
b44e097
Add pod ready timeout out for volume validation pods
rrpolanco Nov 22, 2022
bacfe88
convert pod ready timeout to second units
rrpolanco Nov 22, 2022
26ab28b
accept an int for timeout instead of duration
rrpolanco Nov 22, 2022
59f1f05
specify PVC copy operation timeout in seconds
rrpolanco Nov 23, 2022
a44c4ba
rename flag
rrpolanco Nov 23, 2022
ee23c2c
Refactor WIP
rrpolanco Nov 30, 2022
efe96f3
WIP: PR Review Refactor
rrpolanco Dec 1, 2022
8333438
WIP: More PR review refactor
rrpolanco Dec 2, 2022
87666f5
Map all failures to exit code 1
rrpolanco Dec 2, 2022
a556650
Address Ethan's PR review comments and suggestions
rrpolanco Dec 2, 2022
ff3ea81
tweak Test_pvcForStorageClass test
rrpolanco Dec 2, 2022
f2b558b
convert podready timeout to correct unit
rrpolanco Dec 3, 2022
de8c668
allow os signals to context
rrpolanco Dec 3, 2022
0313d4a
fix pod and pvc cleanup
rrpolanco Dec 3, 2022
cf84ac3
simplify table output
rrpolanco Dec 3, 2022
7c8d9e5
speed up pod deletion
rrpolanco Dec 3, 2022
9e2f86c
validate storage class resource is present in the cluster
rrpolanco Dec 7, 2022
263427f
Change output message
rrpolanco Dec 7, 2022
0659ab5
Add todo
rrpolanco Dec 7, 2022
573e996
Update flags and README
rrpolanco Dec 7, 2022
9d3323d
Update text for pod-ready-timeout flag
rrpolanco Dec 7, 2022
25f7358
Add Ethan's suggestion
rrpolanco Dec 7, 2022
b8382ca
--skip-source-validation works with preflight validation
rrpolanco Dec 9, 2022
92cb2b7
Simplify deleteTmpPVC()
rrpolanco Dec 14, 2022
726a160
Update README with --delete-pv-timeout flag
rrpolanco Dec 14, 2022
54ba092
fix timeout flags
rrpolanco Dec 14, 2022
df71c22
log name of the tempt pvc and not the struct
rrpolanco Dec 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion README.md
Expand Up @@ -3,12 +3,26 @@
pvmigrate allows migrating PVCs between two StorageClasses by creating new PVs, copying over the data, and then changing
PVCs to refer to the new PVs.

## Preflight Validation

`pvmigrate` will run preflight migration validation to catch any potential failures prior to the migration.

Currently supported validations are:
- Checking for existence of storage classes
- Checking existing PVC access modes are supported on the destination storage provider

## Examples

To migrate PVs from the 'default' StorageClass to mynewsc:

```bash
pvmigrate --source-sc default --dest-sc mynewsc
pvmigrate --source-sc "default" --dest-sc "mynewsc"
```

To run preflight migration validation without actually running the migration operation:

```bash
pvmigrate --source-sc "source" --dest-sc "destination" --preflight-validation-only
```

## Flags
Expand All @@ -22,6 +36,9 @@ pvmigrate --source-sc default --dest-sc mynewsc
| --set-defaults | Bool | | false | change default storage class from source to dest |
| --verbose-copy | Bool | | false | show output from the rsync command used to copy data between PVCs |
| --skip-source-validation | Bool | | false | migrate from PVCs using a particular StorageClass name, even if that StorageClass does not exist |
| --preflight-validation-only | Bool | | false | skip the migration and run preflight validation only |
| --skip-preflight-validation | Bool | | false | skip preflight migration validation on the destination storage provider |
| --pod-ready-timeout | time.Duration | | 60 seconds | length of time to wait (in seconds) for validation pod(s) to go into Ready phase |

## Process

Expand Down
5 changes: 3 additions & 2 deletions cmd/main.go
Expand Up @@ -7,6 +7,7 @@ import (
"log"
"os"
"os/signal"
"time"

"github.com/replicatedhq/pvmigrate/pkg/migrate"
"github.com/replicatedhq/pvmigrate/pkg/preflight"
Expand Down Expand Up @@ -34,8 +35,8 @@ func main() {
flag.BoolVar(&options.SetDefaults, "set-defaults", false, "change default storage class from source to dest")
flag.BoolVar(&options.VerboseCopy, "verbose-copy", false, "show output from the rsync command used to copy data between PVCs")
flag.BoolVar(&options.SkipSourceValidation, "skip-source-validation", false, "migrate from PVCs using a particular StorageClass name, even if that StorageClass does not exist")
flag.IntVar(&options.PodReadyTimeout, "pod-ready-timeout", 60, "length of time to wait (in seconds) for volume validation pod(s) to go into Ready phase")
flag.BoolVar(&skipPreflightValidation, "skip-preflight-validation", false, "skip the volume access modes validation on the destination storage provider")
flag.DurationVar(&options.PodReadyTimeout, "pod-ready-timeout", 60*time.Second, "length of time to wait (in seconds) for validation pod(s) to go into Ready phase")
flag.BoolVar(&skipPreflightValidation, "skip-preflight-validation", false, "skip preflight migration validation on the destination storage provider")
flag.BoolVar(&preflightValidationOnly, "preflight-validation-only", false, "skip the migration and run preflight validation only")

flag.Parse()
Expand Down
8 changes: 4 additions & 4 deletions pkg/migrate/migrate.go
Expand Up @@ -24,7 +24,7 @@ const (
scaleAnnotation = baseAnnotation + "-scale"
kindAnnotation = baseAnnotation + "-kind"
sourceNsAnnotation = baseAnnotation + "-sourcens"
sourcePvcAnnotation = baseAnnotation + "-sourcepvc"
sourcePVCAnnotation = baseAnnotation + "-sourcepvc"
desiredReclaimAnnotation = baseAnnotation + "-reclaim"
)

Expand All @@ -47,7 +47,7 @@ type Options struct {
SetDefaults bool
VerboseCopy bool
SkipSourceValidation bool
PodReadyTimeout int
PodReadyTimeout time.Duration
}

// Migrate moves data and PVCs from one StorageClass to another
Expand Down Expand Up @@ -537,7 +537,6 @@ func getPVCs(ctx context.Context, w *log.Logger, clientset k8sclient.Interface,
return matchingPVCs, pvcNamespaces, nil
}

// TODO: move this to the validation package
func validateStorageClasses(ctx context.Context, w *log.Logger, clientset k8sclient.Interface, sourceSCName string, destSCName string, skipSourceValidation bool) error {
// get storage providers
storageClasses, err := clientset.StorageV1().StorageClasses().List(ctx, metav1.ListOptions{})
Expand Down Expand Up @@ -579,6 +578,7 @@ const nameSuffix = "-pvcmigrate"
// 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) <= 63 {
Expand Down Expand Up @@ -957,7 +957,7 @@ func swapPVs(ctx context.Context, w *log.Logger, clientset k8sclient.Interface,
err = mutatePV(ctx, w, clientset, migratedPVC.Spec.VolumeName, func(volume *corev1.PersistentVolume) *corev1.PersistentVolume {
// add annotations describing what PVC this data came from in case of a failure later
volume.Annotations[sourceNsAnnotation] = ns
volume.Annotations[sourcePvcAnnotation] = pvcName
volume.Annotations[sourcePVCAnnotation] = pvcName

// this will be used to set the reclaim policy after attaching a new PVC
volume.Annotations[desiredReclaimAnnotation] = string(originalReclaim)
Expand Down
2 changes: 1 addition & 1 deletion pkg/migrate/migrate_test.go
Expand Up @@ -1389,7 +1389,7 @@ func Test_swapPVs(t *testing.T) {
Annotations: map[string]string{
desiredReclaimAnnotation: "Delete",
sourceNsAnnotation: "testns",
sourcePvcAnnotation: "sourcepvc",
sourcePVCAnnotation: "sourcepvc",
"testannotation": "dest-pv",
},
},
Expand Down
43 changes: 41 additions & 2 deletions pkg/preflight/validate.go
Expand Up @@ -41,12 +41,23 @@ type ValidationFailure struct {

// Validate runs preflight check on storage volumes returning a list of failures
func Validate(ctx context.Context, w *log.Logger, clientset k8sclient.Interface, options migrate.Options) ([]ValidationFailure, error) {
// validate storage classes
scFailures, err := validateStorageClasses(ctx, w, clientset, options.SourceSCName, options.DestSCName)
if err != nil {
return nil, fmt.Errorf("failed to validate storage classes: %w", err)
}

// if there are storage class validation failures it doesn't make sense to proceed
if scFailures != nil {
return scFailures, nil
}

// validate access modes for all PVCs using the source storage class
pvcs, err := pvcsForStorageClass(ctx, w, clientset, options.SourceSCName, options.Namespace)
if err != nil {
return nil, fmt.Errorf("failed to get PVCs for storage %s: %w", options.SourceSCName, err)
}
pvcAccesModeFailures, err := validateVolumeAccessModes(ctx, w, clientset, options.DestSCName, options.RsyncImage, time.Duration(options.PodReadyTimeout)*time.Second, pvcs)
pvcAccesModeFailures, err := validateVolumeAccessModes(ctx, w, clientset, options.DestSCName, options.RsyncImage, options.PodReadyTimeout, pvcs)
if err != nil {
return nil, fmt.Errorf("failed to validate PVC access modes: %w", err)
}
Expand All @@ -56,7 +67,7 @@ func Validate(ctx context.Context, w *log.Logger, clientset k8sclient.Interface,
// PrintPVAccessModeErrors prints and formats the volume access mode errors in pvcErrors
func PrintValidationFailures(stream io.Writer, failures []ValidationFailure) {
tw := tabwriter.NewWriter(stream, 0, 8, 8, '\t', 0)
fmt.Fprintf(tw, "The following resources cannot be migrated:\n\n")
fmt.Fprintf(tw, "The following resources failed validation:\n")
fmt.Fprintln(tw, "NAMESPACE\tRESOURCE\tMESSAGE")
fmt.Fprintf(tw, "---------\t--------\t-------\n")
for _, failure := range failures {
Expand Down Expand Up @@ -98,6 +109,34 @@ func validateVolumeAccessModes(ctx context.Context, l *log.Logger, client k8scli
return volAccessModeFailures, nil
}

// validateStorageClasses returns any failures encountered when discovering the source and destination
// storage classes
func validateStorageClasses(ctx context.Context, l *log.Logger, clientset k8sclient.Interface, sourceSCName, destSCName string) ([]ValidationFailure, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you copy the logic from the migrate version more precisely? This drops support for the 'skip-source-validation' flag

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

// get storage providers
storageClasses, err := clientset.StorageV1().StorageClasses().List(ctx, metav1.ListOptions{})
if err != nil {
return nil, fmt.Errorf("failed to list storage classes: %w", err)
}

var sourceScFound, destScFound bool
var scFailures []ValidationFailure
for _, sc := range storageClasses.Items {
if sc.Name == sourceSCName {
sourceScFound = true
}
if sc.Name == destSCName {
destScFound = true
}
}
if !sourceScFound {
scFailures = append(scFailures, ValidationFailure{Resource: "sc/" + sourceSCName, Message: "Resource not found"})
}
if !destScFound {
scFailures = append(scFailures, ValidationFailure{Resource: "sc/" + destSCName, Message: "Resource not found"})
}
return scFailures, nil
}

// buildTmpPVCConsumerPod creates a pod spec for consuming a pvc
func buildTmpPVCConsumerPod(pvcName, namespace, image string) *corev1.Pod {
return &corev1.Pod{
Expand Down
78 changes: 78 additions & 0 deletions pkg/preflight/validate_test.go
Expand Up @@ -868,3 +868,81 @@ func Test_pvcsForStorageClass(t *testing.T) {
})
}
}

func Test_validateStorageClasses(t *testing.T) {
for _, tt := range []struct {
name string
resources []runtime.Object
sourceSC string
destSC string
wantErr bool
expected []ValidationFailure
}{
{
name: "When both StorageClasses exist and are distinct expect no failures",
sourceSC: "sourcesc",
destSC: "destsc",
resources: []runtime.Object{
&storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: "sourcesc",
},
},
&storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: "destsc",
},
},
},
},
{
name: "When source storage class does not exist expect validation failure",
sourceSC: "sourcesc",
destSC: "destsc",
resources: []runtime.Object{
&storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: "destsc",
},
},
},
expected: []ValidationFailure{
{
Resource: "sc/sourcesc",
Message: "Resource not found",
},
},
},
{
name: "When destination storage class does not exist expect validation failure",
sourceSC: "sourcesc",
destSC: "destsc",
resources: []runtime.Object{
&storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: "sourcesc",
},
},
},
expected: []ValidationFailure{
{
Resource: "sc/destsc",
Message: "Resource not found",
},
},
},
} {
t.Run(tt.name, func(t *testing.T) {
req := require.New(t)
clientset := fake.NewSimpleClientset(tt.resources...)
logger := log.New(io.Discard, "", 0)
result, err := validateStorageClasses(context.Background(), logger, clientset, tt.sourceSC, tt.destSC)
if !tt.wantErr {
req.NoError(err)
} else {
req.Error(err)
}
req.Equal(result, tt.expected)
})
}
}