stages/disks: retry sgdisk --zap-all invocation

When `wipeTable` is enabled, we run `sgdisk --zap-all`. But if the table
was corrupted in the first place, `sgdisk` will exit with code 2 which
then breaks boot.

As a workaround, Ignition used to retry the invocation but the context
around it was lost in coreos#544 and coreos#1149 and the retry was removed and
the error-checking was added.

So this patch effectively re-applies 94c98bc ("sgdisk: retry zap-all
operation on failure"), but now with a comment and a test to make sure
we don't regress again.

Closes: coreos/fedora-coreos-tracker#1596

docs/release-notes.md

@@ -21,6 +21,7 @@ nav_order: 9
 ### Bug fixes
 
 - Prevent races with udev after disk editing
+- Don't fail to wipe partition table if it's corrupted
 
 
 ## Ignition 2.16.2 (2023-07-12)

internal/exec/stages/disks/partitions.go

@@ -324,7 +324,12 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error {
 		s.Logger.Info("wiping partition table requested on %q", devAlias)
 		op.WipeTable(true)
 		if err := op.Commit(); err != nil {
-			return err
+			// `sgdisk --zap-all` will exit code 2 if the table was corrupted; retry it
+			// https://github.com/coreos/fedora-coreos-tracker/issues/1596
+			s.Logger.Info("potential error encountered while wiping table... retrying")
+			if err := op.Commit(); err != nil {
+				return err
+			}
 		}
 	}
 

tests/filesystem.go

@@ -19,6 +19,7 @@ import (
 	"bytes"
 	"compress/bzip2"
 	"context"
+	"crypto/rand"
 	"encoding/base64"
 	"fmt"
 	"io"
@@ -226,6 +227,20 @@ func setupDisk(ctx context.Context, disk *types.Disk, diskIndex int, imageSize i
 			return err
 		}
 	}
+
+	if disk.CorruptTable {
+		bytes := make([]byte, 1536)
+		if _, err := rand.Read(bytes); err != nil {
+			return err
+		}
+		f, err := os.OpenFile(disk.ImageFile, os.O_WRONLY, 0666)
+		if err != nil {
+			return err
+		}
+		f.Write(bytes)
+		defer f.Close()
+	}
+
 	return nil
 }
 

tests/positive/partitions/wipe.go

@@ -0,0 +1,63 @@
+// Copyright 2023 CoreOS, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package partitions
+
+import (
+	"github.com/coreos/ignition/v2/tests/register"
+	"github.com/coreos/ignition/v2/tests/types"
+)
+
+func init() {
+	// Tests that wipe tables
+	register.Register(register.PositiveTest, WipeBadTable())
+}
+
+func WipeBadTable() types.Test {
+	name := "partition.wipebadtable"
+	in := append(types.GetBaseDisk(), types.Disk{
+		Alignment: types.IgnitionAlignment,
+		Partitions: types.Partitions{
+			{
+				Label:  "deleteme",
+				Number: 1,
+				Length: 65536,
+			},
+		},
+		CorruptTable: true,
+	})
+	out := append(types.GetBaseDisk(), types.Disk{Alignment: types.IgnitionAlignment})
+	config := `{
+		"ignition": {
+			"version": "$version"
+		},
+		"storage": {
+			"disks": [
+			{
+				"device": "$disk1",
+				"wipeTable": true
+			}
+			]
+		}
+	}`
+	configMinVersion := "3.0.0"
+
+	return types.Test{
+		Name:             name,
+		In:               in,
+		Out:              out,
+		Config:           config,
+		ConfigMinVersion: configMinVersion,
+	}
+}

tests/types/types.go

@@ -57,10 +57,11 @@ type Node struct {
 }
 
 type Disk struct {
-	ImageFile  string
-	Device     string
-	Alignment  int
-	Partitions Partitions
+	ImageFile    string
+	Device       string
+	Alignment    int
+	Partitions   Partitions
+	CorruptTable bool // set to true to corrupt the partition table
 }
 
 type Partitions []*Partition