Rename Disk stuff -> Drive and teach diskwipe example more tricks (#163)

### What does this PR do

The main reason for this PR is to make the diskwipe example a better
example of both using ironlib to detect hardware presence and
capabilities and using the information to influence actions to be taken.
After collecting all the hardware the new example code detects nvme vs
sata and detects the features supported by the disk all to decide which
is the best tool to wipe the drive.

Along the way many refactors stood out as necessary. The main one was
blkdiscard did not implement the DiskWiper interface. I also noticed we
were using Device when Drive already existed and was more appropriate
and that the args and log lines could do with some homogeneity so that
they don't appear to be originating from different repositories.

Further breakdown of the changes are shown below:

#### Make blkdiscard look more like other DiskWipers & utilities

I missed this when doing review on the original PR, so doing it now.

#### Change wipe examples to all take the same cli args

Best to make the examples all actually usable and there's no point to
have different arguments so lets make them all similar.

#### Rename Disk, device -> Drive, drive

I don't want to have 2 names for the same thing and even worse reuse a
name that means something completely different in a different context.
bmc-toolbox/common already has Drive and Device so lets use Drive and
drive for disks and device for Devices.

#### Make all WipeDrive methods logs look similar

Similar to the cli arg homogeneity argument a few commits ago, lets do
the same thing for log messages in WipeDrive implementations. This makes
it so every line has the drive name, has the wipe method under the same
key and has similar start/error messages.

#### Change WipeDrive to take a common.Drive instead of string

We use a statically typed language so why devolve down to strings when
we also have a better type lying around, so lets use it.

#### Get rid of Nvme.wipe

Now that Nvme.WipeDrive takes a common.Drive that has the capabilities
saved, WipeDrive was just a thin wrapper around wipe. Hoist the code for
wipe into WipeDrive then.

#### Better tests in fill_zero_test

Use stretchr/testify in fill_zero_test for easier to read tests.
Actually test different sized files in fill_zero_test.Test_WipeDrive,
previously there was a bug and all files were 4096 bytes long. The test
were also updated to actually verify that FillZero actually fills the
file with 0s, before it was just ensuring the file was not shrunk or
grown.

#### Make Blkdiscard.WipeDrive & FillZeroCmd.WipeDrive verify the wipe

Nvme wipe has multiple sub-methods for wiping and it verifies the method
worked correctly or tries the next one. FillZeroCmd nor Blkdiscard did
not do this. I was on the fence if we should change Nvme or FillZero and
Blkdiscard and chose to stay with the way Nvme does it because
ultimately, I want callers to not have to worry about this. If a
WipeDrive method returns success then the wipe is good.

#### Make diskwipe a better example

I thought it would be nice to have a diskwipe example that could do a
better job of using ironlib's hardware collection capabilities vs just
statically using a tool, it serves as a better example of using ironlib
vs just being a go wrapper for various unix cli utilities.

So I got rid of the separate blkdiscard example and wipe code from
nvme-wipe (so it just does nvme namespace clearing/creating so I renamed
it to nvme-ns-wipe) and taught diskwipe to know when to use each tool.

### The HW vendor this change applies to (if applicable)

N.A.

### The HW model number, product name this change applies to (if
applicable)

N.A.

### The BMC firmware and/or BIOS versions that this change applies to
(if applicable)

N.A.

### What version of tooling - vendor specific or opensource does this
change depend on (if applicable)

N.A.

### How can this change be tested by a PR reviewer?

These are mostly refactors of code but can mess around with the
examples/diskwipe and examples/nvme-ns-wipe binaries.

### Description for changelog/release notes

Reword changelog/release notes for DiskWiper/WipeDisk ->
DriveWiper/WipeDrive. Reword changelog/release notes regarding the
diskwipe example and how it uses the best tool for the job.

actions/interface.go

@@ -201,7 +201,8 @@ type VirtualDiskManager interface {
 	VirtualDisks(ctx context.Context) ([]*utils.MvcliDevice, error)
 }
 
-// DiskWiper defines an interface to override disk data
-type DiskWiper interface {
-	WipeDisk(ctx context.Context, log *logrus.Logger, logicalName string) error
+// DriveWiper defines an interface to override disk data
+type DriveWiper interface {
+	// WipeDrive wipes away all data from the drive, wipe is always verified to have succeeded
+	WipeDrive(context.Context, *logrus.Logger, *common.Drive) error
 }

actions/storage_controller.go

@@ -78,28 +78,28 @@ func (s *StorageControllerAction) GetControllerUtility(vendorName, modelName str
 }
 
 // GetWipeUtility returns the wipe utility based on the disk wipping features
-func (s *StorageControllerAction) GetWipeUtility(logicalName string) (DiskWiper, error) {
-	s.Logger.Tracef("%s | Detecting wipe utility", logicalName)
+func (s *StorageControllerAction) GetWipeUtility(drive *common.Drive) (DriveWiper, error) {
+	s.Logger.Tracef("%s | Detecting wipe utility", drive.LogicalName)
 	// TODO: use disk wipping features to return the best wipe utility, currently only one available
 
 	return utils.NewFillZeroCmd(s.trace), nil
 }
 
-func (s *StorageControllerAction) WipeDisk(ctx context.Context, log *logrus.Logger, logicalName string) error {
-	util, err := s.GetWipeUtility(logicalName)
+func (s *StorageControllerAction) WipeDrive(ctx context.Context, log *logrus.Logger, drive *common.Drive) error {
+	util, err := s.GetWipeUtility(drive)
 	if err != nil {
 		return err
 	}
 
 	// Watermark disk
 	// Before wiping the disk, we apply watermarks to later verify successful deletion
-	check, err := utils.ApplyWatermarks(logicalName)
+	check, err := utils.ApplyWatermarks(drive)
 	if err != nil {
 		return err
 	}
 
 	// Wipe the disk
-	err = util.WipeDisk(ctx, log, logicalName)
+	err = util.WipeDrive(ctx, log, drive)
 	if err != nil {
 		return err
 	}

examples/diskwipe/.gitignore

@@ -0,0 +1 @@
+diskwipe

examples/diskwipe/main.go

@@ -2,26 +2,101 @@ package main
 
 import (
 	"context"
+	"flag"
+	"strings"
 	"time"
 
+	"github.com/bmc-toolbox/common"
+	"github.com/metal-toolbox/ironlib"
 	"github.com/metal-toolbox/ironlib/actions"
+	"github.com/metal-toolbox/ironlib/utils"
 	"github.com/sirupsen/logrus"
 )
 
-// This example invokes ironlib and wipes the disk /dev/sdZZZ with a timeout of 1 day
+var (
+	defaultTimeout = time.Minute
+
+	logicalName = flag.String("drive", "/dev/someN", "disk to wipe by filling with zeros")
+	timeout     = flag.String("timeout", defaultTimeout.String(), "time to wait for command to complete")
+	verbose     = flag.Bool("verbose", false, "show command runs and output")
+)
 
 func main() {
+	flag.Parse()
+
 	logger := logrus.New()
-	logger.Formatter = new(logrus.JSONFormatter)
-	logger.SetLevel(logrus.TraceLevel)
+	logger.Formatter = new(logrus.TextFormatter)
+	if *verbose {
+		logger.SetLevel(logrus.TraceLevel)
+	}
+	l := logger.WithField("drive", *logicalName)
 
-	sca := actions.NewStorageControllerAction(logger)
-	ctx, cancel := context.WithTimeout(context.Background(), 86400*time.Second)
+	timeout, err := time.ParseDuration(*timeout)
+	if err != nil {
+		l.WithError(err).Fatal("failed to parse timeout duration")
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
 	defer cancel()
 
-	err := sca.WipeDisk(ctx, logger, "/dev/sdZZZ")
+	collector, err := ironlib.New(logger)
+	if err != nil {
+		l.WithError(err).Fatal("exiting")
+	}
+
+	inventory, err := collector.GetInventory(ctx, actions.WithDynamicCollection())
+	if err != nil {
+		l.WithError(err).Fatal("exiting")
+	}
+
+	var drive *common.Drive
+	for _, d := range inventory.Drives {
+		if d.LogicalName == *logicalName {
+			drive = d
+			break
+		}
+	}
+	if drive == nil {
+		l.Fatal("unable to find disk")
+	}
+
+	// Pick the most appropriate wipe based on the disk type and features supported
+	var wiper actions.DriveWiper
+	switch drive.Protocol {
+	case "nvme":
+		wiper = utils.NewNvmeCmd(*verbose)
+	case "sata":
+		// Lets see if drive supports TRIM, if so we'll use blkdiscard
+		for _, cap := range drive.Capabilities {
+			if strings.HasPrefix(cap.Description, "Data Set Management TRIM supported") {
+				if cap.Enabled {
+					wiper = utils.NewBlkdiscardCmd(*verbose)
+				}
+				break
+			}
+		}
+
+		// drive does not support TRIM so we fall back to filling it up with zero
+		if wiper == nil {
+			wiper = utils.NewFillZeroCmd(*verbose)
+
+			// If the user supplied a non-default timeout then we'll honor it, otherwise we just go with a huge timeout.
+			// If this were *real* code and not an example some work could be done to guesstimate a timeout based on disk size.
+			if timeout == defaultTimeout {
+				l.WithField("timeout", timeout.String()).Info("increasing timeout")
+				timeout = 24 * time.Hour
+				ctx, cancel = context.WithTimeout(context.WithoutCancel(ctx), timeout)
+				defer cancel()
+			}
+		}
+	}
+
+	if wiper == nil {
+		l.Fatal("failed find appropriate wiper drive")
+	}
+
+	err = wiper.WipeDrive(ctx, logger, drive)
 	if err != nil {
-		logger.Fatal(err)
+		l.Fatal("failed to wipe drive")
 	}
-	logger.Println("Wiped successfully!")
 }

examples/nvme-ns-wipe/.gitignore

@@ -0,0 +1 @@
+nvme-ns-wipe

examples/blkdiscard/main.go → examples/nvme-ns-wipe/main.go

@@ -10,30 +10,32 @@ import (
 )
 
 var (
-	device  = flag.String("device", "/dev/sdZZZ", "disk to wipe using blkdiscard")
-	timeout = flag.String("timeout", (2 * time.Minute).String(), "time to wait for command to complete")
+	logicalName = flag.String("drive", "/dev/nvmeX", "nvme disk to wipe")
+	timeout     = flag.String("timeout", (1 * time.Minute).String(), "time to wait for command to complete")
+	verbose     = flag.Bool("verbose", false, "show command runs and output")
 )
 
-// This example invokes ironlib and runs blkdiscard on the disk /dev/sdZZZ
 func main() {
 	flag.Parse()
 
 	logger := logrus.New()
 	logger.Formatter = new(logrus.TextFormatter)
-	logger.SetLevel(logrus.TraceLevel)
+	if *verbose {
+		logger.SetLevel(logrus.TraceLevel)
+	}
 
 	timeout, err := time.ParseDuration(*timeout)
 	if err != nil {
 		logger.WithError(err).Fatal("failed to parse timeout duration")
 	}
 
-	blkdiscard := utils.NewBlkdiscardCmd()
+	nvme := utils.NewNvmeCmd(*verbose)
 
 	ctx, cancel := context.WithTimeout(context.Background(), timeout)
 	defer cancel()
 
-	logger.Info("running blkdiscard on ", *device)
-	err = blkdiscard.Discard(ctx, *device)
+	logger.Info("resetting namespaces")
+	err = nvme.ResetNS(ctx, *logicalName)
 	if err != nil {
 		logger.WithError(err).Fatal("exiting")
 	}

utils/blkdiscard.go

@@ -4,6 +4,9 @@ import (
 	"cmp"
 	"context"
 	"os"
+
+	"github.com/bmc-toolbox/common"
+	"github.com/sirupsen/logrus"
 )
 
 const (
@@ -15,26 +18,41 @@ type Blkdiscard struct {
 }
 
 // Return a new blkdiscard executor
-func NewBlkdiscardCmd() *Blkdiscard {
+func NewBlkdiscardCmd(trace bool) *Blkdiscard {
 	// lookup env var for util
 	utility := cmp.Or(os.Getenv(EnvBlkdiscardUtility), "blkdiscard")
 
 	e := NewExecutor(utility)
 	e.SetEnv([]string{"LC_ALL=C.UTF-8"})
 
+	if !trace {
+		e.SetQuiet()
+	}
+
 	return &Blkdiscard{Executor: e}
 }
 
 // Discard runs blkdiscard on the given device (--force is always used)
-func (b *Blkdiscard) Discard(ctx context.Context, device string) error {
-	b.Executor.SetArgs("--force", device)
+func (b *Blkdiscard) Discard(ctx context.Context, drive *common.Drive) error {
+	b.Executor.SetArgs("--force", drive.LogicalName)
 
-	_, err := b.Executor.Exec(ctx)
+	verify, err := ApplyWatermarks(drive)
 	if err != nil {
 		return err
 	}
 
-	return nil
+	_, err = b.Executor.Exec(ctx)
+	if err != nil {
+		return err
+	}
+
+	return verify()
+}
+
+// WipeDrive implements DriveWipe by calling Discard
+func (b *Blkdiscard) WipeDrive(ctx context.Context, logger *logrus.Logger, drive *common.Drive) error {
+	logger.WithField("drive", drive.LogicalName).WithField("method", "blkdiscard").Info("wiping")
+	return b.Discard(ctx, drive)
 }
 
 // NewFakeBlkdiscard returns a mock implementation of the Blkdiscard interface for use in tests.

utils/blkdiscard_test.go

@@ -2,12 +2,28 @@ package utils
 
 import (
 	"context"
+	"os"
 	"testing"
 
+	"github.com/bmc-toolbox/common"
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
 )
 
 func Test_blkdiscard(t *testing.T) {
-	err := NewFakeBlkdiscard().Discard(context.TODO(), "/dev/sdZZZ")
+	logger, hook := test.NewNullLogger()
+	defer hook.Reset()
+
+	d := t.TempDir()
+	tmpfile, err := os.CreateTemp(d, "fake-block-device")
+	assert.NoError(t, err)
+
+	err = os.WriteFile(tmpfile.Name(), make([]byte, 8192), 0o600)
 	assert.NoError(t, err)
+
+	drive := &common.Drive{Common: common.Common{LogicalName: tmpfile.Name()}}
+	err = NewFakeBlkdiscard().WipeDrive(context.TODO(), logger, drive)
+
+	// fake-block-device isn't a blockdevice that supports TRIM so we expect an error
+	assert.Error(t, err)
 }

utils/fill_zero.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"time"
 
+	"github.com/bmc-toolbox/common"
 	"github.com/sirupsen/logrus"
 )
 
@@ -23,12 +24,17 @@ func NewFillZeroCmd(trace bool) *FillZero {
 	return &z
 }
 
-func (z *FillZero) WipeDisk(ctx context.Context, l *logrus.Logger, logicalName string) error {
-	log := l.WithField("device", logicalName)
-	log.Info("starting zero-fill")
+func (z *FillZero) WipeDrive(ctx context.Context, logger *logrus.Logger, drive *common.Drive) error {
+	log := logger.WithField("drive", drive.LogicalName).WithField("method", "zero-fill")
+	log.Info("wiping")
+
+	verify, err := ApplyWatermarks(drive)
+	if err != nil {
+		return err
+	}
 
 	// Write open
-	file, err := os.OpenFile(logicalName, os.O_WRONLY, 0)
+	file, err := os.OpenFile(drive.LogicalName, os.O_WRONLY, 0)
 	if err != nil {
 		return err
 	}
@@ -78,7 +84,8 @@ func (z *FillZero) WipeDisk(ctx context.Context, l *logrus.Logger, logicalName s
 	if err != nil {
 		return err
 	}
-	return nil
+
+	return verify()
 }
 
 func printProgress(log *logrus.Entry, totalBytesWritten, partitionSize int64, start time.Time, bytesSinceLastPrint int64) {