From 45d3bbb4eeb060e62ce79f82f00cdc919c9dcc0a Mon Sep 17 00:00:00 2001 From: Hakan Memisoglu Date: Tue, 13 Aug 2019 13:40:39 -0700 Subject: [PATCH 1/3] Fix the bug where ListSnapshot failing to query deleted fs --- pkg/blockstorage/awsefs/awsefs.go | 7 +++---- pkg/blockstorage/awsefs/conversion.go | 7 ++++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/blockstorage/awsefs/awsefs.go b/pkg/blockstorage/awsefs/awsefs.go index 26e9ef89d9..a86cbf5c41 100644 --- a/pkg/blockstorage/awsefs/awsefs.go +++ b/pkg/blockstorage/awsefs/awsefs.go @@ -453,10 +453,9 @@ func (e *efs) snapshotsFromRecoveryPoints(ctx context.Context, rps []*backup.Rec if err != nil { return nil, errors.Wrap(err, "Failed to get volume ID from recovery point ARN") } - vol, err := e.VolumeGet(ctx, volID, "") - if err != nil { - return nil, errors.Wrap(err, "Failed to get EFS volume") - } + // VolumeGet might return error since originating filesystem might have + // been deleted. + vol, _ := e.VolumeGet(ctx, volID, "") snap, err := snapshotFromRecoveryPointByVault(rp, vol, tags, e.region) if err != nil { return nil, errors.Wrap(err, "Failed to get snapshot from the vault") diff --git a/pkg/blockstorage/awsefs/conversion.go b/pkg/blockstorage/awsefs/conversion.go index a7ddee2afc..f300c12c71 100644 --- a/pkg/blockstorage/awsefs/conversion.go +++ b/pkg/blockstorage/awsefs/conversion.go @@ -90,8 +90,9 @@ func snapshotFromRecoveryPointByVault(rp *backup.RecoveryPointByBackupVault, vol if rp.RecoveryPointArn == nil { return nil, errors.New("Recovery point has no RecoveryPointArn") } - if volume == nil { - return nil, errors.New("Nil volume as argument") + encrypted := false + if volume != nil { + encrypted = volume.Encrypted } return &blockstorage.Snapshot{ ID: *rp.RecoveryPointArn, @@ -100,7 +101,7 @@ func snapshotFromRecoveryPointByVault(rp *backup.RecoveryPointByBackupVault, vol Region: region, Type: blockstorage.TypeEFS, Volume: volume, - Encrypted: volume.Encrypted, + Encrypted: encrypted, Tags: blockstorage.MapToKeyValue(tags), }, nil } From 138f3448de26b1e8773df72825a0c93b9e0ac763 Mon Sep 17 00:00:00 2001 From: Hakan Memisoglu Date: Tue, 13 Aug 2019 14:10:56 -0700 Subject: [PATCH 2/3] Make isVolumeNotFound recursive and fix the problem in both locations. --- pkg/blockstorage/awsefs/awsefs.go | 9 ++++++--- pkg/blockstorage/awsefs/conversion.go | 6 +++++- pkg/blockstorage/awsefs/error.go | 11 ++++++++--- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/pkg/blockstorage/awsefs/awsefs.go b/pkg/blockstorage/awsefs/awsefs.go index a86cbf5c41..ca6c828ee4 100644 --- a/pkg/blockstorage/awsefs/awsefs.go +++ b/pkg/blockstorage/awsefs/awsefs.go @@ -372,8 +372,8 @@ func (e *efs) SnapshotGet(ctx context.Context, id string) (*blockstorage.Snapsho return nil, errors.Wrap(err, "Failed to get volume ID from recovery point ARN") } vol, err := e.VolumeGet(ctx, volID, "") - if err != nil { - return nil, errors.Wrap(err, "Failed to get originating volume") + if err != nil && !isVolumeNotFound(err) { + return nil, errors.Wrap(err, "Failed to get filesystem") } return snapshotFromRecoveryPoint(resp, vol, e.region) } @@ -455,7 +455,10 @@ func (e *efs) snapshotsFromRecoveryPoints(ctx context.Context, rps []*backup.Rec } // VolumeGet might return error since originating filesystem might have // been deleted. - vol, _ := e.VolumeGet(ctx, volID, "") + vol, err := e.VolumeGet(ctx, volID, "") + if err != nil && !isVolumeNotFound(err) { + return nil, errors.Wrap(err, "Failed to get filesystem") + } snap, err := snapshotFromRecoveryPointByVault(rp, vol, tags, e.region) if err != nil { return nil, errors.Wrap(err, "Failed to get snapshot from the vault") diff --git a/pkg/blockstorage/awsefs/conversion.go b/pkg/blockstorage/awsefs/conversion.go index f300c12c71..55cb2881c1 100644 --- a/pkg/blockstorage/awsefs/conversion.go +++ b/pkg/blockstorage/awsefs/conversion.go @@ -65,6 +65,10 @@ func snapshotFromRecoveryPoint(rp *backup.DescribeRecoveryPointOutput, volume *b if rp.RecoveryPointArn == nil { return nil, errors.New("Recovery point has no RecoveryPointArn") } + encrypted := false + if volume != nil { + encrypted = volume.Encrypted + } return &blockstorage.Snapshot{ ID: *rp.RecoveryPointArn, CreationTime: blockstorage.TimeStamp(*rp.CreationDate), @@ -72,7 +76,7 @@ func snapshotFromRecoveryPoint(rp *backup.DescribeRecoveryPointOutput, volume *b Region: region, Type: blockstorage.TypeEFS, Volume: volume, - Encrypted: volume.Encrypted, + Encrypted: encrypted, Tags: nil, }, nil } diff --git a/pkg/blockstorage/awsefs/error.go b/pkg/blockstorage/awsefs/error.go index a9bd8a38fb..6bbc18ed76 100644 --- a/pkg/blockstorage/awsefs/error.go +++ b/pkg/blockstorage/awsefs/error.go @@ -4,13 +4,18 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/backup" awsefs "github.com/aws/aws-sdk-go/service/efs" + "github.com/pkg/errors" ) func isVolumeNotFound(err error) bool { - if awsErr, ok := err.(awserr.Error); ok { - return awsErr.Code() == awsefs.ErrCodeFileSystemNotFound + switch errV := err.(type) { + case awserr.Error: + return errV.Code() == awsefs.ErrCodeFileSystemNotFound + case errors.Causer: + return isVolumeNotFound(errV.Cause()) + default: + return false } - return false } func isBackupVaultAlreadyExists(err error) bool { From fd08822e2a37800ab74d9e8e1b321f5ebaf99ba0 Mon Sep 17 00:00:00 2001 From: Hakan Memisoglu Date: Tue, 13 Aug 2019 15:34:20 -0700 Subject: [PATCH 3/3] Simplify the error check method --- pkg/blockstorage/awsefs/error.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/blockstorage/awsefs/error.go b/pkg/blockstorage/awsefs/error.go index 6bbc18ed76..c75ca88236 100644 --- a/pkg/blockstorage/awsefs/error.go +++ b/pkg/blockstorage/awsefs/error.go @@ -8,11 +8,9 @@ import ( ) func isVolumeNotFound(err error) bool { - switch errV := err.(type) { + switch errV := errors.Cause(err).(type) { case awserr.Error: return errV.Code() == awsefs.ErrCodeFileSystemNotFound - case errors.Causer: - return isVolumeNotFound(errV.Cause()) default: return false }