Merge pull request #1735 from jlebon/pr/re-retry

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/blackbox_test.go

@@ -278,24 +278,24 @@ func outer(t *testing.T, test types.Test, negativeTests bool) error {
 	appendEnv = append(appendEnv, "IGNITION_SYSTEM_CONFIG_DIR="+systemConfigDir)
 
 	if !negativeTests {
-		if err := runIgnition(t, ctx, "fetch", "", tmpDirectory, appendEnv); err != nil {
+		if err := runIgnition(t, ctx, "fetch", "", tmpDirectory, appendEnv, test.SkipCriticalCheck); err != nil {
 			return err
 		}
 
-		if err := runIgnition(t, ctx, "disks", "", tmpDirectory, appendEnv); err != nil {
+		if err := runIgnition(t, ctx, "disks", "", tmpDirectory, appendEnv, test.SkipCriticalCheck); err != nil {
 			return err
 		}
 
 		if err := mountPartition(ctx, rootPartition); err != nil {
 			return err
 		}
 
-		if err := runIgnition(t, ctx, "mount", rootPartition.MountPath, tmpDirectory, appendEnv); err != nil {
+		if err := runIgnition(t, ctx, "mount", rootPartition.MountPath, tmpDirectory, appendEnv, test.SkipCriticalCheck); err != nil {
 			return err
 		}
 
-		filesErr := runIgnition(t, ctx, "files", rootPartition.MountPath, tmpDirectory, appendEnv)
-		if err := runIgnition(t, ctx, "umount", rootPartition.MountPath, tmpDirectory, appendEnv); err != nil {
+		filesErr := runIgnition(t, ctx, "files", rootPartition.MountPath, tmpDirectory, appendEnv, test.SkipCriticalCheck)
+		if err := runIgnition(t, ctx, "umount", rootPartition.MountPath, tmpDirectory, appendEnv, test.SkipCriticalCheck); err != nil {
 			return err
 		}
 		if err := umountPartition(rootPartition); err != nil {
@@ -318,24 +318,24 @@ func outer(t *testing.T, test types.Test, negativeTests bool) error {
 		}
 		return nil
 	} else {
-		if err := runIgnition(t, ctx, "fetch", "", tmpDirectory, appendEnv); err != nil {
+		if err := runIgnition(t, ctx, "fetch", "", tmpDirectory, appendEnv, test.SkipCriticalCheck); err != nil {
 			return nil // error is expected
 		}
 
-		if err := runIgnition(t, ctx, "disks", "", tmpDirectory, appendEnv); err != nil {
+		if err := runIgnition(t, ctx, "disks", "", tmpDirectory, appendEnv, test.SkipCriticalCheck); err != nil {
 			return nil // error is expected
 		}
 
 		if err := mountPartition(ctx, rootPartition); err != nil {
 			return err
 		}
 
-		if err := runIgnition(t, ctx, "mount", rootPartition.MountPath, tmpDirectory, appendEnv); err != nil {
+		if err := runIgnition(t, ctx, "mount", rootPartition.MountPath, tmpDirectory, appendEnv, test.SkipCriticalCheck); err != nil {
 			return nil // error is expected
 		}
 
-		filesErr := runIgnition(t, ctx, "files", rootPartition.MountPath, tmpDirectory, appendEnv)
-		if err := runIgnition(t, ctx, "umount", rootPartition.MountPath, tmpDirectory, appendEnv); err != nil {
+		filesErr := runIgnition(t, ctx, "files", rootPartition.MountPath, tmpDirectory, appendEnv, test.SkipCriticalCheck)
+		if err := runIgnition(t, ctx, "umount", rootPartition.MountPath, tmpDirectory, appendEnv, test.SkipCriticalCheck); err != nil {
 			return nil
 		}
 		if err := umountPartition(rootPartition); err != nil {

tests/filesystem.go

@@ -19,6 +19,7 @@ import (
 	"bytes"
 	"compress/bzip2"
 	"context"
+	"crypto/rand"
 	"encoding/base64"
 	"fmt"
 	"io"
@@ -115,7 +116,7 @@ func umountPartition(p *types.Partition) error {
 }
 
 // returns true if no error, false if error
-func runIgnition(t *testing.T, ctx context.Context, stage, root, cwd string, appendEnv []string) error {
+func runIgnition(t *testing.T, ctx context.Context, stage, root, cwd string, appendEnv []string, skipCriticalCheck bool) error {
 	args := []string{"-platform", "file", "-stage", stage,
 		"-root", root, "-log-to-stdout",
 		"-config-cache", filepath.Join(cwd, "ignition.json"),
@@ -140,7 +141,7 @@ func runIgnition(t *testing.T, ctx context.Context, stage, root, cwd string, app
 	if strings.Contains(string(out), "panic") {
 		return fmt.Errorf("ignition panicked")
 	}
-	if strings.Contains(string(out), "CRITICAL") {
+	if !skipCriticalCheck && strings.Contains(string(out), "CRITICAL") {
 		return fmt.Errorf("found critical ignition log")
 	}
 	return err
@@ -226,6 +227,22 @@ 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
+		}
+		if _, err := f.Write(bytes); err != nil {
+			return err
+		}
+		defer f.Close()
+	}
+
 	return nil
 }
 

tests/positive/partitions/wipe.go

@@ -0,0 +1,65 @@
+// 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,
+		// the first `sgdisk --zap-all` is expected to fail
+		SkipCriticalCheck: true,
+	}
+}

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
@@ -103,6 +104,7 @@ type Test struct {
 	ConfigMinVersion  string
 	ConfigVersion     string
 	ConfigShouldBeBad bool // Set to true to skip config validation step
+	SkipCriticalCheck bool // Set to true to skip critical logging check
 }
 
 func (ps Partitions) GetPartition(label string) *Partition {