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 -->