From 01cfb51fe989f5b3d810961e6813653f10ea541a Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 25 Jun 2021 15:58:39 +0200 Subject: [PATCH] auto-update: make output more user friendly The rather raw and scarce output of `podman auto-update` has been a thorn in my eyes for a longer while. So far, Podman would only print updated systemd units, one per line, without further formatting. Motivated by issue #9949 which is asking for some more useful information in combination with a dry-run feature, I sat down and reflected which information may come in handy. Running `podman auto-update` will now look as follows: ``` $ podman auto-update Trying to pull [...] UNIT CONTAINER IMAGE POLICY UPDATED container-test.service 08fd34e533fd (test) localhost:5000/busybox registry false ``` Also refactor the spaghetti code in the backend a bit to make it easier to digest and maintain. For easier testing and for the sake of consistency with other commands listing output, add a `--format` flag. The man page will get an overhaul in a follow up commit. Signed-off-by: Valentin Rothberg --- cmd/podman/auto-update.go | 99 +++++++++- docs/source/markdown/podman-auto-update.1.md | 24 ++- pkg/autoupdate/autoupdate.go | 186 +++++++++++++------ pkg/domain/entities/auto-update.go | 15 +- pkg/domain/entities/engine_container.go | 2 +- pkg/domain/infra/abi/auto-update.go | 2 +- pkg/domain/infra/tunnel/auto-update.go | 2 +- test/system/250-systemd.bats | 2 +- test/system/255-auto-update.bats | 15 +- 9 files changed, 263 insertions(+), 84 deletions(-) diff --git a/cmd/podman/auto-update.go b/cmd/podman/auto-update.go index 34cc7d74be..6ccf4e1670 100644 --- a/cmd/podman/auto-update.go +++ b/cmd/podman/auto-update.go @@ -1,10 +1,15 @@ package main import ( + "encoding/json" "fmt" + "os" + "strings" "github.com/containers/common/pkg/auth" "github.com/containers/common/pkg/completion" + "github.com/containers/common/pkg/report" + "github.com/containers/podman/v3/cmd/podman/common" "github.com/containers/podman/v3/cmd/podman/registry" "github.com/containers/podman/v3/pkg/domain/entities" "github.com/containers/podman/v3/pkg/errorhandling" @@ -12,8 +17,13 @@ import ( "github.com/spf13/cobra" ) +type cliAutoUpdateOptions struct { + entities.AutoUpdateOptions + format string +} + var ( - autoUpdateOptions = entities.AutoUpdateOptions{} + autoUpdateOptions = cliAutoUpdateOptions{} autoUpdateDescription = `Auto update containers according to their auto-update policy. Auto-update policies are specified with the "io.containers.autoupdate" label. @@ -42,6 +52,9 @@ func init() { authfileFlagName := "authfile" flags.StringVar(&autoUpdateOptions.Authfile, authfileFlagName, auth.GetDefaultAuthFile(), "Path to the authentication file. Use REGISTRY_AUTH_FILE environment variable to override") _ = autoUpdateCommand.RegisterFlagCompletionFunc(authfileFlagName, completion.AutocompleteDefault) + + flags.StringVar(&autoUpdateOptions.format, "format", "", "Change the output format to JSON or a Go template") + _ = autoUpdateCommand.RegisterFlagCompletionFunc("format", common.AutocompleteFormat(autoUpdateOutput{})) } func autoUpdate(cmd *cobra.Command, args []string) error { @@ -49,13 +62,83 @@ func autoUpdate(cmd *cobra.Command, args []string) error { // Backwards compat. System tests expect this error string. return errors.Errorf("`%s` takes no arguments", cmd.CommandPath()) } - report, failures := registry.ContainerEngine().AutoUpdate(registry.GetContext(), autoUpdateOptions) - if report != nil && len(report.Units) > 0 { - // Make it more obvious to users what the output means. - fmt.Println("\nRestarted the following systemd units:") - for _, unit := range report.Units { - fmt.Println(unit) - } + + allReports, failures := registry.ContainerEngine().AutoUpdate(registry.GetContext(), autoUpdateOptions.AutoUpdateOptions) + if allReports == nil { + return errorhandling.JoinErrors(failures) + } + + if err := writeTemplate(allReports, autoUpdateOptions.format); err != nil { + failures = append(failures, err) } + return errorhandling.JoinErrors(failures) } + +type autoUpdateOutput struct { + Unit string + Container string + ContainerName string + ContainerID string + Image string + Policy string + Updated string +} + +func reportsToOutput(allReports []*entities.AutoUpdateReport) []autoUpdateOutput { + output := make([]autoUpdateOutput, len(allReports)) + for i, r := range allReports { + output[i] = autoUpdateOutput{ + Unit: r.SystemdUnit, + Container: fmt.Sprintf("%s (%s)", r.ContainerID[:12], r.ContainerName), + ContainerName: r.ContainerName, + ContainerID: r.ContainerID, + Image: r.ImageName, + Policy: r.Policy, + Updated: r.Updated, + } + } + return output +} + +func writeTemplate(allReports []*entities.AutoUpdateReport, inputFormat string) error { + var format string + var printHeader bool + + output := reportsToOutput(allReports) + switch inputFormat { + case "": + rows := []string{"{{.Unit}}", "{{.Container}}", "{{.Image}}", "{{.Policy}}", "{{.Updated}}"} + format = "{{range . }}" + strings.Join(rows, "\t") + "\n{{end -}}" + printHeader = true + case "json": + prettyJSON, err := json.MarshalIndent(output, "", " ") + if err != nil { + return err + } + fmt.Println(string(prettyJSON)) + return nil + default: + format = "{{range . }}" + inputFormat + "\n{{end -}}" + } + + tmpl, err := report.NewTemplate("auto-update").Parse(format) + if err != nil { + return err + } + + w, err := report.NewWriterDefault(os.Stdout) + if err != nil { + return err + } + defer w.Flush() + + if printHeader { + headers := report.Headers(autoUpdateOutput{}, nil) + if err := tmpl.Execute(w, headers); err != nil { + return err + } + } + + return tmpl.Execute(w, output) +} diff --git a/docs/source/markdown/podman-auto-update.1.md b/docs/source/markdown/podman-auto-update.1.md index 24b9104707..4f2608be4a 100644 --- a/docs/source/markdown/podman-auto-update.1.md +++ b/docs/source/markdown/podman-auto-update.1.md @@ -41,6 +41,22 @@ If the authorization state is not found there, `$HOME/.docker/config.json` is ch Note: There is also the option to override the default path of the authentication file by setting the `REGISTRY_AUTH_FILE` environment variable. This can be done with **export REGISTRY_AUTH_FILE=_path_**. +#### **--format**=*format* + +Change the default output format. This can be of a supported type like 'json' or a Go template. +Valid placeholders for the Go template are listed below: + +| **Placeholder** | **Description** | +| --------------- | -------------------------------------- | +| .Unit | Name of the systemd unit | +| .ContainerName | Name of the container | +| .ContainerID | ID of the container | +| .Container | ID and name of the container | +| .Image | Name of the image | +| .Policy | Auto-update policy of the container | +| .Updated | Update status: true,false,failed | + + ## EXAMPLES Autoupdate with registry policy @@ -53,7 +69,7 @@ bc219740a210455fa27deacc96d50a9e20516492f1417507c13ce1533dbdcd9d ### Generate a systemd unit for this container $ podman generate systemd --new --files bc219740a210455fa27deacc96d50a9e20516492f1417507c13ce1533dbdcd9d -/home/user/containers/libpod/container-bc219740a210455fa27deacc96d50a9e20516492f1417507c13ce1533dbdcd9d.service +/home/user/container-bc219740a210455fa27deacc96d50a9e20516492f1417507c13ce1533dbdcd9d.service ### Load the new systemd unit and start it $ mv ./container-bc219740a210455fa27deacc96d50a9e20516492f1417507c13ce1533dbdcd9d.service ~/.config/systemd/user @@ -67,7 +83,7 @@ $ systemctl --user start container-bc219740a210455fa27deacc96d50a9e20516492f1417 ### Auto-update the container $ podman auto-update -container-bc219740a210455fa27deacc96d50a9e20516492f1417507c13ce1533dbdcd9d.service +[...] ``` Autoupdate with local policy @@ -80,7 +96,7 @@ be0889fd06f252a2e5141b37072c6bada68563026cb2b2649f53394d87ccc338 ### Generate a systemd unit for this container $ podman generate systemd --new --files be0889fd06f252a2e5141b37072c6bada68563026cb2b2649f53394d87ccc338 -/home/user/containers/libpod/container-be0889fd06f252a2e5141b37072c6bada68563026cb2b2649f53394d87ccc338.service +/home/user/container-be0889fd06f252a2e5141b37072c6bada68563026cb2b2649f53394d87ccc338.service ### Load the new systemd unit and start it $ mv ./container-be0889fd06f252a2e5141b37072c6bada68563026cb2b2649f53394d87ccc338.service ~/.config/systemd/user @@ -102,7 +118,7 @@ $ podman commit --change CMD=/bin/bash inspiring_galileo busybox:latest ### Auto-update the container $ podman auto-update -container-be0889fd06f252a2e5141b37072c6bada68563026cb2b2649f53394d87ccc338.service +[...] ``` ## SEE ALSO diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go index 6c21d1788f..fd95c319c2 100644 --- a/pkg/autoupdate/autoupdate.go +++ b/pkg/autoupdate/autoupdate.go @@ -15,6 +15,7 @@ import ( "github.com/containers/podman/v3/pkg/domain/entities" "github.com/containers/podman/v3/pkg/systemd" systemdDefine "github.com/containers/podman/v3/pkg/systemd/define" + "github.com/coreos/go-systemd/v22/dbus" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -119,7 +120,7 @@ func ValidateImageReference(imageName string) error { // // It returns a slice of successfully restarted systemd units and a slice of // errors encountered during auto update. -func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options Options) (*entities.AutoUpdateReport, []error) { +func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options Options) ([]*entities.AutoUpdateReport, []error) { // Create a map from `image ID -> []*Container`. containerMap, errs := imageContainersMap(runtime) if len(containerMap) == 0 { @@ -147,8 +148,8 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options Options) ( } defer conn.Close() - // Update images. - containersToRestart := []*libpod.Container{} + // Update all images/container according to their auto-update policy. + var allReports []*entities.AutoUpdateReport updatedRawImages := make(map[string]bool) for imageID, policyMapper := range containerMap { image, exists := imageMap[imageID] @@ -156,76 +157,139 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options Options) ( errs = append(errs, errors.Errorf("container image ID %q not found in local storage", imageID)) return nil, errs } - // Now we have to check if the image of any containers must be updated. - // Note that the image ID is NOT enough for this check as a given image - // may have multiple tags. - for _, registryCtr := range policyMapper[PolicyRegistryImage] { - cid := registryCtr.ID() - rawImageName := registryCtr.RawImageName() - if rawImageName == "" { - errs = append(errs, errors.Errorf("error registry auto-updating container %q: raw-image name is empty", cid)) - } - authfile := getAuthfilePath(registryCtr, options) - needsUpdate, err := newerRemoteImageAvailable(ctx, runtime, image, rawImageName, authfile) + + for _, ctr := range policyMapper[PolicyRegistryImage] { + report, err := autoUpdateRegistry(ctx, image, ctr, updatedRawImages, &options, conn, runtime) if err != nil { - errs = append(errs, errors.Wrapf(err, "error registry auto-updating container %q: image check for %q failed", cid, rawImageName)) - continue + errs = append(errs, err) } - - if needsUpdate { - logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName) - if _, updated := updatedRawImages[rawImageName]; !updated { - _, err = updateImage(ctx, runtime, rawImageName, options) - if err != nil { - errs = append(errs, errors.Wrapf(err, "error registry auto-updating container %q: image update for %q failed", cid, rawImageName)) - continue - } - updatedRawImages[rawImageName] = true - } - containersToRestart = append(containersToRestart, registryCtr) + if report != nil { + allReports = append(allReports, report) } } - for _, localCtr := range policyMapper[PolicyLocalImage] { - cid := localCtr.ID() - rawImageName := localCtr.RawImageName() - if rawImageName == "" { - errs = append(errs, errors.Errorf("error locally auto-updating container %q: raw-image name is empty", cid)) - } - // This avoids restarting containers unnecessarily. - needsUpdate, err := newerLocalImageAvailable(runtime, image, rawImageName) + for _, ctr := range policyMapper[PolicyLocalImage] { + report, err := autoUpdateLocally(ctx, image, ctr, &options, conn, runtime) if err != nil { - errs = append(errs, errors.Wrapf(err, "error locally auto-updating container %q: image check for %q failed", cid, rawImageName)) - continue + errs = append(errs, err) } - - if needsUpdate { - logrus.Infof("Auto-updating container %q using local image %q", cid, rawImageName) - containersToRestart = append(containersToRestart, localCtr) + if report != nil { + allReports = append(allReports, report) } } } - // Restart containers. - updatedUnits := []string{} - for _, ctr := range containersToRestart { - labels := ctr.Labels() - unit, exists := labels[systemdDefine.EnvVariable] - if !exists { - // Shouldn't happen but let's be sure of it. - errs = append(errs, errors.Errorf("error auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable)) - continue - } - _, err := conn.RestartUnit(unit, "replace", nil) - if err != nil { - errs = append(errs, errors.Wrapf(err, "error auto-updating container %q: restarting systemd unit %q failed", ctr.ID(), unit)) - continue + return allReports, errs +} + +// autoUpdateRegistry updates the image/container according to the "registry" policy. +func autoUpdateRegistry(ctx context.Context, image *libimage.Image, ctr *libpod.Container, updatedRawImages map[string]bool, options *Options, conn *dbus.Conn, runtime *libpod.Runtime) (*entities.AutoUpdateReport, error) { + cid := ctr.ID() + rawImageName := ctr.RawImageName() + if rawImageName == "" { + return nil, errors.Errorf("error registry auto-updating container %q: raw-image name is empty", cid) + } + + labels := ctr.Labels() + unit, exists := labels[systemdDefine.EnvVariable] + if !exists { + return nil, errors.Errorf("error auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable) + } + + report := &entities.AutoUpdateReport{ + ContainerID: cid, + ContainerName: ctr.Name(), + ImageName: rawImageName, + Policy: PolicyRegistryImage, + SystemdUnit: unit, + Updated: "failed", + } + + if _, updated := updatedRawImages[rawImageName]; updated { + logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName) + if err := restartSystemdUnit(ctr, unit, conn); err != nil { + return report, err } - logrus.Infof("Successfully restarted systemd unit %q", unit) - updatedUnits = append(updatedUnits, unit) + report.Updated = "true" + return report, nil } - return &entities.AutoUpdateReport{Units: updatedUnits}, errs + authfile := getAuthfilePath(ctr, options) + needsUpdate, err := newerRemoteImageAvailable(ctx, runtime, image, rawImageName, authfile) + if err != nil { + return report, errors.Wrapf(err, "error registry auto-updating container %q: image check for %q failed", cid, rawImageName) + } + + if !needsUpdate { + report.Updated = "false" + return report, nil + } + + if _, err := updateImage(ctx, runtime, rawImageName, options); err != nil { + return report, errors.Wrapf(err, "error registry auto-updating container %q: image update for %q failed", cid, rawImageName) + } + updatedRawImages[rawImageName] = true + + logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName) + if err := restartSystemdUnit(ctr, unit, conn); err != nil { + return report, err + } + + report.Updated = "true" + return report, nil +} + +// autoUpdateRegistry updates the image/container according to the "local" policy. +func autoUpdateLocally(ctx context.Context, image *libimage.Image, ctr *libpod.Container, options *Options, conn *dbus.Conn, runtime *libpod.Runtime) (*entities.AutoUpdateReport, error) { + cid := ctr.ID() + rawImageName := ctr.RawImageName() + if rawImageName == "" { + return nil, errors.Errorf("error locally auto-updating container %q: raw-image name is empty", cid) + } + + labels := ctr.Labels() + unit, exists := labels[systemdDefine.EnvVariable] + if !exists { + return nil, errors.Errorf("error auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable) + } + + report := &entities.AutoUpdateReport{ + ContainerID: cid, + ContainerName: ctr.Name(), + ImageName: rawImageName, + Policy: PolicyLocalImage, + SystemdUnit: unit, + Updated: "failed", + } + + needsUpdate, err := newerLocalImageAvailable(runtime, image, rawImageName) + if err != nil { + return report, errors.Wrapf(err, "error locally auto-updating container %q: image check for %q failed", cid, rawImageName) + } + + if !needsUpdate { + report.Updated = "false" + return report, nil + } + + logrus.Infof("Auto-updating container %q using local image %q", cid, rawImageName) + if err := restartSystemdUnit(ctr, unit, conn); err != nil { + return report, err + } + + report.Updated = "true" + return report, nil +} + +// restartSystemdUnit restarts the systemd unit the container is running in. +func restartSystemdUnit(ctr *libpod.Container, unit string, conn *dbus.Conn) error { + _, err := conn.RestartUnit(unit, "replace", nil) + if err != nil { + return errors.Wrapf(err, "error auto-updating container %q: restarting systemd unit %q failed", ctr.ID(), unit) + } + + logrus.Infof("Successfully restarted systemd unit %q of container %q", unit, ctr.ID()) + return nil } // imageContainersMap generates a map[image ID] -> [containers using the image] @@ -282,7 +346,7 @@ func imageContainersMap(runtime *libpod.Runtime) (map[string]policyMapper, []err // getAuthfilePath returns an authfile path, if set. The authfile label in the // container, if set, as precedence over the one set in the options. -func getAuthfilePath(ctr *libpod.Container, options Options) string { +func getAuthfilePath(ctr *libpod.Container, options *Options) string { labels := ctr.Labels() authFilePath, exists := labels[AuthfileLabel] if exists { @@ -311,7 +375,7 @@ func newerLocalImageAvailable(runtime *libpod.Runtime, img *libimage.Image, rawI } // updateImage pulls the specified image. -func updateImage(ctx context.Context, runtime *libpod.Runtime, name string, options Options) (*libimage.Image, error) { +func updateImage(ctx context.Context, runtime *libpod.Runtime, name string, options *Options) (*libimage.Image, error) { pullOptions := &libimage.PullOptions{} pullOptions.AuthFilePath = options.Authfile pullOptions.Writer = os.Stderr diff --git a/pkg/domain/entities/auto-update.go b/pkg/domain/entities/auto-update.go index c51158816b..d74462b86c 100644 --- a/pkg/domain/entities/auto-update.go +++ b/pkg/domain/entities/auto-update.go @@ -8,6 +8,17 @@ type AutoUpdateOptions struct { // AutoUpdateReport contains the results from running auto-update. type AutoUpdateReport struct { - // Units - the restarted systemd units during auto-update. - Units []string + // ID of the container *before* an update. + ContainerID string + // Name of the container *before* an update. + ContainerName string + // Name of the image. + ImageName string + // The configured auto-update policy. + Policy string + // SystemdUnit running a container configured for auto updates. + SystemdUnit string + // Indicates whether the image was updated and the container (and + // systemd unit) restarted. + Updated string } diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go index 28e5160db4..62e83fab38 100644 --- a/pkg/domain/entities/engine_container.go +++ b/pkg/domain/entities/engine_container.go @@ -14,7 +14,7 @@ import ( type ContainerCopyFunc func() error type ContainerEngine interface { - AutoUpdate(ctx context.Context, options AutoUpdateOptions) (*AutoUpdateReport, []error) + AutoUpdate(ctx context.Context, options AutoUpdateOptions) ([]*AutoUpdateReport, []error) Config(ctx context.Context) (*config.Config, error) ContainerAttach(ctx context.Context, nameOrID string, options AttachOptions) error ContainerCheckpoint(ctx context.Context, namesOrIds []string, options CheckpointOptions) ([]*CheckpointReport, error) diff --git a/pkg/domain/infra/abi/auto-update.go b/pkg/domain/infra/abi/auto-update.go index b9af4c92c4..daa882ecfd 100644 --- a/pkg/domain/infra/abi/auto-update.go +++ b/pkg/domain/infra/abi/auto-update.go @@ -7,7 +7,7 @@ import ( "github.com/containers/podman/v3/pkg/domain/entities" ) -func (ic *ContainerEngine) AutoUpdate(ctx context.Context, options entities.AutoUpdateOptions) (*entities.AutoUpdateReport, []error) { +func (ic *ContainerEngine) AutoUpdate(ctx context.Context, options entities.AutoUpdateOptions) ([]*entities.AutoUpdateReport, []error) { // Convert the entities options to the autoupdate ones. We can't use // them in the entities package as low-level packages must not leak // into the remote client. diff --git a/pkg/domain/infra/tunnel/auto-update.go b/pkg/domain/infra/tunnel/auto-update.go index 41165cc747..038c60537c 100644 --- a/pkg/domain/infra/tunnel/auto-update.go +++ b/pkg/domain/infra/tunnel/auto-update.go @@ -7,6 +7,6 @@ import ( "github.com/pkg/errors" ) -func (ic *ContainerEngine) AutoUpdate(ctx context.Context, options entities.AutoUpdateOptions) (*entities.AutoUpdateReport, []error) { +func (ic *ContainerEngine) AutoUpdate(ctx context.Context, options entities.AutoUpdateOptions) ([]*entities.AutoUpdateReport, []error) { return nil, []error{errors.New("not implemented")} } diff --git a/test/system/250-systemd.bats b/test/system/250-systemd.bats index 4ea1920098..aafe385c8c 100644 --- a/test/system/250-systemd.bats +++ b/test/system/250-systemd.bats @@ -119,7 +119,7 @@ function service_cleanup() { # Run auto-update and check that it restarted the container run_podman commit --change "CMD=/bin/bash" $cname $IMAGE run_podman auto-update - is $output $SERVICE_NAME "autoupdate local restarted container" + is "$output" ".*$SERVICE_NAME.*" "autoupdate local restarted container" # All good. Stop service, clean up. service_cleanup diff --git a/test/system/255-auto-update.bats b/test/system/255-auto-update.bats index cce8973fc7..4959bb6aa9 100644 --- a/test/system/255-auto-update.bats +++ b/test/system/255-auto-update.bats @@ -121,10 +121,10 @@ function _confirm_update() { generate_service alpine image _wait_service_ready container-$cname.service - run_podman auto-update + run_podman auto-update --format "{{.Unit}},{{.Image}},{{.Updated}},{{.Policy}}" is "$output" "Trying to pull.*" "Image is updated." - is "$output" ".*Restarted the following systemd units: -container-$cname.service" "Systemd unit has been restarted" + is "$output" ".*container-$cname.service,quay.io/libpod/alpine:latest,true,registry.*" "Image is updated." + _confirm_update $cname $ori_image } @@ -153,10 +153,15 @@ container-$cname.service" "Systemd unit has been restarted" @test "podman auto-update - label io.containers.autoupdate=local" { generate_service localtest local - podman commit --change CMD=/bin/bash $cname quay.io/libpod/localtest:latest + image=quay.io/libpod/localtest:latest + podman commit --change CMD=/bin/bash $cname $image + podman image inspect --format "{{.ID}}" $image + imageID="$output" _wait_service_ready container-$cname.service - run_podman auto-update + run_podman auto-update --format "{{.Unit}},{{.Image}},{{.Updated}},{{.Policy}}" + is "$output" ".*container-$cname.service,quay.io/libpod/localtest:latest,true,local.*" "Image is updated." + _confirm_update $cname $ori_image }