Skip to content

Commit 7700122

Browse files
jharrodjharrod
committed
Luks format enhancements
Update cryptsetup LUKS format options for increased performance Clear formatting upon LUKS format failures and timeouts
1 parent 58bd13b commit 7700122

File tree

8 files changed

+143
-9
lines changed

8 files changed

+143
-9
lines changed

‎mocks/mock_utils/mock_devices/mock_devices_client.go‎

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎utils/devices/devices.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ type Devices interface {
7777
) error
7878
RemoveMultipathDeviceMappingWithRetries(ctx context.Context, devicePath string, retries uint64,
7979
sleep time.Duration) error
80+
ClearFormatting(ctx context.Context, devicePath string) error
8081
}
8182

8283
type SizeGetter interface {
@@ -852,6 +853,57 @@ func (c *Client) ScanTargetLUN(ctx context.Context, deviceAddresses []models.Scs
852853
return nil
853854
}
854855

856+
// ClearFormatting clears any formatting on the device. Use with caution.
857+
// This is a destructive operation and should only be used when you are sure you want to clear the device.
858+
// The original purpose for this was to clear incomplete LUKS formatting where the inital format errored or timed out.
859+
// This allows us to retry the LUKS format. If we don't clear the formatting, we encounter an issue where we think
860+
// the device is already formatted and don't attempt to format it again.
861+
func (c *Client) ClearFormatting(ctx context.Context, devicePath string) error {
862+
err := c.wipeFs(ctx, devicePath)
863+
if err != nil {
864+
// Log happens in wipeFs()
865+
return err
866+
}
867+
err = c.zeroDeviceHeader(ctx, devicePath)
868+
if err != nil {
869+
// Log happens in zeroDeviceHeader()
870+
return err
871+
}
872+
return nil
873+
}
874+
875+
// zeroDeviceHeader zeros the first 4 KiB of the device header.
876+
func (c *Client) zeroDeviceHeader(ctx context.Context, devicePath string) error {
877+
Logc(ctx).WithField("device", devicePath).Debug(">>>> devices.zeroDeviceHeader")
878+
defer Logc(ctx).Debug("<<<< devices.zeroDeviceHeader")
879+
880+
const zeroDeviceTimeout = 5 * time.Second
881+
args := []string{"if=/dev/zero", "of=" + devicePath, "bs=4096", "count=512", "status=none"}
882+
out, err := c.command.ExecuteWithTimeout(ctx, "dd", zeroDeviceTimeout, false, args...)
883+
if err != nil {
884+
Logc(ctx).WithFields(LogFields{"error": err, "output": string(out), "device": devicePath}).
885+
Error("Failed to zero the device.")
886+
return fmt.Errorf("failed to zero the device %v; %v", devicePath, string(out))
887+
}
888+
889+
return nil
890+
}
891+
892+
// wipeFs wipes the filesystem signature from the device. This is a destructive action.
893+
func (c *Client) wipeFs(ctx context.Context, devicePath string) error {
894+
Logc(ctx).WithField("device", devicePath).Debug(">>>> devices.wipeFs")
895+
defer Logc(ctx).Debug("<<<< devices.wipeFs")
896+
897+
const wipeFsTimeout = 10 * time.Second
898+
out, err := c.command.ExecuteWithTimeout(ctx, "wipefs", wipeFsTimeout, false, "-a", devicePath)
899+
if err != nil {
900+
Logc(ctx).WithFields(LogFields{"error": err, "output": string(out), "device": devicePath}).
901+
Error("Failed to wipe the filesystem signature.")
902+
return fmt.Errorf("failed to wipe FS %v: %v", devicePath, string(out))
903+
}
904+
return nil
905+
}
906+
855907
// convertToDeviceAddressValue converts the device address value to string.
856908
func convertToDeviceAddressValue(value int) string {
857909
if value < 0 {

‎utils/devices/devices_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,3 +1023,55 @@ func TestRemoveMultipathDeviceMappingWithRetries(t *testing.T) {
10231023
})
10241024
}
10251025
}
1026+
1027+
func TestClearFormatting(t *testing.T) {
1028+
devicePath := "/dev/mock-0"
1029+
tests := map[string]struct {
1030+
getMockCmd func() exec.Command
1031+
expectError bool
1032+
}{
1033+
"Formatting cleared successfully": {
1034+
getMockCmd: func() exec.Command {
1035+
mockCommand := mockexec.NewMockCommand(gomock.NewController(t))
1036+
mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "wipefs", 10*time.Second, false, "-a", devicePath).
1037+
Return([]byte{}, nil).Times(1)
1038+
mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "dd", 5*time.Second, false, "if=/dev/zero", "of="+devicePath, "bs=4096", "count=512", "status=none").
1039+
Return([]byte{}, nil).Times(1)
1040+
return mockCommand
1041+
},
1042+
expectError: false,
1043+
},
1044+
"Error clearing filesystem signature": {
1045+
getMockCmd: func() exec.Command {
1046+
mockCommand := mockexec.NewMockCommand(gomock.NewController(t))
1047+
mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "wipefs", 10*time.Second, false, "-a", devicePath).
1048+
Return(nil, fmt.Errorf("wipefs error")).Times(1)
1049+
return mockCommand
1050+
},
1051+
expectError: true,
1052+
},
1053+
"Error zeroing device header": {
1054+
getMockCmd: func() exec.Command {
1055+
mockCommand := mockexec.NewMockCommand(gomock.NewController(t))
1056+
mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "wipefs", 10*time.Second, false, "-a", devicePath).
1057+
Return([]byte{}, nil).Times(1)
1058+
mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "dd", 5*time.Second, false, "if=/dev/zero", "of="+devicePath, "bs=4096", "count=512", "status=none").
1059+
Return(nil, fmt.Errorf("dd error")).Times(1)
1060+
return mockCommand
1061+
},
1062+
expectError: true,
1063+
},
1064+
}
1065+
1066+
for name, params := range tests {
1067+
t.Run(name, func(t *testing.T) {
1068+
deviceClient := NewDetailed(params.getMockCmd(), afero.NewMemMapFs(), NewDiskSizeGetter())
1069+
err := deviceClient.ClearFormatting(context.Background(), devicePath)
1070+
if params.expectError {
1071+
assert.Error(t, err)
1072+
} else {
1073+
assert.NoError(t, err)
1074+
}
1075+
})
1076+
}
1077+
}

‎utils/devices/luks/luks.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,10 @@ func (d *LUKSDevice) EnsureDeviceMappedOnHost(ctx context.Context, name string,
8787
"luks-passphrase-name": luksPassphraseName,
8888
}).Info("Opening encrypted volume.")
8989
luksFormatted, err := d.EnsureFormattedAndOpen(ctx, luksPassphrase)
90-
if err == nil {
91-
return luksFormatted, nil
90+
91+
// If we fail due to a format issue there is no need to try to open the device.
92+
if err == nil || errors.IsFormatError(err) {
93+
return luksFormatted, err
9294
}
9395

9496
// If we failed to open, try previous passphrase

‎utils/devices/luks/luks_linux.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
)
2020

2121
const (
22-
luksCommandTimeout time.Duration = time.Second * 60
22+
luksCommandTimeout time.Duration = time.Second * 30
2323

2424
luksCypherMode = "aes-xts-plain64"
2525
luksType = "luks2"
@@ -102,7 +102,8 @@ func (d *LUKSDevice) format(ctx context.Context, luksPassphrase string) error {
102102

103103
if output, err := d.command.ExecuteWithTimeoutAndInput(
104104
ctx, "cryptsetup", luksCommandTimeout, true, luksPassphrase, "luksFormat", device,
105-
"--type", "luks2", "-c", "aes-xts-plain64",
105+
"--type", "luks2", "-c", "aes-xts-plain64", "--hash", "sha256", "--pbkdf", "pbkdf2",
106+
"--pbkdf-force-iterations", "1000",
106107
); err != nil {
107108
Logc(ctx).WithFields(LogFields{
108109
"device": device,
@@ -137,6 +138,15 @@ func (d *LUKSDevice) formatUnformattedDevice(ctx context.Context, luksPassphrase
137138

138139
// Attempt LUKS format.
139140
if err := d.format(ctx, luksPassphrase); err != nil {
141+
if errors.IsFormatError(err) {
142+
Logc(ctx).Debug("Failed to format LUKS device. Clearing partial formatting.")
143+
// We clear the formatting here to allow retires, else we would detect the device as already formatted
144+
// or dirty and not retry the format on the next nodeStage attempt.
145+
clearFormatErr := d.devices.ClearFormatting(ctx, d.rawDevicePath)
146+
if clearFormatErr != nil {
147+
Logc(ctx).WithError(clearFormatErr).Error("Failed to clear LUKS format. Format retries may fail.")
148+
}
149+
}
140150
return fmt.Errorf("failed to LUKS format device; %w", err)
141151
}
142152

‎utils/devices/luks/luks_linux_test.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ func mockCryptsetupLuksFormat(mock *mockexec.MockCommand) *gomock.Call {
3333
return mock.EXPECT().ExecuteWithTimeoutAndInput(
3434
gomock.Any(), "cryptsetup", luksCommandTimeout, true, gomock.Any(),
3535
"luksFormat", gomock.Any(), "--type", "luks2", "-c", "aes-xts-plain64",
36+
"--hash", "sha256", "--pbkdf", "pbkdf2", "--pbkdf-force-iterations", "1000",
3637
)
3738
}
3839

@@ -294,6 +295,7 @@ func TestEnsureLUKSDevice_IsOpen(t *testing.T) {
294295
func TestEnsureLUKSDevice_LUKSFormatFails(t *testing.T) {
295296
mockCtrl := gomock.NewController(t)
296297
mockCommand := mockexec.NewMockCommand(mockCtrl)
298+
rawDevicePath := "/dev/sdb"
297299

298300
// Setup mock calls and reassign any clients to their mock counterparts.
299301
mockCryptsetupLuksStatus(mockCommand).Return([]byte{},
@@ -304,8 +306,9 @@ func TestEnsureLUKSDevice_LUKSFormatFails(t *testing.T) {
304306

305307
mockDevices := mock_devices.NewMockDevices(mockCtrl)
306308
mockDevices.EXPECT().IsDeviceUnformatted(gomock.Any(), gomock.Any()).Return(true, nil)
309+
mockDevices.EXPECT().ClearFormatting(gomock.Any(), rawDevicePath).Return(nil)
307310

308-
luksDevice := NewDetailed("/dev/sdb", devicePrefix+"pvc-test", mockCommand, mockDevices, afero.NewMemMapFs())
311+
luksDevice := NewDetailed(rawDevicePath, devicePrefix+"pvc-test", mockCommand, mockDevices, afero.NewMemMapFs())
309312

310313
luksFormatted, err := luksDevice.ensureLUKSDevice(context.Background(), "mysecretlukspassphrase")
311314
assert.Error(t, err)

‎utils/iscsi/iscsi_linux_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ tcp: [4] 127.0.0.2:3260,1029 ` + targetIQN + ` (non-flash)`
7272
"config").Return([]byte(multipathConfig("no", false)), nil)
7373
mockCommand.EXPECT().Execute(context.TODO(), "iscsiadm", "-m",
7474
"session").Return([]byte(iscsiadmSessionOutput), nil)
75-
mockCommand.EXPECT().ExecuteWithTimeoutAndInput(context.TODO(), "cryptsetup", time.Minute, true, "",
75+
mockCommand.EXPECT().ExecuteWithTimeoutAndInput(context.TODO(), "cryptsetup", 30*time.Second, true, "",
7676
"status", "/dev/mapper/luks-test-volume")
7777
return mockCommand
7878
},
@@ -164,7 +164,7 @@ tcp: [4] 127.0.0.2:3260,1029 ` + targetIQN + ` (non-flash)`
164164
"config").Return([]byte(multipathConfig("no", false)), nil)
165165
mockCommand.EXPECT().Execute(context.TODO(), "iscsiadm", "-m",
166166
"session").Return([]byte(iscsiadmSessionOutput), nil)
167-
mockCommand.EXPECT().ExecuteWithTimeoutAndInput(context.TODO(), "cryptsetup", time.Minute, true, "",
167+
mockCommand.EXPECT().ExecuteWithTimeoutAndInput(context.TODO(), "cryptsetup", 30*time.Second, true, "",
168168
"status",
169169
"/dev/mapper/luks-test-volume")
170170
return mockCommand

‎utils/nvme/nvme_linux_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -601,10 +601,11 @@ func TestNVMeMountVolume(t *testing.T) {
601601
getMockCommand: func(ctrl *gomock.Controller) exec.Command {
602602
mockCommand := mockexec.NewMockCommand(ctrl)
603603
mockCommand.EXPECT().ExecuteWithTimeoutAndInput(
604-
gomock.Any(), "cryptsetup", time.Minute, true, "", "status",
604+
gomock.Any(), "cryptsetup", 30*time.Second, true, "", "status",
605605
"/dev/mapper/luks-mockName").Return([]byte{}, mockexec.NewMockExitError(4,
606606
"device does not exist"))
607-
mockCommand.EXPECT().ExecuteWithTimeoutAndInput(gomock.Any(), "cryptsetup", time.Minute, true, "", "status",
607+
mockCommand.EXPECT().ExecuteWithTimeoutAndInput(gomock.Any(), "cryptsetup", 30*time.Second, true, "",
608+
"status",
608609
"/dev/mapper/luks-mockName").Return([]byte{}, nil)
609610
return mockCommand
610611
},

0 commit comments

Comments
 (0)