Skip to content

Commit

Permalink
[ISSUE-656] Support SLES 15 SP3 in CSI (#657)
Browse files Browse the repository at this point in the history
Signed-off-by: Nikita Mikhnenko <nikita_mikhnenko@dell.com>
  • Loading branch information
Aber4Nod authored Dec 16, 2021
1 parent 0b28cab commit 3c4d226
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 102 deletions.
5 changes: 5 additions & 0 deletions .github/.codecov.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
coverage:
status:
project:
default:
threshold: 1%
79 changes: 79 additions & 0 deletions docs/kernel-distribution-dependenant-tools.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
Currently we are using specific system calls in the node daemonset, which in some times depends on kernel version.

For example, parted command at Ubuntu18 acts differently from such host OS as Ubuntu20 or SLES15SP3. Under the hood it uses, `udevadm settle` command two times, which has by default timeout in 120 seconds and these commands hangs:

strace results (from CONTAINER):
```
strace -tT udevadm settle
03:34:46 close(3) = 0 <0.000093>
03:34:46 openat(AT_FDCWD, "/proc/self/stat", O_RDONLY|O_CLOEXEC) = 3 <0.000185>
03:34:46 fstat(3, {st_mode=S_IFREG|0444, st_size=0, ...}) = 0 <0.000036>
03:34:46 read(3, "1187761 (udevadm) R 1187759 1187"..., 1024) = 326 <0.000049>
03:34:46 close(3) = 0 <0.000722>
03:34:46 getuid() = 0 <0.000472>
03:34:46 socket(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC|SOCK_NONBLOCK, 0) = 3 <0.000047>
03:34:46 setsockopt(3, SOL_SOCKET, SO_PASSCRED, [1], 4) = 0 <0.000057>
03:34:46 connect(3, {sa_family=AF_UNIX, sun_path="/run/udev/control"}, 19) = 0 <0.000156>
03:34:46 sendto(3, "udev-237\0\0\0\0\0\0\0\0\352\35\255\336\7\0\0\0\0\0\0\0\0\0\0\0"..., 280, 0, NULL, 0) = 280 <0.000055>
03:34:46 poll([{fd=3, events=POLLIN}], 1, 120000 <--- hanging here
```

strace results for same command from host on which container are running:
```
strace -tT udevadm settle
00:36:35 getpid() = 2822353 <0.000026>
00:36:35 openat(AT_FDCWD, "/proc/self/stat", O_RDONLY|O_CLOEXEC) = 3 <0.000046>
00:36:35 fstat(3, {st_mode=S_IFREG|0444, st_size=0, ...}) = 0 <0.000040>
00:36:35 read(3, "2822353 (udevadm) R 2822350 2822"..., 1024) = 327 <0.000044>
00:36:35 ioctl(3, TCGETS, 0x7fff3bcc2370) = -1 ENOTTY (Inappropriate ioctl for device) <0.000028>
00:36:35 read(3, "", 1024) = 0 <0.000021>
00:36:35 close(3) = 0 <0.000033>
00:36:35 newfstatat(AT_FDCWD, "/proc/1/root", {st_mode=S_IFDIR|0755, st_size=156, ...}, 0) = 0 <0.000039>
00:36:35 newfstatat(AT_FDCWD, "/", {st_mode=S_IFDIR|0755, st_size=156, ...}, 0) = 0 <0.000027>
00:36:35 getuid() = 0 <0.000025>
00:36:35 socket(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC|SOCK_NONBLOCK, 0) = 3 <0.000036>
00:36:35 setsockopt(3, SOL_SOCKET, SO_PASSCRED, [1], 4) = 0 <0.000028>
00:36:35 connect(3, {sa_family=AF_UNIX, sun_path="/run/udev/control"}, 20) = 0 <0.000048>
00:36:35 sendto(3, "udev-246\0\0\0\0\0\0\0\0\352\35\255\336\7\0\0\0\0\0\0\0\0\0\0\0"..., 280, 0, NULL, 0) = 280 <0.000031>
00:36:35 sendto(3, "udev-246\0\0\0\0\0\0\0\0\352\35\255\336\0\0\0\0\0\0\0\0\0\0\0\0"..., 280, 0, NULL, 0) = 280 <0.000028>
00:36:35 epoll_create1(EPOLL_CLOEXEC) = 4 <0.000028>
00:36:35 gettid() = 2822353 <0.000026>
00:36:35 epoll_ctl(4, EPOLL_CTL_ADD, 3, {EPOLLIN, {u32=73863808, u64=94055562678912}}) = 0 <0.000027>
...
```

And so, partprobe has the same udevadm commands executed as child processes.

Currently there is one workaround, which we are using to handle this situation: https://github.com/dell/csi-baremetal/blob/master/docs/proposals/specific-node-kernel-version.md.
With specific-node-kernel-version we choose the image version, which we need for node daemonset depends on host os version.
But as we discovered in case of SLES15SP3, we need not only node-kernel but distribution as well, which becomes cumbersome in perspective.

With issue https://github.com/dell/csi-baremetal/issues/656 we moved from udev based tools (parted, partprobe) to not based ones (sgdisk, blockdev).
This changes helps us to support cross kernel host/guest dependency up to now. And currently there is no need in specific-node-kernel-version:
https://github.com/dell/csi-baremetal/issues/660

While completing the issue https://github.com/dell/csi-baremetal/issues/656 we used following versions of udev:

udev version at SLES15SP2:

S | Name | Type | Version | Arch | Repository
---|-----------------------|---------|--------------|--------|------------------------------------
i | libudev1 | package | 234-24.93.1 | x86_64 | SLE-Module-Basesystem15-SP2-Updates
i | udev | package | 234-24.93.1 | x86_64 | SLE-Module-Basesystem15-SP2-Updates


udev version at SLES15SP3:

S | Name | Type | Version | Arch | Repository
---|-----------------------|---------|---------------|--------|------------------------------------
i | libudev1 | package | 246.16-7.21.1 | x86_64 | SLE-Module-Basesystem15-SP3-Updates
i | udev | package | 246.16-7.21.1 | x86_64 | SLE-Module-Basesystem15-SP3-Updates

udev based tool worked fine with host OS - SLES15SP2 and guest - Ubuntu18. But with host os SLES15SP3, guest's udev based tools start to hang.

In basic there are some differences which was done to udev package at SLES15SP3. As shown at strace output below, epoll reactor is used insted of poll mechanism at udev settle command.

In future it's worth to consider completely move away from tools to use directly sysfs, /dev catalogue and /run/udev/data for discovery as it is done in https://github.com/minio/direct-csi
Issue for this proposal: https://github.com/dell/csi-baremetal/issues/661
53 changes: 20 additions & 33 deletions pkg/base/linuxutils/partitionhelper/partition_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ type WrapPartition interface {
IsPartitionExists(device, partNum string) (exists bool, err error)
GetPartitionTableType(device string) (ptType string, err error)
CreatePartitionTable(device, partTableType string) (err error)
CreatePartition(device, label string) (err error)
CreatePartition(device, label, partUUID string, setUUID bool) (err error)
DeletePartition(device, partNum string) (err error)
SetPartitionUUID(device, partNum, partUUID string) error
GetPartitionUUID(device, partNum string) (string, error)
SyncPartitionTable(device string) error
GetPartitionNameByUUID(device, partUUID string) (string, error)
Expand All @@ -48,33 +47,33 @@ type WrapPartition interface {
const (
// PartitionGPT is the const for GPT partition table
PartitionGPT = "gpt"
// parted is a name of system util
parted = "parted "
// partprobe is a name of system util
partprobe = "partprobe "
// sgdisk is a name of system util
sgdisk = "sgdisk "
// fdiks is a name of system util
fdisk = "fdisk "
// blockdev is a name of system util
blockdev = "blockdev "

// PartprobeDeviceCmdTmpl check that device has partition cmd
PartprobeDeviceCmdTmpl = partprobe + "-d -s %s"
// PartprobeCmdTmpl check device has partition with partprobe cmd
PartprobeCmdTmpl = partprobe + "%s"
// BlockdevCmdTmpl synchronize the partition table
BlockdevCmdTmpl = blockdev + "--rereadpt -v %s"

// CreatePartitionTableCmdTmpl create partition table on provided device of provided type cmd template
// fill device and partition table type
CreatePartitionTableCmdTmpl = parted + "-s %s mklabel %s"
CreatePartitionTableCmdTmpl = sgdisk + "%s -o"
// CreatePartitionCmdTmpl create partition on provided device cmd template, fill device and partition label
CreatePartitionCmdTmpl = parted + "-s %s mkpart --align optimal %s 0%% 100%%"
CreatePartitionCmdTmpl = sgdisk + "-a1 -n 1:0:0 -c 1:%s %s"
// CreatePartitionCmdWithUUIDTmpl create partition on provided device with uuid cmd template, fill device and partition label
CreatePartitionCmdWithUUIDTmpl = sgdisk + "-a1 -n 1:0:0 -c 1:%s -u 1:%s %s"
// DeletePartitionCmdTmpl delete partition from provided device cmd template, fill device and partition number
DeletePartitionCmdTmpl = parted + "-s %s rm %s"
DeletePartitionCmdTmpl = sgdisk + "-d %s %s"

// DetectPartitionTableCmdTmpl is used to print information, which contain partition table
DetectPartitionTableCmdTmpl = fdisk + "--list %s"

// SetPartitionUUIDCmdTmpl command for set GUID of the partition, fill device, part number and part UUID
SetPartitionUUIDCmdTmpl = sgdisk + "%s --partition-guid=%s:%s"
// GetPartitionUUIDCmdTmpl command for read GUID of the first partition, fill device and part number
GetPartitionUUIDCmdTmpl = sgdisk + "%s --info=%s"
)
Expand Down Expand Up @@ -138,10 +137,10 @@ func (p *WrapPartitionImpl) CreatePartitionTable(device, partTableType string) e
device, partTableType)
}

cmd := fmt.Sprintf(CreatePartitionTableCmdTmpl, device, partTableType)
cmd := fmt.Sprintf(CreatePartitionTableCmdTmpl, device)
_, _, err := p.e.RunCmd(cmd,
command.UseMetrics(true),
command.CmdName(strings.TrimSpace(fmt.Sprintf(CreatePartitionTableCmdTmpl, "", ""))))
command.CmdName(strings.TrimSpace(fmt.Sprintf(CreatePartitionTableCmdTmpl, ""))))

if err != nil {
return fmt.Errorf("unable to create partition table for device %s", device)
Expand Down Expand Up @@ -175,8 +174,11 @@ func (p *WrapPartitionImpl) GetPartitionTableType(device string) (string, error)
// CreatePartition creates partition with name partName on a device
// Receives device path to create a partition
// Returns error if something went wrong
func (p *WrapPartitionImpl) CreatePartition(device, label string) error {
cmd := fmt.Sprintf(CreatePartitionCmdTmpl, device, label)
func (p *WrapPartitionImpl) CreatePartition(device, label, partUUID string, setUUID bool) error {
cmd := fmt.Sprintf(CreatePartitionCmdTmpl, label, device)
if setUUID {
cmd = fmt.Sprintf(CreatePartitionCmdWithUUIDTmpl, label, partUUID, device)
}

p.opMutex.Lock()
_, _, err := p.e.RunCmd(cmd,
Expand All @@ -195,7 +197,7 @@ func (p *WrapPartitionImpl) CreatePartition(device, label string) error {
// Receives device path and it's partition which should be deleted
// Returns error if something went wrong
func (p *WrapPartitionImpl) DeletePartition(device, partNum string) error {
cmd := fmt.Sprintf(DeletePartitionCmdTmpl, device, partNum)
cmd := fmt.Sprintf(DeletePartitionCmdTmpl, partNum, device)

p.opMutex.Lock()
_, stderr, err := p.e.RunCmd(cmd,
Expand All @@ -211,21 +213,6 @@ func (p *WrapPartitionImpl) DeletePartition(device, partNum string) error {
return nil
}

// SetPartitionUUID writes partUUID as GUID for the partition partNum of a provided device
// Receives device path and partUUID as strings
// Returns error if something went wrong
func (p *WrapPartitionImpl) SetPartitionUUID(device, partNum, partUUID string) error {
cmd := fmt.Sprintf(SetPartitionUUIDCmdTmpl, device, partNum, partUUID)

if _, _, err := p.e.RunCmd(cmd,
command.UseMetrics(true),
command.CmdName(strings.TrimSpace(fmt.Sprintf(SetPartitionUUIDCmdTmpl, "", "", "")))); err != nil {
return err
}

return nil
}

// GetPartitionUUID reads partition unique GUID from the partition partNum of a provided device
// Receives device path from which to read
// Returns unique GUID as a string or error if something went wrong
Expand Down Expand Up @@ -268,12 +255,12 @@ func (p *WrapPartitionImpl) GetPartitionUUID(device, partNum string) (string, er
// Receives device path to sync with partprobe, device could be an empty string (sync for all devices in the system)
// Returns error if something went wrong
func (p *WrapPartitionImpl) SyncPartitionTable(device string) error {
cmd := fmt.Sprintf(PartprobeCmdTmpl, device)
cmd := fmt.Sprintf(BlockdevCmdTmpl, device)

p.opMutex.Lock()
_, _, err := p.e.RunCmd(cmd,
command.UseMetrics(true),
command.CmdName(strings.TrimSpace(fmt.Sprintf(PartprobeCmdTmpl, ""))))
command.CmdName(strings.TrimSpace(fmt.Sprintf(BlockdevCmdTmpl, ""))))
p.opMutex.Unlock()

if err != nil {
Expand Down
19 changes: 6 additions & 13 deletions pkg/base/linuxutils/partitionhelper/partitionhelper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,18 @@ func TestCreatePartitionTableFail(t *testing.T) {
}

func TestCreatePartition(t *testing.T) {
err := testPartitioner.CreatePartition("/dev/sde", testCSILabel)
err := testPartitioner.CreatePartition("/dev/sde", testCSILabel, testPartUUID, true)
assert.Nil(t, err)

err = testPartitioner.CreatePartition("/dev/sde", testCSILabel, "", false)
assert.Nil(t, err)
}

func TestCreatePartitionFail(t *testing.T) {
err := testPartitioner.CreatePartition("/dev/sdf", testCSILabel)
err := testPartitioner.CreatePartition("/dev/sdf", testCSILabel, testPartUUID, true)
assert.NotNil(t, err)

err = testPartitioner.CreatePartition("/dev/sdww", testCSILabel)
err = testPartitioner.CreatePartition("/dev/sdww", testCSILabel, testPartUUID, true)
assert.NotNil(t, err)
}

Expand All @@ -98,16 +101,6 @@ func TestDeletePartitionFail(t *testing.T) {
assert.NotNil(t, err)
}

func TestSetPartitionUUID(t *testing.T) {
err := testPartitioner.SetPartitionUUID("/dev/sda", testPartNum, testPartUUID)
assert.Nil(t, err)
}

func TestSetPartitionUUIDFail(t *testing.T) {
err := testPartitioner.SetPartitionUUID("/dev/sdb", testPartNum, testPartUUID)
assert.NotNil(t, err)
}

func TestGetPartitionUUID(t *testing.T) {
uuid, err := testPartitioner.GetPartitionUUID("/dev/sda", testPartNum)
assert.Equal(t, "64be631b-62a5-11e9-a756-00505680d67f", uuid)
Expand Down
36 changes: 18 additions & 18 deletions pkg/mocks/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,35 +57,35 @@ var DiskCommands = map[string]CmdOut{
Stderr: "",
Err: errors.New("unable to check partition existence for /dev/sdd"),
},
"partprobe -d -s /dev/sde": EmptyOutSuccess,
"partprobe /dev/sde": EmptyOutSuccess,
"partprobe /dev/sda": EmptyOutSuccess,
"partprobe -d -s /dev/sde": EmptyOutSuccess,
"blockdev --rereadpt -v /dev/sde": EmptyOutSuccess,
"partprobe -d -s /dev/sdqwe": {
Stdout: "",
Stderr: "",
Err: errors.New("unable to get partition table"),
},
"partprobe": EmptyOutSuccess,
"parted -s /dev/sda mklabel gpt": EmptyOutSuccess,
"parted -s /dev/sdd mklabel gpt": {
Stdout: "",
"sgdisk /dev/sda -o": EmptyOutSuccess,
"sgdisk /dev/sdc -o": EmptyOutSuccess,
"sgdisk -d 1 /dev/sda": {
Stdout: "The operation has completed successfully.",
Stderr: "",
Err: errors.New("unable to create partition table"),
Err: nil,
},
"parted -s /dev/sdc mklabel gpt": EmptyOutSuccess,
"parted -s /dev/sda rm 1": EmptyOutSuccess,
"parted -s /dev/sdb rm 1": EmptyOutFail,
"parted -s /dev/sde mkpart --align optimal CSI 0% 100%": EmptyOutSuccess,
"parted -s /dev/sdf mkpart --align optimal CSI 0% 100%": EmptyOutFail,
"sgdisk /dev/sda --partition-guid=1:64be631b-62a5-11e9-a756-00505680d67f": {
Stdout: "The operation has completed successfully.",
"sgdisk -a1 -n 1:0:0 -c 1:CSI -u 1:64be631b-62a5-11e9-a756-00505680d67f /dev/sde": {
Stdout: `Creating new GPT entries.
Setting name!
partNum is 0
The operation has completed successfully`,
Stderr: "",
Err: nil,
},
"sgdisk /dev/sdb --partition-guid=1:64be631b-62a5-11e9-a756-00505680d67f": {
Stdout: "The operation has completed successfully.",
"sgdisk -a1 -n 1:0:0 -c 1:CSI /dev/sde": {
Stdout: `Creating new GPT entries.
Setting name!
partNum is 0
The operation has completed successfully`,
Stderr: "",
Err: Err,
Err: nil,
},
"sgdisk /dev/sda --info=1": {
Stdout: `Partition GUID code: 0FC63DAF-8483-4772-8E79-3D69D8477DE4 (Linux filesystem)
Expand Down
11 changes: 2 additions & 9 deletions pkg/mocks/linuxutils/partitionhelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ func (m *MockWrapPartition) CreatePartitionTable(device, partTableType string) (
}

// CreatePartition is a mock implementations
func (m *MockWrapPartition) CreatePartition(device, label string) (err error) {
args := m.Mock.Called(device, label)
func (m *MockWrapPartition) CreatePartition(device, label, partUUID string, setUUID bool) (err error) {
args := m.Mock.Called(device, label, partUUID, setUUID)

return args.Error(0)
}
Expand All @@ -74,13 +74,6 @@ func (m *MockWrapPartition) DeletePartition(device, partNum string) (err error)
return args.Error(0)
}

// SetPartitionUUID is a mock implementations
func (m *MockWrapPartition) SetPartitionUUID(device, partNum, partUUID string) error {
args := m.Mock.Called(device, partNum, partUUID)

return args.Error(0)
}

// GetPartitionUUID is a mock implementations
func (m *MockWrapPartition) GetPartitionUUID(device, partNum string) (string, error) {
args := m.Mock.Called(device, partNum)
Expand Down
2 changes: 0 additions & 2 deletions pkg/node/Dockerfile.build
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,3 @@ ADD health_probe health_probe

# Remove bash packet to get rid of related CVEs
RUN apt update --no-install-recommends -y -q; apt remove --no-install-recommends -y --allow-remove-essential -q bash; apt install --no-install-recommends -y -q util-linux parted xfsprogs lvm2 gdisk strace udev net-tools


4 changes: 1 addition & 3 deletions pkg/node/provisioners/utilwrappers/partition_operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (d *PartitionOperationsImpl) PreparePartition(p Partition) (*Partition, err
}

// create partition
if err = d.CreatePartition(p.Device, p.Label); err != nil {
if err = d.CreatePartition(p.Device, p.Label, p.PartUUID, !p.Ephemeral); err != nil {
return nil, fmt.Errorf("unable to create partition: %v", err)
}
_ = d.SyncPartitionTable(p.Device)
Expand All @@ -135,8 +135,6 @@ func (d *PartitionOperationsImpl) PreparePartition(p Partition) (*Partition, err
if err != nil {
return nil, fmt.Errorf("unable to get partition UUID for ephemeral volume: %v", err)
}
} else if err = d.SetPartitionUUID(p.Device, p.Num, p.PartUUID); err != nil {
return nil, fmt.Errorf("unable to set partition UUID: %v", err)
}

p.Name = d.SearchPartName(p.Device, p.PartUUID)
Expand Down
Loading

0 comments on commit 3c4d226

Please sign in to comment.