fix: detect CD devices, fix user disks wipe test

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 <[email protected]>
siderolabs · Jun 10, 2024 · 7cbdce7 · 7cbdce7
1 parent aca475c
commit 7cbdce7
Show file tree

Hide file tree

Showing 15 changed files with 137 additions and 41 deletions.
diff --git a/api/resource/definitions/block/block.proto b/api/resource/definitions/block/block.proto
@@ -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.

diff --git a/api/storage/storage.proto b/api/storage/storage.proto
@@ -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;

diff --git a/go.mod b/go.mod
@@ -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

diff --git a/go.sum b/go.sum
@@ -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=

diff --git a/go.work b/go.work
@@ -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
+)
diff --git a/internal/app/machined/pkg/controllers/block/disks.go b/internal/app/machined/pkg/controllers/block/disks.go
@@ -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
@@ -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))
@@ -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

diff --git a/internal/app/storaged/server.go b/internal/app/storaged/server.go
@@ -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":

diff --git a/internal/integration/api/reset.go b/internal/integration/api/reset.go
@@ -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

diff --git a/internal/integration/api/volumes.go b/internal/integration/api/volumes.go
@@ -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())

diff --git a/internal/integration/base/api.go b/internal/integration/base/api.go
@@ -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"
@@ -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
 			}
 

diff --git a/pkg/machinery/api/resource/definitions/block/block.pb.go b/pkg/machinery/api/resource/definitions/block/block.pb.go
diff --git a/pkg/machinery/api/resource/definitions/block/block_vtproto.pb.go b/pkg/machinery/api/resource/definitions/block/block_vtproto.pb.go
diff --git a/pkg/machinery/api/storage/storage.pb.go b/pkg/machinery/api/storage/storage.pb.go
diff --git a/pkg/machinery/resources/block/disk.go b/pkg/machinery/resources/block/disk.go
@@ -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"`

diff --git a/website/content/v1.8/reference/api.md b/website/content/v1.8/reference/api.md
@@ -836,6 +836,7 @@ DiskSpec is the spec for Disks status.
 | sub_system | [string](#string) |  |  |
 | transport | [string](#string) |  |  |
 | rotational | [bool](#bool) |  |  |
+| cdrom | [bool](#bool) |  |  |
 
 
 
@@ -8107,6 +8108,7 @@ DisksResponse represents the response of the `Disks` RPC.
 | HDD | 2 |  |
 | NVME | 3 |  |
 | SD | 4 |  |
+| CD | 5 |  |
 
 
  <!-- end enums -->