From 12f455b76eb36ee84f190f80db5ae813f27e56b7 Mon Sep 17 00:00:00 2001 From: Andrew Jeddeloh Date: Fri, 13 Dec 2019 13:08:10 -0800 Subject: [PATCH 1/4] travis: bump min go to 1.12 We want to be able to use os.ProcessState.ExitCode, see https://golang.org/pkg/os/#ProcessState.ExitCode --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 845c786fd..658d15815 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ language: go go_import_path: github.com/coreos/ignition/v2 go: - - "1.11.x" + - "1.12.x" - "1.13.x" arch: From 1cfb161d9da4c334c89fd077ee5811e5c28f0878 Mon Sep 17 00:00:00 2001 From: Andrew Jeddeloh Date: Fri, 13 Dec 2019 13:09:52 -0800 Subject: [PATCH 2/4] log: use os.ProcessState.ExitCode instead of unix The existing code fails the typecast. Go added a function for what we want to do anyway, use that. --- internal/log/log.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/log/log.go b/internal/log/log.go index cc0b29224..aabc6f4d2 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -20,8 +20,6 @@ import ( "log/syslog" "os/exec" "strings" - - "golang.org/x/sys/unix" ) type LoggerOps interface { @@ -150,7 +148,7 @@ func (l *Logger) LogCmd(cmd *exec.Cmd, format string, a ...interface{}) (int, er cmd.Stderr = stderr if err := cmd.Run(); err != nil { if exitErr, ok := err.(*exec.ExitError); ok { - code = exitErr.Sys().(unix.WaitStatus).ExitStatus() + code = exitErr.ExitCode() } return fmt.Errorf("%v: Cmd: %s Stdout: %q Stderr: %q", err, cmdLine, stdout.Bytes(), stderr.Bytes()) } From 95d0651bf94742945fa88588ce350c25373b63ae Mon Sep 17 00:00:00 2001 From: Andrew Jeddeloh Date: Fri, 13 Dec 2019 13:11:12 -0800 Subject: [PATCH 3/4] blackbox tests: don't swallow errors Don't hide errors we should report. --- tests/blackbox_test.go | 2 +- tests/validator.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/blackbox_test.go b/tests/blackbox_test.go index 14f2530d5..189453e2b 100644 --- a/tests/blackbox_test.go +++ b/tests/blackbox_test.go @@ -332,7 +332,7 @@ func outer(t *testing.T, test types.Test, negativeTests bool) error { return nil } if err := umountPartition(rootPartition); err != nil { - return nil + return err } if filesErr != nil { return nil // error is expected diff --git a/tests/validator.go b/tests/validator.go index 884515295..986420257 100644 --- a/tests/validator.go +++ b/tests/validator.go @@ -183,7 +183,7 @@ func validatePartitionNodes(t *testing.T, ctx context.Context, partition *types. defer func() { if err := umountPartition(partition); err != nil { // failing to unmount is not a validation failure - t.Logf("Failed to unmount %s: %v", partition.MountPath, err) + t.Fatalf("Failed to unmount %s: %v", partition.MountPath, err) } }() for _, file := range partition.Files { From 634508423b631b615b881ae284a515ce5f04e112 Mon Sep 17 00:00:00 2001 From: Andrew Jeddeloh Date: Fri, 13 Dec 2019 13:12:39 -0800 Subject: [PATCH 4/4] tests/filesystems: fix error handling Don't swallow unmounting errors. Use os.ProcessState.ExitCode() instead of the current cast to unit.WaitStatus (which fails). --- tests/filesystem.go | 83 ++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 42 deletions(-) diff --git a/tests/filesystem.go b/tests/filesystem.go index 985f9797c..708ff02ed 100644 --- a/tests/filesystem.go +++ b/tests/filesystem.go @@ -49,11 +49,14 @@ func runWithoutContext(command string, args ...string) ([]byte, error) { return out, nil } -func prepareRootPartitionForPasswd(ctx context.Context, root *types.Partition) error { - if err := mountPartition(ctx, root); err != nil { +func prepareRootPartitionForPasswd(ctx context.Context, root *types.Partition) (err error) { + err = mountPartition(ctx, root) + if err != nil { return err } - defer umountPartition(root) + defer func() { + err = umountPartition(root) + }() mountPath := root.MountPath dirs := []string{ @@ -64,26 +67,25 @@ func prepareRootPartitionForPasswd(ctx context.Context, root *types.Partition) e filepath.Join(mountPath, "etc"), } for _, dir := range dirs { - err := os.MkdirAll(dir, 0755) + err = os.MkdirAll(dir, 0755) if err != nil { - return err + return } } symlinks := []string{"lib64", "bin", "sbin"} for _, symlink := range symlinks { - err := os.Symlink( + err = os.Symlink( filepath.Join(mountPath, "usr", symlink), filepath.Join(mountPath, symlink)) if err != nil { - return err + return } } - // TODO: use the architecture, not hardcode amd64 // TODO: needed for user_group_lookup.c - _, err := run(ctx, "cp", "/lib64/libnss_files.so.2", filepath.Join(mountPath, "usr", "lib64")) - return err + _, err = run(ctx, "cp", "/lib64/libnss_files.so.2", filepath.Join(mountPath, "usr", "lib64")) + return } func getRootPartition(partitions []*types.Partition) *types.Partition { @@ -116,11 +118,8 @@ func runGetExit(cmd string, args ...string) (int, string, error) { if !ok { return -1, logs, err } - status, ok2 := exitErr.Sys().(unix.WaitStatus) - if !ok2 { - return -1, logs, err - } - return status.ExitStatus(), logs, nil + status := exitErr.ExitCode() + return status, logs, nil } func umountPartition(p *types.Partition) error { @@ -132,24 +131,17 @@ func umountPartition(p *types.Partition) error { // specific case. See https://github.com/coreos/bootengine/commit/8bf46fe78ec59bcd5148ce9ab8ec5fb805600151 // for more context. for i := 0; i < 3; i++ { - status, logs, err := runGetExit("umount", p.MountPath) - if status == 0 { - return nil - } - if err != nil { - return fmt.Errorf("exec'ing `umount %s` failed: %v", p.MountPath, err) - } - if status != 32 { - return fmt.Errorf("`umount %s` failed with exit status %d: %s", p.MountPath, status, logs) + if err := unix.Unmount(p.MountPath, unix.MNT_FORCE); err != nil { + time.Sleep(time.Second) + continue } - // wait a sec to see if things clear up - time.Sleep(time.Second) - if unmounted, _, err := runGetExit("mountpoint", "-q", p.MountPath); err != nil { return fmt.Errorf("exec'ing `mountpoint -q %s` failed: %v", p.MountPath, err) } else if unmounted == 1 { return nil } + // wait a sec to see if things clear up + time.Sleep(time.Second) } return fmt.Errorf("umount failed after 3 tries (exit status 32) for %s", p.MountPath) } @@ -399,26 +391,33 @@ func removeEmpty(strings []string) []string { return r } +func createFilesForPartition(ctx context.Context, partition *types.Partition) (err error) { + err = mountPartition(ctx, partition) + if err != nil { + return + } + defer func() { + err = umountPartition(partition) + }() + + err = createDirectoriesFromSlice(partition.MountPath, partition.Directories) + if err != nil { + return + } + err = createFilesFromSlice(partition.MountPath, partition.Files) + if err != nil { + return + } + err = createLinksFromSlice(partition.MountPath, partition.Links) + return +} + func createFilesForPartitions(ctx context.Context, partitions []*types.Partition) error { for _, partition := range partitions { if partition.FilesystemType == "swap" || partition.FilesystemType == "" || partition.FilesystemType == "blank" { continue } - if err := mountPartition(ctx, partition); err != nil { - return err - } - defer umountPartition(partition) - - err := createDirectoriesFromSlice(partition.MountPath, partition.Directories) - if err != nil { - return err - } - createFilesFromSlice(partition.MountPath, partition.Files) - if err != nil { - return err - } - createLinksFromSlice(partition.MountPath, partition.Links) - if err != nil { + if err := createFilesForPartition(ctx, partition); err != nil { return err } }