diff --git a/mantle/cli/cli.go b/mantle/cli/cli.go index 85ce5fe80b6847e579103d51cde4d9154e9e58ed..cb8c50cfe87f767fe107008029306dcf71c3288c 100644 --- a/mantle/cli/cli.go +++ b/mantle/cli/cli.go @@ -20,6 +20,7 @@ import ( "github.com/coreos/pkg/capnslog" "github.com/spf13/cobra" + "github.com/coreos/coreos-assembler/mantle/kola" "github.com/coreos/coreos-assembler/mantle/system/exec" "github.com/coreos/coreos-assembler/mantle/version" ) @@ -65,7 +66,17 @@ func Execute(main *cobra.Command) { }) if err := main.Execute(); err != nil { - plog.Fatal(err) + // If this isn't a warn:true test failure then go ahead and die + if err != kola.ErrWarnOnTestFail { + plog.Fatal(err) + } + // Now check if the user wants exit code 77 on warn:true test fail + // If they do then we'll exit 77. If they don't we'll do nothing + // and drop out to the os.Exit(0) + if kola.Options.UseWarnExitCode77 { + plog.Warning(err) + os.Exit(77) + } } os.Exit(0) } diff --git a/mantle/cmd/kola/kola.go b/mantle/cmd/kola/kola.go index 0b0998ae8d4faf15513c779603c0abb23ec9730a..44341a1295bddc57346728dc6a38958b91b3bdf8 100644 --- a/mantle/cmd/kola/kola.go +++ b/mantle/cmd/kola/kola.go @@ -23,6 +23,7 @@ import ( "path/filepath" "regexp" "sort" + "strings" "text/tabwriter" "time" @@ -126,7 +127,7 @@ This can be useful for e.g. serving locally built OSTree repos to qemu. runExternals []string runMultiply int runRerunFlag bool - allowRerunSuccess bool + allowRerunSuccess string nonexclusiveWrapperMatch = regexp.MustCompile(`^non-exclusive-test-bucket-[0-9]$`) ) @@ -136,7 +137,7 @@ func init() { cmdRun.Flags().StringArrayVarP(&runExternals, "exttest", "E", nil, "Externally defined tests (will be found in DIR/tests/kola)") cmdRun.Flags().IntVar(&runMultiply, "multiply", 0, "Run the provided tests N times (useful to find race conditions)") cmdRun.Flags().BoolVar(&runRerunFlag, "rerun", false, "re-run failed tests once") - cmdRun.Flags().BoolVar(&allowRerunSuccess, "allow-rerun-success", false, "Allow kola test run to be successful when tests pass during re-run") + cmdRun.Flags().StringVar(&allowRerunSuccess, "allow-rerun-success", "", "Allow kola test run to be successful when tests with given 'tags=...[,...]' pass during re-run") root.AddCommand(cmdList) cmdList.Flags().StringArrayVarP(&runExternals, "exttest", "E", nil, "Externally defined tests in directory") @@ -151,6 +152,7 @@ func init() { cmdRunUpgrade.Flags().BoolVar(&findParentImage, "find-parent-image", false, "automatically find parent image if not provided -- note on qemu, this will download the image") cmdRunUpgrade.Flags().StringVar(&qemuImageDir, "qemu-image-dir", "", "directory in which to cache QEMU images if --fetch-parent-image is enabled") cmdRunUpgrade.Flags().BoolVar(&runRerunFlag, "rerun", false, "re-run failed tests once") + cmdRunUpgrade.Flags().StringVar(&allowRerunSuccess, "allow-rerun-success", "", "Allow kola test run to be successful when tests with given 'tags=...[,...]' pass during re-run") root.AddCommand(cmdRerun) @@ -237,6 +239,27 @@ func runRerun(cmd *cobra.Command, args []string) error { return kolaRunPatterns(patterns, false) } +// parseRerunSuccess converts rerun specification into a tags +func parseRerunSuccess() ([]string, error) { + // In the future we may extend format to something like: [:] + // SELECTOR + // tags=...,...,... + // tests=...,...,... + // OPTIONS + // n=... + // allow-single=.. + tags := []string{} + if len(allowRerunSuccess) == 0 { + return tags, nil + } + if !strings.HasPrefix(allowRerunSuccess, "tags=") { + return nil, fmt.Errorf("invalid rerun spec %s", allowRerunSuccess) + } + split := strings.TrimPrefix(allowRerunSuccess, "tags=") + tags = append(tags, strings.Split(split, ",")...) + return tags, nil +} + func kolaRunPatterns(patterns []string, rerun bool) error { var err error outputDir, err = kola.SetupOutputDir(outputDir, kolaPlatform) @@ -248,7 +271,12 @@ func kolaRunPatterns(patterns []string, rerun bool) error { return err } - runErr := kola.RunTests(patterns, runMultiply, rerun, allowRerunSuccess, kolaPlatform, outputDir, !kola.Options.NoTestExitError) + rerunSuccessTags, err := parseRerunSuccess() + if err != nil { + return err + } + + runErr := kola.RunTests(patterns, runMultiply, rerun, rerunSuccessTags, kolaPlatform, outputDir) // needs to be after RunTests() because harness empties the directory if err := writeProps(); err != nil { @@ -393,7 +421,8 @@ func runList(cmd *cobra.Command, args []string) error { test.ExcludeArchitectures, test.Distros, test.ExcludeDistros, - test.Tags} + test.Tags, + test.Description} item.updateValues() testlist = append(testlist, item) } @@ -402,39 +431,44 @@ func runList(cmd *cobra.Command, args []string) error { return testlist[i].Name < testlist[j].Name }) - if !listJSON { - var w = tabwriter.NewWriter(os.Stdout, 0, 8, 0, '\t', 0) - - fmt.Fprintln(w, "Test Name\tPlatforms\tArchitectures\tDistributions\tTags") - fmt.Fprintln(w, "\t") - for _, item := range testlist { - platformFound := (listPlatform == "all") - if listPlatform != "all" { - for _, platform := range item.Platforms { - if listPlatform == "all" || platform == "all" || platform == listPlatform { - platformFound = true - break - } + var newtestlist []*item + for _, item := range testlist { + platformFound := (listPlatform == "all") + if listPlatform != "all" { + for _, platform := range item.Platforms { + if listPlatform == "all" || platform == "all" || platform == listPlatform { + platformFound = true + break } } + } - distroFound := (listDistro == "all") - if listDistro != "all" { - for _, distro := range item.Distros { - if listDistro == "all" || distro == "all" || distro == listDistro { - distroFound = true - break - } + distroFound := (listDistro == "all") + if listDistro != "all" { + for _, distro := range item.Distros { + if listDistro == "all" || distro == "all" || distro == listDistro { + distroFound = true + break } } + } - if platformFound && distroFound { - fmt.Fprintf(w, "%v\n", item) - } + if platformFound && distroFound { + newtestlist = append(newtestlist, item) + } + } + + if !listJSON { + var w = tabwriter.NewWriter(os.Stdout, 0, 8, 0, '\t', 0) + + fmt.Fprintln(w, "Test Name\tPlatforms\tArchitectures\tDistributions\tTags") + fmt.Fprintln(w, "\t") + for _, item := range newtestlist { + fmt.Fprintf(w, "%v\n", item) } w.Flush() } else { - out, err := json.MarshalIndent(testlist, "", "\t") + out, err := json.MarshalIndent(newtestlist, "", "\t") if err != nil { return errors.Wrapf(err, "marshalling test list") } @@ -453,6 +487,7 @@ type item struct { Distros []string ExcludeDistros []string `json:"-"` Tags []string + Description string } func (i *item) updateValues() { @@ -679,7 +714,7 @@ func runRunUpgrade(cmd *cobra.Command, args []string) error { patterns = args } - runErr := kola.RunUpgradeTests(patterns, runRerunFlag, kolaPlatform, outputDir, !kola.Options.NoTestExitError) + runErr := kola.RunUpgradeTests(patterns, runRerunFlag, kolaPlatform, outputDir) // needs to be after RunTests() because harness empties the directory if err := writeProps(); err != nil { diff --git a/mantle/cmd/kola/options.go b/mantle/cmd/kola/options.go index 012847da49ac2c68b5eebca82d1103ce0aa39892..bfd86894690e0dc870a1784f696079efb4b840b3 100644 --- a/mantle/cmd/kola/options.go +++ b/mantle/cmd/kola/options.go @@ -56,7 +56,7 @@ func init() { root.PersistentFlags().StringVarP(&kola.Options.Distribution, "distro", "b", "", "Distribution: "+strings.Join(kolaDistros, ", ")) root.PersistentFlags().StringVarP(&kolaParallelArg, "parallel", "j", "1", "number of tests to run in parallel, or \"auto\" to match CPU count") sv(&kola.TAPFile, "tapfile", "", "file to write TAP results to") - root.PersistentFlags().BoolVarP(&kola.Options.NoTestExitError, "no-test-exit-error", "T", false, "Don't exit with non-zero if tests fail") + root.PersistentFlags().BoolVarP(&kola.Options.UseWarnExitCode77, "on-warn-failure-exit-77", "", false, "Exit with code 77 if 'warn: true' tests fail") sv(&kola.Options.BaseName, "basename", "kola", "Cluster name prefix") ss("debug-systemd-unit", []string{}, "full-unit-name.service to enable SYSTEMD_LOG_LEVEL=debug on. Can be specified multiple times.") ssv(&kola.DenylistedTests, "denylist-test", []string{}, "Test pattern to add to denylist. Can be specified multiple times.") @@ -68,6 +68,8 @@ func init() { sv(&kola.Options.CosaWorkdir, "workdir", "", "coreos-assembler working directory") sv(&kola.Options.CosaBuildId, "build", "", "coreos-assembler build ID") sv(&kola.Options.CosaBuildArch, "arch", coreosarch.CurrentRpmArch(), "The target architecture of the build") + // we make this a percentage to avoid having to deal with floats + root.PersistentFlags().UintVar(&kola.Options.ExtendTimeoutPercent, "extend-timeout-percentage", 0, "Extend all test timeouts by N percent") // rhcos-specific options sv(&kola.Options.OSContainer, "oscontainer", "", "oscontainer image pullspec for pivot (RHCOS only)") @@ -154,6 +156,7 @@ func init() { bv(&kola.QEMUOptions.Native4k, "qemu-native-4k", false, "Force 4k sectors for main disk") bv(&kola.QEMUOptions.Nvme, "qemu-nvme", false, "Use NVMe for main disk") bv(&kola.QEMUOptions.Swtpm, "qemu-swtpm", true, "Create temporary software TPM") + ssv(&kola.QEMUOptions.BindRO, "qemu-bind-ro", nil, "Inject a host directory; this does not automatically mount in the guest") sv(&kola.QEMUIsoOptions.IsoPath, "qemu-iso", "", "path to CoreOS ISO image") bv(&kola.QEMUIsoOptions.AsDisk, "qemu-iso-as-disk", false, "attach ISO image as regular disk") @@ -216,8 +219,6 @@ func syncOptionsImpl(useCosa bool) error { kola.QEMUOptions.Firmware = "uefi" } else if kola.Options.CosaBuildArch == "x86_64" && kola.QEMUOptions.Native4k { kola.QEMUOptions.Firmware = "uefi" - } else { - kola.QEMUOptions.Firmware = "bios" } } diff --git a/mantle/cmd/kola/testiso.go b/mantle/cmd/kola/testiso.go index 6787f2c90b8f8f503bc6255e4384b07c681b51f9..62a4e6b302d57fda0dd6b6343cdcc6d48cac2732 100644 --- a/mantle/cmd/kola/testiso.go +++ b/mantle/cmd/kola/testiso.go @@ -66,6 +66,11 @@ var ( isOffline bool isISOFromRAM bool + // These tests only run on RHCOS + tests_RHCOS_uefi = []string{ + "iso-fips.uefi", + } + // The iso-as-disk tests are only supported in x86_64 because other // architectures don't have the required hybrid partition table. tests_x86_64 = []string{ @@ -133,9 +138,8 @@ var ( ) const ( - installTimeout = 10 * time.Minute + installTimeoutMins = 10 // https://github.com/coreos/fedora-coreos-config/pull/2544 - //liveISOFromRAMKarg = "coreos.liveiso.fromram" liveISOFromRAMKarg = "nestos.liveiso.fromram" ) @@ -330,7 +334,7 @@ func liveArtifactExistsInBuild() error { return nil } -func getArchPatternsList() []string { +func getAllTests(build *util.LocalBuild) []string { arch := coreosarch.CurrentRpmArch() var tests []string switch arch { @@ -343,6 +347,9 @@ func getArchPatternsList() []string { case "aarch64": tests = tests_aarch64 } + if kola.CosaBuild.Meta.Name == "rhcos" && arch != "s390x" && arch != "ppc64le" { + tests = append(tests, tests_RHCOS_uefi...) + } return tests } @@ -443,7 +450,10 @@ func filterTests(tests []string, patterns []string) ([]string, error) { func runTestIso(cmd *cobra.Command, args []string) error { var err error - tests := getArchPatternsList() + if kola.CosaBuild == nil { + return fmt.Errorf("Must provide --build") + } + tests := getAllTests(kola.CosaBuild) if len(args) != 0 { if tests, err = filterTests(tests, args); err != nil { return err @@ -451,10 +461,7 @@ func runTestIso(cmd *cobra.Command, args []string) error { return harness.SuiteEmpty } } - - if kola.CosaBuild == nil { - return fmt.Errorf("Must provide --build") - } + ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -560,6 +567,8 @@ func runTestIso(cmd *cobra.Command, args []string) error { duration, err = testAsDisk(ctx, filepath.Join(outputDir, test)) case "iso-live-login": duration, err = testLiveLogin(ctx, filepath.Join(outputDir, test)) + case "iso-fips": + duration, err = testLiveFIPS(ctx, filepath.Join(outputDir, test)) case "iso-install", "iso-offline-install", "iso-offline-install-fromram": duration, err = testLiveIso(ctx, inst, filepath.Join(outputDir, test), false) case "miniso-install": @@ -583,8 +592,9 @@ func awaitCompletion(ctx context.Context, inst *platform.QemuInstance, outdir st start := time.Now() errchan := make(chan error) go func() { - time.Sleep(installTimeout) - errchan <- fmt.Errorf("timed out after %v", installTimeout) + timeout := (time.Duration(installTimeoutMins*(100+kola.Options.ExtendTimeoutPercent)) * time.Minute) / 100 + time.Sleep(timeout) + errchan <- fmt.Errorf("timed out after %v", timeout) }() if !console { go func() { @@ -607,6 +617,7 @@ func awaitCompletion(ctx context.Context, inst *platform.QemuInstance, outdir st go func() { err := inst.Wait() // only one Wait() gets process data, so also manually check for signal + plog.Debugf("qemu exited err=%v", err) if err == nil && inst.Signaled() { err = errors.New("process killed") } @@ -637,7 +648,9 @@ func awaitCompletion(ctx context.Context, inst *platform.QemuInstance, outdir st errchan <- fmt.Errorf("Unexpected string from completion channel: %s expected: %s", line, exp) return } + plog.Debugf("Matched expected message %s", exp) } + plog.Debugf("Matched all expected messages") // OK! errchan <- nil }() @@ -672,23 +685,23 @@ func testPXE(ctx context.Context, inst platform.Install, outdir string) (time.Du } tmpd, err := os.MkdirTemp("", "kola-testiso") if err != nil { - return 0, err + return 0, errors.Wrapf(err, "creating tempdir") } defer os.RemoveAll(tmpd) sshPubKeyBuf, _, err := util.CreateSSHAuthorizedKey(tmpd) if err != nil { - return 0, err + return 0, errors.Wrapf(err, "creating SSH AuthorizedKey") } builder, virtioJournalConfig, err := newQemuBuilderWithDisk(outdir) if err != nil { - return 0, err + return 0, errors.Wrapf(err, "creating QemuBuilder") } inst.Builder = builder completionChannel, err := inst.Builder.VirtioChannelRead("testisocompletion") if err != nil { - return 0, err + return 0, errors.Wrapf(err, "setting up virtio-serial channel") } var keys []string @@ -799,6 +812,66 @@ func testLiveIso(ctx context.Context, inst platform.Install, outdir string, mini return awaitCompletion(ctx, mach.QemuInst, outdir, completionChannel, mach.BootStartedErrorChannel, []string{liveOKSignal, signalCompleteString}) } +// testLiveFIPS verifies that adding fips=1 to the ISO results in a FIPS mode system +func testLiveFIPS(ctx context.Context, outdir string) (time.Duration, error) { + tmpd, err := os.MkdirTemp("", "kola-testiso") + if err != nil { + return 0, err + } + defer os.RemoveAll(tmpd) + + builddir := kola.CosaBuild.Dir + isopath := filepath.Join(builddir, kola.CosaBuild.Meta.BuildArtifacts.LiveIso.Path) + builder, config, err := newQemuBuilder(outdir) + if err != nil { + return 0, err + } + defer builder.Close() + if err := builder.AddIso(isopath, "", false); err != nil { + return 0, err + } + + // This is the core change under test - adding the `fips=1` kernel argument via + // coreos-installer iso kargs modify should enter fips mode. + // Removing this line should cause this test to fail. + builder.AppendKernelArgs = "fips=1" + + completionChannel, err := builder.VirtioChannelRead("testisocompletion") + if err != nil { + return 0, err + } + + config.AddSystemdUnit("fips-verify.service", ` +[Unit] +OnFailure=emergency.target +OnFailureJobMode=isolate +Before=fips-signal-ok.service + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=grep 1 /proc/sys/crypto/fips_enabled +ExecStart=grep FIPS etc/crypto-policies/config + +[Install] +RequiredBy=fips-signal-ok.service +`, conf.Enable) + config.AddSystemdUnit("fips-signal-ok.service", liveSignalOKUnit, conf.Enable) + config.AddSystemdUnit("fips-emergency-target.service", signalFailureUnit, conf.Enable) + + // Just for reliability, we'll run fully offline + builder.Append("-net", "none") + + builder.SetConfig(config) + mach, err := builder.Exec() + if err != nil { + return 0, errors.Wrapf(err, "running iso") + } + defer mach.Destroy() + + return awaitCompletion(ctx, mach, outdir, completionChannel, nil, []string{liveOKSignal}) +} + func testLiveLogin(ctx context.Context, outdir string) (time.Duration, error) { builddir := kola.CosaBuild.Dir isopath := filepath.Join(builddir, kola.CosaBuild.Meta.BuildArtifacts.LiveIso.Path) diff --git a/mantle/harness/harness.go b/mantle/harness/harness.go index bb96fc8bdfc552ea3d1926b6890bd96dadf62fc0..1edb6a5738d0055c659e78c1e836a5a40537a3af 100644 --- a/mantle/harness/harness.go +++ b/mantle/harness/harness.go @@ -79,6 +79,7 @@ type H struct { warningOnFailure bool timeout time.Duration // Duration for which the test will be allowed to run + timedout bool // A timeout was reached execTimer *time.Timer // Used to interrupt the test after timeout // To signal that a timeout has occured to observers timeoutContext context.Context @@ -101,6 +102,7 @@ func (t *H) runTimeoutCheck(ctx context.Context, timeout time.Duration, f func() // Timeout if call to function f takes too long select { case <-ctx.Done(): + t.timedout = true t.Fatalf("TIMEOUT[%v]: %s\n", timeout, errMsg) case <-ioCompleted: // Finish the test @@ -303,6 +305,11 @@ func (c *H) Failed() bool { return c.failed } +// Reports if the test was stopped because a timeout was reached. +func (c *H) TimedOut() bool { + return c.timedout +} + // FailNow marks the function as having failed and stops its execution. // Execution will continue at the next test. // FailNow must be called from the goroutine running the @@ -617,7 +624,7 @@ func (t *H) report() { status := t.status() if status == testresult.Fail || t.suite.opts.Verbose { - t.flushToParent(format, status, t.name, dstr) + t.flushToParent(format, status.Display(), t.name, dstr) } // TODO: store multiple buffers for subtests without indentation diff --git a/mantle/harness/testresult/status.go b/mantle/harness/testresult/status.go index 53daec575bd7e129b69e490eb2f7d04e28149c67..c1f641d14e8a67fe3751028de700a61bae48c071 100644 --- a/mantle/harness/testresult/status.go +++ b/mantle/harness/testresult/status.go @@ -14,6 +14,8 @@ package testresult +import "os" + const ( Fail TestResult = "FAIL" Warn TestResult = "WARN" @@ -22,3 +24,25 @@ const ( ) type TestResult string + +func (s TestResult) Display() string { + if term, has_term := os.LookupEnv("TERM"); !has_term || term == "" { + return string(s) + } + + red := "\033[31m" + yellow := "\033[33m" + blue := "\033[34m" + green := "\033[32m" + reset := "\033[0m" + + if s == Fail { + return red + string(s) + reset + } else if s == Warn { + return yellow + string(s) + reset + } else if s == Skip { + return blue + string(s) + reset + } else { + return green + string(s) + reset + } +} diff --git a/mantle/kola/harness.go b/mantle/kola/harness.go index dc71dd27fa6fa6aecf771b271902a3b24911f3b9..176acf35129780ea276d666fd793179fb5a25ed5 100644 --- a/mantle/kola/harness.go +++ b/mantle/kola/harness.go @@ -18,10 +18,12 @@ import ( "bufio" "encoding/json" "fmt" + "hash/fnv" "io" "os" "path/filepath" "regexp" + "strconv" "strings" "sync" "time" @@ -83,6 +85,11 @@ const NeedsInternetTag = "needs-internet" // For more, see the doc in external-tests.md. const PlatformIndependentTag = "platform-independent" +// The string for the tag that indicates a test has been marked as allowing rerun success. +// In some cases the internal test framework will add this tag to a test to indicate if +// the test passes a rerun to allow the run to succeed. +const AllowRerunSuccessTag = "allow-rerun-success" + // defaultPlatformIndependentPlatform is the platform where we run tests that claim platform independence const defaultPlatformIndependentPlatform = "qemu" @@ -92,6 +99,10 @@ const SkipBaseChecksTag = "skip-base-checks" // Date format for snooze date specified in kola-denylist.yaml (YYYY-MM-DD) const snoozeFormat = "2006-01-02" +// SkipConsoleWarningsTag will cause kola not to check console for kernel errors. +// This overlaps with SkipBaseChecksTag above, but is really a special flag for kola-denylist.yaml. +const SkipConsoleWarningsTag = "skip-console-warnings" + var ( plog = capnslog.NewPackageLogger("github.com/coreos/coreos-assembler/mantle", "kola") @@ -114,6 +125,8 @@ var ( // ForceRunPlatformIndependent will cause tests that claim platform-independence to run ForceRunPlatformIndependent bool + // SkipConsoleWarnings is set via SkipConsoleWarningsTag in kola-denylist.yaml + SkipConsoleWarnings bool DenylistedTests []string // tests which are on the denylist WarnOnErrorTests []string // denylisted tests we are going to run and warn in case of error Tags []string // tags to be ran @@ -125,16 +138,18 @@ var ( nonexclusiveWrapperMatch = regexp.MustCompile(`^non-exclusive-test-bucket-[0-9]$`) consoleChecks = []struct { - desc string - match *regexp.Regexp - warnOnly bool - skipFlag *register.Flag + desc string + match *regexp.Regexp + warnOnly bool + allowRerunSuccess bool + skipFlag *register.Flag }{ { - desc: "emergency shell", - match: regexp.MustCompile("Press Enter for emergency shell|Starting Emergency Shell|You are in emergency mode"), + desc: "emergency shell", + match: regexp.MustCompile("Press Enter for emergency shell|Starting Emergency Shell|You are in emergency mode"), warnOnly: false, - skipFlag: &[]register.Flag{register.NoEmergencyShellCheck}[0], + allowRerunSuccess: false, + skipFlag: &[]register.Flag{register.NoEmergencyShellCheck}[0], }, { desc: "dracut fatal", @@ -148,6 +163,17 @@ var ( desc: "kernel oops", match: regexp.MustCompile("Oops:"), }, + { + // For this one we see it sometimes when I/O is really slow, which is often + // more of an indication of a problem with resources in our pipeline rather + // than a problem with the software we are testing. We'll mark it as warnOnly + // so it's non-fatal and also allow for a rerun of a test that goes on to + // fail that had this problem to ultimately result in success. + desc: "kernel soft lockup", + match: regexp.MustCompile("watchdog: BUG: soft lockup - CPU"), + warnOnly: true, + allowRerunSuccess: true, + }, { desc: "kernel warning", match: regexp.MustCompile(`WARNING: CPU: \d+ PID: \d+ at (.+)`), @@ -204,9 +230,11 @@ var ( { // https://github.com/coreos/fedora-coreos-config/pull/1797 desc: "systemd generator failure", - match: regexp.MustCompile(`systemd\[[0-9]+\]: (.*) failed with exit status`), + match: regexp.MustCompile(`(/.*/system-generators/.*) (failed with exit status|terminated by signal|failed due to unknown reason)`), }, } + + ErrWarnOnTestFail = errors.New("A test marked as warn:true failed.") ) const ( @@ -281,8 +309,8 @@ func NewFlight(pltfrm string) (flight platform.Flight, err error) { return } -// matchesPatterns returns true if `s` matches one of the patterns in `patterns`. -func matchesPatterns(s string, patterns []string) (bool, error) { +// MatchesPatterns returns true if `s` matches one of the patterns in `patterns`. +func MatchesPatterns(s string, patterns []string) (bool, error) { for _, pattern := range patterns { match, err := filepath.Match(pattern, s) if err != nil { @@ -295,8 +323,8 @@ func matchesPatterns(s string, patterns []string) (bool, error) { return false, nil } -// hasString returns true if `s` equals one of the strings in `slice`. -func hasString(s string, slice []string) bool { +// HasString returns true if `s` equals one of the strings in `slice`. +func HasString(s string, slice []string) bool { for _, e := range slice { if e == s { return true @@ -306,27 +334,18 @@ func hasString(s string, slice []string) bool { } func testSkipBaseChecks(test *register.Test) bool { - for _, tag := range test.Tags { - if tag == SkipBaseChecksTag { - return true - } - } - return false + return HasString(SkipBaseChecksTag, test.Tags) } func testRequiresInternet(test *register.Test) bool { - for _, flag := range test.Flags { - if flag == register.RequiresInternetAccess { - return true - } - } - // Also parse the newer tag for this - for _, tag := range test.Tags { - if tag == NeedsInternetTag { - return true - } + return HasString(NeedsInternetTag, test.Tags) +} + +func markTestForRerunSuccess(test *register.Test, msg string) { + if !HasString(AllowRerunSuccessTag, test.Tags) { + plog.Warningf("%s Adding as candidate for rerun success: %s", msg, test.Name) + test.Tags = append(test.Tags, AllowRerunSuccessTag) } - return false } type DenyListObj struct { @@ -429,23 +448,39 @@ func ParseDenyListYaml(pltfrm string) error { continue } + // Process "special" patterns which aren't test names, but influence overall behavior + if obj.Pattern == SkipConsoleWarningsTag { + SkipConsoleWarnings = true + continue + } + if obj.SnoozeDate != "" { snoozeDate, err := time.Parse(snoozeFormat, obj.SnoozeDate) if err != nil { return err - } else if today.After(snoozeDate) { - continue } - - fmt.Printf("🕒 Snoozing kola test pattern \"%s\" until %s:\n", obj.Pattern, snoozeDate.Format("Jan 02 2006")) + if today.After(snoozeDate) { + fmt.Printf("⏰ Snooze for kola test pattern \"%s\" expired on %s\n", obj.Pattern, snoozeDate.Format("Jan 02 2006")) + if obj.Warn { + fmt.Printf("⚠️ Will warn on failure for kola test pattern \"%s\":\n", obj.Pattern) + WarnOnErrorTests = append(WarnOnErrorTests, obj.Pattern) + } + } else { + fmt.Printf("🕒 Snoozing kola test pattern \"%s\" until %s\n", obj.Pattern, snoozeDate.Format("Jan 02 2006")) + DenylistedTests = append(DenylistedTests, obj.Pattern) + } } else { - fmt.Printf("⚠️ Skipping kola test pattern \"%s\":\n", obj.Pattern) + if obj.Warn { + fmt.Printf("⚠️ Will warn on failure for kola test pattern \"%s\":\n", obj.Pattern) + WarnOnErrorTests = append(WarnOnErrorTests, obj.Pattern) + } else { + fmt.Printf("⏭️ Skipping kola test pattern \"%s\":\n", obj.Pattern) + DenylistedTests = append(DenylistedTests, obj.Pattern) + } } - if obj.Tracker != "" { fmt.Printf(" 👉 %s\n", obj.Tracker) } - DenylistedTests = append(DenylistedTests, obj.Pattern) } return nil @@ -470,59 +505,21 @@ func filterTests(tests map[string]*register.Test, patterns []string, pltfrm stri // Higher-level functions default to '*' if the user didn't pass anything. // Notice this. (This totally ignores the corner case where the user // actually typed '*'). - userTypedPattern := !hasString("*", patterns) + userTypedPattern := !HasString("*", patterns) for name, t := range tests { if NoNet && testRequiresInternet(t) { plog.Debugf("Skipping test that requires network: %s", t.Name) continue } - var denylisted bool - // Drop anything which is denylisted directly or by pattern - for _, bl := range DenylistedTests { - nameMatch, err := filepath.Match(bl, t.Name) - if err != nil { - return nil, err - } - // If it matched the pattern this test is denylisted - if nameMatch { - denylisted = true - break - } - - // Check if any native tests are denylisted. To exclude native tests, specify the high level - // test and a "/" and then the glob pattern. - // - basic/TestNetworkScripts: excludes only TestNetworkScripts - // - basic/* - excludes all - // - If no pattern is specified after / , excludes none - nativedenylistindex := strings.Index(bl, "/") - if nativedenylistindex > -1 { - // Check native tests for arch specific exclusion - for nativetestname := range t.NativeFuncs { - nameMatch, err := filepath.Match(bl[nativedenylistindex+1:], nativetestname) - if err != nil { - return nil, err - } - if nameMatch { - delete(t.NativeFuncs, nativetestname) - } - } - } - } - // If the test is denylisted, skip it and continue to the next test - if denylisted { - plog.Debugf("Skipping denylisted test %s", t.Name) - continue - } - - nameMatch, err := matchesPatterns(t.Name, patterns) + nameMatch, err := MatchesPatterns(t.Name, patterns) if err != nil { return nil, err } tagMatch := false for _, tag := range positiveTags { - tagMatch = hasString(tag, t.Tags) || tag == t.RequiredTag + tagMatch = HasString(tag, t.Tags) || tag == t.RequiredTag if tagMatch { break } @@ -530,7 +527,7 @@ func filterTests(tests map[string]*register.Test, patterns []string, pltfrm stri negativeTagMatch := false for _, tag := range negativeTags { - negativeTagMatch = hasString(tag, t.Tags) + negativeTagMatch = HasString(tag, t.Tags) if negativeTagMatch { break } @@ -540,7 +537,7 @@ func filterTests(tests map[string]*register.Test, patterns []string, pltfrm stri } if t.RequiredTag != "" && // if the test has a required tag... - !tagMatch && // and that tag was not provided by the user... + !HasString(t.RequiredTag, positiveTags) && // and that tag was not provided by the user... (!userTypedPattern || !nameMatch) { // and the user didn't request it by name... continue // then skip it } @@ -634,13 +631,58 @@ func filterTests(tests map[string]*register.Test, patterns []string, pltfrm stri return r, nil } +func filterDenylistedTests(tests map[string]*register.Test) (map[string]*register.Test, error) { + r := make(map[string]*register.Test) + for name, t := range tests { + denylisted := false + // Detect anything which is denylisted directly or by pattern + for _, bl := range DenylistedTests { + nameMatch, err := filepath.Match(bl, t.Name) + if err != nil { + return nil, err + } + // If it matched the pattern this test is denylisted + if nameMatch { + denylisted = true + break + } + + // Check if any native tests are denylisted. To exclude native tests, specify the high level + // test and a "/" and then the glob pattern. + // - basic/TestNetworkScripts: excludes only TestNetworkScripts + // - basic/* - excludes all + // - If no pattern is specified after / , excludes none + nativedenylistindex := strings.Index(bl, "/") + if nativedenylistindex > -1 { + // Check native tests for arch specific exclusion + for nativetestname := range t.NativeFuncs { + nameMatch, err := filepath.Match(bl[nativedenylistindex+1:], nativetestname) + if err != nil { + return nil, err + } + if nameMatch { + delete(t.NativeFuncs, nativetestname) + } + } + } + } + // If the test is denylisted, skip it and continue to the next test + if denylisted { + plog.Debugf("Skipping denylisted test %s", t.Name) + continue + } + r[name] = t + } + return r, nil +} + // runProvidedTests is a harness for running multiple tests in parallel. // Filters tests based on a glob pattern and by platform. Has access to all // tests either registered in this package or by imported packages that // register tests in their init() function. outputDir is where various test // logs and data will be written for analysis after the test run. If it already // exists it will be erased! -func runProvidedTests(testsBank map[string]*register.Test, patterns []string, multiply int, rerun bool, allowRerunSuccess bool, pltfrm, outputDir string, propagateTestErrors bool) error { +func runProvidedTests(testsBank map[string]*register.Test, patterns []string, multiply int, rerun bool, rerunSuccessTags []string, pltfrm, outputDir string) error { var versionStr string // Avoid incurring cost of starting machine in getClusterSemver when @@ -650,16 +692,41 @@ func runProvidedTests(testsBank map[string]*register.Test, patterns []string, mu // either way // Add denylisted tests in kola-denylist.yaml to DenylistedTests - err := parseDenyListYaml(pltfrm) + err := ParseDenyListYaml(pltfrm) if err != nil { plog.Fatal(err) } + // Make sure all given patterns by the user match at least one test + for _, pattern := range patterns { + match, err := patternMatchesTests(pattern, testsBank) + if err != nil { + plog.Fatal(err) + } + if !match { + plog.Fatalf("The user provided pattern didn't match any tests: %s", pattern) + } + } + tests, err := filterTests(testsBank, patterns, pltfrm) if err != nil { plog.Fatal(err) } + if len(tests) == 0 { + plog.Fatalf("There are no matching tests to run on this architecture/platform: %s %s", coreosarch.CurrentRpmArch(), pltfrm) + } + + tests, err = filterDenylistedTests(tests) + if err != nil { + plog.Fatal(err) + } + + if len(tests) == 0 { + fmt.Printf("There are no tests to run because all tests are denylisted. Output in %v\n", outputDir) + return nil + } + flight, err := NewFlight(pltfrm) if err != nil { plog.Fatalf("Flight failed: %v", err) @@ -677,7 +744,11 @@ func runProvidedTests(testsBank map[string]*register.Test, patterns []string, mu } } - if len(nonExclusiveTests) > 0 { + if len(nonExclusiveTests) == 1 { + // If there is only one test then it can just be run by itself + // so add it back to the tests map. + tests[nonExclusiveTests[0].Name] = nonExclusiveTests[0] + } else if len(nonExclusiveTests) > 0 { buckets := createTestBuckets(nonExclusiveTests) numBuckets := len(buckets) for i := 0; i < numBuckets; { @@ -746,16 +817,12 @@ func runProvidedTests(testsBank map[string]*register.Test, patterns []string, mu // At the end of the test, its cluster is destroyed runTest(h, test, pltfrm, flight) } - htests.Add(test.Name, run, test.Timeout) + htests.Add(test.Name, run, (test.Timeout*time.Duration(100+(Options.ExtendTimeoutPercent)))/100) } handleSuiteErrors := func(outputDir string, suiteErr error) error { caughtTestError := suiteErr != nil - if !propagateTestErrors { - suiteErr = nil - } - if TAPFile != "" { src := filepath.Join(outputDir, "test.tap") err := system.CopyRegularFile(src, TAPFile) @@ -769,78 +836,170 @@ func runProvidedTests(testsBank map[string]*register.Test, patterns []string, mu } else { fmt.Printf("PASS, output in %v\n", outputDir) } + return suiteErr } suite := harness.NewSuite(opts, htests) - firstRunErr := suite.Run() - firstRunErr = handleSuiteErrors(outputDir, firstRunErr) + runErr := suite.Run() + runErr = handleSuiteErrors(outputDir, runErr) - testsToRerun := getRerunnable(testResults.getResults()) + detectedFailedWarnTrueTests := len(getWarnTrueFailedTests(testResults.getResults())) != 0 + + testsToRerun := getRerunnable(testsBank, testResults.getResults()) + numFailedTests := len(testsToRerun) if len(testsToRerun) > 0 && rerun { newOutputDir := filepath.Join(outputDir, "rerun") fmt.Printf("\n\n======== Re-running failed tests (flake detection) ========\n\n") - reRunErr := runProvidedTests(testsBank, testsToRerun, multiply, false, allowRerunSuccess, pltfrm, newOutputDir, propagateTestErrors) - if allowRerunSuccess { - return reRunErr + reRunErr := runProvidedTests(testsToRerun, []string{"*"}, multiply, false, rerunSuccessTags, pltfrm, newOutputDir) + if reRunErr == nil && allTestsAllowRerunSuccess(testsToRerun, rerunSuccessTags) { + runErr = nil // reset to success since all tests allowed rerun success + numFailedTests = 0 // zero out the tally of failed tests + } else { + runErr = reRunErr } + } - // If the intial run failed and the rerun passed, we still return an error - return firstRunErr + // Return ErrWarnOnTestFail when ONLY tests with warn:true feature failed + if detectedFailedWarnTrueTests && numFailedTests == 0 { + return ErrWarnOnTestFail + } else { + return runErr + } } -func GetRerunnableTestName(testName string) (string, bool) { - // The current nonexclusive test wrapper would rerun all non-exclusive tests. - // Instead, we only want to rerun the one(s) that failed, so we will not consider - // the wrapper as "rerunnable". +func getWarnTrueFailedTests(tests []*harness.H) []string { + var warnTrueFailedTests []string + for _, test := range tests { + if !test.Failed() { + continue + } + name := GetBaseTestName(test.Name()) + if name == "" { + continue // skip non-exclusive test wrapper + } + if IsWarningOnFailure(name) { + warnTrueFailedTests = append(warnTrueFailedTests, name) + } + } + return warnTrueFailedTests +} + +func allTestsAllowRerunSuccess(testsToRerun map[string]*register.Test, rerunSuccessTags []string) bool { + // Always consider the special AllowRerunSuccessTag that is added + // by the test harness in some failure scenarios. + rerunSuccessTags = append(rerunSuccessTags, AllowRerunSuccessTag) + // Build up a map of rerunSuccessTags so that we can easily check + // if a given tag is in the map. + rerunSuccessTagMap := make(map[string]bool) + for _, tag := range rerunSuccessTags { + if tag == "all" || tag == "*" { + // If `all` or `*` is in rerunSuccessTags then we can return early + return true + } + rerunSuccessTagMap[tag] = true + } + // Iterate over the tests that were re-ran. If any of them don't + // allow rerun success then just exit early. + for _, test := range testsToRerun { + testAllowsRerunSuccess := false + for _, tag := range test.Tags { + if rerunSuccessTagMap[tag] { + testAllowsRerunSuccess = true + } + } + if !testAllowsRerunSuccess { + return false + } + } + return true +} +func GetBaseTestName(testName string) string { + // If this is a non-exclusive wrapper then just return the empty string if nonexclusiveWrapperMatch.MatchString(testName) { + return "" + } + + // If the given test is a non-exclusive test with the prefix in + // the name we'll need to pull it apart. For example: + // non-exclusive-test-bucket-0/ext.config.files.license -> ext.config.files.license + substrings := nonexclusivePrefixMatch.Split(testName, 2) + return substrings[len(substrings)-1] +} + +func GetRerunnableTestName(testName string) (string, bool) { + name := GetBaseTestName(testName) + if name == "" { // Test is not rerunnable if the test name matches the wrapper pattern return "", false - } else { - // Failed non-exclusive tests will have a prefix in the name - // since they are run as subtests of a wrapper test, we need to - // remove this prefix to match the true name of the test - substrings := nonexclusivePrefixMatch.Split(testName, 2) - name := substrings[len(substrings)-1] + } + if strings.Contains(name, "/") { + // In the case that a test is exclusive, we may + // be adding a subtest. We don't want to do this + return "", false + } - if strings.Contains(name, "/") { - // In the case that a test is exclusive, we may - // be adding a subtest. We don't want to do this - return "", false + // Tests with 'warn: true' are not rerunnable + if IsWarningOnFailure(name) { + return "", false + } + + // The test is not a nonexclusive wrapper, and its not a + // subtest of an exclusive test + return name, true +} + +func IsWarningOnFailure(testName string) bool { + for _, pattern := range WarnOnErrorTests { + found, err := filepath.Match(pattern, testName) + if err != nil { + plog.Fatal(err) + return false + } + if found { + return true } - // The test is not a nonexclusive wrapper, and its not a - // subtest of an exclusive test - return name, true } + return false } -func getRerunnable(tests []*harness.H) []string { - var testsToRerun []string - for _, h := range tests { +func getRerunnable(testsBank map[string]*register.Test, testResults []*harness.H) map[string]*register.Test { + // First get the names of the tests that need to rerun + var testNamesToRerun []string + for _, h := range testResults { // The current nonexclusive test wrapper would have all non-exclusive tests. // We would add all those tests for rerunning if none of the non-exclusive // subtests start due to some initial failure. if nonexclusiveWrapperMatch.MatchString(h.Name()) && !h.GetNonExclusiveTestStarted() { if h.Failed() { - testsToRerun = append(testsToRerun, h.Subtests()...) + testNamesToRerun = append(testNamesToRerun, h.Subtests()...) } } else { name, isRerunnable := GetRerunnableTestName(h.Name()) if h.Failed() && isRerunnable { - testsToRerun = append(testsToRerun, name) + testNamesToRerun = append(testNamesToRerun, name) + } + } + } + // Then convert the list of names into a list of a register.Test objects + testsToRerun := make(map[string]*register.Test) + for name, t := range testsBank { + for _, rerun := range testNamesToRerun { + if name == rerun { + testsToRerun[name] = t } } } return testsToRerun } -func RunTests(patterns []string, multiply int, rerun bool, allowRerunSuccess bool, pltfrm, outputDir string, propagateTestErrors bool) error { - return runProvidedTests(register.Tests, patterns, multiply, rerun, allowRerunSuccess, pltfrm, outputDir, propagateTestErrors) +func RunTests(patterns []string, multiply int, rerun bool, rerunSuccessTags []string, pltfrm, outputDir string) error { + return runProvidedTests(register.Tests, patterns, multiply, rerun, rerunSuccessTags, pltfrm, outputDir) } -func RunUpgradeTests(patterns []string, rerun bool, pltfrm, outputDir string, propagateTestErrors bool) error { - return runProvidedTests(register.UpgradeTests, patterns, 0, rerun, false, pltfrm, outputDir, propagateTestErrors) +func RunUpgradeTests(patterns []string, rerun bool, pltfrm, outputDir string) error { + return runProvidedTests(register.UpgradeTests, patterns, 0, rerun, nil, pltfrm, outputDir) } // externalTestMeta is parsed from kola.json in external tests @@ -862,6 +1021,7 @@ type externalTestMeta struct { Conflicts []string `json:"conflicts" yaml:"conflicts"` AllowConfigWarnings bool `json:"allowConfigWarnings" yaml:"allowConfigWarnings"` NoInstanceCreds bool `json:"noInstanceCreds" yaml:"noInstanceCreds"` + Description string `json:"description" yaml:"description"` } // metadataFromTestBinary extracts JSON-in-comment like: @@ -1045,9 +1205,10 @@ ExecStart=%s config.AddSystemdUnit(unitName, unit, conf.NoState) // Architectures using 64k pages use slightly more memory, ask for more than requested - // to make sure that we don't run out of it. Currently ppc64le and aarch64 use 64k pages. + // to make sure that we don't run out of it. Currently, only ppc64le uses 64k pages by default. + // See similar logic in boot-mirror.go and luks.go. switch coreosarch.CurrentRpmArch() { - case "ppc64le", "aarch64": + case "ppc64le": if targetMeta.MinMemory <= 4096 { targetMeta.MinMemory = targetMeta.MinMemory * 2 } @@ -1055,6 +1216,7 @@ ExecStart=%s t := ®ister.Test{ Name: testname, + Description: targetMeta.Description, ClusterSize: 1, // Hardcoded for now ExternalTest: executable, DependencyDir: destDirs, @@ -1141,14 +1303,29 @@ func testIsDenyListed(testname string) (bool, error) { return false, nil } +// Function that returns true if at least one test matches the given pattern +func patternMatchesTests(pattern string, tests map[string]*register.Test) (bool, error) { + for testname := range tests { + if match, err := filepath.Match(pattern, testname); err != nil { + return false, err + } else if match { + return true, nil + } + } + return false, nil +} + // registerTestDir parses one test directory and registers it as a test -func registerTestDir(dir, testprefix string, children []os.FileInfo) error { +func registerTestDir(dir, testprefix string, children []os.DirEntry) error { var dependencydir string var meta externalTestMeta - var err error userdata := conf.EmptyIgnition() executables := []string{} - for _, c := range children { + for _, e := range children { + c, err := e.Info() + if err != nil { + return fmt.Errorf("getting info for %q: %w", e.Name(), err) + } fpath := filepath.Join(dir, c.Name()) // follow symlinks; oddly, there's no IsSymlink() if c.Mode()&os.ModeSymlink != 0 { @@ -1370,7 +1547,7 @@ mainloop: func makeNonExclusiveTest(bucket int, tests []*register.Test, flight platform.Flight) register.Test { // Parse test flags and gather configs internetAccess := false - var flags []register.Flag + var tags []string var nonExclusiveTestConfs []*conf.Conf dependencyDirs := make(register.DepDirMap) var subtests []string @@ -1389,7 +1566,7 @@ func makeNonExclusiveTest(bucket int, tests []*register.Test, flight platform.Fl plog.Fatalf("Non-exclusive test %v cannot have AppendKernelArgs", test.Name) } if !internetAccess && testRequiresInternet(test) { - flags = append(flags, register.RequiresInternetAccess) + tags = append(tags, NeedsInternetTag) internetAccess = true } @@ -1453,10 +1630,9 @@ func makeNonExclusiveTest(bucket int, tests []*register.Test, flight platform.Fl UserData: mergedConfig, Subtests: subtests, // This will allow runTest to copy kolet to machine - NativeFuncs: make(map[string]register.NativeFuncWrap), - ClusterSize: 1, - Flags: flags, - + NativeFuncs: make(map[string]register.NativeFuncWrap), + ClusterSize: 1, + Tags: tags, DependencyDir: dependencyDirs, } @@ -1493,6 +1669,10 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig defer func() { h.StopExecTimer() c.Destroy() + if h.TimedOut() { + // We'll allow tests that time out to succeed on rerun. + markTestForRerunSuccess(t, "Test timed out.") + } if testSkipBaseChecks(t) { plog.Debugf("Skipping base checks for %s", t.Name) return @@ -1544,6 +1724,9 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig return err }) if err != nil { + // The platform failed starting machines, which usually isn't *CoreOS + // fault. Maybe it will have better luck in the rerun. + markTestForRerunSuccess(t, "Platform failed starting machines.") h.Fatalf("Cluster failed starting machines: %v", err) } } @@ -1562,6 +1745,10 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig FailFast: t.FailFast, } + if IsWarningOnFailure(t.Name) { + tcluster.H.WarningOnFailure() + } + // Note that we passed in SkipStartMachine=true in our machine // options. This means NewMachines() didn't block on the machines // being up with SSH access before returning; i.e. it skipped running @@ -1655,25 +1842,38 @@ func scpKolet(machines []platform.Machine) error { } // CheckConsole checks some console output for badness and returns short -// descriptions of any badness it finds. If t is specified, its flags are -// respected. -func CheckConsole(output []byte, t *register.Test) []string { - var ret []string +// descriptions of any bad lines it finds along with a boolean +// indicating if the configuration has the bad lines marked as +// warnOnly or not (for things we don't want to error for). If t is +// specified, its flags are respected and tags possibly updated for +// rerun success. +func CheckConsole(output []byte, t *register.Test) (bool, []string) { + var badlines []string + warnOnly, allowRerunSuccess := true, true for _, check := range consoleChecks { if check.skipFlag != nil && t != nil && t.HasFlag(*check.skipFlag) { continue } match := check.match.FindSubmatch(output) if match != nil { - badness := check.desc + badline := check.desc if len(match) > 1 { // include first subexpression - badness += fmt.Sprintf(" (%s)", match[1]) + badline += fmt.Sprintf(" (%s)", match[1]) + } + badlines = append(badlines, badline) + if !check.warnOnly { + warnOnly = false + } + if !check.allowRerunSuccess { + allowRerunSuccess = false } - ret = append(ret, badness) } } - return ret + if len(badlines) > 0 && allowRerunSuccess && t != nil { + markTestForRerunSuccess(t, "CheckConsole:") + } + return warnOnly, badlines } func SetupOutputDir(outputDir, platform string) (string, error) { diff --git a/mantle/kola/register/register.go b/mantle/kola/register/register.go index 63cb4f57e602dd3ce3c35a1d188b2ae1496cdafe..65bedb205c63d59b111074fe2660bff739776f0f 100644 --- a/mantle/kola/register/register.go +++ b/mantle/kola/register/register.go @@ -25,12 +25,11 @@ import ( type Flag int const ( - NoSSHKeyInUserData Flag = iota // don't inject SSH key into Ignition/cloud-config - NoSSHKeyInMetadata // don't add SSH key to platform metadata - NoInstanceCreds // don't grant credentials (AWS instance profile, GCP service account) to the instance - NoEmergencyShellCheck // don't check console output for emergency shell invocation - RequiresInternetAccess // run the test only if the platform supports Internet access - AllowConfigWarnings // ignore Ignition and Butane warnings instead of failing + NoSSHKeyInUserData Flag = iota // don't inject SSH key into Ignition/cloud-config + NoSSHKeyInMetadata // don't add SSH key to platform metadata + NoInstanceCreds // don't grant credentials (AWS instance profile, GCP service account) to the instance + NoEmergencyShellCheck // don't check console output for emergency shell invocation + AllowConfigWarnings // ignore Ignition and Butane warnings instead of failing ) // NativeFuncWrap is a wrapper for the NativeFunc which includes an optional string of arches and/or distributions to @@ -69,6 +68,7 @@ type Test struct { Tags []string // list of tags that can be matched against -- defaults to none Timeout time.Duration // the duration for which a test will be allowed to run RequiredTag string // if specified, test is filtered by default unless tag is provided -- defaults to none + Description string // test description // Whether the primary disk is multipathed. MultiPathDisk bool diff --git a/mantle/network/journal/format.go b/mantle/network/journal/format.go index 5875f55b1cc1293055eee8e982141cc1dfab00be..2f7ee01e8a525c79d455bd8618d9f97d2e3b4ef0 100644 --- a/mantle/network/journal/format.go +++ b/mantle/network/journal/format.go @@ -65,7 +65,13 @@ func (s *shortWriter) WriteEntry(entry Entry) error { var buf bytes.Buffer buf.WriteString(realtime.In(s.tz).Format(time.StampMicro)) - if identifier, ok := entry[FIELD_SYSLOG_IDENTIFIER]; ok { + // Default to equivalent of journalctl -o with-unit, because its value is + // trusted, and the syslog identifier (commonly when executing bash via ExecStart) + // can be garbage. + if unit, ok := entry[FIELD_SYSTEMD_UNIT]; ok { + buf.WriteByte(' ') + buf.WriteString(string(unit)) + } else if identifier, ok := entry[FIELD_SYSLOG_IDENTIFIER]; ok { buf.WriteByte(' ') buf.Write(identifier) } else { diff --git a/mantle/network/journal/format_test.go b/mantle/network/journal/format_test.go index f401b67fa5b75d09375a1092e248defa88d40d10..183f778cc013545ec268f7c35b0e46c8ff5c37f4 100644 --- a/mantle/network/journal/format_test.go +++ b/mantle/network/journal/format_test.go @@ -171,8 +171,8 @@ func TestFormatShortFromExport(t *testing.T) { const expect = `Jul 17 16:01:01.413961 gdm-password][587]: AccountsService-DEBUG(+): ActUserManager: ignoring unspecified session '8' since it's not graphical: Success Jul 17 16:01:01.416351 /USR/SBIN/CROND[8278]: (root) CMD (run-parts /etc/cron.hourly) -- Reboot -- -Feb 14 20:15:16.372858 python3[16853]: foo - bar +Feb 14 20:15:16.372858 session-35898.scope[16853]: foo + bar ` if d := diff.Diff(buf.String(), expect); d != "" { t.Errorf("unexpected output:\n%s", d) diff --git a/mantle/platform/cluster.go b/mantle/platform/cluster.go index a3a72a7bb7a88a4ef2f8236f43dea3316ec64547..7a9b283e807ff739fa175fb9f9de792a79c470b5 100644 --- a/mantle/platform/cluster.go +++ b/mantle/platform/cluster.go @@ -216,8 +216,7 @@ func (bc *BaseCluster) RenderUserData(userdata *platformConf.UserData, ignitionV // disable Zincati by default if bc.Distribution() == "fcos" { - conf.AddFile("/etc/zincati/config.d/90-disable-auto-updates.toml", `[updates] -enabled = false`, 0644) + conf.DisableAutomaticUpdates() } if bc.bf.baseopts.OSContainer != "" { @@ -230,6 +229,8 @@ After=network-online.target [Service] Type=oneshot +# Newer zincati more explicitly fails on containers; fully disable it +ExecStart=systemctl mask --now zincati ExecStart=rpm-ostree rebase --experimental %s ExecStart=touch /etc/kola-rebase-done ExecStart=systemctl reboot diff --git a/mantle/platform/conf/conf.go b/mantle/platform/conf/conf.go index 7ac78d9456c41ee78f1427c6a1763d06d8caac81..d890c6a8733357565a587e194e8eba50d9c0821f 100644 --- a/mantle/platform/conf/conf.go +++ b/mantle/platform/conf/conf.go @@ -38,13 +38,16 @@ import ( v32types "github.com/coreos/ignition/v2/config/v3_2/types" v33 "github.com/coreos/ignition/v2/config/v3_3" v33types "github.com/coreos/ignition/v2/config/v3_3/types" - v34exp "github.com/coreos/ignition/v2/config/v3_4_experimental" - v34exptypes "github.com/coreos/ignition/v2/config/v3_4_experimental/types" + v34 "github.com/coreos/ignition/v2/config/v3_4" + v34types "github.com/coreos/ignition/v2/config/v3_4/types" + v35exp "github.com/coreos/ignition/v2/config/v3_5_experimental" + v35exptypes "github.com/coreos/ignition/v2/config/v3_5_experimental/types" "github.com/coreos/ignition/v2/config/validate" "github.com/coreos/pkg/capnslog" "github.com/coreos/vcontext/report" "github.com/vincent-petithory/dataurl" "golang.org/x/crypto/ssh/agent" + "gopkg.in/yaml.v3" ) type kind int @@ -88,8 +91,9 @@ type Conf struct { ignitionV31 *v31types.Config ignitionV32 *v32types.Config ignitionV33 *v33types.Config + ignitionV34 *v34types.Config - ignitionV34exp *v34exptypes.Config + ignitionV35exp *v35exptypes.Config } // Empty creates a completely empty configuration. Any configuration addition @@ -232,13 +236,20 @@ func (u *UserData) Render(warnings WarningsAction) (*Conf, error) { return err } c.ignitionV33 = &ignc - case semver.Version{Major: 3, Minor: 4, PreRelease: "experimental"}: - ignc, report, err := v34exp.Parse(data) + case semver.Version{Major: 3, Minor: 4}: + ignc, report, err := v34.Parse(data) if err != nil { plog.Errorf("invalid userdata: %v", report) return err } - c.ignitionV34exp = &ignc + c.ignitionV34 = &ignc + case semver.Version{Major: 3, Minor: 5, PreRelease: "experimental"}: + ignc, report, err := v35exp.Parse(data) + if err != nil { + plog.Errorf("invalid userdata: %v", report) + return err + } + c.ignitionV35exp = &ignc // Special case for the next stable version: wrap it in a // config of the current stable version, so we can still add // our config fragments without understanding the specified @@ -247,17 +258,17 @@ func (u *UserData) Render(warnings WarningsAction) (*Conf, error) { // tests using the experimental spec, since CI only needs to // ensure that the installed Ignition can parse the config, // not that Mantle can also parse it. - case semver.Version{Major: 3, Minor: 4}: - plog.Warningf("Generating wrapper for unsupported config spec") + case semver.Version{Major: 3, Minor: 5}: + plog.Warningf("mantle has not been updated for Ignition spec %s; applying workaround", ver) url, err := makeGzipDataUrl(data) if err != nil { return fmt.Errorf("generating data URL: %w", err) } - c.ignitionV33 = &v33types.Config{ - Ignition: v33types.Ignition{ - Version: "3.3.0", - Config: v33types.IgnitionConfig{ - Merge: []v33types.Resource{ + c.ignitionV34 = &v34types.Config{ + Ignition: v34types.Ignition{ + Version: "3.4.0", + Config: v34types.IgnitionConfig{ + Merge: []v34types.Resource{ { Source: ignutil.StrToPtr(url), Compression: ignutil.StrToPtr("gzip"), @@ -281,10 +292,7 @@ func (u *UserData) Render(warnings WarningsAction) (*Conf, error) { return nil, err } case kindButane: - ignc, report, err := butane.TranslateBytes([]byte(u.data), butaneCommon.TranslateBytesOptions{ - // allow variant: openshift but don't generate a MachineConfig - Raw: true, - }) + ignc, report, err := u.translateButane() if err != nil { return nil, err } @@ -308,6 +316,87 @@ func (u *UserData) Render(warnings WarningsAction) (*Conf, error) { return c, nil } +// wrapper function to translate a Butane config to an Ignition config +func (u *UserData) translateButane() ([]byte, report.Report, error) { + if u.kind != kindButane { + panic("translateButane() called on non-Butane UserData") + } + + // First, try a normal translation + ignc, report, err := butane.TranslateBytes([]byte(u.data), butaneCommon.TranslateBytesOptions{ + // allow variant: openshift but don't generate a MachineConfig + Raw: true, + }) + butaneVersion, ok := err.(butaneCommon.ErrUnknownVersion) + if !ok { + // succeeded, or failed for reasons other than the + // config version + return ignc, report, err + } + + // This is an unrecognized spec version. Possibly the tests have + // been updated to stabilize a new version, but mantle hasn't been + // updated to parse it yet. Staple "-experimental" onto the config + // version and see if it parses. + if butaneVersion.Version.PreRelease != "" { + return ignc, report, fmt.Errorf("Butane config has unrecognized spec version and prerelease is already set to %q; cannot work around", butaneVersion.Version.PreRelease) + } + var parsed map[string]interface{} + if err := yaml.Unmarshal([]byte(u.data), &parsed); err != nil { + return ignc, report, err + } + version := butaneVersion.Version + version.PreRelease = "experimental" + parsed["version"] = version.String() + buc, err := yaml.Marshal(parsed) + if err != nil { + return ignc, report, err + } + ignc, report, err = butane.TranslateBytes(buc, butaneCommon.TranslateBytesOptions{ + Raw: true, + }) + if err != nil { + return ignc, report, fmt.Errorf("Butane config has unrecognized spec version and workaround didn't help: %w", err) + } + + // Stapling on "-experimental" worked. Now we need to check whether + // the resulting Ignition config has an -experimental version. + version, _, err = ignutil.GetConfigVersion(ignc) + if err != nil { + return ignc, report, err + } + if version.PreRelease == "" { + // no -experimental; we're done + plog.Warningf("mantle's vendored Butane has not been updated for %s spec %s; applying workaround", butaneVersion.Variant, butaneVersion.Version) + return ignc, report, nil + } + + // The Ignition config version has an -experimental suffix. Since + // we're going through a spec stabilization, Ignition in the image + // will probably not accept this config any longer. Remove the + // -experimental suffix and reserialize. + parsed = nil + if err := json.Unmarshal(ignc, &parsed); err != nil { + return ignc, report, err + } + ignitionSection, ok := parsed["ignition"] + if !ok { + return ignc, report, fmt.Errorf("no ignition section in Butane output") + } + ignitionSectionMap, ok := ignitionSection.(map[string]interface{}) + if !ok { + return ignc, report, fmt.Errorf("ignition section is not a map") + } + version.PreRelease = "" + ignitionSectionMap["version"] = version.String() + ignc, err = json.Marshal(parsed) + if err != nil { + return ignc, report, err + } + plog.Warningf("mantle's vendored Butane has not been updated for %s spec %s or Ignition spec %s; applying workaround", butaneVersion.Variant, butaneVersion.Version, version) + return ignc, report, nil +} + // String returns the string representation of the userdata in Conf. func (c *Conf) String() string { if c.ignitionV3 != nil { @@ -322,8 +411,11 @@ func (c *Conf) String() string { } else if c.ignitionV33 != nil { buf, _ := json.Marshal(c.ignitionV33) return string(buf) - } else if c.ignitionV34exp != nil { - buf, _ := json.Marshal(c.ignitionV34exp) + } else if c.ignitionV34 != nil { + buf, _ := json.Marshal(c.ignitionV34) + return string(buf) + } else if c.ignitionV35exp != nil { + buf, _ := json.Marshal(c.ignitionV35exp) return string(buf) } @@ -355,25 +447,31 @@ func (c *Conf) MergeV33(newConfig v33types.Config) { } // MergeV34exp merges a config with the ignitionV34exp config via Ignition's merging function. -func (c *Conf) MergeV34exp(newConfig v34exptypes.Config) { - mergeConfig := v34exp.Merge(*c.ignitionV34exp, newConfig) - c.ignitionV34exp = &mergeConfig +func (c *Conf) MergeV34(newConfig v34types.Config) { + mergeConfig := v34.Merge(*c.ignitionV34, newConfig) + c.ignitionV34 = &mergeConfig +} + +// MergeV35exp merges a config with the ignitionV35exp config via Ignition's merging function. +func (c *Conf) MergeV35exp(newConfig v35exptypes.Config) { + mergeConfig := v35exp.Merge(*c.ignitionV35exp, newConfig) + c.ignitionV35exp = &mergeConfig } -// Merge all configs into a V3.3 config +// Merge all configs into a V3.1 config func MergeAllConfigs(confObjs []*Conf) (*UserData, error) { config := Conf{ - ignitionV33: &v33types.Config{ - Ignition: v33types.Ignition{ - Version: "3.3.0", + ignitionV3: &v3types.Config{ + Ignition: v3types.Ignition{ + Version: "3.0.0", }, }, } - objectsToMerge := &config.ignitionV33.Ignition.Config.Merge + objectsToMerge := &config.ignitionV3.Ignition.Config.Merge for _, conf := range confObjs { ud := conf.String() url := dataurl.EncodeBytes([]byte(ud)) - obj := v33types.Resource{ + obj := v3types.ConfigReference{ Source: &url, } *objectsToMerge = append(*objectsToMerge, obj) @@ -444,8 +542,11 @@ func (c *Conf) ValidConfig() bool { } else if c.ignitionV33 != nil { rpt := validate.ValidateWithContext(c.ignitionV33, nil) return !rpt.IsFatal() - } else if c.ignitionV34exp != nil { - rpt := validate.ValidateWithContext(c.ignitionV34exp, nil) + } else if c.ignitionV34 != nil { + rpt := validate.ValidateWithContext(c.ignitionV34, nil) + return !rpt.IsFatal() + } else if c.ignitionV35exp != nil { + rpt := validate.ValidateWithContext(c.ignitionV35exp, nil) return !rpt.IsFatal() } else { return false @@ -562,20 +663,20 @@ func (c *Conf) addFileV33(path, contents string, mode int) { c.MergeV33(newConfig) } -func (c *Conf) addFileV34exp(path, contents string, mode int) { +func (c *Conf) addFileV34(path, contents string, mode int) { source := dataurl.EncodeBytes([]byte(contents)) - newConfig := v34exptypes.Config{ - Ignition: v34exptypes.Ignition{ - Version: "3.4.0-experimental", + newConfig := v34types.Config{ + Ignition: v34types.Ignition{ + Version: "3.4.0", }, - Storage: v34exptypes.Storage{ - Files: []v34exptypes.File{ + Storage: v34types.Storage{ + Files: []v34types.File{ { - Node: v34exptypes.Node{ + Node: v34types.Node{ Path: path, }, - FileEmbedded1: v34exptypes.FileEmbedded1{ - Contents: v34exptypes.Resource{ + FileEmbedded1: v34types.FileEmbedded1{ + Contents: v34types.Resource{ Source: &source, }, Mode: &mode, @@ -584,7 +685,32 @@ func (c *Conf) addFileV34exp(path, contents string, mode int) { }, }, } - c.MergeV34exp(newConfig) + c.MergeV34(newConfig) +} + +func (c *Conf) addFileV35exp(path, contents string, mode int) { + source := dataurl.EncodeBytes([]byte(contents)) + newConfig := v35exptypes.Config{ + Ignition: v35exptypes.Ignition{ + Version: "3.5.0-experimental", + }, + Storage: v35exptypes.Storage{ + Files: []v35exptypes.File{ + { + Node: v35exptypes.Node{ + Path: path, + }, + FileEmbedded1: v35exptypes.FileEmbedded1{ + Contents: v35exptypes.Resource{ + Source: &source, + }, + Mode: &mode, + }, + }, + }, + }, + } + c.MergeV35exp(newConfig) } func (c *Conf) AddFile(path, contents string, mode int) { @@ -596,8 +722,10 @@ func (c *Conf) AddFile(path, contents string, mode int) { c.addFileV32(path, contents, mode) } else if c.ignitionV33 != nil { c.addFileV33(path, contents, mode) - } else if c.ignitionV34exp != nil { - c.addFileV34exp(path, contents, mode) + } else if c.ignitionV34 != nil { + c.addFileV34(path, contents, mode) + } else if c.ignitionV35exp != nil { + c.addFileV35exp(path, contents, mode) } } @@ -677,13 +805,13 @@ func (c *Conf) addSystemdUnitV33(name, contents string, enable, mask bool) { c.MergeV33(newConfig) } -func (c *Conf) addSystemdUnitV34exp(name, contents string, enable, mask bool) { - newConfig := v34exptypes.Config{ - Ignition: v34exptypes.Ignition{ - Version: "3.4.0-experimental", +func (c *Conf) addSystemdUnitV34(name, contents string, enable, mask bool) { + newConfig := v34types.Config{ + Ignition: v34types.Ignition{ + Version: "3.4.0", }, - Systemd: v34exptypes.Systemd{ - Units: []v34exptypes.Unit{ + Systemd: v34types.Systemd{ + Units: []v34types.Unit{ { Name: name, Contents: &contents, @@ -693,7 +821,26 @@ func (c *Conf) addSystemdUnitV34exp(name, contents string, enable, mask bool) { }, }, } - c.MergeV34exp(newConfig) + c.MergeV34(newConfig) +} + +func (c *Conf) addSystemdUnitV35exp(name, contents string, enable, mask bool) { + newConfig := v35exptypes.Config{ + Ignition: v35exptypes.Ignition{ + Version: "3.5.0-experimental", + }, + Systemd: v35exptypes.Systemd{ + Units: []v35exptypes.Unit{ + { + Name: name, + Contents: &contents, + Enabled: &enable, + Mask: &mask, + }, + }, + }, + } + c.MergeV35exp(newConfig) } func (c *Conf) AddSystemdUnit(name, contents string, state systemdUnitState) { @@ -712,8 +859,10 @@ func (c *Conf) AddSystemdUnit(name, contents string, state systemdUnitState) { c.addSystemdUnitV32(name, contents, enable, mask) } else if c.ignitionV33 != nil { c.addSystemdUnitV33(name, contents, enable, mask) - } else if c.ignitionV34exp != nil { - c.addSystemdUnitV34exp(name, contents, enable, mask) + } else if c.ignitionV34 != nil { + c.addSystemdUnitV34(name, contents, enable, mask) + } else if c.ignitionV35exp != nil { + c.addSystemdUnitV35exp(name, contents, enable, mask) } } @@ -805,16 +954,16 @@ func (c *Conf) addSystemdDropinV33(service, name, contents string) { c.MergeV33(newConfig) } -func (c *Conf) addSystemdDropinV34exp(service, name, contents string) { - newConfig := v34exptypes.Config{ - Ignition: v34exptypes.Ignition{ - Version: "3.4.0-experimental", +func (c *Conf) addSystemdDropinV34(service, name, contents string) { + newConfig := v34types.Config{ + Ignition: v34types.Ignition{ + Version: "3.4.0", }, - Systemd: v34exptypes.Systemd{ - Units: []v34exptypes.Unit{ + Systemd: v34types.Systemd{ + Units: []v34types.Unit{ { Name: service, - Dropins: []v34exptypes.Dropin{ + Dropins: []v34types.Dropin{ { Name: name, Contents: &contents, @@ -824,7 +973,29 @@ func (c *Conf) addSystemdDropinV34exp(service, name, contents string) { }, }, } - c.MergeV34exp(newConfig) + c.MergeV34(newConfig) +} + +func (c *Conf) addSystemdDropinV35exp(service, name, contents string) { + newConfig := v35exptypes.Config{ + Ignition: v35exptypes.Ignition{ + Version: "3.5.0-experimental", + }, + Systemd: v35exptypes.Systemd{ + Units: []v35exptypes.Unit{ + { + Name: service, + Dropins: []v35exptypes.Dropin{ + { + Name: name, + Contents: &contents, + }, + }, + }, + }, + }, + } + c.MergeV35exp(newConfig) } func (c *Conf) AddSystemdUnitDropin(service, name, contents string) { @@ -836,8 +1007,10 @@ func (c *Conf) AddSystemdUnitDropin(service, name, contents string) { c.addSystemdDropinV32(service, name, contents) } else if c.ignitionV33 != nil { c.addSystemdDropinV33(service, name, contents) - } else if c.ignitionV34exp != nil { - c.addSystemdDropinV34exp(service, name, contents) + } else if c.ignitionV34 != nil { + c.addSystemdDropinV34(service, name, contents) + } else if c.ignitionV35exp != nil { + c.addSystemdDropinV35exp(service, name, contents) } } @@ -925,17 +1098,38 @@ func (c *Conf) addAuthorizedKeysV33(username string, keys map[string]struct{}) { c.MergeV33(newConfig) } -func (c *Conf) addAuthorizedKeysV34exp(username string, keys map[string]struct{}) { - var keyObjs []v34exptypes.SSHAuthorizedKey +func (c *Conf) addAuthorizedKeysV34(username string, keys map[string]struct{}) { + var keyObjs []v34types.SSHAuthorizedKey + for key := range keys { + keyObjs = append(keyObjs, v34types.SSHAuthorizedKey(key)) + } + newConfig := v34types.Config{ + Ignition: v34types.Ignition{ + Version: "3.4.0", + }, + Passwd: v34types.Passwd{ + Users: []v34types.PasswdUser{ + { + Name: username, + SSHAuthorizedKeys: keyObjs, + }, + }, + }, + } + c.MergeV34(newConfig) +} + +func (c *Conf) addAuthorizedKeysV35exp(username string, keys map[string]struct{}) { + var keyObjs []v35exptypes.SSHAuthorizedKey for key := range keys { - keyObjs = append(keyObjs, v34exptypes.SSHAuthorizedKey(key)) + keyObjs = append(keyObjs, v35exptypes.SSHAuthorizedKey(key)) } - newConfig := v34exptypes.Config{ - Ignition: v34exptypes.Ignition{ - Version: "3.4.0-experimental", + newConfig := v35exptypes.Config{ + Ignition: v35exptypes.Ignition{ + Version: "3.5.0-experimental", }, - Passwd: v34exptypes.Passwd{ - Users: []v34exptypes.PasswdUser{ + Passwd: v35exptypes.Passwd{ + Users: []v35exptypes.PasswdUser{ { Name: username, SSHAuthorizedKeys: keyObjs, @@ -943,7 +1137,7 @@ func (c *Conf) addAuthorizedKeysV34exp(username string, keys map[string]struct{} }, }, } - c.MergeV34exp(newConfig) + c.MergeV35exp(newConfig) } // AddAuthorizedKeys adds an Ignition config to add the given keys to the SSH @@ -962,8 +1156,10 @@ func (c *Conf) AddAuthorizedKeys(user string, keys []string) { c.addAuthorizedKeysV32(user, keysSet) } else if c.ignitionV33 != nil { c.addAuthorizedKeysV33(user, keysSet) - } else if c.ignitionV34exp != nil { - c.addAuthorizedKeysV34exp(user, keysSet) + } else if c.ignitionV34 != nil { + c.addAuthorizedKeysV34(user, keysSet) + } else if c.ignitionV35exp != nil { + c.addAuthorizedKeysV35exp(user, keysSet) } } @@ -1059,26 +1255,48 @@ func (c *Conf) addConfigSourceV33(source string) { c.MergeV33(newConfig) } -func (c *Conf) addConfigSourceV34exp(source string) { - var resources []v34exptypes.Resource - var headers []v34exptypes.HTTPHeader - resources = append(resources, v34exptypes.Resource{ +func (c *Conf) addConfigSourceV34(source string) { + var resources []v34types.Resource + var headers []v34types.HTTPHeader + resources = append(resources, v34types.Resource{ Compression: nil, HTTPHeaders: headers, Source: &source, - Verification: v34exptypes.Verification{ + Verification: v34types.Verification{ Hash: nil, }, }) - newConfig := v34exptypes.Config{ - Ignition: v34exptypes.Ignition{ - Version: "3.4.0-experimental", - Config: v34exptypes.IgnitionConfig{ + newConfig := v34types.Config{ + Ignition: v34types.Ignition{ + Version: "3.4.0", + Config: v34types.IgnitionConfig{ Merge: resources, }, }, } - c.MergeV34exp(newConfig) + c.MergeV34(newConfig) +} + +func (c *Conf) addConfigSourceV35exp(source string) { + var resources []v35exptypes.Resource + var headers []v35exptypes.HTTPHeader + resources = append(resources, v35exptypes.Resource{ + Compression: nil, + HTTPHeaders: headers, + Source: &source, + Verification: v35exptypes.Verification{ + Hash: nil, + }, + }) + newConfig := v35exptypes.Config{ + Ignition: v35exptypes.Ignition{ + Version: "3.5.0-experimental", + Config: v35exptypes.IgnitionConfig{ + Merge: resources, + }, + }, + } + c.MergeV35exp(newConfig) } // AddConfigSource adds an Ignition config to merge (v3) the @@ -1092,15 +1310,17 @@ func (c *Conf) AddConfigSource(source string) { c.addConfigSourceV32(source) } else if c.ignitionV33 != nil { c.addConfigSourceV33(source) - } else if c.ignitionV34exp != nil { - c.addConfigSourceV34exp(source) + } else if c.ignitionV34 != nil { + c.addConfigSourceV34(source) + } else if c.ignitionV35exp != nil { + c.addConfigSourceV35exp(source) } } // IsIgnition returns true if the config is for Ignition. // Returns false in the case of empty configs func (c *Conf) IsIgnition() bool { - return c.ignitionV3 != nil || c.ignitionV31 != nil || c.ignitionV32 != nil || c.ignitionV33 != nil || c.ignitionV34exp != nil + return c.ignitionV3 != nil || c.ignitionV31 != nil || c.ignitionV32 != nil || c.ignitionV33 != nil || c.ignitionV34 != nil || c.ignitionV35exp != nil } func (c *Conf) IsEmpty() bool { @@ -1120,6 +1340,12 @@ func (c *Conf) AddAutoLogin() { c.AddSystemdUnitDropin("serial-getty@.service", "10-autologin.conf", getAutologinUnit("serial-getty@.service", "--keep-baud 115200,38400,9600")) } +// DisableAutomaticUpdates turns off zincati +func (c *Conf) DisableAutomaticUpdates() { + c.AddFile("/etc/zincati/config.d/90-disable-auto-updates.toml", `[updates] + enabled = false`, 0644) +} + // AddAutoResize adds an Ignition config for a `resize` function to resize serial consoles func (c *Conf) AddAutoResize() { c.AddFile("/etc/profile.d/autoresize.sh", ` diff --git a/mantle/platform/conf/conf_test.go b/mantle/platform/conf/conf_test.go index f8e099fdd819ea1c79896601b376de27e7648ae8..c40a89cac34675964842a2ca0937dc190f69606a 100644 --- a/mantle/platform/conf/conf_test.go +++ b/mantle/platform/conf/conf_test.go @@ -41,7 +41,7 @@ func TestConfCopyKey(t *testing.T) { Ignition(`{ "ignition": { "version": "2.2.0" } }`), Ignition(`{ "ignition": { "version": "2.3.0" } }`), Ignition(`{ "ignition": { "version": "2.4.0" } }`), - Ignition(`{ "ignition": { "version": "3.5.0" } }`), + Ignition(`{ "ignition": { "version": "3.6.0" } }`), } for _, tt := range tests { @@ -58,9 +58,10 @@ func TestConfCopyKey(t *testing.T) { Ignition(`{ "ignition": { "version": "3.1.0" } }`), Ignition(`{ "ignition": { "version": "3.2.0" } }`), Ignition(`{ "ignition": { "version": "3.3.0" } }`), - Ignition(`{ "ignition": { "version": "3.4.0-experimental" } }`), - // special-case handling of next stable spec Ignition(`{ "ignition": { "version": "3.4.0" } }`), + Ignition(`{ "ignition": { "version": "3.5.0-experimental" } }`), + // special-case handling of next stable spec + Ignition(`{ "ignition": { "version": "3.5.0" } }`), } for i, tt := range tests { diff --git a/mantle/platform/metal.go b/mantle/platform/metal.go index a2c9e0d45018c05f5fbcae1c5930b2d00c57c1eb..063b372bcef5d3b07e5c7607a6f861ea0d485a4c 100644 --- a/mantle/platform/metal.go +++ b/mantle/platform/metal.go @@ -47,7 +47,7 @@ var baseKargs = []string{"rd.neednet=1", "ip=dhcp", "ignition.firstboot", "ignit var ( // TODO expose this as an API that can be used by cosa too consoleKernelArgument = map[string]string{ - "x86_64": "ttyS0", + "x86_64": "ttyS0,115200n8", "ppc64le": "hvc0", "aarch64": "ttyAMA0", "s390x": "ttysclp0", @@ -122,6 +122,20 @@ func (inst *Install) PXE(kargs []string, liveIgnition, ignition conf.Conf, offli return nil, err } + installerConfig := installerConfig{ + Console: []string{consoleKernelArgument[coreosarch.CurrentRpmArch()]}, + } + installerConfigData, err := yaml.Marshal(installerConfig) + if err != nil { + return nil, err + } + mode := 0644 + + // XXX: https://github.com/coreos/coreos-installer/issues/1171 + if coreosarch.CurrentRpmArch() != "s390x" { + liveIgnition.AddFile("/etc/nestos/installer.d/mantle.yaml", string(installerConfigData), mode) + } + inst.kargs = kargs inst.ignition = ignition inst.liveIgnition = liveIgnition @@ -367,7 +381,7 @@ func (t *installerRun) completePxeSetup(kargs []string) error { case "pxe": pxeconfigdir := filepath.Join(t.tftpdir, "pxelinux.cfg") if err := os.Mkdir(pxeconfigdir, 0777); err != nil { - return err + return errors.Wrapf(err, "creating dir %s", pxeconfigdir) } pxeimages := []string{"pxelinux.0", "ldlinux.c32"} pxeconfig := []byte(fmt.Sprintf(` @@ -381,8 +395,9 @@ func (t *installerRun) completePxeSetup(kargs []string) error { if coreosarch.CurrentRpmArch() == "s390x" { pxeconfig = []byte(kargsStr) } - if err := os.WriteFile(filepath.Join(pxeconfigdir, "default"), pxeconfig, 0777); err != nil { - return err + pxeconfig_path := filepath.Join(pxeconfigdir, "default") + if err := os.WriteFile(pxeconfig_path, pxeconfig, 0777); err != nil { + return errors.Wrapf(err, "writing file %s", pxeconfig_path) } // this is only for s390x where the pxe image has to be created; @@ -393,25 +408,31 @@ func (t *installerRun) completePxeSetup(kargs []string) error { err := exec.Command("/usr/bin/mk-s390image", kernelpath, "-r", initrdpath, "-p", filepath.Join(pxeconfigdir, "default"), filepath.Join(t.tftpdir, pxeimages[0])).Run() if err != nil { - return err + return errors.Wrap(err, "running mk-s390image") } } else { for _, img := range pxeimages { srcpath := filepath.Join("/usr/share/syslinux", img) - if err := exec.Command("/usr/lib/coreos-assembler/cp-reflink", srcpath, t.tftpdir).Run(); err != nil { - return err + cp_cmd := exec.Command("/usr/lib/coreos-assembler/cp-reflink", srcpath, t.tftpdir) + cp_cmd.Stderr = os.Stderr + if err := cp_cmd.Run(); err != nil { + return errors.Wrapf(err, "running cp-reflink %s %s", srcpath, t.tftpdir) } } } t.pxe.bootfile = "/" + pxeimages[0] case "grub": - if err := exec.Command("grub2-mknetdir", "--net-directory="+t.tftpdir).Run(); err != nil { - return err + grub2_mknetdir_cmd := exec.Command("grub2-mknetdir", "--net-directory="+t.tftpdir) + grub2_mknetdir_cmd.Stderr = os.Stderr + if err := grub2_mknetdir_cmd.Run(); err != nil { + return errors.Wrap(err, "running grub2-mknetdir") } if t.pxe.pxeimagepath != "" { dstpath := filepath.Join(t.tftpdir, "boot/grub2") - if err := exec.Command("/usr/lib/coreos-assembler/cp-reflink", t.pxe.pxeimagepath, dstpath).Run(); err != nil { - return err + cp_cmd := exec.Command("/usr/lib/coreos-assembler/cp-reflink", t.pxe.pxeimagepath, dstpath) + cp_cmd.Stderr = os.Stderr + if err := cp_cmd.Run(); err != nil { + return errors.Wrapf(err, "running cp-reflink %s %s", t.pxe.pxeimagepath, dstpath) } } if err := os.WriteFile(filepath.Join(t.tftpdir, "boot/grub2/grub.cfg"), []byte(fmt.Sprintf(` @@ -424,7 +445,7 @@ func (t *installerRun) completePxeSetup(kargs []string) error { initrd %s } `, t.kern.kernel, kargsStr, t.kern.initramfs)), 0777); err != nil { - return err + return errors.Wrap(err, "writing grub.cfg") } default: panic("Unhandled boottype " + t.pxe.boottype) @@ -518,7 +539,7 @@ func (t *installerRun) run() (*QemuInstance, error) { func (inst *Install) runPXE(kern *kernelSetup, offline bool) (*InstalledMachine, error) { t, err := inst.setup(kern) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "setting up install") } defer func() { err = t.destroy() @@ -526,7 +547,7 @@ func (inst *Install) runPXE(kern *kernelSetup, offline bool) (*InstalledMachine, bootStartedChan, err := inst.Builder.VirtioChannelRead("bootstarted") if err != nil { - return nil, err + return nil, errors.Wrapf(err, "setting up bootstarted virtio-serial channel") } kargs := renderBaseKargs() @@ -535,11 +556,11 @@ func (inst *Install) runPXE(kern *kernelSetup, offline bool) (*InstalledMachine, kargs = append(kargs, renderInstallKargs(t, offline)...) if err := t.completePxeSetup(kargs); err != nil { - return nil, err + return nil, errors.Wrapf(err, "completing PXE setup") } qinst, err := t.run() if err != nil { - return nil, err + return nil, errors.Wrapf(err, "running PXE install") } tempdir := t.tempdir t.tempdir = "" // Transfer ownership @@ -557,7 +578,8 @@ type installerConfig struct { Insecure bool `yaml:",omitempty"` AppendKargs []string `yaml:"append-karg,omitempty"` CopyNetwork bool `yaml:"copy-network,omitempty"` - DestDevice string `yaml:"dest-device"` + DestDevice string `yaml:"dest-device,omitempty"` + Console []string `yaml:"console,omitempty"` } func (inst *Install) InstallViaISOEmbed(kargs []string, liveIgnition, targetIgnition conf.Conf, outdir string, offline, minimal bool) (*InstalledMachine, error) { @@ -577,16 +599,16 @@ func (inst *Install) InstallViaISOEmbed(kargs []string, liveIgnition, targetIgni return nil, fmt.Errorf("Cannot use `--add-nm-keyfile` with offline mode") } - // XXX: we do support this now, via `coreos-installer iso kargs` - if len(inst.kargs) > 0 { - return nil, errors.New("injecting kargs is not supported yet, see https://github.com/coreos/coreos-installer/issues/164") - } - installerConfig := installerConfig{ IgnitionFile: "/var/opt/pointer.ign", DestDevice: "/dev/vda", } + // XXX: https://github.com/coreos/coreos-installer/issues/1171 + if coreosarch.CurrentRpmArch() != "s390x" { + installerConfig.Console = []string{consoleKernelArgument[coreosarch.CurrentRpmArch()]} + } + if inst.MultiPathDisk { // we only have one multipath device so it has to be that installerConfig.DestDevice = "/dev/mapper/mpatha" @@ -618,6 +640,20 @@ func (inst *Install) InstallViaISOEmbed(kargs []string, liveIgnition, targetIgni builddir := inst.CosaBuild.Dir srcisopath := filepath.Join(builddir, inst.CosaBuild.Meta.BuildArtifacts.LiveIso.Path) + + // Copy the ISO to a new location for modification. + // This is a bit awkward; we copy here, but QemuBuilder will also copy + // again (in `setupIso()`). I didn't want to lower the NM keyfile stuff + // into QemuBuilder. And plus, both tempdirs should be in /var/tmp so + // the `cp --reflink=auto` that QemuBuilder does should just reflink. + newIso := filepath.Join(tempdir, "install.iso") + cmd := exec.Command("cp", "--reflink=auto", srcisopath, newIso) + cmd.Stderr = os.Stderr + if err := cmd.Run(); err != nil { + return nil, errors.Wrapf(err, "copying iso") + } + srcisopath = newIso + var metalimg string if inst.Native4k { metalimg = inst.CosaBuild.Meta.BuildArtifacts.Metal4KNative.Path @@ -705,18 +741,8 @@ func (inst *Install) InstallViaISOEmbed(kargs []string, liveIgnition, targetIgni keyfileArgs = append(keyfileArgs, "--keyfile", path) } if len(keyfileArgs) > 0 { - // This is a bit awkward; we copy here, but QemuBuilder will also copy - // again (in `setupIso()`). I didn't want to lower the NM keyfile stuff - // into QemuBuilder. And plus, both tempdirs should be in /var/tmp so - // the `cp --reflink=auto` that QemuBuilder does should just reflink. - newIso := filepath.Join(tempdir, "install.iso") - cmd := exec.Command("cp", "--reflink=auto", srcisopath, newIso) - cmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { - return nil, errors.Wrapf(err, "copying iso") - } - args := []string{"iso", "network", "embed", newIso} + args := []string{"iso", "network", "embed", srcisopath} args = append(args, keyfileArgs...) cmd = exec.Command("nestos-installer", args...) cmd.Stderr = os.Stderr @@ -724,14 +750,22 @@ func (inst *Install) InstallViaISOEmbed(kargs []string, liveIgnition, targetIgni return nil, errors.Wrapf(err, "running nestos-installer iso network embed") } + installerConfig.CopyNetwork = true + // force networking on in the initrd to verify the keyfile was used - cmd = exec.Command("nestos-installer", "iso", "kargs", "modify", newIso, "--append", "rd.neednet=1") + inst.kargs = append(inst.kargs, "rd.neednet=1") + } + + if len(inst.kargs) > 0 { + args := []string{"iso", "kargs", "modify", srcisopath} + for _, karg := range inst.kargs { + args = append(args, "--append", karg) + } + cmd = exec.Command("nestos-installer", args...) cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { - return nil, errors.Wrapf(err, "running nestos-installer iso kargs modify") + return nil, errors.Wrapf(err, "running nestos-installer iso kargs") } - srcisopath = newIso - installerConfig.CopyNetwork = true } if inst.Insecure { diff --git a/mantle/platform/platform.go b/mantle/platform/platform.go index e777317c2b6b309fdee2985fec91c1a76ed387a8..7e038d55795d3738c014c4ca62a696c0d519dfc1 100644 --- a/mantle/platform/platform.go +++ b/mantle/platform/platform.go @@ -182,7 +182,7 @@ type Options struct { CosaBuildId string CosaBuildArch string - NoTestExitError bool + UseWarnExitCode77 bool // OSContainer is an image pull spec that can be given to the pivot service // in RHCOS machines to perform machine content upgrades. @@ -191,6 +191,8 @@ type Options struct { OSContainer string SSHOnTestFailure bool + + ExtendTimeoutPercent uint } // RuntimeConfig contains cluster-specific configuration. @@ -407,38 +409,11 @@ func NewMachines(c Cluster, userdata *conf.UserData, n int, options MachineOptio return machs, nil } -// checkSystemdUnitFailures ensures that no system unit is in a failed state, -// temporarily ignoring some non-fatal flakes on RHCOS. -// See: https://bugzilla.redhat.com/show_bug.cgi?id=1914362 -func checkSystemdUnitFailures(output string, distribution string) error { - if len(output) == 0 { - return nil - } - - var ignoredUnits []string - if distribution == "rhcos" { - ignoredUnits = append(ignoredUnits, "user@1000.service") - ignoredUnits = append(ignoredUnits, "user-runtime-dir@1000.service") +// checkSystemdUnitFailures ensures that no system unit is in a failed state. +func checkSystemdUnitFailures(output string) error { + if len(output) > 0 { + return fmt.Errorf("some systemd units failed: %s", output) } - - var failedUnits []string - for _, unit := range strings.Split(output, "\n") { - // Filter ignored units - ignored := false - for _, i := range ignoredUnits { - if unit == i { - ignored = true - break - } - } - if !ignored { - failedUnits = append(failedUnits, unit) - } - } - if len(failedUnits) > 0 { - return fmt.Errorf("some systemd units failed: %s", failedUnits) - } - return nil } @@ -473,7 +448,10 @@ func CheckMachine(ctx context.Context, m Machine) error { if err := ctx.Err(); err != nil { return err } - out, stderr, err := m.SSH("systemctl is-system-running") + // By design, `systemctl is-system-running` returns nonzero codes based on its state. + // We want to explicitly accept some nonzero states and test instead by the string so + // add `|| :`. + out, stderr, err := m.SSH("systemctl is-system-running || :") if !bytes.Contains([]byte("initializing starting running stopping"), out) { return nil // stop retrying if the system went haywire } @@ -487,24 +465,8 @@ func CheckMachine(ctx context.Context, m Machine) error { return errors.Wrapf(err, "ssh unreachable") } - out, stderr, err := m.SSH(`. /etc/os-release && echo "$ID-$VARIANT_ID"`) - if err != nil { - return fmt.Errorf("no /etc/os-release file: %v: %s", err, stderr) - } - - // ensure we're talking to a supported system - var distribution string - switch string(out) { - case `nestos-container`, `NestOS-nestos`: - distribution = "nestos" - // Reserved for older versions of NestOS - case `fedora-coreos`: - distribution = "fcos" - case `scos-`, `scos-coreos`, `rhcos-`, `rhcos-coreos`: - distribution = "rhcos" - default: - return fmt.Errorf("not a supported instance: %v", string(out)) - } + // NestOS don't want to check '/etc/os-release' to check if it is supported + // check systemd version on host to see if we can use `busctl --json=short` var systemdVer int @@ -531,7 +493,7 @@ func CheckMachine(ctx context.Context, m Machine) error { if err != nil { return fmt.Errorf("failed to query systemd for failed units: %s: %v: %s", out, err, stderr) } - err = checkSystemdUnitFailures(string(out), distribution) + err = checkSystemdUnitFailures(string(out)) if err != nil { plog.Error(err) systemdFailures = true diff --git a/mantle/platform/platform_test.go b/mantle/platform/platform_test.go deleted file mode 100644 index 9e277e4e84cf538a7ddd992c17a6babc0874a34f..0000000000000000000000000000000000000000 --- a/mantle/platform/platform_test.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2021 Red Hat, 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 platform - -import "testing" - -func TestSystemUnitFiltering(t *testing.T) { - // Output from: - // $ busctl --json=short call \ - // org.freedesktop.systemd1 \ - // /org/freedesktop/systemd1 \ - // org.freedesktop.systemd1.Manager \ - // ListUnitsFiltered as 2 state failed \ - // | jq -r '.data[][][0]' - var output string - - output = `abrt-oops.service -systemd-timesyncd.service` - if checkSystemdUnitFailures(output, "fcos") == nil { - t.Errorf("Should have failed") - } - if checkSystemdUnitFailures(output, "rhcos") == nil { - t.Errorf("Should have failed") - } - - output = `user@1000.service -user-runtime-dir@1000.service` - if checkSystemdUnitFailures(output, "fcos") == nil { - t.Errorf("Should have failed") - } - if checkSystemdUnitFailures(output, "rhcos") != nil { - t.Errorf("Should have passed") - } - - output = `abrt-oops.service -user@1000.service -user-runtime-dir@1000.service` - if checkSystemdUnitFailures(output, "fcos") == nil { - t.Errorf("Should have failed") - } - if checkSystemdUnitFailures(output, "rhcos") == nil { - t.Errorf("Should have failed") - } -} diff --git a/mantle/platform/qemu.go b/mantle/platform/qemu.go index 833af78ac8300b77bb239a8d62c9ff3200c6db2c..1d1f46f75afcaa1e5d564bce456894eceefda3db 100644 --- a/mantle/platform/qemu.go +++ b/mantle/platform/qemu.go @@ -339,7 +339,7 @@ func (inst *QemuInstance) SwitchBootOrder() (err2 error) { switch dev.Device { case "installiso": bootdev = devpath - case "d1", "mpath10": + case "disk-1", "mpath10": primarydev = devpath case "mpath11": secondarydev = devpath @@ -383,8 +383,8 @@ func (inst *QemuInstance) SwitchBootOrder() (err2 error) { } // RemovePrimaryBlockDevice deletes the primary device from a qemu instance -// and sets the seconday device as primary. It expects that all block devices -// are mirrors. +// and sets the secondary device as primary. It expects that all block devices +// with device name disk- are mirrors. func (inst *QemuInstance) RemovePrimaryBlockDevice() (err2 error) { var primaryDevice string var secondaryDevicePath string @@ -397,7 +397,7 @@ func (inst *QemuInstance) RemovePrimaryBlockDevice() (err2 error) { // a `BackingFileDepth` parameter of a device and check if // it is a removable and part of `virtio-blk-pci` devices. for _, dev := range blkdevs.Return { - if !dev.Removable && strings.Contains(dev.DevicePath, "virtio-backend") { + if !dev.Removable && strings.HasPrefix(dev.Device, "disk-") { if dev.Inserted.BackingFileDepth == 1 { primaryDevice = dev.DevicePath } else { @@ -1131,7 +1131,7 @@ func (builder *QemuBuilder) addDiskImpl(disk *Disk, primary bool) error { opts = "," + strings.Join(diskOpts, ",") } - id := fmt.Sprintf("d%d", builder.diskID) + id := fmt.Sprintf("disk-%d", builder.diskID) // Avoid file locking detection, and the disks we create // here are always currently ephemeral. @@ -1918,6 +1918,43 @@ func (builder *QemuBuilder) Exec() (*QemuInstance, error) { return nil, fmt.Errorf("failed to connect over qmp to qemu instance") } + // Hacky code to test https://github.com/openshift/os/pull/1346 + if timeout, ok := os.LookupEnv("COSA_TEST_CDROM_UNPLUG"); ok { + val, err := time.ParseDuration(timeout) + if err != nil { + return nil, err + } + go func() { + devs, err := inst.listDevices() + if err != nil { + plog.Error("failed to list devices") + return + } + + var cdrom string + for _, dev := range devs.Return { + switch dev.Type { + case "child": + cdrom = filepath.Join("/machine/peripheral-anon", dev.Name) + default: + break + } + } + if cdrom == "" { + plog.Errorf("failed to get scsi-cd id") + return + } + + plog.Debugf("get cdrom id %s", cdrom) + time.Sleep(val) + if err := inst.deleteBlockDevice(cdrom); err != nil { + plog.Errorf("failed to delete block device: %s", cdrom) + return + } + plog.Info("delete cdrom") + }() + } + return &inst, nil } diff --git a/mantle/platform/qmp_util.go b/mantle/platform/qmp_util.go index 1cea8c8ec7fe648cf9eb0993f8944aafe9d83518..d485e193ed33979ff96d61dcdcef810c7bf6b543 100644 --- a/mantle/platform/qmp_util.go +++ b/mantle/platform/qmp_util.go @@ -56,12 +56,12 @@ func (inst *QemuInstance) listDevices() (*QOMDev, error) { listcmd := `{ "execute": "qom-list", "arguments": { "path": "/machine/peripheral-anon" } }` out, err := inst.runQmpCommand(listcmd) if err != nil { - return nil, errors.Wrapf(err, "Running QMP list command") + return nil, errors.Wrapf(err, "Running QMP qom-list command") } var devs QOMDev if err = json.Unmarshal(out, &devs); err != nil { - return nil, errors.Wrapf(err, "De-serializing QMP output") + return nil, errors.Wrapf(err, "De-serializing QMP qom-list output") } return &devs, nil } @@ -71,22 +71,22 @@ func (inst *QemuInstance) listBlkDevices() (*QOMBlkDev, error) { listcmd := `{ "execute": "query-block" }` out, err := inst.runQmpCommand(listcmd) if err != nil { - return nil, errors.Wrapf(err, "Running QMP list command") + return nil, errors.Wrapf(err, "Running QMP query-block command") } var devs QOMBlkDev if err = json.Unmarshal(out, &devs); err != nil { - return nil, errors.Wrapf(err, "De-serializing QMP output") + return nil, errors.Wrapf(err, "De-serializing QMP query-block output") } return &devs, nil } -// setBootIndenx uses the qmp socket to the bootindex for the particular device. +// setBootIndexForDevice uses the qmp socket to the bootindex for the particular device. func (inst *QemuInstance) setBootIndexForDevice(device string, bootindex int) error { cmd := fmt.Sprintf(`{ "execute":"qom-set", "arguments": { "path":"%s", "property":"bootindex", "value":%d } }`, device, bootindex) if _, err := inst.runQmpCommand(cmd); err != nil { - return errors.Wrapf(err, "Running QMP command") + return errors.Wrapf(err, "Setting bootindex of device %s to %d", device, bootindex) } return nil } @@ -95,7 +95,7 @@ func (inst *QemuInstance) setBootIndexForDevice(device string, bootindex int) er func (inst *QemuInstance) deleteBlockDevice(device string) error { cmd := fmt.Sprintf(`{ "execute": "device_del", "arguments": { "id":"%s" } }`, device) if _, err := inst.runQmpCommand(cmd); err != nil { - return errors.Wrapf(err, "Running QMP command") + return errors.Wrapf(err, "Deleting block device %s", device) } return nil }