From 16a008585c3df0a890029af46cb544beb006ae1c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 20 Oct 2022 08:40:35 -0400 Subject: [PATCH] Add new `build-cimage` command This is a big step towards https://github.com/coreos/coreos-assembler/issues/2685 I know there's a lot going on with the pipeline, and I don't want to conflict with all that work - but at the same time, in my opinion we are just too dependent on complex Jenkins flows and our bespoke "meta.json in S3". The core of CoreOS *is a container image* now. This new command adds an opinionated flow where one can do: ``` $ cosa init $ cosa build-cimage quay.io/cgwalters/ostest ``` And *that's it* - we do proper change detection, reading and writing from the remote container image. We don't do silly things like storing an `.ociarchive` in S3 when we have native registries available. Later, we can build on this and rework our disk images to derive from that container image, as #2685 calls for. Also in the near term future, I think we can rework `cmd-build` such that it reuses this flow, but outputs to an `.ociarchive` instead. However, this code is going to need a bit more work to run in supermin. --- cmd/buildcimage.go | 188 ++++++++++++++++++++++++++++++++++ cmd/coreos-assembler.go | 2 + internal/pkg/cmdrun/cmdrun.go | 31 ++++++ internal/pkg/cosash/cosash.go | 5 + src/cmd-fetch | 1 + 5 files changed, 227 insertions(+) create mode 100644 cmd/buildcimage.go create mode 100644 internal/pkg/cmdrun/cmdrun.go diff --git a/cmd/buildcimage.go b/cmd/buildcimage.go new file mode 100644 index 0000000000..5038393324 --- /dev/null +++ b/cmd/buildcimage.go @@ -0,0 +1,188 @@ +// See usage below +package main + +// build-cimage is a wrapper for `rpm-ostree compose image`; for more +// on that, see https://coreos.github.io/rpm-ostree/container/ +// +// A key motivation here is to sever the dependency on S3 (and meta.json) for +// our container image builds. As part of the ostree native container work, +// the core of CoreOS becomes a container image. Our disk images +// have always been derivatives of the container, and this is pushing things +// farther in that direction. +// See https://github.com/coreos/coreos-assembler/issues/2685 +// +// This command is opinionated on reading and writing to a remote registry, +// whereas the underlying `rpm-ostree compose image` defaults to +// an ociarchive. + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "os" + + "github.com/coreos/coreos-assembler/internal/pkg/cmdrun" + "github.com/coreos/coreos-assembler/internal/pkg/cosash" + "github.com/spf13/cobra" +) + +const initConfigPath = "src/config.json" +const defaultManifest = "src/config/manifest.yaml" + +type BuildCImageOptions struct { + authfile string + initialize bool +} + +var ( + BuildCImageOpts BuildCImageOptions + + cmdBuildCImage = &cobra.Command{ + Use: "build-cimage", + Short: "cosa build-cimage [repository]", + Args: cobra.ExactArgs(1), + Long: "Initialize directory for ostree container image build", + RunE: implRunBuildCImage, + } +) + +func init() { + cmdBuildCImage.Flags().BoolVarP( + &BuildCImageOpts.initialize, "initialize", "i", false, + "Assume target image does not exist") + cmdBuildCImage.Flags().StringVar( + &BuildCImageOpts.authfile, "authfile", "", + "Path to container authentication file") +} + +func runBuildCImage(argv []string) error { + cmdBuildCImage.SetArgs(argv) + return cmdBuildCImage.Execute() +} + +// This is a Go reipmlementation of pick_yaml_or_else_json() from cmdlib.sh +func pickConfigFileYamlOrJson(name string, preferJson bool) (string, error) { + jsonPath := fmt.Sprintf("src/config/%s.json", name) + yamlPath := fmt.Sprintf("src/config/%s.yaml", name) + if _, err := os.Stat(jsonPath); err != nil { + if !os.IsNotExist(err) { + return "", err + } + jsonPath = "" + } + if _, err := os.Stat(yamlPath); err != nil { + if !os.IsNotExist(err) { + return "", err + } + yamlPath = "" + } + if jsonPath != "" && yamlPath != "" { + return "", fmt.Errorf("found both %s and %s", jsonPath, yamlPath) + } + if jsonPath != "" { + return jsonPath, nil + } + return yamlPath, nil +} + +type configVariant struct { + Variant string `json:"coreos-assembler.config-variant"` +} + +func getVariant() (string, error) { + contents, err := ioutil.ReadFile(initConfigPath) + if err != nil { + if !os.IsNotExist(err) { + return "", err + } + contents = []byte{} + } + + var variantData configVariant + if err := json.Unmarshal(contents, &variantData); err != nil { + return "", fmt.Errorf("parsing %s: %w", initConfigPath, err) + } + + return variantData.Variant, nil +} + +func implRunBuildCImage(c *cobra.Command, args []string) error { + if err := cmdrun.RunCmdSyncV("cosa", "build", "--prepare-only"); err != nil { + return err + } + + csh, err := cosash.NewCosaSh() + if err != nil { + return err + } + + basearch, err := csh.BaseArch() + if err != nil { + return err + } + variant, err := getVariant() + if err != nil { + return err + } + manifest := defaultManifest + if variant != "" { + manifest = fmt.Sprintf("src/config/manifest-%s.yaml", variant) + } + + repository := args[0] + + buildArgs := []string{"compose", "image", "--format", "registry", "--layer-repo", "tmp/repo"} + if BuildCImageOpts.initialize { + buildArgs = append(buildArgs, "--initialize") + } + if BuildCImageOpts.authfile != "" { + buildArgs = append(buildArgs, "--authfile", BuildCImageOpts.authfile) + } + if _, err := os.Stat("tmp/cosa-transient"); err != nil { + if !os.IsNotExist(err) { + return err + } + cachedir := "cache/buildcimage-cache" + if err := os.MkdirAll(cachedir, 0o755); err != nil { + return err + } + buildArgs = append(buildArgs, "--cachedir", cachedir) + } + manifestLock, err := pickConfigFileYamlOrJson(fmt.Sprintf("manifest-lock.%s", basearch), true) + if err != nil { + return err + } + manifestLockOverrides, err := pickConfigFileYamlOrJson("manifest-lock.overrides", false) + if err != nil { + return err + } + manifestLockArchOverrides, err := pickConfigFileYamlOrJson(fmt.Sprintf("manifest-lock.overrides.%s", basearch), false) + if err != nil { + return err + } + for _, lock := range []string{manifestLock, manifestLockOverrides, manifestLockArchOverrides} { + if lock != "" { + buildArgs = append(buildArgs, "--lockfile", lock) + } + } + buildArgs = append(buildArgs, manifest) + buildArgs = append(buildArgs, repository) + + argv0 := "rpm-ostree" + priv, err := csh.HasPrivileges() + if err != nil { + return err + } + if priv { + argv0 = "sudo" + buildArgs = append([]string{"rpm-ostree"}, buildArgs...) + } else { + return fmt.Errorf("this command currently requires the ability to create nested containers") + } + + if err := cmdrun.RunCmdSyncV(argv0, buildArgs...); err != nil { + return err + } + + return nil +} diff --git a/cmd/coreos-assembler.go b/cmd/coreos-assembler.go index dca810f94e..e4ba0e908f 100644 --- a/cmd/coreos-assembler.go +++ b/cmd/coreos-assembler.go @@ -88,6 +88,8 @@ func run(argv []string) error { switch cmd { case "clean": return runClean(argv) + case "build-cimage": + return runBuildCImage(argv) case "update-variant": return runUpdateVariant(argv) case "remote-session": diff --git a/internal/pkg/cmdrun/cmdrun.go b/internal/pkg/cmdrun/cmdrun.go new file mode 100644 index 0000000000..bc75fa090c --- /dev/null +++ b/internal/pkg/cmdrun/cmdrun.go @@ -0,0 +1,31 @@ +package cmdrun + +import ( + "fmt" + "os" + "os/exec" + "strings" + "syscall" +) + +// Synchronously invoke a command, logging the command arguments +// to stdout. +func RunCmdSyncV(cmdName string, args ...string) error { + fmt.Printf("Running: %s %s\n", cmdName, strings.Join(args, " ")) + return RunCmdSync(cmdName, args...) +} + +// Synchronously invoke a command, passing both stdout and stderr. +func RunCmdSync(cmdName string, args ...string) error { + cmd := exec.Command(cmdName, args...) + cmd.SysProcAttr = &syscall.SysProcAttr{ + Pdeathsig: syscall.SIGTERM, + } + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("error running %s %s: %w", cmdName, strings.Join(args, " "), err) + } + + return nil +} diff --git a/internal/pkg/cosash/cosash.go b/internal/pkg/cosash/cosash.go index 49c91a3470..48395df72e 100644 --- a/internal/pkg/cosash/cosash.go +++ b/internal/pkg/cosash/cosash.go @@ -173,6 +173,11 @@ pwd >&3 `) } +// BaseArch returns the base architecture +func (sh *CosaSh) BaseArch() (string, error) { + return sh.ProcessWithReply(`echo $basearch >&3`) +} + // HasPrivileges checks if we can use sudo func (sh *CosaSh) HasPrivileges() (bool, error) { r, err := sh.ProcessWithReply(` diff --git a/src/cmd-fetch b/src/cmd-fetch index 2d909b6569..bf447b1652 100755 --- a/src/cmd-fetch +++ b/src/cmd-fetch @@ -85,6 +85,7 @@ fi prepare_build +# NOTE: Keep this logic in sync with buildcimage.go args= if [ -n "${UPDATE_LOCKFILE}" ]; then # Put this under tmprepo so it gets automatically chown'ed if needed