From 8dba2d64c6f515c2192bd30f1bce94b878ac846e Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 25 Mar 2020 10:15:57 -0400 Subject: [PATCH] kola: promote concept of sdk.LocalBuild throughout Instead of keeping separate the build metadata object and the build directory, let's instead raise the visibility of `sdk.LocalBuild`, which bundles those two things together already. What I initially set out to solve was that `cosa kola --build ID` no longer worked, because we weren't passing `--cosa-build` to kola anymore. But even if we did, it's silly to have `cosa kola` dig into build dirs to fetch the path to the `meta.json` when `kola` now knows how to do that. Concretely, this changes three things: 1. we add a `--cosa-workdir` to allow specifying a cosa dir other than the current working dir. 2. the `--cosa-build` argument is now a build ID, not a path to a `meta.json`. 3. `kola.CosaBuild` is now a `LocalBuild` so that we also have access to the build directory everywhere. --- mantle/cmd/kola/kola.go | 14 ++++---- mantle/cmd/kola/options.go | 55 +++++++++++++++++++----------- mantle/cmd/kola/testiso.go | 23 ++++++------- mantle/kola/harness.go | 4 +-- mantle/kola/tests/upgrade/basic.go | 12 +++---- mantle/platform/metal.go | 43 ++++++++++++----------- mantle/platform/platform.go | 3 +- mantle/sdk/repo.go | 11 ++---- src/cmd-kola | 2 ++ 9 files changed, 89 insertions(+), 78 deletions(-) diff --git a/mantle/cmd/kola/kola.go b/mantle/cmd/kola/kola.go index 77ad7fbca7..f5671af2b9 100644 --- a/mantle/cmd/kola/kola.go +++ b/mantle/cmd/kola/kola.go @@ -481,10 +481,10 @@ func runArtifactIgnitionVersion(cmd *cobra.Command, args []string) error { } func preRunUpgrade(cmd *cobra.Command, args []string) error { - // unlike `kola run`, we *require* the --cosa-build arg -- XXX: figure out + // unlike `kola run`, we *require* the --build arg -- XXX: figure out // how to get this working using cobra's MarkFlagRequired() - if kola.Options.CosaBuild == "" { - errors.New("Error: missing required argument --cosa-build") + if kola.Options.CosaBuildId == "" { + errors.New("Error: missing required argument --build") } err := syncOptionsImpl(false) @@ -592,15 +592,15 @@ func getParentFcosBuildBase() (string, error) { // parse commitmeta.json for `fedora-coreos.stream`, then fetch the stream // metadata for that stream, then fetch the release metadata - if kola.CosaBuild.BuildRef == "" { + if kola.CosaBuild.Meta.BuildRef == "" { return "", errors.New("no ref in build metadata") } - stream := filepath.Base(kola.CosaBuild.BuildRef) + stream := filepath.Base(kola.CosaBuild.Meta.BuildRef) var parentVersion string - if kola.CosaBuild.FedoraCoreOsParentVersion != "" { - parentVersion = kola.CosaBuild.FedoraCoreOsParentVersion + if kola.CosaBuild.Meta.FedoraCoreOsParentVersion != "" { + parentVersion = kola.CosaBuild.Meta.FedoraCoreOsParentVersion } else { // ok, we're probably operating on a local dev build since the pipeline // always injects the parent; just instead fetch the release index diff --git a/mantle/cmd/kola/options.go b/mantle/cmd/kola/options.go index 96a756ad65..b9291cbaa6 100644 --- a/mantle/cmd/kola/options.go +++ b/mantle/cmd/kola/options.go @@ -21,7 +21,6 @@ import ( "strings" "github.com/coreos/mantle/auth" - "github.com/coreos/mantle/cosa" "github.com/coreos/mantle/kola" "github.com/coreos/mantle/platform" "github.com/coreos/mantle/sdk" @@ -58,7 +57,8 @@ func init() { sv(&kola.Options.IgnitionVersion, "ignition-version", "", "Ignition version override: v2, v3") ssv(&kola.BlacklistedTests, "blacklist-test", []string{}, "List of tests to blacklist") bv(&kola.Options.SSHOnTestFailure, "ssh-on-test-failure", false, "SSH into a machine when tests fail") - sv(&kola.Options.CosaBuild, "cosa-build", "", "File path for coreos-assembler meta.json") + sv(&kola.Options.CosaWorkdir, "workdir", "", "coreos-assembler working directory") + sv(&kola.Options.CosaBuildId, "build", "", "coreos-assembler build ID") // rhcos-specific options sv(&kola.Options.OSContainer, "oscontainer", "", "oscontainer image pullspec for pivot (RHCOS only)") @@ -142,8 +142,6 @@ func init() { // Sync up the command line options if there is dependency func syncOptionsImpl(useCosa bool) error { - var err error - kola.PacketOptions.Board = kola.QEMUOptions.Board kola.PacketOptions.GSOptions = &kola.GCEOptions @@ -175,28 +173,45 @@ func syncOptionsImpl(useCosa bool) error { } foundCosa := false - if kola.Options.CosaBuild == "" { - isroot, err := sdk.IsCosaRoot(".") + if kola.Options.CosaBuildId != "" { + // specified --build? fetch that build. in this path we *require* a + // cosa workdir, either assumed as PWD or via --workdir. + + if kola.Options.CosaWorkdir == "" { + kola.Options.CosaWorkdir = "." + } + + localbuild, err := sdk.GetLocalBuild(kola.Options.CosaWorkdir, kola.Options.CosaBuildId) if err != nil { return err } - if isroot { - localbuild, err := sdk.GetLatestLocalBuild() + + kola.CosaBuild = localbuild + foundCosa = true + } else { + if kola.Options.CosaWorkdir == "" { + // specified neither --build nor --workdir; only opportunistically + // try to use the PWD as the workdir, but don't error out if it's + // not + if isroot, err := sdk.IsCosaRoot("."); err != nil { + return err + } else if isroot { + kola.Options.CosaWorkdir = "." + } + } + + if kola.Options.CosaWorkdir != "" { + localbuild, err := sdk.GetLatestLocalBuild(kola.Options.CosaWorkdir) if err != nil { return err } - kola.Options.CosaBuild = filepath.Join(localbuild.Dir, "meta.json") - kola.CosaBuild = localbuild.Meta + kola.Options.CosaBuildId = localbuild.Meta.BuildID + kola.CosaBuild = localbuild foundCosa = true } - } else { - kola.CosaBuild, err = cosa.ParseBuild(kola.Options.CosaBuild) - if err != nil { - return err - } - foundCosa = true } + if foundCosa && useCosa { if err := syncCosaOptions(); err != nil { return err @@ -247,19 +262,19 @@ func syncOptions() error { func syncCosaOptions() error { switch kolaPlatform { case "qemu-unpriv", "qemu": - if kola.QEMUOptions.DiskImage == "" && kola.CosaBuild.BuildArtifacts.Qemu != nil { - kola.QEMUOptions.DiskImage = filepath.Join(filepath.Dir(kola.Options.CosaBuild), kola.CosaBuild.BuildArtifacts.Qemu.Path) + if kola.QEMUOptions.DiskImage == "" && kola.CosaBuild.Meta.BuildArtifacts.Qemu != nil { + kola.QEMUOptions.DiskImage = filepath.Join(kola.CosaBuild.Dir, kola.CosaBuild.Meta.BuildArtifacts.Qemu.Path) } } if kola.Options.IgnitionVersion == "" { if kola.CosaBuild != nil { - kola.Options.IgnitionVersion = sdk.TargetIgnitionVersion(kola.CosaBuild) + kola.Options.IgnitionVersion = sdk.TargetIgnitionVersion(kola.CosaBuild.Meta) } } if kola.Options.Distribution == "" { - distro, err := sdk.TargetDistro(kola.CosaBuild) + distro, err := sdk.TargetDistro(kola.CosaBuild.Meta) if err != nil { return err } diff --git a/mantle/cmd/kola/testiso.go b/mantle/cmd/kola/testiso.go index 6e7d25230c..dbaeeebd31 100644 --- a/mantle/cmd/kola/testiso.go +++ b/mantle/cmd/kola/testiso.go @@ -86,8 +86,7 @@ func runTestIso(cmd *cobra.Command, args []string) error { } baseInst := platform.Install{ - CosaBuildDir: kola.Options.CosaBuild, - CosaBuild: kola.CosaBuild, + CosaBuild: kola.CosaBuild, Console: console, Firmware: kola.QEMUOptions.Firmware, @@ -109,13 +108,13 @@ func runTestIso(cmd *cobra.Command, args []string) error { "-device", "virtio-serial", "-device", "virtserialport,chardev=completion,name=completion", "-chardev", "file,id=completion,path=" + completionfile} - if kola.CosaBuild.BuildArtifacts.Metal == nil { - return fmt.Errorf("Build %s must have a `metal` artifact", kola.CosaBuild.OstreeVersion) + if kola.CosaBuild.Meta.BuildArtifacts.Metal == nil { + return fmt.Errorf("Build %s must have a `metal` artifact", kola.CosaBuild.Meta.OstreeVersion) } ranTest := false - foundLegacy := baseInst.CosaBuild.BuildArtifacts.Kernel != nil + foundLegacy := baseInst.CosaBuild.Meta.BuildArtifacts.Kernel != nil if foundLegacy { if legacy { ranTest = true @@ -125,16 +124,16 @@ func runTestIso(cmd *cobra.Command, args []string) error { if err := testPXE(inst, completionfile); err != nil { return err } - fmt.Printf("Successfully tested legacy installer for %s\n", kola.CosaBuild.OstreeVersion) + fmt.Printf("Successfully tested legacy installer for %s\n", kola.CosaBuild.Meta.OstreeVersion) } } else if legacy { - return fmt.Errorf("build %s has no legacy installer kernel", kola.CosaBuild.Name) + return fmt.Errorf("build %s has no legacy installer kernel", kola.CosaBuild.Meta.Name) } - foundLive := kola.CosaBuild.BuildArtifacts.LiveKernel != nil + foundLive := kola.CosaBuild.Meta.BuildArtifacts.LiveKernel != nil if !nolive { if !foundLive { - return fmt.Errorf("build %s has no live installer kernel", kola.CosaBuild.Name) + return fmt.Errorf("build %s has no live installer kernel", kola.CosaBuild.Meta.Name) } if !nopxe { ranTest = true @@ -143,7 +142,7 @@ func runTestIso(cmd *cobra.Command, args []string) error { if err := testPXE(instPxe, completionfile); err != nil { return err } - fmt.Printf("Successfully tested PXE live installer for %s\n", kola.CosaBuild.OstreeVersion) + fmt.Printf("Successfully tested PXE live installer for %s\n", kola.CosaBuild.Meta.OstreeVersion) } ranTest = true @@ -151,7 +150,7 @@ func runTestIso(cmd *cobra.Command, args []string) error { if err := testLiveIso(instIso, completionfile); err != nil { return err } - fmt.Printf("Successfully tested ISO live installer for %s\n", kola.CosaBuild.OstreeVersion) + fmt.Printf("Successfully tested ISO live installer for %s\n", kola.CosaBuild.Meta.OstreeVersion) } if !ranTest { @@ -179,7 +178,7 @@ func testPXE(inst platform.Install, completionfile string) error { }, } var configStr string - if sdk.TargetIgnitionVersion(kola.CosaBuild) == "v2" { + if sdk.TargetIgnitionVersion(kola.CosaBuild.Meta) == "v2" { ignc2, err := ignconverter.Translate3to2(config) if err != nil { return err diff --git a/mantle/kola/harness.go b/mantle/kola/harness.go index 276991c941..12eb5a2af1 100644 --- a/mantle/kola/harness.go +++ b/mantle/kola/harness.go @@ -33,7 +33,6 @@ import ( ignv3 "github.com/coreos/ignition/v2/config/v3_0" ignv3types "github.com/coreos/ignition/v2/config/v3_0/types" - "github.com/coreos/mantle/cosa" "github.com/coreos/mantle/harness" "github.com/coreos/mantle/harness/reporters" "github.com/coreos/mantle/kola/cluster" @@ -55,6 +54,7 @@ import ( "github.com/coreos/mantle/platform/machine/openstack" "github.com/coreos/mantle/platform/machine/packet" "github.com/coreos/mantle/platform/machine/unprivqemu" + "github.com/coreos/mantle/sdk" "github.com/coreos/mantle/system" "github.com/coreos/mantle/util" ) @@ -72,7 +72,7 @@ var ( PacketOptions = packetapi.Options{Options: &Options} // glue to set platform options from main QEMUOptions = unprivqemu.Options{Options: &Options} // glue to set platform options from main - CosaBuild *cosa.Build // this is the parsed object of --cosa-build + CosaBuild *sdk.LocalBuild // this is a parsed cosa build TestParallelism int //glue var to set test parallelism from main TAPFile string // if not "", write TAP results here diff --git a/mantle/kola/tests/upgrade/basic.go b/mantle/kola/tests/upgrade/basic.go index ca24512350..ee64b696af 100644 --- a/mantle/kola/tests/upgrade/basic.go +++ b/mantle/kola/tests/upgrade/basic.go @@ -120,19 +120,19 @@ func fcosUpgradeBasic(c cluster.TestCluster) { // optimized for qemu testing locally where this won't leave localhost at // all. cloud testing should mostly be a pipeline thing, where the infra // connection should be much faster - ostreeTarPath := filepath.Join(filepath.Dir(kola.Options.CosaBuild), kola.CosaBuild.BuildArtifacts.Ostree.Path) + ostreeTarPath := filepath.Join(kola.CosaBuild.Dir, kola.CosaBuild.Meta.BuildArtifacts.Ostree.Path) if err := cluster.DropFile(c.Machines(), ostreeTarPath); err != nil { c.Fatal(err) } - c.MustSSHf(m, "tar -xf %s -C %s", kola.CosaBuild.BuildArtifacts.Ostree.Path, ostreeRepo) + c.MustSSHf(m, "tar -xf %s -C %s", kola.CosaBuild.Meta.BuildArtifacts.Ostree.Path, ostreeRepo) graph.seedFromMachine(c, m) - graph.addUpdate(c, m, kola.CosaBuild.OstreeVersion, kola.CosaBuild.OstreeCommit) + graph.addUpdate(c, m, kola.CosaBuild.Meta.OstreeVersion, kola.CosaBuild.Meta.OstreeCommit) }) c.Run("upgrade-from-previous", func(c cluster.TestCluster) { - waitForUpgradeToVersion(c, m, kola.CosaBuild.OstreeVersion) + waitForUpgradeToVersion(c, m, kola.CosaBuild.Meta.OstreeVersion) }) // Now, synthesize an update and serve that -- this is similar to @@ -141,9 +141,9 @@ func fcosUpgradeBasic(c cluster.TestCluster) { // Zincati & HTTP). Essentially, this sanity-checks that old starting state // + new content set can update. c.Run("upgrade-from-current", func(c cluster.TestCluster) { - newVersion := kola.CosaBuild.OstreeVersion + ".kola" + newVersion := kola.CosaBuild.Meta.OstreeVersion + ".kola" newCommit := c.MustSSHf(m, "ostree commit --repo %s -b %s --tree ref=%s --add-metadata-string version=%s", - ostreeRepo, kola.CosaBuild.BuildRef, kola.CosaBuild.OstreeCommit, newVersion) + ostreeRepo, kola.CosaBuild.Meta.BuildRef, kola.CosaBuild.Meta.OstreeCommit, newVersion) graph.addUpdate(c, m, newVersion, string(newCommit)) diff --git a/mantle/platform/metal.go b/mantle/platform/metal.go index 960926c4c0..263c05206f 100644 --- a/mantle/platform/metal.go +++ b/mantle/platform/metal.go @@ -31,8 +31,8 @@ import ( "github.com/pkg/errors" "github.com/vincent-petithory/dataurl" - "github.com/coreos/mantle/cosa" "github.com/coreos/mantle/platform/conf" + "github.com/coreos/mantle/sdk" "github.com/coreos/mantle/system" "github.com/coreos/mantle/system/exec" "github.com/coreos/mantle/util" @@ -77,8 +77,7 @@ var ( ) type Install struct { - CosaBuildDir string - CosaBuild *cosa.Build + CosaBuild *sdk.LocalBuild Firmware string Console bool @@ -99,8 +98,8 @@ type InstalledMachine struct { } func (inst *Install) PXE(kargs []string, ignition string) (*InstalledMachine, error) { - if inst.CosaBuild.BuildArtifacts.Metal == nil { - return nil, fmt.Errorf("Build %s must have a `metal` artifact", inst.CosaBuild.OstreeVersion) + if inst.CosaBuild.Meta.BuildArtifacts.Metal == nil { + return nil, fmt.Errorf("Build %s must have a `metal` artifact", inst.CosaBuild.Meta.OstreeVersion) } inst.kargs = kargs @@ -109,23 +108,23 @@ func (inst *Install) PXE(kargs []string, ignition string) (*InstalledMachine, er var err error var mach *InstalledMachine if inst.LegacyInstaller { - if inst.CosaBuild.BuildArtifacts.Kernel == nil { - return nil, fmt.Errorf("build %s has no legacy installer kernel", inst.CosaBuild.OstreeVersion) + if inst.CosaBuild.Meta.BuildArtifacts.Kernel == nil { + return nil, fmt.Errorf("build %s has no legacy installer kernel", inst.CosaBuild.Meta.OstreeVersion) } mach, err = inst.runPXE(&kernelSetup{ - kernel: inst.CosaBuild.BuildArtifacts.Kernel.Path, - initramfs: inst.CosaBuild.BuildArtifacts.Initramfs.Path, + kernel: inst.CosaBuild.Meta.BuildArtifacts.Kernel.Path, + initramfs: inst.CosaBuild.Meta.BuildArtifacts.Initramfs.Path, }, true) if err != nil { return nil, errors.Wrapf(err, "legacy installer") } } else { - if inst.CosaBuild.BuildArtifacts.LiveKernel == nil { - return nil, fmt.Errorf("build %s has no live installer kernel", inst.CosaBuild.Name) + if inst.CosaBuild.Meta.BuildArtifacts.LiveKernel == nil { + return nil, fmt.Errorf("build %s has no live installer kernel", inst.CosaBuild.Meta.Name) } mach, err = inst.runPXE(&kernelSetup{ - kernel: inst.CosaBuild.BuildArtifacts.LiveKernel.Path, - initramfs: inst.CosaBuild.BuildArtifacts.LiveInitramfs.Path, + kernel: inst.CosaBuild.Meta.BuildArtifacts.LiveKernel.Path, + initramfs: inst.CosaBuild.Meta.BuildArtifacts.LiveInitramfs.Path, }, false) if err != nil { return nil, errors.Wrapf(err, "testing live installer") @@ -262,7 +261,7 @@ func (inst *Install) setup(kern *kernelSetup) (*installerRun, error) { return nil, err } - builddir := filepath.Dir(inst.CosaBuildDir) + builddir := inst.CosaBuild.Dir serializedConfig := []byte(inst.ignition) if err := ioutil.WriteFile(filepath.Join(tftpdir, "config.ign"), serializedConfig, 0644); err != nil { return nil, err @@ -274,7 +273,7 @@ func (inst *Install) setup(kern *kernelSetup) (*installerRun, error) { } } - metalimg := inst.CosaBuild.BuildArtifacts.Metal.Path + metalimg := inst.CosaBuild.Meta.BuildArtifacts.Metal.Path metalname, err := setupMetalImage(builddir, metalimg, tftpdir) if err != nil { return nil, errors.Wrapf(err, "setting up metal image") @@ -504,11 +503,11 @@ func generatePointerIgnitionString(target string) string { } func (inst *Install) InstallViaISOEmbed(kargs []string, liveIgniton, targetIgnition string) (*InstalledMachine, error) { - if inst.CosaBuild.BuildArtifacts.Metal == nil { - return nil, fmt.Errorf("Build %s must have a `metal` artifact", inst.CosaBuild.OstreeVersion) + if inst.CosaBuild.Meta.BuildArtifacts.Metal == nil { + return nil, fmt.Errorf("Build %s must have a `metal` artifact", inst.CosaBuild.Meta.OstreeVersion) } - if inst.CosaBuild.BuildArtifacts.LiveIso == nil { - return nil, fmt.Errorf("Build %s must have a live ISO", inst.CosaBuild.Name) + if inst.CosaBuild.Meta.BuildArtifacts.LiveIso == nil { + return nil, fmt.Errorf("Build %s must have a live ISO", inst.CosaBuild.Meta.Name) } if len(inst.kargs) > 0 { @@ -534,9 +533,9 @@ func (inst *Install) InstallViaISOEmbed(kargs []string, liveIgniton, targetIgnit return nil, err } - builddir := filepath.Dir(inst.CosaBuildDir) - srcisopath := filepath.Join(builddir, inst.CosaBuild.BuildArtifacts.LiveIso.Path) - metalimg := inst.CosaBuild.BuildArtifacts.Metal.Path + builddir := inst.CosaBuild.Dir + srcisopath := filepath.Join(builddir, inst.CosaBuild.Meta.BuildArtifacts.LiveIso.Path) + metalimg := inst.CosaBuild.Meta.BuildArtifacts.Metal.Path metalname, err := setupMetalImage(builddir, metalimg, tempdir) if err != nil { return nil, errors.Wrapf(err, "setting up metal image") diff --git a/mantle/platform/platform.go b/mantle/platform/platform.go index 02fe721b7b..a723d0a447 100644 --- a/mantle/platform/platform.go +++ b/mantle/platform/platform.go @@ -161,7 +161,8 @@ type Options struct { IgnitionVersion string SystemdDropins []SystemdDropin - CosaBuild string + CosaWorkdir string + CosaBuildId string NoTestExitError bool diff --git a/mantle/sdk/repo.go b/mantle/sdk/repo.go index d86ad632ec..e00ec740b1 100644 --- a/mantle/sdk/repo.go +++ b/mantle/sdk/repo.go @@ -70,16 +70,11 @@ func RequireCosaRoot(root string) error { return nil } -func GetLatestLocalBuild() (*LocalBuild, error) { - return GetLocalBuild("latest") +func GetLatestLocalBuild(root string) (*LocalBuild, error) { + return GetLocalBuild(root, "latest") } -func GetLocalBuild(buildid string) (*LocalBuild, error) { - // Maybe in the future we support an environment variable or CLI switch - root, err := os.Getwd() - if err != nil { - return nil, err - } +func GetLocalBuild(root, buildid string) (*LocalBuild, error) { if err := RequireCosaRoot(root); err != nil { return nil, err } diff --git a/src/cmd-kola b/src/cmd-kola index 037bf57887..6035c68602 100755 --- a/src/cmd-kola +++ b/src/cmd-kola @@ -56,6 +56,8 @@ kolaargs = ['kola'] if os.getuid() != 0 and not ('-p' in unknown_args): kolaargs.extend(['-p', 'qemu-unpriv']) +if args.build is not None: + kolaargs.extend(['--build', args.build]) outputdir = args.output_dir or default_output_dir kolaargs.extend(['--output-dir', outputdir]) subargs = args.subargs or [default_cmd]