Skip to content

Commit

Permalink
stages/disks: retry sgdisk --zap-all invocation
Browse files Browse the repository at this point in the history
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 #544 and #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
  • Loading branch information
jlebon committed Oct 25, 2023
1 parent 3cb0210 commit 1434a1a
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 5 deletions.
1 change: 1 addition & 0 deletions docs/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion internal/exec/stages/disks/partitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand Down
15 changes: 15 additions & 0 deletions tests/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"bytes"
"compress/bzip2"
"context"
"crypto/rand"
"encoding/base64"
"fmt"
"io"
Expand Down Expand Up @@ -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
}

Expand Down
63 changes: 63 additions & 0 deletions tests/positive/partitions/wipe.go
Original file line number Diff line number Diff line change
@@ -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,
}
}
9 changes: 5 additions & 4 deletions tests/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1434a1a

Please sign in to comment.