Skip to content

Commit

Permalink
fix: detect CD devices, fix user disks wipe test
Browse files Browse the repository at this point in the history
Detect CD devices, and set size to 0 for CD without media.

In user disk wipe tests, skip device mapper devices and CD-ROM.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
  • Loading branch information
smira committed Jun 10, 2024
1 parent aca475c commit 7cbdce7
Show file tree
Hide file tree
Showing 15 changed files with 137 additions and 41 deletions.
1 change: 1 addition & 0 deletions api/resource/definitions/block/block.proto
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ message DiskSpec {
string sub_system = 10;
string transport = 11;
bool rotational = 12;
bool cdrom = 13;
}

// SystemDiskSpec is the spec for SystemDisks status.
Expand Down
1 change: 1 addition & 0 deletions api/storage/storage.proto
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ message Disk {
HDD = 2;
NVME = 3;
SD = 4;
CD = 5;
}
// Type is a type of the disk: nvme, ssd, hdd, sd card.
DiskType type = 9;
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ require (
github.com/siderolabs/gen v0.5.0
github.com/siderolabs/go-api-signature v0.3.2
github.com/siderolabs/go-blockdevice v0.4.7
github.com/siderolabs/go-blockdevice/v2 v2.0.0-20240607145058-1a51f162a09e
github.com/siderolabs/go-blockdevice/v2 v2.0.0-20240610010119-f4a4030394f4
github.com/siderolabs/go-circular v0.2.0
github.com/siderolabs/go-cmd v0.1.1
github.com/siderolabs/go-copy v0.1.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -661,8 +661,8 @@ github.com/siderolabs/go-api-signature v0.3.2 h1:blqrZF1GM7TWgq7mY7CsR+yQ93u6az0
github.com/siderolabs/go-api-signature v0.3.2/go.mod h1:punhUOaXa7LELYBRCUhfgUGH6ieVz68GrP98apCKXj8=
github.com/siderolabs/go-blockdevice v0.4.7 h1:2bk4WpEEflGxjrNwp57ye24Pr+cYgAiAeNMWiQOuWbQ=
github.com/siderolabs/go-blockdevice v0.4.7/go.mod h1:4PeOuk71pReJj1JQEXDE7kIIQJPVe8a+HZQa+qjxSEA=
github.com/siderolabs/go-blockdevice/v2 v2.0.0-20240607145058-1a51f162a09e h1:PQhtHJj3zwaqehthq0fs2TyW8bW/mlOYoHfZIeSYQ3M=
github.com/siderolabs/go-blockdevice/v2 v2.0.0-20240607145058-1a51f162a09e/go.mod h1:5GnL7VLNp5/vgiwYP74fi6KuTUfqGcRxQxtto2tzD+I=
github.com/siderolabs/go-blockdevice/v2 v2.0.0-20240610010119-f4a4030394f4 h1:78l10So9aPgv8VLmccaPfLTLmyTT4vdkF1MNmIcAGDg=
github.com/siderolabs/go-blockdevice/v2 v2.0.0-20240610010119-f4a4030394f4/go.mod h1:5GnL7VLNp5/vgiwYP74fi6KuTUfqGcRxQxtto2tzD+I=
github.com/siderolabs/go-circular v0.2.0 h1:Xca8zrjF/YsujLbwDSojkKzJe7ngetnpuIJn8N78DJI=
github.com/siderolabs/go-circular v0.2.0/go.mod h1:rrYCwHLYWmxqrmZP+LjYtwB2a55lxzQi0Ztu1VpWZSc=
github.com/siderolabs/go-cmd v0.1.1 h1:nTouZUSxLeiiEe7hFexSVvaTsY/3O8k1s08BxPRrsps=
Expand Down
11 changes: 11 additions & 0 deletions go.work
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
go 1.22.4

use (
.
./hack/cloud-image-uploader
./hack/docgen
./hack/gotagsrewrite
./hack/module-sig-verify
./hack/structprotogen
./pkg/machinery
)
9 changes: 9 additions & 0 deletions internal/app/machined/pkg/controllers/block/disks.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ func (ctrl *DisksController) analyzeBlockDevice(ctx context.Context, r controlle
return nil
}

defer bd.Close() //nolint:errcheck

size, err := bd.GetSize()
if err != nil || size == 0 {
return nil //nolint:nilerr
Expand All @@ -128,6 +130,12 @@ func (ctrl *DisksController) analyzeBlockDevice(ctx context.Context, r controlle
return nil
}

isCD := bd.IsCD()
if isCD && bd.IsCDNoMedia() {
// Linux reports non-zero size for CD-ROMs even when there is no media.
size = 0
}

ioSize, err := bd.GetIOSize()
if err != nil {
logger.Debug("failed to get io size", zap.Error(err))
Expand All @@ -152,6 +160,7 @@ func (ctrl *DisksController) analyzeBlockDevice(ctx context.Context, r controlle
d.TypedSpec().IOSize = ioSize
d.TypedSpec().SectorSize = sectorSize
d.TypedSpec().Readonly = readOnly
d.TypedSpec().CDROM = isCD

d.TypedSpec().Model = props.Model
d.TypedSpec().Serial = props.Serial
Expand Down
2 changes: 2 additions & 0 deletions internal/app/storaged/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ func (s *Server) Disks(ctx context.Context, in *emptypb.Empty) (reply *storage.D
var diskType storage.Disk_DiskType

switch {
case d.TypedSpec().CDROM:
diskType = storage.Disk_CD
case d.TypedSpec().Transport == "nvme":
diskType = storage.Disk_NVME
case d.TypedSpec().Transport == "mmc":
Expand Down
19 changes: 15 additions & 4 deletions internal/integration/api/reset.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,24 +122,35 @@ func (suite *ResetSuite) TestResetWithSpecEphemeral() {
}, true)
}

// TestResetWithSpecState resets only state partition on the node.
// TestResetWithSpecStateAndUserDisks resets state partition and user disks on the node.
//
// As ephemeral partition is not reset, so kubelet cert shouldn't change.
func (suite *ResetSuite) TestResetWithSpecState() {
func (suite *ResetSuite) TestResetWithSpecStateAndUserDisks() {
if suite.Capabilities().SecureBooted {
// this is because in secure boot mode, the machine config is only applied and cannot be passed as kernel args
suite.T().Skip("skipping as talos is explicitly trusted booted")
}

node := suite.RandomDiscoveredNodeInternalIP()
node := suite.RandomDiscoveredNodeInternalIP(machine.TypeWorker)

disks, err := suite.Client.Disks(client.WithNode(suite.ctx, node))
suite.Require().NoError(err)
suite.Require().NotEmpty(disks.Messages)

userDisksToWipe := xslices.Map(
xslices.Filter(disks.Messages[0].Disks, func(disk *storage.Disk) bool {
return !disk.SystemDisk
switch {
case disk.SystemDisk:
return false
case disk.Type == storage.Disk_UNKNOWN, disk.Type == storage.Disk_CD, disk.Type == storage.Disk_SD:
return false
case disk.Readonly:
return false
case disk.BusPath == "/virtual":
return false
}

return true
}),
func(disk *storage.Disk) string {
return disk.DeviceName
Expand Down
11 changes: 9 additions & 2 deletions internal/integration/api/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,20 @@ func (suite *VolumesSuite) TestDisks() {
continue
}

suite.Assert().NotEmpty(disk.TypedSpec().Size, "disk: %s", disk.Metadata().ID())
if !disk.TypedSpec().CDROM {
suite.Assert().NotEmpty(disk.TypedSpec().Size, "disk: %s", disk.Metadata().ID())
}

suite.Assert().NotEmpty(disk.TypedSpec().IOSize, "disk: %s", disk.Metadata().ID())
suite.Assert().NotEmpty(disk.TypedSpec().SectorSize, "disk: %s", disk.Metadata().ID())

if suite.Cluster != nil {
// running on our own provider, transport should be always detected
suite.Assert().NotEmpty(disk.TypedSpec().Transport, "disk: %s", disk.Metadata().ID())
if disk.TypedSpec().BusPath == "/virtual" {
suite.Assert().Empty(disk.TypedSpec().Transport, "disk: %s", disk.Metadata().ID())
} else {
suite.Assert().NotEmpty(disk.TypedSpec().Transport, "disk: %s", disk.Metadata().ID())
}
}

diskNames = append(diskNames, disk.Metadata().ID())
Expand Down
7 changes: 6 additions & 1 deletion internal/integration/base/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/siderolabs/talos/pkg/cluster/check"
"github.com/siderolabs/talos/pkg/machinery/api/common"
machineapi "github.com/siderolabs/talos/pkg/machinery/api/machine"
"github.com/siderolabs/talos/pkg/machinery/api/storage"
"github.com/siderolabs/talos/pkg/machinery/client"
clientconfig "github.com/siderolabs/talos/pkg/machinery/client/config"
"github.com/siderolabs/talos/pkg/machinery/config"
Expand Down Expand Up @@ -507,7 +508,11 @@ func (apiSuite *APISuite) UserDisks(ctx context.Context, node string, sizeGreate

for _, msg := range resp.Messages {
for _, disk := range msg.Disks {
if disk.SystemDisk {
if disk.SystemDisk || disk.Readonly || disk.Type == storage.Disk_CD {
continue
}

if disk.BusPath == "/virtual" {
continue
}

Expand Down
29 changes: 19 additions & 10 deletions pkg/machinery/api/resource/definitions/block/block.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 33 additions & 0 deletions pkg/machinery/api/resource/definitions/block/block_vtproto.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 25 additions & 21 deletions pkg/machinery/api/storage/storage.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/machinery/resources/block/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type DiskSpec struct {
SectorSize uint `yaml:"sectorSize" protobuf:"3"`

Readonly bool `yaml:"readonly" protobuf:"4"`
CDROM bool `yaml:"cdrom" protobuf:"13"`

Model string `yaml:"model,omitempty" protobuf:"5"`
Serial string `yaml:"serial,omitempty" protobuf:"6"`
Expand Down
2 changes: 2 additions & 0 deletions website/content/v1.8/reference/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,7 @@ DiskSpec is the spec for Disks status.
| sub_system | [string](#string) | | |
| transport | [string](#string) | | |
| rotational | [bool](#bool) | | |
| cdrom | [bool](#bool) | | |



Expand Down Expand Up @@ -8107,6 +8108,7 @@ DisksResponse represents the response of the `Disks` RPC.
| HDD | 2 | |
| NVME | 3 | |
| SD | 4 | |
| CD | 5 | |


<!-- end enums -->
Expand Down

0 comments on commit 7cbdce7

Please sign in to comment.