From 9d964ffb9fc98ed13f6fec974c917b84c175d39a Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 8 Jun 2020 13:34:12 -0400 Subject: Ensure Conmon is alive before waiting for exit file This came out of a conversation with Valentin about systemd-managed Podman. He discovered that unit files did not properly handle cases where Conmon was dead - the ExecStopPost `podman rm --force` line was not actually removing the container, but interestingly, adding a `podman cleanup --rm` line would remove it. Both of these commands do the same thing (minus the `podman cleanup --rm` command not force-removing running containers). Without a running Conmon instance, the container process is still running (assuming you killed Conmon with SIGKILL and it had no chance to kill the container it managed), but you can still kill the container itself with `podman stop` - Conmon is not involved, only the OCI Runtime. (`podman rm --force` and `podman stop` use the same code to kill the container). The problem comes when we want to get the container's exit code - we expect Conmon to make us an exit file, which it's obviously not going to do, being dead. The first `podman rm` would fail because of this, but importantly, it would (after failing to retrieve the exit code correctly) set container status to Exited, so that the second `podman cleanup` process would succeed. To make sure the first `podman rm --force` succeeds, we need to catch the case where Conmon is already dead, and instead of waiting for an exit file that will never come, immediately set the Stopped state and remove an error that can be caught and handled. Signed-off-by: Matthew Heon --- libpod/container_internal.go | 24 ++++++++++++++++++++++-- libpod/define/errors.go | 3 +++ libpod/oci.go | 7 +++++++ libpod/oci_conmon_linux.go | 25 +++++++++++++++++++++++++ libpod/oci_missing.go | 5 +++++ libpod/runtime_ctr.go | 6 ++++-- 6 files changed, 66 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 43e873bd6..f6fc3c1a4 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1209,13 +1209,35 @@ func (c *Container) stop(timeout uint) error { } } + // Check if conmon is still alive. + // If it is not, we won't be getting an exit file. + conmonAlive, err := c.ociRuntime.CheckConmonRunning(c) + if err != nil { + return err + } + if err := c.ociRuntime.StopContainer(c, timeout, all); err != nil { return err } + c.newContainerEvent(events.Stop) + c.state.PID = 0 c.state.ConmonPID = 0 c.state.StoppedByUser = true + + if !conmonAlive { + // Conmon is dead, so we can't epect an exit code. + c.state.ExitCode = -1 + c.state.FinishedTime = time.Now() + c.state.State = define.ContainerStateStopped + if err := c.save(); err != nil { + logrus.Errorf("Error saving container %s status: %v", c.ID(), err) + } + + return errors.Wrapf(define.ErrConmonDead, "container %s conmon process missing, cannot retrieve exit code", c.ID()) + } + if err := c.save(); err != nil { return errors.Wrapf(err, "error saving container %s state after stopping", c.ID()) } @@ -1225,8 +1247,6 @@ func (c *Container) stop(timeout uint) error { return err } - c.newContainerEvent(events.Stop) - return nil } diff --git a/libpod/define/errors.go b/libpod/define/errors.go index 16df2a1cc..083553b7e 100644 --- a/libpod/define/errors.go +++ b/libpod/define/errors.go @@ -141,6 +141,9 @@ var ( // ErrConmonOutdated indicates the version of conmon found (whether via the configuration or $PATH) // is out of date for the current podman version ErrConmonOutdated = errors.New("outdated conmon version") + // ErrConmonDead indicates that the container's conmon process has been + // killed, preventing normal operation. + ErrConmonDead = errors.New("conmon process killed") // ErrImageInUse indicates the requested operation failed because the image was in use ErrImageInUse = errors.New("image is being used") diff --git a/libpod/oci.go b/libpod/oci.go index 684a7ba42..c2f0041b1 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -107,6 +107,13 @@ type OCIRuntime interface { // error. CheckpointContainer(ctr *Container, options ContainerCheckpointOptions) error + // CheckConmonRunning verifies that the given container's Conmon + // instance is still running. Runtimes without Conmon, or systems where + // the PID of conmon is not available, should mock this as True. + // True indicates that Conmon for the instance is running, False + // indicates it is not. + CheckConmonRunning(ctr *Container) (bool, error) + // SupportsCheckpoint returns whether this OCI runtime // implementation supports the CheckpointContainer() operation. SupportsCheckpoint() bool diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 9c92b036e..0921a532b 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -669,6 +669,31 @@ func (r *ConmonOCIRuntime) CheckpointContainer(ctr *Container, options Container return utils.ExecCmdWithStdStreams(os.Stdin, os.Stdout, os.Stderr, nil, r.path, args...) } +func (r *ConmonOCIRuntime) CheckConmonRunning(ctr *Container) (bool, error) { + if ctr.state.ConmonPID == 0 { + // If the container is running or paused, assume Conmon is + // running. We didn't record Conmon PID on some old versions, so + // that is likely what's going on... + // Unusual enough that we should print a warning message though. + if ctr.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) { + logrus.Warnf("Conmon PID is not set, but container is running!") + return true, nil + } + // Container's not running, so conmon PID being unset is + // expected. Conmon is not running. + return false, nil + } + + // We have a conmon PID. Ping it with signal 0. + if err := unix.Kill(ctr.state.ConmonPID, 0); err != nil { + if err == unix.ESRCH { + return false, nil + } + return false, errors.Wrapf(err, "error pinging container %s conmon with signal 0", ctr.ID()) + } + return true, nil +} + // SupportsCheckpoint checks if the OCI runtime supports checkpointing // containers. func (r *ConmonOCIRuntime) SupportsCheckpoint() bool { diff --git a/libpod/oci_missing.go b/libpod/oci_missing.go index 4da16876c..90e90cc6c 100644 --- a/libpod/oci_missing.go +++ b/libpod/oci_missing.go @@ -163,6 +163,11 @@ func (r *MissingRuntime) CheckpointContainer(ctr *Container, options ContainerCh return r.printError() } +// CheckConmonRunning is not available as the runtime is missing +func (r *MissingRuntime) CheckConmonRunning(ctr *Container) (bool, error) { + return false, r.printError() +} + // SupportsCheckpoint returns false as checkpointing requires a working runtime func (r *MissingRuntime) SupportsCheckpoint() bool { return false diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 655b42e51..aa91dff03 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -464,9 +464,11 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool, } } - // Check that the container's in a good state to be removed + // Check that the container's in a good state to be removed. if c.state.State == define.ContainerStateRunning { - if err := c.stop(c.StopTimeout()); err != nil { + // Ignore ErrConmonDead - we couldn't retrieve the container's + // exit code properly, but it's still stopped. + if err := c.stop(c.StopTimeout()); err != nil && errors.Cause(err) != define.ErrConmonDead { return errors.Wrapf(err, "cannot remove container %s as it could not be stopped", c.ID()) } } -- cgit v1.2.3-54-g00ecf From 4bb43b898d72cb938d317055e4ade2771cfc9c54 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 9 Jun 2020 16:47:30 -0400 Subject: Fixup issues found by golint Signed-off-by: Daniel J Walsh --- .golangci.yml | 2 +- cmd/podman/common/default.go | 2 +- cmd/podman/common/specgen.go | 2 +- cmd/podman/containers/container.go | 2 +- cmd/podman/containers/stats.go | 6 +++--- cmd/podman/images/image.go | 2 +- cmd/podman/images/list.go | 4 ++-- cmd/podman/networks/list.go | 14 +++++++------- cmd/podman/pods/pod.go | 2 +- cmd/podman/pods/stats.go | 4 ++-- cmd/podman/registry/json.go | 4 ++-- cmd/podman/report/report.go | 2 +- cmd/podman/root.go | 8 ++++---- cmd/podman/system/df.go | 6 +++--- cmd/podman/system/service.go | 4 ++-- cmd/podman/system/system.go | 2 +- cmd/podman/volumes/volume.go | 2 +- libpod/filters/pods.go | 8 ++++---- libpod/image/filters.go | 6 +++--- pkg/api/handlers/compat/info.go | 2 +- pkg/api/handlers/compat/networks.go | 4 ---- pkg/api/handlers/libpod/pods.go | 6 +++--- pkg/api/handlers/libpod/volumes.go | 2 +- pkg/api/handlers/utils/containers.go | 2 +- pkg/api/server/handler_api.go | 6 +++--- pkg/api/server/server.go | 2 +- pkg/bindings/bindings.go | 3 +-- pkg/bindings/connection.go | 8 ++++---- pkg/bindings/generate/generate.go | 2 +- pkg/bindings/play/play.go | 2 +- pkg/domain/entities/engine.go | 4 ++-- pkg/domain/entities/images.go | 2 +- pkg/domain/infra/abi/containers.go | 4 ++-- pkg/domain/infra/abi/parse/parse.go | 2 +- pkg/domain/infra/abi/play.go | 2 +- pkg/domain/infra/abi/system.go | 8 ++++---- pkg/domain/infra/abi/trust.go | 8 ++++---- pkg/domain/infra/abi/volumes.go | 2 +- pkg/domain/infra/runtime_abi.go | 4 ++-- pkg/domain/infra/runtime_tunnel.go | 4 ++-- pkg/domain/infra/tunnel/containers.go | 2 +- pkg/domain/infra/tunnel/generate.go | 2 +- pkg/domain/infra/tunnel/play.go | 2 +- pkg/domain/infra/tunnel/pods.go | 2 +- pkg/domain/utils/utils.go | 2 +- pkg/network/network.go | 2 +- pkg/parallel/parallel_linux.go | 6 +++--- pkg/spec/createconfig.go | 2 +- pkg/specgen/container_validate.go | 2 +- pkg/specgen/generate/container_create.go | 2 +- pkg/specgen/generate/namespaces.go | 4 ++-- pkg/specgen/pod_validate.go | 2 +- pkg/systemd/generate/systemdgen_test.go | 4 ++-- pkg/trust/config.go | 4 ++-- pkg/varlinkapi/create.go | 4 ++-- pkg/varlinkapi/volumes.go | 2 +- 56 files changed, 100 insertions(+), 105 deletions(-) (limited to 'libpod') diff --git a/.golangci.yml b/.golangci.yml index 5480b02bb..33a8b4f59 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,6 +7,7 @@ run: - contrib - dependencies - test + - pkg/spec - pkg/varlink - pkg/varlinkapi skip-files: @@ -21,7 +22,6 @@ linters: - gochecknoinits - goconst - gocyclo - - golint - gosec - lll - maligned diff --git a/cmd/podman/common/default.go b/cmd/podman/common/default.go index 7233b2091..6e5994b18 100644 --- a/cmd/podman/common/default.go +++ b/cmd/podman/common/default.go @@ -16,5 +16,5 @@ var ( // DefaultImageVolume default value DefaultImageVolume = "bind" // Pull in configured json library - json = registry.JsonLibrary() + json = registry.JSONLibrary() ) diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index 26003b40f..2286e67de 100644 --- a/cmd/podman/common/specgen.go +++ b/cmd/podman/common/specgen.go @@ -669,7 +669,7 @@ func makeHealthCheckFromCli(inCmd, interval string, retries uint, timeout, start hc.Interval = intervalDuration if retries < 1 { - return nil, errors.New("healthcheck-retries must be greater than 0.") + return nil, errors.New("healthcheck-retries must be greater than 0") } hc.Retries = int(retries) timeoutDuration, err := time.ParseDuration(timeout) diff --git a/cmd/podman/containers/container.go b/cmd/podman/containers/container.go index a102318fb..3ff341dcd 100644 --- a/cmd/podman/containers/container.go +++ b/cmd/podman/containers/container.go @@ -10,7 +10,7 @@ import ( var ( // Pull in configured json library - json = registry.JsonLibrary() + json = registry.JSONLibrary() // Command: podman _container_ containerCmd = &cobra.Command{ diff --git a/cmd/podman/containers/stats.go b/cmd/podman/containers/stats.go index c61b161e4..357ab7d38 100644 --- a/cmd/podman/containers/stats.go +++ b/cmd/podman/containers/stats.go @@ -87,13 +87,13 @@ func init() { func checkStatOptions(cmd *cobra.Command, args []string) error { opts := 0 if statsOptions.All { - opts += 1 + opts++ } if statsOptions.Latest { - opts += 1 + opts++ } if len(args) > 0 { - opts += 1 + opts++ } if opts > 1 { return errors.Errorf("--all, --latest and containers cannot be used together") diff --git a/cmd/podman/images/image.go b/cmd/podman/images/image.go index 790c16c05..ebef126c0 100644 --- a/cmd/podman/images/image.go +++ b/cmd/podman/images/image.go @@ -9,7 +9,7 @@ import ( var ( // Pull in configured json library - json = registry.JsonLibrary() + json = registry.JSONLibrary() // Command: podman _image_ imageCmd = &cobra.Command{ diff --git a/cmd/podman/images/list.go b/cmd/podman/images/list.go index 23757104b..236ae15b4 100644 --- a/cmd/podman/images/list.go +++ b/cmd/podman/images/list.go @@ -100,7 +100,7 @@ func images(cmd *cobra.Command, args []string) error { switch { case listFlag.quiet: - return writeId(summaries) + return writeID(summaries) case cmd.Flag("format").Changed && listFlag.format == "json": return writeJSON(summaries) default: @@ -108,7 +108,7 @@ func images(cmd *cobra.Command, args []string) error { } } -func writeId(imageS []*entities.ImageSummary) error { +func writeID(imageS []*entities.ImageSummary) error { var ids = map[string]struct{}{} for _, e := range imageS { i := "sha256:" + e.ID diff --git a/cmd/podman/networks/list.go b/cmd/podman/networks/list.go index 24604c055..498a4dc18 100644 --- a/cmd/podman/networks/list.go +++ b/cmd/podman/networks/list.go @@ -33,8 +33,8 @@ var ( var ( networkListOptions entities.NetworkListOptions - headers string = "NAME\tVERSION\tPLUGINS\n" - defaultListRow string = "{{.Name}}\t{{.Version}}\t{{.Plugins}}\n" + headers = "NAME\tVERSION\tPLUGINS\n" + defaultListRow = "{{.Name}}\t{{.Version}}\t{{.Plugins}}\n" ) func networkListFlags(flags *pflag.FlagSet) { @@ -57,7 +57,7 @@ func init() { func networkList(cmd *cobra.Command, args []string) error { var ( - nlprs []NetworkListPrintReports + nlprs []ListPrintReports ) // validate the filter pattern. @@ -83,7 +83,7 @@ func networkList(cmd *cobra.Command, args []string) error { } for _, r := range responses { - nlprs = append(nlprs, NetworkListPrintReports{r}) + nlprs = append(nlprs, ListPrintReports{r}) } row := networkListOptions.Format @@ -125,14 +125,14 @@ func jsonOut(responses []*entities.NetworkListReport) error { return nil } -type NetworkListPrintReports struct { +type ListPrintReports struct { *entities.NetworkListReport } -func (n NetworkListPrintReports) Version() string { +func (n ListPrintReports) Version() string { return n.CNIVersion } -func (n NetworkListPrintReports) Plugins() string { +func (n ListPrintReports) Plugins() string { return network.GetCNIPlugins(n.NetworkConfigList) } diff --git a/cmd/podman/pods/pod.go b/cmd/podman/pods/pod.go index ed265ef90..9dc538c71 100644 --- a/cmd/podman/pods/pod.go +++ b/cmd/podman/pods/pod.go @@ -10,7 +10,7 @@ import ( var ( // Pull in configured json library - json = registry.JsonLibrary() + json = registry.JSONLibrary() // Command: podman _pod_ podCmd = &cobra.Command{ diff --git a/cmd/podman/pods/stats.go b/cmd/podman/pods/stats.go index d3950fdbc..d14632f01 100644 --- a/cmd/podman/pods/stats.go +++ b/cmd/podman/pods/stats.go @@ -71,7 +71,7 @@ func stats(cmd *cobra.Command, args []string) error { } format := statsOptions.Format - doJson := strings.ToLower(format) == formats.JSONString + doJSON := strings.ToLower(format) == formats.JSONString header := getPodStatsHeader(format) for { @@ -80,7 +80,7 @@ func stats(cmd *cobra.Command, args []string) error { return err } // Print the stats in the requested format and configuration. - if doJson { + if doJSON { if err := printJSONPodStats(reports); err != nil { return err } diff --git a/cmd/podman/registry/json.go b/cmd/podman/registry/json.go index f25406c3c..a8a1623f5 100644 --- a/cmd/podman/registry/json.go +++ b/cmd/podman/registry/json.go @@ -11,8 +11,8 @@ var ( jsonSync sync.Once ) -// JsonLibrary provides a "encoding/json" compatible API -func JsonLibrary() jsoniter.API { +// JSONLibrary provides a "encoding/json" compatible API +func JSONLibrary() jsoniter.API { jsonSync.Do(func() { json = jsoniter.ConfigCompatibleWithStandardLibrary }) diff --git a/cmd/podman/report/report.go b/cmd/podman/report/report.go index 8392f10e0..ce349ef35 100644 --- a/cmd/podman/report/report.go +++ b/cmd/podman/report/report.go @@ -3,4 +3,4 @@ package report import "github.com/containers/libpod/cmd/podman/registry" // Pull in configured json library -var json = registry.JsonLibrary() +var json = registry.JSONLibrary() diff --git a/cmd/podman/root.go b/cmd/podman/root.go index b62ee144a..4f834e87d 100644 --- a/cmd/podman/root.go +++ b/cmd/podman/root.go @@ -119,10 +119,10 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error { } if cmd.Flag("cpu-profile").Changed { - f, err := os.Create(cfg.CpuProfile) + f, err := os.Create(cfg.CPUProfile) if err != nil { return errors.Wrapf(err, "unable to create cpu profiling file %s", - cfg.CpuProfile) + cfg.CPUProfile) } if err := pprof.StartCPUProfile(f); err != nil { return err @@ -212,13 +212,13 @@ func rootFlags(opts *entities.PodmanConfig, flags *pflag.FlagSet) { // V2 flags flags.BoolVarP(&opts.Remote, "remote", "r", false, "Access remote Podman service (default false)") // TODO Read uri from containers.config when available - flags.StringVar(&opts.Uri, "url", registry.DefaultAPIAddress(), "URL to access Podman service (CONTAINER_HOST)") + flags.StringVar(&opts.URI, "url", registry.DefaultAPIAddress(), "URL to access Podman service (CONTAINER_HOST)") flags.StringSliceVar(&opts.Identities, "identity", []string{}, "path to SSH identity file, (CONTAINER_SSHKEY)") flags.StringVar(&opts.PassPhrase, "passphrase", "", "passphrase for identity file (not secure, CONTAINER_PASSPHRASE), ssh-agent always supported") cfg := opts.Config flags.StringVar(&cfg.Engine.CgroupManager, "cgroup-manager", cfg.Engine.CgroupManager, "Cgroup manager to use (\"cgroupfs\"|\"systemd\")") - flags.StringVar(&opts.CpuProfile, "cpu-profile", "", "Path for the cpu profiling results") + flags.StringVar(&opts.CPUProfile, "cpu-profile", "", "Path for the cpu profiling results") flags.StringVar(&opts.ConmonPath, "conmon", "", "Path of the conmon binary") flags.StringVar(&cfg.Engine.NetworkCmdPath, "network-cmd-path", cfg.Engine.NetworkCmdPath, "Path to the command for configuring the network") flags.StringVar(&cfg.Network.NetworkConfigDir, "cni-config-dir", cfg.Network.NetworkConfigDir, "Path of the configuration directory for CNI networks") diff --git a/cmd/podman/system/df.go b/cmd/podman/system/df.go index 8fe035209..9318bba12 100644 --- a/cmd/podman/system/df.go +++ b/cmd/podman/system/df.go @@ -63,7 +63,7 @@ func printSummary(reports *entities.SystemDfReport, userFormat string) error { dfSummaries []*dfSummary active int size, reclaimable int64 - format string = "{{.Type}}\t{{.Total}}\t{{.Active}}\t{{.Size}}\t{{.Reclaimable}}\n" + format = "{{.Type}}\t{{.Total}}\t{{.Active}}\t{{.Size}}\t{{.Reclaimable}}\n" w io.Writer = os.Stdout ) @@ -74,7 +74,7 @@ func printSummary(reports *entities.SystemDfReport, userFormat string) error { for _, i := range reports.Images { if i.Containers > 0 { - active += 1 + active++ } size += i.Size if i.Containers < 1 { @@ -99,7 +99,7 @@ func printSummary(reports *entities.SystemDfReport, userFormat string) error { ) for _, c := range reports.Containers { if c.Status == "running" { - conActive += 1 + conActive++ } else { conReclaimable += c.RWSize } diff --git a/cmd/podman/system/service.go b/cmd/podman/system/service.go index 1b07ee301..ecd17c251 100644 --- a/cmd/podman/system/service.go +++ b/cmd/podman/system/service.go @@ -64,7 +64,7 @@ func aliasTimeoutFlag(_ *pflag.FlagSet, name string) pflag.NormalizedName { } func service(cmd *cobra.Command, args []string) error { - apiURI, err := resolveApiURI(args) + apiURI, err := resolveAPIURI(args) if err != nil { return err } @@ -103,7 +103,7 @@ func service(cmd *cobra.Command, args []string) error { return restService(opts, cmd.Flags(), registry.PodmanConfig()) } -func resolveApiURI(_url []string) (string, error) { +func resolveAPIURI(_url []string) (string, error) { // When determining _*THE*_ listening endpoint -- // 1) User input wins always // 2) systemd socket activation diff --git a/cmd/podman/system/system.go b/cmd/podman/system/system.go index d9691ad2a..acf41a32d 100644 --- a/cmd/podman/system/system.go +++ b/cmd/podman/system/system.go @@ -9,7 +9,7 @@ import ( var ( // Pull in configured json library - json = registry.JsonLibrary() + json = registry.JSONLibrary() // Command: podman _system_ systemCmd = &cobra.Command{ diff --git a/cmd/podman/volumes/volume.go b/cmd/podman/volumes/volume.go index 3e90d178c..12947a6b1 100644 --- a/cmd/podman/volumes/volume.go +++ b/cmd/podman/volumes/volume.go @@ -9,7 +9,7 @@ import ( var ( // Pull in configured json library - json = registry.JsonLibrary() + json = registry.JSONLibrary() // Command: podman _volume_ volumeCmd = &cobra.Command{ diff --git a/libpod/filters/pods.go b/libpod/filters/pods.go index 9bf436eab..0edb9fbf2 100644 --- a/libpod/filters/pods.go +++ b/libpod/filters/pods.go @@ -57,13 +57,13 @@ func GeneratePodFilterFunc(filter, filterValue string) ( return nil, errors.Errorf("%s is not a valid status", filterValue) } return func(p *libpod.Pod) bool { - ctr_statuses, err := p.Status() + ctrStatuses, err := p.Status() if err != nil { return false } - for _, ctr_status := range ctr_statuses { - state := ctr_status.String() - if ctr_status == define.ContainerStateConfigured { + for _, ctrStatus := range ctrStatuses { + state := ctrStatus.String() + if ctrStatus == define.ContainerStateConfigured { state = "created" } if state == filterValue { diff --git a/libpod/image/filters.go b/libpod/image/filters.go index 747eba165..9d99fb344 100644 --- a/libpod/image/filters.go +++ b/libpod/image/filters.go @@ -102,8 +102,8 @@ func ReferenceFilter(ctx context.Context, referenceFilter string) ResultFilter { } } -// IdFilter allows you to filter by image Id -func IdFilter(idFilter string) ResultFilter { +// IDFilter allows you to filter by image Id +func IDFilter(idFilter string) ResultFilter { return func(i *Image) bool { return i.ID() == idFilter } @@ -172,7 +172,7 @@ func (ir *Runtime) createFilterFuncs(filters []string, img *Image) ([]ResultFilt case "reference": filterFuncs = append(filterFuncs, ReferenceFilter(ctx, splitFilter[1])) case "id": - filterFuncs = append(filterFuncs, IdFilter(splitFilter[1])) + filterFuncs = append(filterFuncs, IDFilter(splitFilter[1])) default: return nil, errors.Errorf("invalid filter %s ", splitFilter[0]) } diff --git a/pkg/api/handlers/compat/info.go b/pkg/api/handlers/compat/info.go index e9756a03f..d4a933c54 100644 --- a/pkg/api/handlers/compat/info.go +++ b/pkg/api/handlers/compat/info.go @@ -177,7 +177,7 @@ func getContainersState(r *libpod.Runtime) map[define.ContainerStatus]int { if err != nil { continue } - states[state] += 1 + states[state]++ } } return states diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index c52ca093f..8734ba405 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -20,10 +20,6 @@ import ( "github.com/pkg/errors" ) -type CompatInspectNetwork struct { - types.NetworkResource -} - func InspectNetwork(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) diff --git a/pkg/api/handlers/libpod/pods.go b/pkg/api/handlers/libpod/pods.go index c3f8d5d66..dd6262a20 100644 --- a/pkg/api/handlers/libpod/pods.go +++ b/pkg/api/handlers/libpod/pods.go @@ -31,11 +31,11 @@ func PodCreate(w http.ResponseWriter, r *http.Request) { } pod, err := generate.MakePod(&psg, runtime) if err != nil { - http_code := http.StatusInternalServerError + httpCode := http.StatusInternalServerError if errors.Cause(err) == define.ErrPodExists { - http_code = http.StatusConflict + httpCode = http.StatusConflict } - utils.Error(w, "Something went wrong.", http_code, err) + utils.Error(w, "Something went wrong.", httpCode, err) return } utils.WriteResponse(w, http.StatusCreated, handlers.IDResponse{ID: pod.ID()}) diff --git a/pkg/api/handlers/libpod/volumes.go b/pkg/api/handlers/libpod/volumes.go index c42ca407b..b5574b87b 100644 --- a/pkg/api/handlers/libpod/volumes.go +++ b/pkg/api/handlers/libpod/volumes.go @@ -46,7 +46,7 @@ func CreateVolume(w http.ResponseWriter, r *http.Request) { volumeOptions = append(volumeOptions, libpod.WithVolumeLabels(input.Label)) } if len(input.Options) > 0 { - parsedOptions, err := parse.ParseVolumeOptions(input.Options) + parsedOptions, err := parse.VolumeOptions(input.Options) if err != nil { utils.InternalServerError(w, err) return diff --git a/pkg/api/handlers/utils/containers.go b/pkg/api/handlers/utils/containers.go index a46b308b5..4bcac6e72 100644 --- a/pkg/api/handlers/utils/containers.go +++ b/pkg/api/handlers/utils/containers.go @@ -62,7 +62,7 @@ func WaitContainer(w http.ResponseWriter, r *http.Request) (int32, error) { func CreateContainer(ctx context.Context, w http.ResponseWriter, runtime *libpod.Runtime, cc *createconfig.CreateConfig) { var pod *libpod.Pod - ctr, err := createconfig.CreateContainerFromCreateConfig(runtime, cc, ctx, pod) + ctr, err := createconfig.CreateContainerFromCreateConfig(ctx, runtime, cc, pod) if err != nil { Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "CreateContainerFromCreateConfig()")) return diff --git a/pkg/api/server/handler_api.go b/pkg/api/server/handler_api.go index 7a7db12f3..dbdb7f17b 100644 --- a/pkg/api/server/handler_api.go +++ b/pkg/api/server/handler_api.go @@ -34,9 +34,9 @@ func (s *APIServer) APIHandler(h http.HandlerFunc) http.HandlerFunc { } // TODO: Use r.ConnContext when ported to go 1.13 - c := context.WithValue(r.Context(), "decoder", s.Decoder) - c = context.WithValue(c, "runtime", s.Runtime) - c = context.WithValue(c, "shutdownFunc", s.Shutdown) + c := context.WithValue(r.Context(), "decoder", s.Decoder) //nolint + c = context.WithValue(c, "runtime", s.Runtime) //nolint + c = context.WithValue(c, "shutdownFunc", s.Shutdown) //nolint r = r.WithContext(c) h(w, r) diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go index 499a4c58a..bd6a99b96 100644 --- a/pkg/api/server/server.go +++ b/pkg/api/server/server.go @@ -257,7 +257,7 @@ func (t *IdleTracker) ConnState(conn net.Conn, state http.ConnState) { if oldActive == 0 { t.timer.Stop() } - t.total += 1 + t.total++ case http.StateIdle, http.StateClosed: delete(t.active, conn) // Restart the timer if we've become idle diff --git a/pkg/bindings/bindings.go b/pkg/bindings/bindings.go index da47ea713..94f7a45d0 100644 --- a/pkg/bindings/bindings.go +++ b/pkg/bindings/bindings.go @@ -5,7 +5,6 @@ // This package exposes a series of methods that allow users to firstly // create their connection with the API endpoints. Once the connection // is established, users can then manage the Podman container runtime. - package bindings import ( @@ -28,7 +27,7 @@ var ( pFalse = false PFalse = &pFalse - // _*YES*- podman will fail to run if this value is wrong + // APIVersion - podman will fail to run if this value is wrong APIVersion = semver.MustParse("1.0.0") ) diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go index b130b9598..aa7f3707c 100644 --- a/pkg/bindings/connection.go +++ b/pkg/bindings/connection.go @@ -41,7 +41,7 @@ type APIResponse struct { } type Connection struct { - Uri *url.URL + URI *url.URL Client *http.Client } @@ -137,7 +137,7 @@ func NewConnectionWithIdentity(ctx context.Context, uri string, passPhrase strin func tcpClient(_url *url.URL) (Connection, error) { connection := Connection{ - Uri: _url, + URI: _url, } connection.Client = &http.Client{ Transport: &http.Transport{ @@ -246,7 +246,7 @@ func sshClient(_url *url.URL, secure bool, passPhrase string, identities ...stri return Connection{}, errors.Wrapf(err, "Connection to bastion host (%s) failed.", _url.String()) } - connection := Connection{Uri: _url} + connection := Connection{URI: _url} connection.Client = &http.Client{ Transport: &http.Transport{ DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { @@ -257,7 +257,7 @@ func sshClient(_url *url.URL, secure bool, passPhrase string, identities ...stri } func unixClient(_url *url.URL) (Connection, error) { - connection := Connection{Uri: _url} + connection := Connection{URI: _url} connection.Client = &http.Client{ Transport: &http.Transport{ DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { diff --git a/pkg/bindings/generate/generate.go b/pkg/bindings/generate/generate.go index 161b722f3..5e4be4896 100644 --- a/pkg/bindings/generate/generate.go +++ b/pkg/bindings/generate/generate.go @@ -10,7 +10,7 @@ import ( "github.com/containers/libpod/pkg/domain/entities" ) -func GenerateKube(ctx context.Context, nameOrID string, options entities.GenerateKubeOptions) (*entities.GenerateKubeReport, error) { +func Kube(ctx context.Context, nameOrID string, options entities.GenerateKubeOptions) (*entities.GenerateKubeReport, error) { conn, err := bindings.GetClient(ctx) if err != nil { return nil, err diff --git a/pkg/bindings/play/play.go b/pkg/bindings/play/play.go index 288cca454..9a4f56b6d 100644 --- a/pkg/bindings/play/play.go +++ b/pkg/bindings/play/play.go @@ -13,7 +13,7 @@ import ( "github.com/containers/libpod/pkg/domain/entities" ) -func PlayKube(ctx context.Context, path string, options entities.PlayKubeOptions) (*entities.PlayKubeReport, error) { +func Kube(ctx context.Context, path string, options entities.PlayKubeOptions) (*entities.PlayKubeReport, error) { var report entities.PlayKubeReport conn, err := bindings.GetClient(ctx) if err != nil { diff --git a/pkg/domain/entities/engine.go b/pkg/domain/entities/engine.go index b2bef0eea..1f056bad7 100644 --- a/pkg/domain/entities/engine.go +++ b/pkg/domain/entities/engine.go @@ -39,7 +39,7 @@ type PodmanConfig struct { CGroupUsage string // rootless code determines Usage message ConmonPath string // --conmon flag will set Engine.ConmonPath - CpuProfile string // Hidden: Should CPU profile be taken + CPUProfile string // Hidden: Should CPU profile be taken EngineMode EngineMode // ABI or Tunneling mode Identities []string // ssh identities for connecting to server MaxWorks int // maximum number of parallel threads @@ -52,7 +52,7 @@ type PodmanConfig struct { SpanCtx context.Context // context to use when tracing Syslog bool // write to StdOut and Syslog, not supported when tunneling Trace bool // Hidden: Trace execution - Uri string // URI to RESTful API Service + URI string // URI to RESTful API Service Runroot string StorageDriver string diff --git a/pkg/domain/entities/images.go b/pkg/domain/entities/images.go index 19a2c87f5..5bb110b57 100644 --- a/pkg/domain/entities/images.go +++ b/pkg/domain/entities/images.go @@ -299,7 +299,7 @@ type ShowTrustReport struct { Raw []byte SystemRegistriesDirPath string JSONOutput []byte - Policies []*trust.TrustPolicy + Policies []*trust.Policy } // SetTrustOptions describes the CLI options for setting trust diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 043fcfe7e..f8897b41e 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -162,7 +162,7 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin if err != nil && !(options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr) { return nil, err } - errMap, err := parallel.ParallelContainerOp(ctx, ctrs, func(c *libpod.Container) error { + errMap, err := parallel.ContainerOp(ctx, ctrs, func(c *libpod.Container) error { var err error if options.Timeout != nil { err = c.StopWithTimeout(*options.Timeout) @@ -323,7 +323,7 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, return reports, nil } - errMap, err := parallel.ParallelContainerOp(ctx, ctrs, func(c *libpod.Container) error { + errMap, err := parallel.ContainerOp(ctx, ctrs, func(c *libpod.Container) error { err := ic.Libpod.RemoveContainer(ctx, c, options.Force, options.Volumes) if err != nil { if options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr { diff --git a/pkg/domain/infra/abi/parse/parse.go b/pkg/domain/infra/abi/parse/parse.go index 6c0e1ee55..2320c6a32 100644 --- a/pkg/domain/infra/abi/parse/parse.go +++ b/pkg/domain/infra/abi/parse/parse.go @@ -12,7 +12,7 @@ import ( // Handle volume options from CLI. // Parse "o" option to find UID, GID. -func ParseVolumeOptions(opts map[string]string) ([]libpod.VolumeCreateOption, error) { +func VolumeOptions(opts map[string]string) ([]libpod.VolumeCreateOption, error) { libpodOptions := []libpod.VolumeCreateOption{} volumeOptions := make(map[string]string) diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 6d0919d2b..932974aba 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -243,7 +243,7 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, path string, options en if err != nil { return nil, err } - ctr, err := createconfig.CreateContainerFromCreateConfig(ic.Libpod, conf, ctx, pod) + ctr, err := createconfig.CreateContainerFromCreateConfig(ctx, ic.Libpod, conf, pod) if err != nil { return nil, err } diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index 9b538b301..b91dd513d 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -338,7 +338,7 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System } for _, viu := range inUse { if util.StringInSlice(viu, runningContainers) { - consInUse += 1 + consInUse++ } } report := entities.SystemDfVolumeReport{ @@ -376,12 +376,12 @@ func (se *SystemEngine) Renumber(ctx context.Context, flags *pflag.FlagSet, conf return nil } -func (s SystemEngine) Migrate(ctx context.Context, flags *pflag.FlagSet, config *entities.PodmanConfig, options entities.SystemMigrateOptions) error { +func (se SystemEngine) Migrate(ctx context.Context, flags *pflag.FlagSet, config *entities.PodmanConfig, options entities.SystemMigrateOptions) error { return nil } -func (s SystemEngine) Shutdown(ctx context.Context) { - if err := s.Libpod.Shutdown(false); err != nil { +func (se SystemEngine) Shutdown(ctx context.Context) { + if err := se.Libpod.Shutdown(false); err != nil { logrus.Error(err) } } diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 5b89c91d9..03986ad0e 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -112,8 +112,8 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti return ioutil.WriteFile(policyPath, data, 0644) } -func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistriesDirPath string) ([]*trust.TrustPolicy, error) { - var output []*trust.TrustPolicy +func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistriesDirPath string) ([]*trust.Policy, error) { + var output []*trust.Policy registryConfigs, err := trust.LoadAndMergeConfig(systemRegistriesDirPath) if err != nil { @@ -121,7 +121,7 @@ func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistri } if len(policyContentStruct.Default) > 0 { - defaultPolicyStruct := trust.TrustPolicy{ + defaultPolicyStruct := trust.Policy{ Name: "* (default)", RepoName: "default", Type: trustTypeDescription(policyContentStruct.Default[0].Type), @@ -130,7 +130,7 @@ func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistri } for _, transval := range policyContentStruct.Transports { for repo, repoval := range transval { - tempTrustShowOutput := trust.TrustPolicy{ + tempTrustShowOutput := trust.Policy{ Name: repo, RepoName: repo, Type: repoval[0].Type, diff --git a/pkg/domain/infra/abi/volumes.go b/pkg/domain/infra/abi/volumes.go index 91b2440df..2c9d31a23 100644 --- a/pkg/domain/infra/abi/volumes.go +++ b/pkg/domain/infra/abi/volumes.go @@ -24,7 +24,7 @@ func (ic *ContainerEngine) VolumeCreate(ctx context.Context, opts entities.Volum volumeOptions = append(volumeOptions, libpod.WithVolumeLabels(opts.Label)) } if len(opts.Options) > 0 { - parsedOptions, err := parse.ParseVolumeOptions(opts.Options) + parsedOptions, err := parse.VolumeOptions(opts.Options) if err != nil { return nil, err } diff --git a/pkg/domain/infra/runtime_abi.go b/pkg/domain/infra/runtime_abi.go index 0a82b9a6b..60d0c6e86 100644 --- a/pkg/domain/infra/runtime_abi.go +++ b/pkg/domain/infra/runtime_abi.go @@ -20,7 +20,7 @@ func NewContainerEngine(facts *entities.PodmanConfig) (entities.ContainerEngine, r, err := NewLibpodRuntime(facts.FlagSet, facts) return r, err case entities.TunnelMode: - ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.Uri, facts.PassPhrase, facts.Identities...) + ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.URI, facts.PassPhrase, facts.Identities...) return &tunnel.ContainerEngine{ClientCxt: ctx}, err } return nil, fmt.Errorf("runtime mode '%v' is not supported", facts.EngineMode) @@ -33,7 +33,7 @@ func NewImageEngine(facts *entities.PodmanConfig) (entities.ImageEngine, error) r, err := NewLibpodImageRuntime(facts.FlagSet, facts) return r, err case entities.TunnelMode: - ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.Uri, facts.PassPhrase, facts.Identities...) + ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.URI, facts.PassPhrase, facts.Identities...) return &tunnel.ImageEngine{ClientCxt: ctx}, err } return nil, fmt.Errorf("runtime mode '%v' is not supported", facts.EngineMode) diff --git a/pkg/domain/infra/runtime_tunnel.go b/pkg/domain/infra/runtime_tunnel.go index bba7d2c0c..24a93b888 100644 --- a/pkg/domain/infra/runtime_tunnel.go +++ b/pkg/domain/infra/runtime_tunnel.go @@ -16,7 +16,7 @@ func NewContainerEngine(facts *entities.PodmanConfig) (entities.ContainerEngine, case entities.ABIMode: return nil, fmt.Errorf("direct runtime not supported") case entities.TunnelMode: - ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.Uri, facts.PassPhrase, facts.Identities...) + ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.URI, facts.PassPhrase, facts.Identities...) return &tunnel.ContainerEngine{ClientCxt: ctx}, err } return nil, fmt.Errorf("runtime mode '%v' is not supported", facts.EngineMode) @@ -28,7 +28,7 @@ func NewImageEngine(facts *entities.PodmanConfig) (entities.ImageEngine, error) case entities.ABIMode: return nil, fmt.Errorf("direct image runtime not supported") case entities.TunnelMode: - ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.Uri, facts.PassPhrase, facts.Identities...) + ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.URI, facts.PassPhrase, facts.Identities...) return &tunnel.ImageEngine{ClientCxt: ctx}, err } return nil, fmt.Errorf("runtime mode '%v' is not supported", facts.EngineMode) diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 1981055e2..f9aed4102 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -233,7 +233,7 @@ func (ic *ContainerEngine) ContainerTop(ctx context.Context, options entities.To func (ic *ContainerEngine) ContainerCommit(ctx context.Context, nameOrId string, options entities.CommitOptions) (*entities.CommitReport, error) { var ( repo string - tag string = "latest" + tag = "latest" ) if len(options.ImageName) > 0 { ref, err := reference.Parse(options.ImageName) diff --git a/pkg/domain/infra/tunnel/generate.go b/pkg/domain/infra/tunnel/generate.go index eb5587f89..519dc5907 100644 --- a/pkg/domain/infra/tunnel/generate.go +++ b/pkg/domain/infra/tunnel/generate.go @@ -13,5 +13,5 @@ func (ic *ContainerEngine) GenerateSystemd(ctx context.Context, nameOrID string, } func (ic *ContainerEngine) GenerateKube(ctx context.Context, nameOrID string, options entities.GenerateKubeOptions) (*entities.GenerateKubeReport, error) { - return generate.GenerateKube(ic.ClientCxt, nameOrID, options) + return generate.Kube(ic.ClientCxt, nameOrID, options) } diff --git a/pkg/domain/infra/tunnel/play.go b/pkg/domain/infra/tunnel/play.go index 15383a703..5f6bc4a2a 100644 --- a/pkg/domain/infra/tunnel/play.go +++ b/pkg/domain/infra/tunnel/play.go @@ -8,5 +8,5 @@ import ( ) func (ic *ContainerEngine) PlayKube(ctx context.Context, path string, options entities.PlayKubeOptions) (*entities.PlayKubeReport, error) { - return play.PlayKube(ic.ClientCxt, path, options) + return play.Kube(ic.ClientCxt, path, options) } diff --git a/pkg/domain/infra/tunnel/pods.go b/pkg/domain/infra/tunnel/pods.go index b93c48aab..af302d81f 100644 --- a/pkg/domain/infra/tunnel/pods.go +++ b/pkg/domain/infra/tunnel/pods.go @@ -87,7 +87,7 @@ func (ic *ContainerEngine) PodUnpause(ctx context.Context, namesOrIds []string, func (ic *ContainerEngine) PodStop(ctx context.Context, namesOrIds []string, options entities.PodStopOptions) ([]*entities.PodStopReport, error) { var ( reports []*entities.PodStopReport - timeout int = -1 + timeout = -1 ) foundPods, err := getPodsByContext(ic.ClientCxt, options.All, namesOrIds) if err != nil && !(options.Ignore && errors.Cause(err) == define.ErrNoSuchPod) { diff --git a/pkg/domain/utils/utils.go b/pkg/domain/utils/utils.go index c17769f62..ee213e1b6 100644 --- a/pkg/domain/utils/utils.go +++ b/pkg/domain/utils/utils.go @@ -31,7 +31,7 @@ func ToLibpodFilters(f url.Values) (filters []string) { return } -func ToUrlValues(f []string) (filters url.Values) { +func ToURLValues(f []string) (filters url.Values) { filters = make(url.Values) for _, v := range f { t := strings.SplitN(v, "=", 2) diff --git a/pkg/network/network.go b/pkg/network/network.go index 526ee92d8..3ff664316 100644 --- a/pkg/network/network.go +++ b/pkg/network/network.go @@ -14,7 +14,7 @@ import ( ) // DefaultNetworkDriver is the default network type used -var DefaultNetworkDriver string = "bridge" +var DefaultNetworkDriver = "bridge" // SupportedNetworkDrivers describes the list of supported drivers var SupportedNetworkDrivers = []string{DefaultNetworkDriver} diff --git a/pkg/parallel/parallel_linux.go b/pkg/parallel/parallel_linux.go index e3f086c0e..472571972 100644 --- a/pkg/parallel/parallel_linux.go +++ b/pkg/parallel/parallel_linux.go @@ -9,11 +9,11 @@ import ( "github.com/sirupsen/logrus" ) -// ParallelContainerOp performs the given function on the given set of +// ContainerOp performs the given function on the given set of // containers, using a number of parallel threads. // If no error is returned, each container specified in ctrs will have an entry // in the resulting map; containers with no error will be set to nil. -func ParallelContainerOp(ctx context.Context, ctrs []*libpod.Container, applyFunc func(*libpod.Container) error) (map[*libpod.Container]error, error) { +func ContainerOp(ctx context.Context, ctrs []*libpod.Container, applyFunc func(*libpod.Container) error) (map[*libpod.Container]error, error) { jobControlLock.RLock() defer jobControlLock.RUnlock() @@ -22,7 +22,7 @@ func ParallelContainerOp(ctx context.Context, ctrs []*libpod.Container, applyFun // The expectation is that most of the time is spent in applyFunc // anyways. var ( - errMap map[*libpod.Container]error = make(map[*libpod.Container]error) + errMap = make(map[*libpod.Container]error) errLock sync.Mutex allDone sync.WaitGroup ) diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go index 2cf30a59e..e19c582b5 100644 --- a/pkg/spec/createconfig.go +++ b/pkg/spec/createconfig.go @@ -399,7 +399,7 @@ func AddPrivilegedDevices(g *generate.Generator) error { return addPrivilegedDevices(g) } -func CreateContainerFromCreateConfig(r *libpod.Runtime, createConfig *CreateConfig, ctx context.Context, pod *libpod.Pod) (*libpod.Container, error) { +func CreateContainerFromCreateConfig(ctx context.Context, r *libpod.Runtime, createConfig *CreateConfig, pod *libpod.Pod) (*libpod.Container, error) { runtimeSpec, options, err := createConfig.MakeContainerConfig(r, pod) if err != nil { return nil, err diff --git a/pkg/specgen/container_validate.go b/pkg/specgen/container_validate.go index 2c5891f9a..45179343b 100644 --- a/pkg/specgen/container_validate.go +++ b/pkg/specgen/container_validate.go @@ -10,7 +10,7 @@ import ( var ( // ErrInvalidSpecConfig describes an error that the given SpecGenerator is invalid - ErrInvalidSpecConfig error = errors.New("invalid configuration") + ErrInvalidSpecConfig = errors.New("invalid configuration") // SystemDValues describes the only values that SystemD can be SystemDValues = []string{"true", "false", "always"} // ImageVolumeModeValues describes the only values that ImageVolumeMode can be diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 74ae848af..33075b543 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -230,7 +230,7 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. options = append(options, libpod.WithPrivileged(s.Privileged)) // Get namespace related options - namespaceOptions, err := GenerateNamespaceOptions(ctx, s, rt, pod, img) + namespaceOptions, err := namespaceOptions(ctx, s, rt, pod, img) if err != nil { return nil, err } diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index ffa96a5cf..e67afe1bf 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/pkg/specgen/generate/namespaces.go @@ -72,13 +72,13 @@ func GetDefaultNamespaceMode(nsType string, cfg *config.Config, pod *libpod.Pod) return toReturn, errors.Wrapf(define.ErrInvalidArg, "invalid namespace type %q passed", nsType) } -// GenerateNamespaceOptions generates container creation options for all +// namespaceOptions generates container creation options for all // namespaces in a SpecGenerator. // Pod is the pod the container will join. May be nil is the container is not // joining a pod. // TODO: Consider grouping options that are not directly attached to a namespace // elsewhere. -func GenerateNamespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.Pod, img *image.Image) ([]libpod.CtrCreateOption, error) { +func namespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.Pod, img *image.Image) ([]libpod.CtrCreateOption, error) { toReturn := []libpod.CtrCreateOption{} // If pod is not nil, get infra container. diff --git a/pkg/specgen/pod_validate.go b/pkg/specgen/pod_validate.go index 640447e71..2d57cdb91 100644 --- a/pkg/specgen/pod_validate.go +++ b/pkg/specgen/pod_validate.go @@ -7,7 +7,7 @@ import ( var ( // ErrInvalidPodSpecConfig describes an error given when the podspecgenerator is invalid - ErrInvalidPodSpecConfig error = errors.New("invalid pod spec") + ErrInvalidPodSpecConfig = errors.New("invalid pod spec") // containerConfig has the default configurations defined in containers.conf containerConfig = util.DefaultContainerConfig() ) diff --git a/pkg/systemd/generate/systemdgen_test.go b/pkg/systemd/generate/systemdgen_test.go index 3269405a6..cc5db5e24 100644 --- a/pkg/systemd/generate/systemdgen_test.go +++ b/pkg/systemd/generate/systemdgen_test.go @@ -170,7 +170,7 @@ Type=forking [Install] WantedBy=multi-user.target default.target` - goodIdNew := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service + goodIDNew := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service # autogenerated by Podman CI [Unit] @@ -323,7 +323,7 @@ WantedBy=multi-user.target default.target` New: true, CreateCommand: []string{"I'll get stripped", "container", "run", "awesome-image:latest"}, }, - goodIdNew, + goodIDNew, false, }, } diff --git a/pkg/trust/config.go b/pkg/trust/config.go index 0bafc722b..164df2a90 100644 --- a/pkg/trust/config.go +++ b/pkg/trust/config.go @@ -1,7 +1,7 @@ package trust -// Trust Policy describes a basic trust policy configuration -type TrustPolicy struct { +// Policy describes a basic trust policy configuration +type Policy struct { Name string `json:"name"` RepoName string `json:"repo_name,omitempty"` Keys []string `json:"keys,omitempty"` diff --git a/pkg/varlinkapi/create.go b/pkg/varlinkapi/create.go index 571ce6115..d921130e7 100644 --- a/pkg/varlinkapi/create.go +++ b/pkg/varlinkapi/create.go @@ -220,7 +220,7 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod. } } - ctr, err := CreateContainerFromCreateConfig(runtime, createConfig, ctx, pod) + ctr, err := CreateContainerFromCreateConfig(ctx, runtime, createConfig, pod) if err != nil { return nil, nil, err } @@ -909,7 +909,7 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod. return config, nil } -func CreateContainerFromCreateConfig(r *libpod.Runtime, createConfig *cc.CreateConfig, ctx context.Context, pod *libpod.Pod) (*libpod.Container, error) { +func CreateContainerFromCreateConfig(ctx context.Context, r *libpod.Runtime, createConfig *cc.CreateConfig, pod *libpod.Pod) (*libpod.Container, error) { runtimeSpec, options, err := createConfig.MakeContainerConfig(r, pod) if err != nil { return nil, err diff --git a/pkg/varlinkapi/volumes.go b/pkg/varlinkapi/volumes.go index aa0eb1fb5..3b6276287 100644 --- a/pkg/varlinkapi/volumes.go +++ b/pkg/varlinkapi/volumes.go @@ -25,7 +25,7 @@ func (i *VarlinkAPI) VolumeCreate(call iopodman.VarlinkCall, options iopodman.Vo volumeOptions = append(volumeOptions, libpod.WithVolumeLabels(options.Labels)) } if len(options.Options) > 0 { - parsedOptions, err := parse.ParseVolumeOptions(options.Options) + parsedOptions, err := parse.VolumeOptions(options.Options) if err != nil { return call.ReplyErrorOccurred(err.Error()) } -- cgit v1.2.3-54-g00ecf From a389eab8d116f406aba6a4fd1dd5a545e536e88b Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 10 Jun 2020 14:45:11 +0200 Subject: container: make resolv.conf and hosts accessible in userns when running in a new userns, make sure the resolv.conf and hosts files bind mounted from another container are accessible to root in the userns. Signed-off-by: Giuseppe Scrivano --- libpod/container_internal_linux.go | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index d08e012a6..e0cd39ee5 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -1171,6 +1171,15 @@ func (c *Container) makeBindMounts() error { // finally, save it in the new container c.state.BindMounts["/etc/hosts"] = hostsPath } + + if !hasCurrentUserMapped(c) { + if err := makeAccessible(resolvPath, c.RootUID(), c.RootGID()); err != nil { + return err + } + if err := makeAccessible(hostsPath, c.RootUID(), c.RootGID()); err != nil { + return err + } + } } else { if !c.config.UseImageResolvConf { newResolv, err := c.generateResolvConf() -- cgit v1.2.3-54-g00ecf From 6c27e27b8c263696c78d540a87918c6482507310 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 10 Jun 2020 14:52:08 +0200 Subject: container: do not set hostname when joining uts do not set the hostname when joining an UTS namespace, as it could be owned by a different userns. Signed-off-by: Giuseppe Scrivano --- libpod/container_internal_linux.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index e0cd39ee5..e3ca3f6b2 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -392,7 +392,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { } for _, i := range c.config.Spec.Linux.Namespaces { - if i.Type == spec.UTSNamespace { + if i.Type == spec.UTSNamespace && i.Path == "" { hostname := c.Hostname() g.SetHostname(hostname) g.AddProcessEnv("HOSTNAME", hostname) @@ -591,7 +591,8 @@ func (c *Container) addNamespaceContainer(g *generate.Generator, ns LinuxNS, ctr if specNS == spec.UTSNamespace { hostname := nsCtr.Hostname() - g.SetHostname(hostname) + // Joining an existing namespace, cannot set the hostname + g.SetHostname("") g.AddProcessEnv("HOSTNAME", hostname) } -- cgit v1.2.3-54-g00ecf From 4e2a0b5b9c534a3bdf64ff22ecbca4a43f65e65c Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 8 Jun 2020 16:23:50 -0400 Subject: Enable IPv6 port binding Two areas needed tweaking to accomplish this: port parsing and binding ports on the host. Parsing is an obvious problem - we have to accomodate an IPv6 address enclosed by [] as well as a normal IPv4 address. It was slightly complicated by the fact that we previously just counted the number of colons in the whole port definition (a thousand curses on whoever in the IPv6 standard body decided to reuse colons for address separators), but did not end up being that bad. Libpod also (optionally) binds ports on the host to prevent their reuse by host processes. This code was IPv4 only for TCP, and bound to both for UDP (which I'm fairly certain is not correct, and has been adjusted). This just needed protocols adjusted to read "tcp4"/"tcp6" and "udp4"/"udp6" based on what we wanted to bind to. Fixes #5715 Signed-off-by: Matthew Heon --- cmd/podman/common/util.go | 32 +++++++++++++++++++++++++++++++- libpod/oci_util.go | 36 ++++++++++++++++++++++++++++++++---- test/e2e/run_networking_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 5 deletions(-) (limited to 'libpod') diff --git a/cmd/podman/common/util.go b/cmd/podman/common/util.go index a3626b4e4..0d9f3ba26 100644 --- a/cmd/podman/common/util.go +++ b/cmd/podman/common/util.go @@ -71,14 +71,44 @@ func createPortBindings(ports []string) ([]specgen.PortMapping, error) { return nil, errors.Errorf("invalid port format - protocol can only be specified once") } - splitPort := strings.Split(splitProto[0], ":") + remainder := splitProto[0] + haveV6 := false + + // Check for an IPv6 address in brackets + splitV6 := strings.Split(remainder, "]") + switch len(splitV6) { + case 1: + // Do nothing, proceed as before + case 2: + // We potentially have an IPv6 address + haveV6 = true + if !strings.HasPrefix(splitV6[0], "[") { + return nil, errors.Errorf("invalid port format - IPv6 addresses must be enclosed by []") + } + if !strings.HasPrefix(splitV6[1], ":") { + return nil, errors.Errorf("invalid port format - IPv6 address must be followed by a colon (':')") + } + ipNoPrefix := strings.TrimPrefix(splitV6[0], "[") + hostIP = &ipNoPrefix + remainder = strings.TrimPrefix(splitV6[1], ":") + default: + return nil, errors.Errorf("invalid port format - at most one IPv6 address can be specified in a --publish") + } + + splitPort := strings.Split(remainder, ":") switch len(splitPort) { case 1: + if haveV6 { + return nil, errors.Errorf("invalid port format - must provide host and destination port if specifying an IP") + } ctrPort = splitPort[0] case 2: hostPort = &(splitPort[0]) ctrPort = splitPort[1] case 3: + if haveV6 { + return nil, errors.Errorf("invalid port format - when v6 address specified, must be [ipv6]:hostPort:ctrPort") + } hostIP = &(splitPort[0]) hostPort = &(splitPort[1]) ctrPort = splitPort[2] diff --git a/libpod/oci_util.go b/libpod/oci_util.go index 53567d2d0..8b40dad81 100644 --- a/libpod/oci_util.go +++ b/libpod/oci_util.go @@ -36,14 +36,30 @@ func bindPorts(ports []ocicni.PortMapping) ([]*os.File, error) { var files []*os.File notifySCTP := false for _, i := range ports { + isV6 := net.ParseIP(i.HostIP).To4() == nil + if i.HostIP == "" { + isV6 = false + } switch i.Protocol { case "udp": - addr, err := net.ResolveUDPAddr("udp", fmt.Sprintf("%s:%d", i.HostIP, i.HostPort)) + var ( + addr *net.UDPAddr + err error + ) + if isV6 { + addr, err = net.ResolveUDPAddr("udp6", fmt.Sprintf("[%s]:%d", i.HostIP, i.HostPort)) + } else { + addr, err = net.ResolveUDPAddr("udp4", fmt.Sprintf("%s:%d", i.HostIP, i.HostPort)) + } if err != nil { return nil, errors.Wrapf(err, "cannot resolve the UDP address") } - server, err := net.ListenUDP("udp", addr) + proto := "udp4" + if isV6 { + proto = "udp6" + } + server, err := net.ListenUDP(proto, addr) if err != nil { return nil, errors.Wrapf(err, "cannot listen on the UDP port") } @@ -54,12 +70,24 @@ func bindPorts(ports []ocicni.PortMapping) ([]*os.File, error) { files = append(files, f) case "tcp": - addr, err := net.ResolveTCPAddr("tcp4", fmt.Sprintf("%s:%d", i.HostIP, i.HostPort)) + var ( + addr *net.TCPAddr + err error + ) + if isV6 { + addr, err = net.ResolveTCPAddr("tcp6", fmt.Sprintf("[%s]:%d", i.HostIP, i.HostPort)) + } else { + addr, err = net.ResolveTCPAddr("tcp4", fmt.Sprintf("%s:%d", i.HostIP, i.HostPort)) + } if err != nil { return nil, errors.Wrapf(err, "cannot resolve the TCP address") } - server, err := net.ListenTCP("tcp4", addr) + proto := "tcp4" + if isV6 { + proto = "tcp6" + } + server, err := net.ListenTCP(proto, addr) if err != nil { return nil, errors.Wrapf(err, "cannot listen on the TCP port") } diff --git a/test/e2e/run_networking_test.go b/test/e2e/run_networking_test.go index 9db2f5d49..4fad85f00 100644 --- a/test/e2e/run_networking_test.go +++ b/test/e2e/run_networking_test.go @@ -129,6 +129,32 @@ var _ = Describe("Podman run networking", func() { Expect(inspectOut[0].NetworkSettings.Ports[0].HostIP).To(Equal("127.0.0.1")) }) + It("podman run -p [::1]:8080:80/udp", func() { + name := "testctr" + session := podmanTest.Podman([]string{"create", "-t", "-p", "[::1]:8080:80/udp", "--name", name, ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + inspectOut := podmanTest.InspectContainer(name) + Expect(len(inspectOut)).To(Equal(1)) + Expect(len(inspectOut[0].NetworkSettings.Ports)).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports[0].HostPort).To(Equal(int32(8080))) + Expect(inspectOut[0].NetworkSettings.Ports[0].ContainerPort).To(Equal(int32(80))) + Expect(inspectOut[0].NetworkSettings.Ports[0].Protocol).To(Equal("udp")) + Expect(inspectOut[0].NetworkSettings.Ports[0].HostIP).To(Equal("::1")) + }) + + It("podman run -p [::1]:8080:80/tcp", func() { + name := "testctr" + session := podmanTest.Podman([]string{"create", "-t", "-p", "[::1]:8080:80/tcp", "--name", name, ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + inspectOut := podmanTest.InspectContainer(name) + Expect(len(inspectOut)).To(Equal(1)) + Expect(len(inspectOut[0].NetworkSettings.Ports)).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports[0].HostPort).To(Equal(int32(8080))) + Expect(inspectOut[0].NetworkSettings.Ports[0].ContainerPort).To(Equal(int32(80))) + Expect(inspectOut[0].NetworkSettings.Ports[0].Protocol).To(Equal("tcp")) + Expect(inspectOut[0].NetworkSettings.Ports[0].HostIP).To(Equal("::1")) + }) + It("podman run --expose 80 -P", func() { name := "testctr" session := podmanTest.Podman([]string{"create", "-t", "--expose", "80", "-P", "--name", name, ALPINE, "/bin/sh"}) -- cgit v1.2.3-54-g00ecf From 86ffa552d2af02bdcc06e8a0e93095b7d0110395 Mon Sep 17 00:00:00 2001 From: Will Haines Date: Wed, 10 Jun 2020 12:40:19 -0600 Subject: Fixed bug where 'podman log ' would truncate some lines. Signed-off-by: Will Haines --- libpod/container_log.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/container_log.go b/libpod/container_log.go index bfa303e84..c3a84d048 100644 --- a/libpod/container_log.go +++ b/libpod/container_log.go @@ -19,7 +19,7 @@ func (r *Runtime) Log(containers []*Container, options *logs.LogOptions, logChan return nil } -// ReadLog reads a containers log based on the input options and returns loglines over a channel +// ReadLog reads a containers log based on the input options and returns loglines over a channel. func (c *Container) ReadLog(options *logs.LogOptions, logChannel chan *logs.LogLine) error { // TODO Skip sending logs until journald logs can be read // TODO make this not a magic string @@ -61,7 +61,7 @@ func (c *Container) readFromLogFile(options *logs.LogOptions, logChannel chan *l partial += nll.Msg continue } else if !nll.Partial() && len(partial) > 1 { - nll.Msg = partial + nll.Msg = partial + nll.Msg partial = "" } nll.CID = c.ID() -- cgit v1.2.3-54-g00ecf From 636881ece56f77df4c6a25dc5faa68a740607e3f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 28 May 2020 10:54:17 +0200 Subject: pod config: add a `CreateCommand` field Add a `CreateCommand` field to the pod config which includes the entire `os.Args` at pod-creation. Similar to the already existing field in a container config, we need this information to properly generate generic systemd unit files for pods. It's a prerequisite to support the `--new` flag for pods. Also add the `CreateCommand` to the pod-inspect data, which can come in handy for debugging, general inspection and certainly for the tests that are added along with the other changes. Signed-off-by: Valentin Rothberg --- libpod/define/pod_inspect.go | 3 +++ libpod/options.go | 12 ++++++++++++ libpod/pod.go | 4 ++++ libpod/pod_api.go | 1 + pkg/specgen/generate/pod_create.go | 1 + test/e2e/pod_inspect_test.go | 22 ++++++++++++++++++++++ 6 files changed, 43 insertions(+) (limited to 'libpod') diff --git a/libpod/define/pod_inspect.go b/libpod/define/pod_inspect.go index 26fd2cab4..7f06e16fc 100644 --- a/libpod/define/pod_inspect.go +++ b/libpod/define/pod_inspect.go @@ -18,6 +18,9 @@ type InspectPodData struct { Namespace string `json:"Namespace,omitempty"` // Created is the time when the pod was created. Created time.Time + // CreateCommand is the full command plus arguments of the process the + // container has been created with. + CreateCommand []string `json:"CreateCommand,omitempty"` // State represents the current state of the pod. State string `json:"State"` // Hostname is the hostname that the pod will set. diff --git a/libpod/options.go b/libpod/options.go index 8e0d3df86..75d098815 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1538,6 +1538,18 @@ func WithPodHostname(hostname string) PodCreateOption { } } +// WithPodCreateCommand adds the full command plus arguments of the current +// process to the pod config. +func WithPodCreateCommand() PodCreateOption { + return func(pod *Pod) error { + if pod.valid { + return define.ErrPodFinalized + } + pod.config.CreateCommand = os.Args + return nil + } +} + // WithPodLabels sets the labels of a pod. func WithPodLabels(labels map[string]string) PodCreateOption { return func(pod *Pod) error { diff --git a/libpod/pod.go b/libpod/pod.go index 8afaa6052..38fe1fd2c 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -64,6 +64,10 @@ type PodConfig struct { // Time pod was created CreatedTime time.Time `json:"created"` + // CreateCommand is the full command plus arguments of the process the + // container has been created with. + CreateCommand []string `json:"CreateCommand,omitempty"` + // ID of the pod's lock LockID uint32 `json:"lockID"` } diff --git a/libpod/pod_api.go b/libpod/pod_api.go index e2c4b515d..c8605eb69 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -489,6 +489,7 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) { Name: p.Name(), Namespace: p.Namespace(), Created: p.CreatedTime(), + CreateCommand: p.config.CreateCommand, State: podState, Hostname: p.config.Hostname, Labels: p.Labels(), diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index cd2d69cfb..51b7835b2 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -93,5 +93,6 @@ func createPodOptions(p *specgen.PodSpecGenerator) ([]libpod.PodCreateOption, er options = append(options, libpod.WithInfraContainerPorts(ports)) } options = append(options, libpod.WithPodCgroups()) + options = append(options, libpod.WithPodCreateCommand()) return options, nil } diff --git a/test/e2e/pod_inspect_test.go b/test/e2e/pod_inspect_test.go index 8040adf1e..f1acd3750 100644 --- a/test/e2e/pod_inspect_test.go +++ b/test/e2e/pod_inspect_test.go @@ -57,4 +57,26 @@ var _ = Describe("Podman pod inspect", func() { podData := inspect.InspectPodToJSON() Expect(podData.ID).To(Equal(podid)) }) + + It("podman pod inspect (CreateCommand)", func() { + podName := "myTestPod" + createCommand := []string{"pod", "create", "--name", podName, "--hostname", "rudolph", "--share", "net"} + + // Create the pod. + session := podmanTest.Podman(createCommand) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Inspect the pod and make sure that the create command is + // exactly how we created the pod. + inspect := podmanTest.Podman([]string{"pod", "inspect", podName}) + inspect.WaitWithDefaultTimeout() + Expect(inspect.ExitCode()).To(Equal(0)) + Expect(inspect.IsJSONOutputValid()).To(BeTrue()) + podData := inspect.InspectPodToJSON() + // Let's get the last len(createCommand) items in the command. + inspectCreateCommand := podData.CreateCommand + index := len(inspectCreateCommand) - len(createCommand) + Expect(inspectCreateCommand[index:]).To(Equal(createCommand)) + }) }) -- cgit v1.2.3-54-g00ecf From 402c68b41d3fd8f354a4fd0ba4d4101920f8cfe6 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 29 May 2020 13:20:22 +0200 Subject: pod create: add `--infra-conmon-pidfile` Add an `--infra-conmon-pidfile` flag to `podman-pod-create` to write the infra container's conmon process ID to a specified path. Several container sub-commands already support `--conmon-pidfile` which is especially helpful to allow for systemd to access and track the conmon processes. This allows for easily tracking the conmon process of a pod's infra container. Signed-off-by: Valentin Rothberg --- cmd/podman/pods/create.go | 4 ++++ completions/bash/podman | 1 + docs/source/markdown/podman-pod-create.1.md | 4 ++++ libpod/options.go | 12 ++++++++++++ libpod/pod.go | 1 + libpod/runtime_pod_infra_linux.go | 3 +++ pkg/domain/entities/pods.go | 22 +++++++++++++--------- pkg/specgen/generate/pod_create.go | 3 +++ pkg/specgen/podspecgen.go | 3 +++ test/e2e/pod_start_test.go | 19 +++++++++++++++++++ 10 files changed, 63 insertions(+), 9 deletions(-) (limited to 'libpod') diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index 5ed5fa57c..51b7a7d52 100644 --- a/cmd/podman/pods/create.go +++ b/cmd/podman/pods/create.go @@ -53,6 +53,7 @@ func init() { flags.AddFlagSet(common.GetNetFlags()) flags.StringVar(&createOptions.CGroupParent, "cgroup-parent", "", "Set parent cgroup for the pod") flags.BoolVar(&createOptions.Infra, "infra", true, "Create an infra container associated with the pod to share namespaces with") + flags.StringVar(&createOptions.InfraConmonPidFile, "infra-conmon-pidfile", "", "Path to the file that will receive the POD of the infra container's conmon") flags.StringVar(&createOptions.InfraImage, "infra-image", containerConfig.Engine.InfraImage, "The image of the infra container to associate with the pod") flags.StringVar(&createOptions.InfraCommand, "infra-command", containerConfig.Engine.InfraCommand, "The command to run on the infra container when the pod is started") flags.StringSliceVar(&labelFile, "label-file", []string{}, "Read in a line delimited file of labels") @@ -83,6 +84,9 @@ func create(cmd *cobra.Command, args []string) error { if !createOptions.Infra { logrus.Debugf("Not creating an infra container") + if cmd.Flag("infra-conmon-pidfile").Changed { + return errors.New("cannot set infra-conmon-pid without an infra container") + } if cmd.Flag("infra-command").Changed { return errors.New("cannot set infra-command without an infra container") } diff --git a/completions/bash/podman b/completions/bash/podman index 0e4b60b14..6dbe645fe 100644 --- a/completions/bash/podman +++ b/completions/bash/podman @@ -3098,6 +3098,7 @@ _podman_pod_create() { --dns-opt --dns-search --infra-command + --infra-conmon-pidfile --infra-image --ip --label-file diff --git a/docs/source/markdown/podman-pod-create.1.md b/docs/source/markdown/podman-pod-create.1.md index 489c9b32e..de6b600f0 100644 --- a/docs/source/markdown/podman-pod-create.1.md +++ b/docs/source/markdown/podman-pod-create.1.md @@ -47,6 +47,10 @@ Set a hostname to the pod Create an infra container and associate it with the pod. An infra container is a lightweight container used to coordinate the shared kernel namespace of a pod. Default: true. +**--infra-conmon-pidfile**=*file* + +Write the pid of the infra container's **conmon** process to a file. As **conmon** runs in a separate process than Podman, this is necessary when using systemd to manage Podman containers and pods. + **--infra-command**=*command* The command that will be run to start the infra container. Default: "/pause". diff --git a/libpod/options.go b/libpod/options.go index 75d098815..5a0f60093 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1550,6 +1550,18 @@ func WithPodCreateCommand() PodCreateOption { } } +// WithInfraConmonPidFile sets the path to a custom conmon PID file for the +// infra container. +func WithInfraConmonPidFile(path string) PodCreateOption { + return func(pod *Pod) error { + if pod.valid { + return define.ErrPodFinalized + } + pod.config.InfraContainer.ConmonPidFile = path + return nil + } +} + // WithPodLabels sets the labels of a pod. func WithPodLabels(labels map[string]string) PodCreateOption { return func(pod *Pod) error { diff --git a/libpod/pod.go b/libpod/pod.go index 38fe1fd2c..7af78fa07 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -83,6 +83,7 @@ type podState struct { // InfraContainerConfig is the configuration for the pod's infra container type InfraContainerConfig struct { + ConmonPidFile string `json:"conmonPidFile"` HasInfraContainer bool `json:"makeInfraContainer"` HostNetwork bool `json:"infraHostNetwork,omitempty"` PortBindings []ocicni.PortMapping `json:"infraPortBindings"` diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go index 06a7b3936..a0dee3aa1 100644 --- a/libpod/runtime_pod_infra_linux.go +++ b/libpod/runtime_pod_infra_linux.go @@ -130,6 +130,9 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, rawIm options = append(options, WithRootFSFromImage(imgID, imgName, rawImageName)) options = append(options, WithName(containerName)) options = append(options, withIsInfra()) + if len(p.config.InfraContainer.ConmonPidFile) > 0 { + options = append(options, WithConmonPidFile(p.config.InfraContainer.ConmonPidFile)) + } return r.newContainer(ctx, g.Config, options...) } diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index a85333c75..fc76ddd41 100644 --- a/pkg/domain/entities/pods.go +++ b/pkg/domain/entities/pods.go @@ -103,15 +103,16 @@ type PodRmReport struct { } type PodCreateOptions struct { - CGroupParent string - Hostname string - Infra bool - InfraImage string - InfraCommand string - Labels map[string]string - Name string - Net *NetOptions - Share []string + CGroupParent string + Hostname string + Infra bool + InfraImage string + InfraCommand string + InfraConmonPidFile string + Labels map[string]string + Name string + Net *NetOptions + Share []string } type PodCreateReport struct { @@ -127,6 +128,9 @@ func (p PodCreateOptions) ToPodSpecGen(s *specgen.PodSpecGenerator) { if len(p.InfraCommand) > 0 { s.InfraCommand = strings.Split(p.InfraCommand, " ") } + if len(p.InfraConmonPidFile) > 0 { + s.InfraConmonPidFile = p.InfraConmonPidFile + } s.InfraImage = p.InfraImage s.SharedNamespaces = p.Share diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index 51b7835b2..5ccb1ba80 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -94,5 +94,8 @@ func createPodOptions(p *specgen.PodSpecGenerator) ([]libpod.PodCreateOption, er } options = append(options, libpod.WithPodCgroups()) options = append(options, libpod.WithPodCreateCommand()) + if len(p.InfraConmonPidFile) > 0 { + options = append(options, libpod.WithInfraConmonPidFile(p.InfraConmonPidFile)) + } return options, nil } diff --git a/pkg/specgen/podspecgen.go b/pkg/specgen/podspecgen.go index 11976233a..600d27004 100644 --- a/pkg/specgen/podspecgen.go +++ b/pkg/specgen/podspecgen.go @@ -25,6 +25,9 @@ type PodBasicConfig struct { // InfraCommand and InfraImages in this struct. // Optional. NoInfra bool `json:"no_infra,omitempty"` + // InfraConmonPidFile is a custom path to store the infra container's + // conmon PID. + InfraConmonPidFile string `json:"infra_conmon_pid_file,omitempty"` // InfraCommand sets the command that will be used to start the infra // container. // If not set, the default set in the Libpod configuration file will be diff --git a/test/e2e/pod_start_test.go b/test/e2e/pod_start_test.go index 4502a76ed..99285d1e1 100644 --- a/test/e2e/pod_start_test.go +++ b/test/e2e/pod_start_test.go @@ -193,4 +193,23 @@ var _ = Describe("Podman pod start", func() { Expect(session.ExitCode()).To(Equal(0)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(20)) // 10*(infra+top) }) + + It("podman pod create --infra-conmon-pod create + start", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + tmpFile := tmpDir + "podID" + defer os.RemoveAll(tmpDir) + + podName := "rudolph" + // Create a pod with --infra-conmon-pid. + session := podmanTest.Podman([]string{"pod", "create", "--name", podName, "--infra-conmon-pidfile", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"pod", "start", podName}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1)) // infra + }) + }) -- cgit v1.2.3-54-g00ecf From b4a410215ef0d6ebac6e5b61921f2cbe5d9ac8bd Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Jun 2020 15:34:04 +0200 Subject: add (*Pod).CreateCommand() Add a method to Pod to easily access its .config.CreateCommand. Signed-off-by: Valentin Rothberg --- libpod/pod.go | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'libpod') diff --git a/libpod/pod.go b/libpod/pod.go index 7af78fa07..db87e60f2 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -129,6 +129,12 @@ func (p *Pod) CreatedTime() time.Time { return p.config.CreatedTime } +// CreateCommand returns the os.Args of the process with which the pod has been +// created. +func (p *Pod) CreateCommand() []string { + return p.config.CreateCommand +} + // CgroupParent returns the pod's CGroup parent func (p *Pod) CgroupParent() string { return p.config.CgroupParent -- cgit v1.2.3-54-g00ecf From 35ae53067f01c0194dc13513656e57293de95004 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Jun 2020 16:57:58 +0200 Subject: generate systemd: refactor Refactor the systemd-unit generation code and move all the logic into `pkg/systemd/generate`. The code was already hard to maintain but I found it impossible to wire the `--new` logic for pods in all the chaos. The code refactoring in this commit will make maintaining the code easier and should make it easier to extend as well. Further changes and refactorings may still be needed but they will easier. Signed-off-by: Valentin Rothberg --- libpod/pod.go | 14 ++ pkg/domain/infra/abi/generate.go | 158 +-------------- pkg/systemd/generate/common.go | 36 ++++ pkg/systemd/generate/containers.go | 109 ++++++---- pkg/systemd/generate/containers_test.go | 347 ------------------------------- pkg/systemd/generate/generate_test.go | 349 ++++++++++++++++++++++++++++++++ pkg/systemd/generate/pods.go | 130 ++++++++++++ 7 files changed, 604 insertions(+), 539 deletions(-) create mode 100644 pkg/systemd/generate/common.go delete mode 100644 pkg/systemd/generate/containers_test.go create mode 100644 pkg/systemd/generate/generate_test.go create mode 100644 pkg/systemd/generate/pods.go (limited to 'libpod') diff --git a/libpod/pod.go b/libpod/pod.go index db87e60f2..f53290876 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -257,6 +257,20 @@ func (p *Pod) InfraContainerID() (string, error) { return p.state.InfraContainerID, nil } +// InfraContainer returns the infra container. +func (p *Pod) InfraContainer() (*Container, error) { + if !p.HasInfraContainer() { + return nil, errors.New("pod has no infra container") + } + + id, err := p.InfraContainerID() + if err != nil { + return nil, err + } + + return p.runtime.state.Container(id) +} + // TODO add pod batching // Lock pod to avoid lock contention // Store and lock all containers (no RemoveContainer in batch guarantees cache will not become stale) diff --git a/pkg/domain/infra/abi/generate.go b/pkg/domain/infra/abi/generate.go index 08cb87287..fa0cfb389 100644 --- a/pkg/domain/infra/abi/generate.go +++ b/pkg/domain/infra/abi/generate.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "fmt" - "strings" "github.com/containers/libpod/libpod" "github.com/containers/libpod/libpod/define" @@ -16,165 +15,28 @@ import ( ) func (ic *ContainerEngine) GenerateSystemd(ctx context.Context, nameOrID string, options entities.GenerateSystemdOptions) (*entities.GenerateSystemdReport, error) { - opts := generate.Options{ - Files: options.Files, - New: options.New, - } - // First assume it's a container. - if info, found, err := ic.generateSystemdgenContainerInfo(nameOrID, nil, options); found && err != nil { - return nil, err - } else if found && err == nil { - output, err := generate.CreateContainerSystemdUnit(info, opts) - if err != nil { - return nil, err + ctr, err := ic.Libpod.LookupContainer(nameOrID) + if err == nil { + // Generate the unit for the container. + s, err := generate.ContainerUnit(ctr, options) + if err == nil { + return &entities.GenerateSystemdReport{Output: s}, nil } - return &entities.GenerateSystemdReport{Output: output}, nil - } - - // --new does not support pods. - if options.New { - return nil, errors.Errorf("error generating systemd unit files: cannot generate generic files for a pod") } - // We're either having a pod or garbage. + // If it's not a container, we either have a pod or garbage. pod, err := ic.Libpod.LookupPod(nameOrID) if err != nil { return nil, errors.Errorf("%q does not refer to a container or pod", nameOrID) } - // Error out if the pod has no infra container, which we require to be the - // main service. - if !pod.HasInfraContainer() { - return nil, fmt.Errorf("error generating systemd unit files: Pod %q has no infra container", pod.Name()) - } - - // Generate a systemdgen.ContainerInfo for the infra container. This - // ContainerInfo acts as the main service of the pod. - infraID, err := pod.InfraContainerID() - if err != nil { - return nil, nil - } - podInfo, _, err := ic.generateSystemdgenContainerInfo(infraID, pod, options) + // Generate the units for the pod and all its containers. + s, err := generate.PodUnits(pod, options) if err != nil { return nil, err } - - // Compute the container-dependency graph for the Pod. - containers, err := pod.AllContainers() - if err != nil { - return nil, err - } - if len(containers) == 0 { - return nil, fmt.Errorf("error generating systemd unit files: Pod %q has no containers", pod.Name()) - } - graph, err := libpod.BuildContainerGraph(containers) - if err != nil { - return nil, err - } - - // Traverse the dependency graph and create systemdgen.ContainerInfo's for - // each container. - containerInfos := []*generate.ContainerInfo{podInfo} - for ctr, dependencies := range graph.DependencyMap() { - // Skip the infra container as we already generated it. - if ctr.ID() == infraID { - continue - } - ctrInfo, _, err := ic.generateSystemdgenContainerInfo(ctr.ID(), nil, options) - if err != nil { - return nil, err - } - // Now add the container's dependencies and at the container as a - // required service of the infra container. - for _, dep := range dependencies { - if dep.ID() == infraID { - ctrInfo.BoundToServices = append(ctrInfo.BoundToServices, podInfo.ServiceName) - } else { - _, serviceName := generateServiceName(dep, nil, options) - ctrInfo.BoundToServices = append(ctrInfo.BoundToServices, serviceName) - } - } - podInfo.RequiredServices = append(podInfo.RequiredServices, ctrInfo.ServiceName) - containerInfos = append(containerInfos, ctrInfo) - } - - // Now generate the systemd service for all containers. - builder := strings.Builder{} - for i, info := range containerInfos { - if i > 0 { - builder.WriteByte('\n') - } - out, err := generate.CreateContainerSystemdUnit(info, opts) - if err != nil { - return nil, err - } - builder.WriteString(out) - } - - return &entities.GenerateSystemdReport{Output: builder.String()}, nil -} - -// generateSystemdgenContainerInfo is a helper to generate a -// systemdgen.ContainerInfo for `GenerateSystemd`. -func (ic *ContainerEngine) generateSystemdgenContainerInfo(nameOrID string, pod *libpod.Pod, options entities.GenerateSystemdOptions) (*generate.ContainerInfo, bool, error) { - ctr, err := ic.Libpod.LookupContainer(nameOrID) - if err != nil { - return nil, false, err - } - - timeout := ctr.StopTimeout() - if options.StopTimeout != nil { - timeout = *options.StopTimeout - } - - config := ctr.Config() - conmonPidFile := config.ConmonPidFile - if conmonPidFile == "" { - return nil, true, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") - } - - createCommand := []string{} - if config.CreateCommand != nil { - createCommand = config.CreateCommand - } else if options.New { - return nil, true, errors.Errorf("cannot use --new on container %q: no create command found", nameOrID) - } - - name, serviceName := generateServiceName(ctr, pod, options) - info := &generate.ContainerInfo{ - ServiceName: serviceName, - ContainerName: name, - RestartPolicy: options.RestartPolicy, - PIDFile: conmonPidFile, - StopTimeout: timeout, - GenerateTimestamp: true, - CreateCommand: createCommand, - } - - return info, true, nil -} - -// generateServiceName generates the container name and the service name for systemd service. -func generateServiceName(ctr *libpod.Container, pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, string) { - var kind, name, ctrName string - if pod == nil { - kind = options.ContainerPrefix //defaults to container - name = ctr.ID() - if options.Name { - name = ctr.Name() - } - ctrName = name - } else { - kind = options.PodPrefix //defaults to pod - name = pod.ID() - ctrName = ctr.ID() - if options.Name { - name = pod.Name() - ctrName = ctr.Name() - } - } - return ctrName, fmt.Sprintf("%s%s%s", kind, options.Separator, name) + return &entities.GenerateSystemdReport{Output: s}, nil } func (ic *ContainerEngine) GenerateKube(ctx context.Context, nameOrID string, options entities.GenerateKubeOptions) (*entities.GenerateKubeReport, error) { diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go new file mode 100644 index 000000000..e809b4837 --- /dev/null +++ b/pkg/systemd/generate/common.go @@ -0,0 +1,36 @@ +package generate + +import ( + "github.com/pkg/errors" +) + +// EnvVariable "PODMAN_SYSTEMD_UNIT" is set in all generated systemd units and +// is set to the unit's (unique) name. +const EnvVariable = "PODMAN_SYSTEMD_UNIT" + +// restartPolicies includes all valid restart policies to be used in a unit +// file. +var restartPolicies = []string{"no", "on-success", "on-failure", "on-abnormal", "on-watchdog", "on-abort", "always"} + +// validateRestartPolicy checks that the user-provided policy is valid. +func validateRestartPolicy(restart string) error { + for _, i := range restartPolicies { + if i == restart { + return nil + } + } + return errors.Errorf("%s is not a valid restart policy", restart) +} + +const headerTemplate = `# {{.ServiceName}}.service +# autogenerated by Podman {{.PodmanVersion}} +{{- if .TimeStamp}} +# {{.TimeStamp}} +{{- end}} + +[Unit] +Description=Podman {{.ServiceName}}.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +` diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index fb0ea5cf9..f316d4452 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -11,22 +11,20 @@ import ( "text/template" "time" + "github.com/containers/libpod/libpod" + "github.com/containers/libpod/pkg/domain/entities" "github.com/containers/libpod/version" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) -// EnvVariable "PODMAN_SYSTEMD_UNIT" is set in all generated systemd units and -// is set to the unit's (unique) name. -const EnvVariable = "PODMAN_SYSTEMD_UNIT" - -// ContainerInfo contains data required for generating a container's systemd +// containerInfo contains data required for generating a container's systemd // unit file. -type ContainerInfo struct { +type containerInfo struct { // ServiceName of the systemd service. ServiceName string // Name or ID of the container. - ContainerName string + ContainerNameOrID string // StopTimeout sets the timeout Podman waits before killing the container // during service stop. StopTimeout uint @@ -63,29 +61,7 @@ type ContainerInfo struct { EnvVariable string } -var restartPolicies = []string{"no", "on-success", "on-failure", "on-abnormal", "on-watchdog", "on-abort", "always"} - -// validateRestartPolicy checks that the user-provided policy is valid. -func validateRestartPolicy(restart string) error { - for _, i := range restartPolicies { - if i == restart { - return nil - } - } - return errors.Errorf("%s is not a valid restart policy", restart) -} - -const containerTemplate = `# {{.ServiceName}}.service -# autogenerated by Podman {{.PodmanVersion}} -{{- if .TimeStamp}} -# {{.TimeStamp}} -{{- end}} - -[Unit] -Description=Podman {{.ServiceName}}.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target +const containerTemplate = headerTemplate + ` {{- if .BoundToServices}} RefuseManualStart=yes RefuseManualStop=yes @@ -107,8 +83,8 @@ ExecStop={{.Executable}} stop --ignore --cidfile %t/%n-ctr-id {{if (ge .StopTime ExecStopPost={{.Executable}} rm --ignore -f --cidfile %t/%n-ctr-id PIDFile=%t/%n-pid {{- else}} -ExecStart={{.Executable}} start {{.ContainerName}} -ExecStop={{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerName}} +ExecStart={{.Executable}} start {{.ContainerNameOrID}} +ExecStop={{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}} PIDFile={{.PIDFile}} {{- end}} KillMode=none @@ -117,18 +93,19 @@ Type=forking [Install] WantedBy=multi-user.target default.target` -// Options include different options to control the unit file generation. -type Options struct { - // When set, generate service files in the current working directory and - // return the paths to these files instead of returning all contents in one - // big string. - Files bool - // New controls if a new container is created or if an existing one is started. - New bool +// ContainerUnit generates a systemd unit for the specified container. Based +// on the options, the return value might be the entire unit or a file it has +// been written to. +func ContainerUnit(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, error) { + info, err := generateContainerInfo(ctr, options) + if err != nil { + return "", err + } + return createContainerSystemdUnit(info, options) } -// CreateContainerSystemdUnit creates a systemd unit file for a container. -func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, error) { +// createContainerSystemdUnit creates a systemd unit file for a container. +func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSystemdOptions) (string, error) { if err := validateRestartPolicy(info.RestartPolicy); err != nil { return "", err } @@ -152,7 +129,7 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro // been created via a Python script, which would certainly yield an // invalid `info.CreateCommand`. Hence, we're doing a best effort unit // generation and don't try aiming at completeness. - if opts.New { + if options.New { // The create command must at least have three arguments: // /usr/bin/podman run $IMAGE index := 2 @@ -218,7 +195,7 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro return "", err } - if !opts.Files { + if !options.Files { return buf.String(), nil } @@ -233,3 +210,47 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro } return path, nil } + +func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSystemdOptions) (*containerInfo, error) { + timeout := ctr.StopTimeout() + if options.StopTimeout != nil { + timeout = *options.StopTimeout + } + + config := ctr.Config() + conmonPidFile := config.ConmonPidFile + if conmonPidFile == "" { + return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") + } + + createCommand := []string{} + if config.CreateCommand != nil { + createCommand = config.CreateCommand + } else if options.New { + return nil, errors.Errorf("cannot use --new on container %q: no create command found", ctr.ID()) + } + + nameOrID, serviceName := containerServiceName(ctr, options) + + info := containerInfo{ + ServiceName: serviceName, + ContainerNameOrID: nameOrID, + RestartPolicy: options.RestartPolicy, + PIDFile: conmonPidFile, + StopTimeout: timeout, + GenerateTimestamp: true, + CreateCommand: createCommand, + } + return &info, nil +} + +// containerServiceName returns the nameOrID and the service name of the +// container. +func containerServiceName(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, string) { + nameOrID := ctr.ID() + if options.Name { + nameOrID = ctr.Name() + } + serviceName := fmt.Sprintf("%s%s%s", options.ContainerPrefix, options.Separator, nameOrID) + return nameOrID, serviceName +} diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go deleted file mode 100644 index 9da261a69..000000000 --- a/pkg/systemd/generate/containers_test.go +++ /dev/null @@ -1,347 +0,0 @@ -package generate - -import ( - "testing" -) - -func TestValidateRestartPolicy(t *testing.T) { - type ContainerInfo struct { - restart string - } - tests := []struct { - name string - ContainerInfo ContainerInfo - wantErr bool - }{ - {"good-on", ContainerInfo{restart: "no"}, false}, - {"good-on-success", ContainerInfo{restart: "on-success"}, false}, - {"good-on-failure", ContainerInfo{restart: "on-failure"}, false}, - {"good-on-abnormal", ContainerInfo{restart: "on-abnormal"}, false}, - {"good-on-watchdog", ContainerInfo{restart: "on-watchdog"}, false}, - {"good-on-abort", ContainerInfo{restart: "on-abort"}, false}, - {"good-always", ContainerInfo{restart: "always"}, false}, - {"fail", ContainerInfo{restart: "foobar"}, true}, - {"failblank", ContainerInfo{restart: ""}, true}, - } - for _, tt := range tests { - test := tt - t.Run(tt.name, func(t *testing.T) { - if err := validateRestartPolicy(test.ContainerInfo.restart); (err != nil) != test.wantErr { - t.Errorf("ValidateRestartPolicy() error = %v, wantErr %v", err, test.wantErr) - } - }) - } -} - -func TestCreateContainerSystemdUnit(t *testing.T) { - goodID := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service -# autogenerated by Podman CI - -[Unit] -Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 -ExecStop=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 -PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodName := `# container-foobar.service -# autogenerated by Podman CI - -[Unit] -Description=Podman container-foobar.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStart=/usr/bin/podman start foobar -ExecStop=/usr/bin/podman stop -t 10 foobar -PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodNameBoundTo := `# container-foobar.service -# autogenerated by Podman CI - -[Unit] -Description=Podman container-foobar.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target -RefuseManualStart=yes -RefuseManualStop=yes -BindsTo=a.service b.service c.service pod.service -After=a.service b.service c.service pod.service - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStart=/usr/bin/podman start foobar -ExecStop=/usr/bin/podman stop -t 10 foobar -PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - podGoodName := `# pod-123abc.service -# autogenerated by Podman CI - -[Unit] -Description=Podman pod-123abc.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target -Requires=container-1.service container-2.service -Before=container-1.service container-2.service - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStart=/usr/bin/podman start jadda-jadda-infra -ExecStop=/usr/bin/podman stop -t 10 jadda-jadda-infra -PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodNameNew := `# jadda-jadda.service -# autogenerated by Podman CI - -[Unit] -Description=Podman jadda-jadda.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodNameNewDetach := `# jadda-jadda.service -# autogenerated by Podman CI - -[Unit] -Description=Podman jadda-jadda.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodIDNew := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service -# autogenerated by Podman CI - -[Unit] -Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d awesome-image:latest -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 10 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - tests := []struct { - name string - info ContainerInfo - want string - wantErr bool - }{ - - {"good with id", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - ContainerName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - }, - goodID, - false, - }, - {"good with name", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-foobar", - ContainerName: "foobar", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - }, - goodName, - false, - }, - {"good with name and bound to", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-foobar", - ContainerName: "foobar", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - BoundToServices: []string{"pod", "a", "b", "c"}, - }, - goodNameBoundTo, - false, - }, - {"pod", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "pod-123abc", - ContainerName: "jadda-jadda-infra", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - RequiredServices: []string{"container-1", "container-2"}, - }, - podGoodName, - false, - }, - {"bad restart policy", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - RestartPolicy: "never", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - }, - "", - true, - }, - {"good with name and generic", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "jadda-jadda", - ContainerName: "jadda-jadda", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 42, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, - }, - goodNameNew, - false, - }, - {"good with explicit short detach param", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "jadda-jadda", - ContainerName: "jadda-jadda", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 42, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, - }, - goodNameNew, - false, - }, - {"good with explicit full detach param", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "jadda-jadda", - ContainerName: "jadda-jadda", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 42, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, - }, - goodNameNewDetach, - false, - }, - {"good with id and no param", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - ContainerName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "awesome-image:latest"}, - }, - goodIDNew, - false, - }, - } - for _, tt := range tests { - test := tt - t.Run(tt.name, func(t *testing.T) { - opts := Options{ - Files: false, - New: test.info.New, - } - got, err := CreateContainerSystemdUnit(&test.info, opts) - if (err != nil) != test.wantErr { - t.Errorf("CreateContainerSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr) - return - } - if got != test.want { - t.Errorf("CreateContainerSystemdUnit() = \n%v\n---------> want\n%v", got, test.want) - } - }) - } -} diff --git a/pkg/systemd/generate/generate_test.go b/pkg/systemd/generate/generate_test.go new file mode 100644 index 000000000..11cabb463 --- /dev/null +++ b/pkg/systemd/generate/generate_test.go @@ -0,0 +1,349 @@ +package generate + +import ( + "testing" + + "github.com/containers/libpod/pkg/domain/entities" +) + +func TestValidateRestartPolicy(t *testing.T) { + type containerInfo struct { + restart string + } + tests := []struct { + name string + containerInfo containerInfo + wantErr bool + }{ + {"good-on", containerInfo{restart: "no"}, false}, + {"good-on-success", containerInfo{restart: "on-success"}, false}, + {"good-on-failure", containerInfo{restart: "on-failure"}, false}, + {"good-on-abnormal", containerInfo{restart: "on-abnormal"}, false}, + {"good-on-watchdog", containerInfo{restart: "on-watchdog"}, false}, + {"good-on-abort", containerInfo{restart: "on-abort"}, false}, + {"good-always", containerInfo{restart: "always"}, false}, + {"fail", containerInfo{restart: "foobar"}, true}, + {"failblank", containerInfo{restart: ""}, true}, + } + for _, tt := range tests { + test := tt + t.Run(tt.name, func(t *testing.T) { + if err := validateRestartPolicy(test.containerInfo.restart); (err != nil) != test.wantErr { + t.Errorf("ValidateRestartPolicy() error = %v, wantErr %v", err, test.wantErr) + } + }) + } +} + +func TestCreateContainerSystemdUnit(t *testing.T) { + goodID := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service +# autogenerated by Podman CI + +[Unit] +Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 +ExecStop=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodName := `# container-foobar.service +# autogenerated by Podman CI + +[Unit] +Description=Podman container-foobar.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodNameBoundTo := `# container-foobar.service +# autogenerated by Podman CI + +[Unit] +Description=Podman container-foobar.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +RefuseManualStart=yes +RefuseManualStop=yes +BindsTo=a.service b.service c.service pod.service +After=a.service b.service c.service pod.service + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + podGoodName := `# pod-123abc.service +# autogenerated by Podman CI + +[Unit] +Description=Podman pod-123abc.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +Requires=container-1.service container-2.service +Before=container-1.service container-2.service + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 10 jadda-jadda-infra +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodNameNew := `# jadda-jadda.service +# autogenerated by Podman CI + +[Unit] +Description=Podman jadda-jadda.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id +PIDFile=%t/%n-pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodNameNewDetach := `# jadda-jadda.service +# autogenerated by Podman CI + +[Unit] +Description=Podman jadda-jadda.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id +PIDFile=%t/%n-pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodIDNew := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service +# autogenerated by Podman CI + +[Unit] +Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 10 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id +PIDFile=%t/%n-pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + tests := []struct { + name string + info containerInfo + want string + wantErr bool + }{ + + {"good with id", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + ContainerNameOrID: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + }, + goodID, + false, + }, + {"good with name", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-foobar", + ContainerNameOrID: "foobar", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + }, + goodName, + false, + }, + {"good with name and bound to", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-foobar", + ContainerNameOrID: "foobar", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + BoundToServices: []string{"pod", "a", "b", "c"}, + }, + goodNameBoundTo, + false, + }, + {"pod", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "pod-123abc", + ContainerNameOrID: "jadda-jadda-infra", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + RequiredServices: []string{"container-1", "container-2"}, + }, + podGoodName, + false, + }, + {"bad restart policy", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + RestartPolicy: "never", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + }, + "", + true, + }, + {"good with name and generic", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + New: true, + CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + }, + goodNameNew, + false, + }, + {"good with explicit short detach param", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + New: true, + CreateCommand: []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + }, + goodNameNew, + false, + }, + {"good with explicit full detach param", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + New: true, + CreateCommand: []string{"I'll get stripped", "container", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + }, + goodNameNewDetach, + false, + }, + {"good with id and no param", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + ContainerNameOrID: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + New: true, + CreateCommand: []string{"I'll get stripped", "container", "run", "awesome-image:latest"}, + }, + goodIDNew, + false, + }, + } + for _, tt := range tests { + test := tt + t.Run(tt.name, func(t *testing.T) { + opts := entities.GenerateSystemdOptions{ + Files: false, + New: test.info.New, + } + got, err := createContainerSystemdUnit(&test.info, opts) + if (err != nil) != test.wantErr { + t.Errorf("CreateContainerSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr) + return + } + if got != test.want { + t.Errorf("CreateContainerSystemdUnit() = \n%v\n---------> want\n%v", got, test.want) + } + }) + } +} diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go new file mode 100644 index 000000000..16f324f78 --- /dev/null +++ b/pkg/systemd/generate/pods.go @@ -0,0 +1,130 @@ +package generate + +import ( + "fmt" + "strings" + + "github.com/containers/libpod/libpod" + "github.com/containers/libpod/pkg/domain/entities" + "github.com/pkg/errors" +) + +// PodUnits generates systemd units for the specified pod and its containers. +// Based on the options, the return value might be the content of all units or +// the files they been written to. +func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, error) { + if options.New { + return "", errors.New("--new is not supported for pods") + } + + // Error out if the pod has no infra container, which we require to be the + // main service. + if !pod.HasInfraContainer() { + return "", fmt.Errorf("error generating systemd unit files: Pod %q has no infra container", pod.Name()) + } + + podInfo, err := generatePodInfo(pod, options) + if err != nil { + return "", err + } + + infraID, err := pod.InfraContainerID() + if err != nil { + return "", err + } + + // Compute the container-dependency graph for the Pod. + containers, err := pod.AllContainers() + if err != nil { + return "", err + } + if len(containers) == 0 { + return "", fmt.Errorf("error generating systemd unit files: Pod %q has no containers", pod.Name()) + } + graph, err := libpod.BuildContainerGraph(containers) + if err != nil { + return "", err + } + + // Traverse the dependency graph and create systemdgen.containerInfo's for + // each container. + containerInfos := []*containerInfo{podInfo} + for ctr, dependencies := range graph.DependencyMap() { + // Skip the infra container as we already generated it. + if ctr.ID() == infraID { + continue + } + ctrInfo, err := generateContainerInfo(ctr, options) + if err != nil { + return "", err + } + // Now add the container's dependencies and at the container as a + // required service of the infra container. + for _, dep := range dependencies { + if dep.ID() == infraID { + ctrInfo.BoundToServices = append(ctrInfo.BoundToServices, podInfo.ServiceName) + } else { + _, serviceName := containerServiceName(dep, options) + ctrInfo.BoundToServices = append(ctrInfo.BoundToServices, serviceName) + } + } + podInfo.RequiredServices = append(podInfo.RequiredServices, ctrInfo.ServiceName) + containerInfos = append(containerInfos, ctrInfo) + } + + // Now generate the systemd service for all containers. + builder := strings.Builder{} + for i, info := range containerInfos { + if i > 0 { + builder.WriteByte('\n') + } + out, err := createContainerSystemdUnit(info, options) + if err != nil { + return "", err + } + builder.WriteString(out) + } + + return builder.String(), nil +} + +func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (*containerInfo, error) { + // Generate a systemdgen.containerInfo for the infra container. This + // containerInfo acts as the main service of the pod. + infraCtr, err := pod.InfraContainer() + if err != nil { + return nil, errors.Wrap(err, "could not find infra container") + } + + timeout := infraCtr.StopTimeout() + if options.StopTimeout != nil { + timeout = *options.StopTimeout + } + + config := infraCtr.Config() + conmonPidFile := config.ConmonPidFile + if conmonPidFile == "" { + return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") + } + + createCommand := []string{} + + nameOrID := pod.ID() + ctrNameOrID := infraCtr.ID() + if options.Name { + nameOrID = pod.Name() + ctrNameOrID = infraCtr.Name() + } + serviceName := fmt.Sprintf("%s%s%s", options.PodPrefix, options.Separator, nameOrID) + + info := containerInfo{ + ServiceName: serviceName, + ContainerNameOrID: ctrNameOrID, + RestartPolicy: options.RestartPolicy, + PIDFile: conmonPidFile, + StopTimeout: timeout, + GenerateTimestamp: true, + CreateCommand: createCommand, + } + return &info, nil +} -- cgit v1.2.3-54-g00ecf From 8d8746adeeab8a39ccedb5b06fe8d0a785a97190 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Jun 2020 18:23:12 +0200 Subject: generate systemd: create pod template Create a new template for generating a pod unit file. Eventually, this allows for treating and extending pod and container generation seprately. The `--new` flag now also works on pods. Signed-off-by: Valentin Rothberg --- docs/source/markdown/podman-generate-systemd.1.md | 5 +- libpod/pod.go | 2 +- pkg/systemd/generate/common.go | 14 + pkg/systemd/generate/common_test.go | 25 ++ pkg/systemd/generate/containers.go | 189 ++++++----- pkg/systemd/generate/containers_test.go | 366 ++++++++++++++++++++++ pkg/systemd/generate/generate_test.go | 349 --------------------- pkg/systemd/generate/pods.go | 239 +++++++++++++- pkg/systemd/generate/pods_test.go | 100 ++++++ test/e2e/generate_systemd_test.go | 50 ++- 10 files changed, 891 insertions(+), 448 deletions(-) create mode 100644 pkg/systemd/generate/common_test.go create mode 100644 pkg/systemd/generate/containers_test.go delete mode 100644 pkg/systemd/generate/generate_test.go create mode 100644 pkg/systemd/generate/pods_test.go (limited to 'libpod') diff --git a/docs/source/markdown/podman-generate-systemd.1.md b/docs/source/markdown/podman-generate-systemd.1.md index 72031b19b..2facd754c 100644 --- a/docs/source/markdown/podman-generate-systemd.1.md +++ b/docs/source/markdown/podman-generate-systemd.1.md @@ -26,10 +26,7 @@ Use the name of the container for the start, stop, and description in the unit f **--new** -Create a new container via podman-run instead of starting an existing one. This option relies on container configuration files, which may not map directly to podman CLI flags; please review the generated output carefully before placing in production. -Since we use systemd `Type=forking` service, using this option will force the container run with the detached param `-d`. - -Note: Generating systemd unit files with `--new` flag is not yet supported for pods. +Using this flag will yield unit files that do not expect containers and pods to exist. Instead, new containers and pods are created based on their configuration files. The unit files are created best effort and may need to be further edited; please review the generated files carefully before using them in production. **--time**, **-t**=*value* diff --git a/libpod/pod.go b/libpod/pod.go index f53290876..bf0d7a397 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -260,7 +260,7 @@ func (p *Pod) InfraContainerID() (string, error) { // InfraContainer returns the infra container. func (p *Pod) InfraContainer() (*Container, error) { if !p.HasInfraContainer() { - return nil, errors.New("pod has no infra container") + return nil, errors.Wrap(define.ErrNoSuchCtr, "pod has no infra container") } id, err := p.InfraContainerID() diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index e809b4837..4f995be96 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -34,3 +34,17 @@ Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target ` + +// filterPodFlags removes --pod and --pod-id-file from the specified command. +func filterPodFlags(command []string) []string { + processed := []string{} + for i := 0; i < len(command); i++ { + s := command[i] + if s == "--pod" || s == "--pod-id-file" { + i += 1 + continue + } + processed = append(processed, s) + } + return processed +} diff --git a/pkg/systemd/generate/common_test.go b/pkg/systemd/generate/common_test.go new file mode 100644 index 000000000..f53bb7828 --- /dev/null +++ b/pkg/systemd/generate/common_test.go @@ -0,0 +1,25 @@ +package generate + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFilterPodFlags(t *testing.T) { + + tests := []struct { + input []string + }{ + {[]string{"podman", "pod", "create"}}, + {[]string{"podman", "pod", "create", "--name", "foo"}}, + {[]string{"podman", "pod", "create", "--pod-id-file", "foo"}}, + {[]string{"podman", "run", "--pod", "foo"}}, + } + + for _, test := range tests { + processed := filterPodFlags(test.input) + assert.NotContains(t, processed, "--pod-id-file") + assert.NotContains(t, processed, "--pod") + } +} diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index f316d4452..dced1a3da 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -33,14 +33,13 @@ type containerInfo struct { // PIDFile of the service. Required for forking services. Must point to the // PID of the associated conmon process. PIDFile string + // ContainerIDFile to be used in the unit. + ContainerIDFile string // GenerateTimestamp, if set the generated unit file has a time stamp. GenerateTimestamp bool // BoundToServices are the services this service binds to. Note that this // service runs after them. BoundToServices []string - // RequiredServices are services this service requires. Note that this - // service runs before them. - RequiredServices []string // PodmanVersion for the header. Will be set internally. Will be auto-filled // if left empty. PodmanVersion string @@ -49,16 +48,23 @@ type containerInfo struct { Executable string // TimeStamp at the time of creating the unit file. Will be set internally. TimeStamp string - // New controls if a new container is created or if an existing one is started. - New bool // CreateCommand is the full command plus arguments of the process the // container has been created with. CreateCommand []string - // RunCommand is a post-processed variant of CreateCommand and used for - // the ExecStart field in generic unit files. - RunCommand string // EnvVariable is generate.EnvVariable and must not be set. EnvVariable string + // ExecStartPre of the unit. + ExecStartPre string + // ExecStart of the unit. + ExecStart string + // ExecStop of the unit. + ExecStop string + // ExecStopPost of the unit. + ExecStopPost string + + // If not nil, the container is part of the pod. We can use the + // podInfo to extract the relevant data. + pod *podInfo } const containerTemplate = headerTemplate + ` @@ -68,25 +74,19 @@ RefuseManualStop=yes BindsTo={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} After={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} {{- end}} -{{- if .RequiredServices}} -Requires={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} -Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} -{{- end}} [Service] Environment={{.EnvVariable}}=%n Restart={{.RestartPolicy}} -{{- if .New}} -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart={{.RunCommand}} -ExecStop={{.Executable}} stop --ignore --cidfile %t/%n-ctr-id {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} -ExecStopPost={{.Executable}} rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid -{{- else}} -ExecStart={{.Executable}} start {{.ContainerNameOrID}} -ExecStop={{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}} -PIDFile={{.PIDFile}} +{{- if .ExecStartPre}} +ExecStartPre={{.ExecStartPre}} +{{- end}} +ExecStart={{.ExecStart}} +ExecStop={{.ExecStop}} +{{- if .ExecStopPost}} +ExecStopPost={{.ExecStopPost}} {{- end}} +PIDFile={{.PIDFile}} KillMode=none Type=forking @@ -101,11 +101,58 @@ func ContainerUnit(ctr *libpod.Container, options entities.GenerateSystemdOption if err != nil { return "", err } - return createContainerSystemdUnit(info, options) + return executeContainerTemplate(info, options) +} + +func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSystemdOptions) (*containerInfo, error) { + timeout := ctr.StopTimeout() + if options.StopTimeout != nil { + timeout = *options.StopTimeout + } + + config := ctr.Config() + conmonPidFile := config.ConmonPidFile + if conmonPidFile == "" { + return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") + } + + createCommand := []string{} + if config.CreateCommand != nil { + createCommand = config.CreateCommand + } else if options.New { + return nil, errors.Errorf("cannot use --new on container %q: no create command found", ctr.ID()) + } + + nameOrID, serviceName := containerServiceName(ctr, options) + + info := containerInfo{ + ServiceName: serviceName, + ContainerNameOrID: nameOrID, + RestartPolicy: options.RestartPolicy, + PIDFile: conmonPidFile, + StopTimeout: timeout, + GenerateTimestamp: true, + CreateCommand: createCommand, + } + + return &info, nil +} + +// containerServiceName returns the nameOrID and the service name of the +// container. +func containerServiceName(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, string) { + nameOrID := ctr.ID() + if options.Name { + nameOrID = ctr.Name() + } + serviceName := fmt.Sprintf("%s%s%s", options.ContainerPrefix, options.Separator, nameOrID) + return nameOrID, serviceName } -// createContainerSystemdUnit creates a systemd unit file for a container. -func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSystemdOptions) (string, error) { +// executeContainerTemplate executes the container template on the specified +// containerInfo. Note that the containerInfo is also post processed and +// completed, which allows for an easier unit testing. +func executeContainerTemplate(info *containerInfo, options entities.GenerateSystemdOptions) (string, error) { if err := validateRestartPolicy(info.RestartPolicy); err != nil { return "", err } @@ -121,6 +168,8 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } info.EnvVariable = EnvVariable + info.ExecStart = "{{.Executable}} start {{.ContainerNameOrID}}" + info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}" // Assemble the ExecStart command when creating a new container. // @@ -130,6 +179,8 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy // invalid `info.CreateCommand`. Hence, we're doing a best effort unit // generation and don't try aiming at completeness. if options.New { + info.PIDFile = "%t/" + info.ServiceName + ".pid" + info.ContainerIDFile = "%t/" + info.ServiceName + ".ctr-id" // The create command must at least have three arguments: // /usr/bin/podman run $IMAGE index := 2 @@ -141,13 +192,20 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } // We're hard-coding the first five arguments and append the // CreateCommand with a stripped command and subcomand. - command := []string{ + startCommand := []string{ info.Executable, "run", - "--conmon-pidfile", "%t/%n-pid", - "--cidfile", "%t/%n-ctr-id", + "--conmon-pidfile", "{{.PIDFile}}", + "--cidfile", "{{.ContainerIDFile}}", "--cgroups=no-conmon", } + // If the container is in a pod, make sure that the + // --pod-id-file is set correctly. + if info.pod != nil { + podFlags := []string{"--pod-id-file", info.pod.PodIDFile} + startCommand = append(startCommand, podFlags...) + info.CreateCommand = filterPodFlags(info.CreateCommand) + } // Enforce detaching // @@ -165,12 +223,14 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } } if !hasDetachParam { - command = append(command, "-d") + startCommand = append(startCommand, "-d") } + startCommand = append(startCommand, info.CreateCommand[index:]...) - command = append(command, info.CreateCommand[index:]...) - info.RunCommand = strings.Join(command, " ") - info.New = true + info.ExecStartPre = "/usr/bin/rm -f {{.PIDFile}} {{.ContainerIDFile}}" + info.ExecStart = strings.Join(startCommand, " ") + info.ExecStop = "{{.Executable}} stop --ignore --cidfile {{.ContainerIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}" + info.ExecStopPost = "{{.Executable}} rm --ignore -f --cidfile {{.ContainerIDFile}}" } if info.PodmanVersion == "" { @@ -181,11 +241,17 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } // Sort the slices to assure a deterministic output. - sort.Strings(info.RequiredServices) sort.Strings(info.BoundToServices) // Generate the template and compile it. - templ, err := template.New("systemd_service_file").Parse(containerTemplate) + // + // Note that we need a two-step generation process to allow for fields + // embedding other fields. This way we can replace `A -> B -> C` and + // make the code easier to maintain at the cost of a slightly slower + // generation. That's especially needed for embedding the PID and ID + // files in other fields which will eventually get replaced in the 2nd + // template execution. + templ, err := template.New("container_template").Parse(containerTemplate) if err != nil { return "", errors.Wrap(err, "error parsing systemd service template") } @@ -195,6 +261,17 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy return "", err } + // Now parse the generated template (i.e., buf) and execute it. + templ, err = template.New("container_template").Parse(buf.String()) + if err != nil { + return "", errors.Wrap(err, "error parsing systemd service template") + } + + buf = bytes.Buffer{} + if err := templ.Execute(&buf, info); err != nil { + return "", err + } + if !options.Files { return buf.String(), nil } @@ -210,47 +287,3 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } return path, nil } - -func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSystemdOptions) (*containerInfo, error) { - timeout := ctr.StopTimeout() - if options.StopTimeout != nil { - timeout = *options.StopTimeout - } - - config := ctr.Config() - conmonPidFile := config.ConmonPidFile - if conmonPidFile == "" { - return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") - } - - createCommand := []string{} - if config.CreateCommand != nil { - createCommand = config.CreateCommand - } else if options.New { - return nil, errors.Errorf("cannot use --new on container %q: no create command found", ctr.ID()) - } - - nameOrID, serviceName := containerServiceName(ctr, options) - - info := containerInfo{ - ServiceName: serviceName, - ContainerNameOrID: nameOrID, - RestartPolicy: options.RestartPolicy, - PIDFile: conmonPidFile, - StopTimeout: timeout, - GenerateTimestamp: true, - CreateCommand: createCommand, - } - return &info, nil -} - -// containerServiceName returns the nameOrID and the service name of the -// container. -func containerServiceName(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, string) { - nameOrID := ctr.ID() - if options.Name { - nameOrID = ctr.Name() - } - serviceName := fmt.Sprintf("%s%s%s", options.ContainerPrefix, options.Separator, nameOrID) - return nameOrID, serviceName -} diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go new file mode 100644 index 000000000..8365ecd7a --- /dev/null +++ b/pkg/systemd/generate/containers_test.go @@ -0,0 +1,366 @@ +package generate + +import ( + "testing" + + "github.com/containers/libpod/pkg/domain/entities" +) + +func TestValidateRestartPolicyContainer(t *testing.T) { + type containerInfo struct { + restart string + } + tests := []struct { + name string + containerInfo containerInfo + wantErr bool + }{ + {"good-on", containerInfo{restart: "no"}, false}, + {"good-on-success", containerInfo{restart: "on-success"}, false}, + {"good-on-failure", containerInfo{restart: "on-failure"}, false}, + {"good-on-abnormal", containerInfo{restart: "on-abnormal"}, false}, + {"good-on-watchdog", containerInfo{restart: "on-watchdog"}, false}, + {"good-on-abort", containerInfo{restart: "on-abort"}, false}, + {"good-always", containerInfo{restart: "always"}, false}, + {"fail", containerInfo{restart: "foobar"}, true}, + {"failblank", containerInfo{restart: ""}, true}, + } + for _, tt := range tests { + test := tt + t.Run(tt.name, func(t *testing.T) { + if err := validateRestartPolicy(test.containerInfo.restart); (err != nil) != test.wantErr { + t.Errorf("ValidateRestartPolicy() error = %v, wantErr %v", err, test.wantErr) + } + }) + } +} + +func TestCreateContainerSystemdUnit(t *testing.T) { + goodID := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service +# autogenerated by Podman CI + +[Unit] +Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 +ExecStop=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodName := `# container-foobar.service +# autogenerated by Podman CI + +[Unit] +Description=Podman container-foobar.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodNameBoundTo := `# container-foobar.service +# autogenerated by Podman CI + +[Unit] +Description=Podman container-foobar.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +RefuseManualStart=yes +RefuseManualStop=yes +BindsTo=a.service b.service c.service pod.service +After=a.service b.service c.service pod.service + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodNameNew := `# jadda-jadda.service +# autogenerated by Podman CI + +[Unit] +Description=Podman jadda-jadda.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStartPre=/usr/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodNameNewWithPodFile := `# jadda-jadda.service +# autogenerated by Podman CI + +[Unit] +Description=Podman jadda-jadda.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStartPre=/usr/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon --pod-id-file /tmp/pod-foobar.pod-id-file -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + goodNameNewDetach := `# jadda-jadda.service +# autogenerated by Podman CI + +[Unit] +Description=Podman jadda-jadda.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStartPre=/usr/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + goodIDNew := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service +# autogenerated by Podman CI + +[Unit] +Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStartPre=/usr/bin/rm -f %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid --cidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id --cgroups=no-conmon -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id -t 10 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id +PIDFile=%t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + tests := []struct { + name string + info containerInfo + want string + new bool + wantErr bool + }{ + + {"good with id", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + ContainerNameOrID: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + EnvVariable: EnvVariable, + }, + goodID, + false, + false, + }, + {"good with name", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-foobar", + ContainerNameOrID: "foobar", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + EnvVariable: EnvVariable, + }, + goodName, + false, + false, + }, + {"good with name and bound to", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-foobar", + ContainerNameOrID: "foobar", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + BoundToServices: []string{"pod", "a", "b", "c"}, + EnvVariable: EnvVariable, + }, + goodNameBoundTo, + false, + false, + }, + {"bad restart policy", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + RestartPolicy: "never", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + EnvVariable: EnvVariable, + }, + "", + false, + true, + }, + {"good with name and generic", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + EnvVariable: EnvVariable, + }, + goodNameNew, + true, + false, + }, + {"good with explicit short detach param", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + CreateCommand: []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + EnvVariable: EnvVariable, + }, + goodNameNew, + true, + false, + }, + {"good with explicit short detach param and podInfo", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + CreateCommand: []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + EnvVariable: EnvVariable, + pod: &podInfo{ + PodIDFile: "/tmp/pod-foobar.pod-id-file", + }, + }, + goodNameNewWithPodFile, + true, + false, + }, + {"good with explicit full detach param", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + CreateCommand: []string{"I'll get stripped", "container", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + EnvVariable: EnvVariable, + }, + goodNameNewDetach, + true, + false, + }, + {"good with id and no param", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + ContainerNameOrID: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + CreateCommand: []string{"I'll get stripped", "container", "run", "awesome-image:latest"}, + EnvVariable: EnvVariable, + }, + goodIDNew, + true, + false, + }, + } + for _, tt := range tests { + test := tt + t.Run(tt.name, func(t *testing.T) { + opts := entities.GenerateSystemdOptions{ + Files: false, + New: test.new, + } + got, err := executeContainerTemplate(&test.info, opts) + if (err != nil) != test.wantErr { + t.Errorf("CreateContainerSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr) + return + } + if got != test.want { + t.Errorf("CreateContainerSystemdUnit() = \n%v\n---------> want\n%v", got, test.want) + } + }) + } +} diff --git a/pkg/systemd/generate/generate_test.go b/pkg/systemd/generate/generate_test.go deleted file mode 100644 index 11cabb463..000000000 --- a/pkg/systemd/generate/generate_test.go +++ /dev/null @@ -1,349 +0,0 @@ -package generate - -import ( - "testing" - - "github.com/containers/libpod/pkg/domain/entities" -) - -func TestValidateRestartPolicy(t *testing.T) { - type containerInfo struct { - restart string - } - tests := []struct { - name string - containerInfo containerInfo - wantErr bool - }{ - {"good-on", containerInfo{restart: "no"}, false}, - {"good-on-success", containerInfo{restart: "on-success"}, false}, - {"good-on-failure", containerInfo{restart: "on-failure"}, false}, - {"good-on-abnormal", containerInfo{restart: "on-abnormal"}, false}, - {"good-on-watchdog", containerInfo{restart: "on-watchdog"}, false}, - {"good-on-abort", containerInfo{restart: "on-abort"}, false}, - {"good-always", containerInfo{restart: "always"}, false}, - {"fail", containerInfo{restart: "foobar"}, true}, - {"failblank", containerInfo{restart: ""}, true}, - } - for _, tt := range tests { - test := tt - t.Run(tt.name, func(t *testing.T) { - if err := validateRestartPolicy(test.containerInfo.restart); (err != nil) != test.wantErr { - t.Errorf("ValidateRestartPolicy() error = %v, wantErr %v", err, test.wantErr) - } - }) - } -} - -func TestCreateContainerSystemdUnit(t *testing.T) { - goodID := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service -# autogenerated by Podman CI - -[Unit] -Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 -ExecStop=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 -PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodName := `# container-foobar.service -# autogenerated by Podman CI - -[Unit] -Description=Podman container-foobar.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStart=/usr/bin/podman start foobar -ExecStop=/usr/bin/podman stop -t 10 foobar -PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodNameBoundTo := `# container-foobar.service -# autogenerated by Podman CI - -[Unit] -Description=Podman container-foobar.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target -RefuseManualStart=yes -RefuseManualStop=yes -BindsTo=a.service b.service c.service pod.service -After=a.service b.service c.service pod.service - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStart=/usr/bin/podman start foobar -ExecStop=/usr/bin/podman stop -t 10 foobar -PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - podGoodName := `# pod-123abc.service -# autogenerated by Podman CI - -[Unit] -Description=Podman pod-123abc.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target -Requires=container-1.service container-2.service -Before=container-1.service container-2.service - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStart=/usr/bin/podman start jadda-jadda-infra -ExecStop=/usr/bin/podman stop -t 10 jadda-jadda-infra -PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodNameNew := `# jadda-jadda.service -# autogenerated by Podman CI - -[Unit] -Description=Podman jadda-jadda.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodNameNewDetach := `# jadda-jadda.service -# autogenerated by Podman CI - -[Unit] -Description=Podman jadda-jadda.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - goodIDNew := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service -# autogenerated by Podman CI - -[Unit] -Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target - -[Service] -Environment=PODMAN_SYSTEMD_UNIT=%n -Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d awesome-image:latest -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 10 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid -KillMode=none -Type=forking - -[Install] -WantedBy=multi-user.target default.target` - - tests := []struct { - name string - info containerInfo - want string - wantErr bool - }{ - - {"good with id", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - ContainerNameOrID: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - }, - goodID, - false, - }, - {"good with name", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-foobar", - ContainerNameOrID: "foobar", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - }, - goodName, - false, - }, - {"good with name and bound to", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-foobar", - ContainerNameOrID: "foobar", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - BoundToServices: []string{"pod", "a", "b", "c"}, - }, - goodNameBoundTo, - false, - }, - {"pod", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "pod-123abc", - ContainerNameOrID: "jadda-jadda-infra", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - RequiredServices: []string{"container-1", "container-2"}, - }, - podGoodName, - false, - }, - {"bad restart policy", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - RestartPolicy: "never", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - }, - "", - true, - }, - {"good with name and generic", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "jadda-jadda", - ContainerNameOrID: "jadda-jadda", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 42, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, - }, - goodNameNew, - false, - }, - {"good with explicit short detach param", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "jadda-jadda", - ContainerNameOrID: "jadda-jadda", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 42, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, - }, - goodNameNew, - false, - }, - {"good with explicit full detach param", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "jadda-jadda", - ContainerNameOrID: "jadda-jadda", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 42, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, - }, - goodNameNewDetach, - false, - }, - {"good with id and no param", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - ContainerNameOrID: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "awesome-image:latest"}, - }, - goodIDNew, - false, - }, - } - for _, tt := range tests { - test := tt - t.Run(tt.name, func(t *testing.T) { - opts := entities.GenerateSystemdOptions{ - Files: false, - New: test.info.New, - } - got, err := createContainerSystemdUnit(&test.info, opts) - if (err != nil) != test.wantErr { - t.Errorf("CreateContainerSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr) - return - } - if got != test.want { - t.Errorf("CreateContainerSystemdUnit() = \n%v\n---------> want\n%v", got, test.want) - } - }) - } -} diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index 16f324f78..355103df8 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -1,22 +1,101 @@ package generate import ( + "bytes" "fmt" + "io/ioutil" + "os" + "path/filepath" + "sort" "strings" + "text/template" + "time" "github.com/containers/libpod/libpod" "github.com/containers/libpod/pkg/domain/entities" + "github.com/containers/libpod/version" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) +// podInfo contains data required for generating a pod's systemd +// unit file. +type podInfo struct { + // ServiceName of the systemd service. + ServiceName string + // Name or ID of the infra container. + InfraNameOrID string + // StopTimeout sets the timeout Podman waits before killing the container + // during service stop. + StopTimeout uint + // RestartPolicy of the systemd unit (e.g., no, on-failure, always). + RestartPolicy string + // PIDFile of the service. Required for forking services. Must point to the + // PID of the associated conmon process. + PIDFile string + // PodIDFile of the unit. + PodIDFile string + // GenerateTimestamp, if set the generated unit file has a time stamp. + GenerateTimestamp bool + // RequiredServices are services this service requires. Note that this + // service runs before them. + RequiredServices []string + // PodmanVersion for the header. Will be set internally. Will be auto-filled + // if left empty. + PodmanVersion string + // Executable is the path to the podman executable. Will be auto-filled if + // left empty. + Executable string + // TimeStamp at the time of creating the unit file. Will be set internally. + TimeStamp string + // CreateCommand is the full command plus arguments of the process the + // container has been created with. + CreateCommand []string + // PodCreateCommand - a post-processed variant of CreateCommand to use + // when creating the pod. + PodCreateCommand string + // EnvVariable is generate.EnvVariable and must not be set. + EnvVariable string + // ExecStartPre1 of the unit. + ExecStartPre1 string + // ExecStartPre2 of the unit. + ExecStartPre2 string + // ExecStart of the unit. + ExecStart string + // ExecStop of the unit. + ExecStop string + // ExecStopPost of the unit. + ExecStopPost string +} + +const podTemplate = headerTemplate + `Requires={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} +Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} + +[Service] +Environment={{.EnvVariable}}=%n +Restart={{.RestartPolicy}} +{{- if .ExecStartPre1}} +ExecStartPre={{.ExecStartPre1}} +{{- end}} +{{- if .ExecStartPre2}} +ExecStartPre={{.ExecStartPre2}} +{{- end}} +ExecStart={{.ExecStart}} +ExecStop={{.ExecStop}} +{{- if .ExecStopPost}} +ExecStopPost={{.ExecStopPost}} +{{- end}} +PIDFile={{.PIDFile}} +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + // PodUnits generates systemd units for the specified pod and its containers. // Based on the options, the return value might be the content of all units or // the files they been written to. func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, error) { - if options.New { - return "", errors.New("--new is not supported for pods") - } - // Error out if the pod has no infra container, which we require to be the // main service. if !pod.HasInfraContainer() { @@ -48,7 +127,7 @@ func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, // Traverse the dependency graph and create systemdgen.containerInfo's for // each container. - containerInfos := []*containerInfo{podInfo} + containerInfos := []*containerInfo{} for ctr, dependencies := range graph.DependencyMap() { // Skip the infra container as we already generated it. if ctr.ID() == infraID { @@ -74,11 +153,15 @@ func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, // Now generate the systemd service for all containers. builder := strings.Builder{} - for i, info := range containerInfos { - if i > 0 { - builder.WriteByte('\n') - } - out, err := createContainerSystemdUnit(info, options) + out, err := executePodTemplate(podInfo, options) + if err != nil { + return "", err + } + builder.WriteString(out) + for _, info := range containerInfos { + info.pod = podInfo + builder.WriteByte('\n') + out, err := executeContainerTemplate(info, options) if err != nil { return "", err } @@ -88,7 +171,7 @@ func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, return builder.String(), nil } -func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (*containerInfo, error) { +func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (*podInfo, error) { // Generate a systemdgen.containerInfo for the infra container. This // containerInfo acts as the main service of the pod. infraCtr, err := pod.InfraContainer() @@ -107,7 +190,10 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) ( return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") } - createCommand := []string{} + createCommand := pod.CreateCommand() + if options.New && len(createCommand) == 0 { + return nil, errors.Errorf("cannot use --new on pod %q: no create command found", pod.ID()) + } nameOrID := pod.ID() ctrNameOrID := infraCtr.ID() @@ -117,9 +203,9 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) ( } serviceName := fmt.Sprintf("%s%s%s", options.PodPrefix, options.Separator, nameOrID) - info := containerInfo{ + info := podInfo{ ServiceName: serviceName, - ContainerNameOrID: ctrNameOrID, + InfraNameOrID: ctrNameOrID, RestartPolicy: options.RestartPolicy, PIDFile: conmonPidFile, StopTimeout: timeout, @@ -128,3 +214,128 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) ( } return &info, nil } + +// executePodTemplate executes the pod template on the specified podInfo. Note +// that the podInfo is also post processed and completed, which allows for an +// easier unit testing. +func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) (string, error) { + if err := validateRestartPolicy(info.RestartPolicy); err != nil { + return "", err + } + + // Make sure the executable is set. + if info.Executable == "" { + executable, err := os.Executable() + if err != nil { + executable = "/usr/bin/podman" + logrus.Warnf("Could not obtain podman executable location, using default %s", executable) + } + info.Executable = executable + } + + info.EnvVariable = EnvVariable + info.ExecStart = "{{.Executable}} start {{.InfraNameOrID}}" + info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}" + + // Assemble the ExecStart command when creating a new pod. + // + // Note that we cannot catch all corner cases here such that users + // *must* manually check the generated files. A pod might have been + // created via a Python script, which would certainly yield an invalid + // `info.CreateCommand`. Hence, we're doing a best effort unit + // generation and don't try aiming at completeness. + if options.New { + info.PIDFile = "%t/" + info.ServiceName + ".pid" + info.PodIDFile = "%t/" + info.ServiceName + ".pod-id" + + podCreateIndex := 0 + var podRootArgs, podCreateArgs []string + switch len(info.CreateCommand) { + case 0, 1, 2: + return "", errors.Errorf("pod does not appear to be created via `podman pod create`: %v", info.CreateCommand) + default: + // Make sure that pod was created with `pod create` and + // not something else, such as `run --pod new`. + for i := 1; i < len(info.CreateCommand); i++ { + if info.CreateCommand[i-1] == "pod" && info.CreateCommand[i] == "create" { + podCreateIndex = i + break + } + } + if podCreateIndex == 0 { + return "", errors.Errorf("pod does not appear to be created via `podman pod create`: %v", info.CreateCommand) + } + podRootArgs = info.CreateCommand[1 : podCreateIndex-2] + podCreateArgs = filterPodFlags(info.CreateCommand[podCreateIndex+1:]) + } + // We're hard-coding the first five arguments and append the + // CreateCommand with a stripped command and subcomand. + startCommand := []string{info.Executable} + startCommand = append(startCommand, podRootArgs...) + startCommand = append(startCommand, + []string{"pod", "create", + "--infra-conmon-pidfile", "{{.PIDFile}}", + "--pod-id-file", "{{.PodIDFile}}"}...) + + startCommand = append(startCommand, podCreateArgs...) + + info.ExecStartPre1 = "/usr/bin/rm -f {{.PIDFile}} {{.PodIDFile}}" + info.ExecStartPre2 = strings.Join(startCommand, " ") + info.ExecStart = "{{.Executable}} pod start --pod-id-file {{.PodIDFile}}" + info.ExecStop = "{{.Executable}} pod stop --ignore --pod-id-file {{.PodIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}" + info.ExecStopPost = "{{.Executable}} pod rm --ignore -f --pod-id-file {{.PodIDFile}}" + } + if info.PodmanVersion == "" { + info.PodmanVersion = version.Version + } + if info.GenerateTimestamp { + info.TimeStamp = fmt.Sprintf("%v", time.Now().Format(time.UnixDate)) + } + + // Sort the slices to assure a deterministic output. + sort.Strings(info.RequiredServices) + + // Generate the template and compile it. + // + // Note that we need a two-step generation process to allow for fields + // embedding other fields. This way we can replace `A -> B -> C` and + // make the code easier to maintain at the cost of a slightly slower + // generation. That's especially needed for embedding the PID and ID + // files in other fields which will eventually get replaced in the 2nd + // template execution. + templ, err := template.New("pod_template").Parse(podTemplate) + if err != nil { + return "", errors.Wrap(err, "error parsing systemd service template") + } + + var buf bytes.Buffer + if err := templ.Execute(&buf, info); err != nil { + return "", err + } + + // Now parse the generated template (i.e., buf) and execute it. + templ, err = template.New("pod_template").Parse(buf.String()) + if err != nil { + return "", errors.Wrap(err, "error parsing systemd service template") + } + + buf = bytes.Buffer{} + if err := templ.Execute(&buf, info); err != nil { + return "", err + } + + if !options.Files { + return buf.String(), nil + } + + buf.WriteByte('\n') + cwd, err := os.Getwd() + if err != nil { + return "", errors.Wrap(err, "error getting current working directory") + } + path := filepath.Join(cwd, fmt.Sprintf("%s.service", info.ServiceName)) + if err := ioutil.WriteFile(path, buf.Bytes(), 0644); err != nil { + return "", errors.Wrap(err, "error generating systemd unit") + } + return path, nil +} diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go new file mode 100644 index 000000000..f6e225c35 --- /dev/null +++ b/pkg/systemd/generate/pods_test.go @@ -0,0 +1,100 @@ +package generate + +import ( + "testing" + + "github.com/containers/libpod/pkg/domain/entities" +) + +func TestValidateRestartPolicyPod(t *testing.T) { + type podInfo struct { + restart string + } + tests := []struct { + name string + podInfo podInfo + wantErr bool + }{ + {"good-on", podInfo{restart: "no"}, false}, + {"good-on-success", podInfo{restart: "on-success"}, false}, + {"good-on-failure", podInfo{restart: "on-failure"}, false}, + {"good-on-abnormal", podInfo{restart: "on-abnormal"}, false}, + {"good-on-watchdog", podInfo{restart: "on-watchdog"}, false}, + {"good-on-abort", podInfo{restart: "on-abort"}, false}, + {"good-always", podInfo{restart: "always"}, false}, + {"fail", podInfo{restart: "foobar"}, true}, + {"failblank", podInfo{restart: ""}, true}, + } + for _, tt := range tests { + test := tt + t.Run(tt.name, func(t *testing.T) { + if err := validateRestartPolicy(test.podInfo.restart); (err != nil) != test.wantErr { + t.Errorf("ValidateRestartPolicy() error = %v, wantErr %v", err, test.wantErr) + } + }) + } +} + +func TestCreatePodSystemdUnit(t *testing.T) { + podGoodName := `# pod-123abc.service +# autogenerated by Podman CI + +[Unit] +Description=Podman pod-123abc.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +Requires=container-1.service container-2.service +Before=container-1.service container-2.service + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 10 jadda-jadda-infra +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + tests := []struct { + name string + info podInfo + want string + wantErr bool + }{ + {"pod", + podInfo{ + Executable: "/usr/bin/podman", + ServiceName: "pod-123abc", + InfraNameOrID: "jadda-jadda-infra", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + RequiredServices: []string{"container-1", "container-2"}, + }, + podGoodName, + false, + }, + } + + for _, tt := range tests { + test := tt + t.Run(tt.name, func(t *testing.T) { + opts := entities.GenerateSystemdOptions{ + Files: false, + } + got, err := executePodTemplate(&test.info, opts) + if (err != nil) != test.wantErr { + t.Errorf("CreatePodSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr) + return + } + if got != test.want { + t.Errorf("CreatePodSystemdUnit() = \n%v\n---------> want\n%v", got, test.want) + } + }) + } +} diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go index 64d220776..497e8f71e 100644 --- a/test/e2e/generate_systemd_test.go +++ b/test/e2e/generate_systemd_test.go @@ -3,6 +3,7 @@ package integration import ( + "io/ioutil" "os" . "github.com/containers/libpod/test/utils" @@ -191,7 +192,7 @@ var _ = Describe("Podman generate systemd", func() { found, _ := session.GrepString("# container-foo.service") Expect(found).To(BeTrue()) - found, _ = session.GrepString("stop --ignore --cidfile %t/%n-ctr-id -t 42") + found, _ = session.GrepString("stop --ignore --cidfile %t/container-foo.ctr-id -t 42") Expect(found).To(BeTrue()) }) @@ -230,7 +231,7 @@ var _ = Describe("Podman generate systemd", func() { session := podmanTest.Podman([]string{"generate", "systemd", "--time", "42", "--name", "--new", "foo"}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(Equal(125)) + Expect(session.ExitCode()).To(Equal(0)) }) It("podman generate systemd --container-prefix con", func() { @@ -325,4 +326,49 @@ var _ = Describe("Podman generate systemd", func() { found, _ = session.GrepString("BindsTo=p_foo.service") Expect(found).To(BeTrue()) }) + + It("podman generate systemd pod with containers --new", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + tmpFile := tmpDir + "podID" + defer os.RemoveAll(tmpDir) + + n := podmanTest.Podman([]string{"pod", "create", "--pod-id-file", tmpFile, "--name", "foo"}) + n.WaitWithDefaultTimeout() + Expect(n.ExitCode()).To(Equal(0)) + + n = podmanTest.Podman([]string{"create", "--pod", "foo", "--name", "foo-1", "alpine", "top"}) + n.WaitWithDefaultTimeout() + Expect(n.ExitCode()).To(Equal(0)) + + n = podmanTest.Podman([]string{"create", "--pod", "foo", "--name", "foo-2", "alpine", "top"}) + n.WaitWithDefaultTimeout() + Expect(n.ExitCode()).To(Equal(0)) + + session := podmanTest.Podman([]string{"generate", "systemd", "--new", "--name", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Grepping the output (in addition to unit tests) + found, _ := session.GrepString("# pod-foo.service") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("Requires=container-foo-1.service container-foo-2.service") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("BindsTo=pod-foo.service") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("pod create --infra-conmon-pidfile %t/pod-foo.pid --pod-id-file %t/pod-foo.pod-id --name foo") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("ExecStartPre=/usr/bin/rm -f %t/pod-foo.pid %t/pod-foo.pod-id") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("pod stop --ignore --pod-id-file %t/pod-foo.pod-id -t 10") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("pod rm --ignore -f --pod-id-file %t/pod-foo.pod-id") + Expect(found).To(BeTrue()) + }) }) -- cgit v1.2.3-54-g00ecf From 8ef1b461ae4187cc508cc8d9b73d68d04503467f Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 11 Jun 2020 13:06:21 +0200 Subject: libpod: fix check for slirp4netns netns fix the check for c.state.NetNS == nil. Its value is changed in the first code block, so the condition is always true in the second one and we end up running slirp4netns twice. Closes: https://github.com/containers/libpod/issues/6538 Signed-off-by: Giuseppe Scrivano --- libpod/container_internal_linux.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index e3ca3f6b2..9afe11b2b 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -79,7 +79,8 @@ func (c *Container) prepare() error { go func() { defer wg.Done() // Set up network namespace if not already set up - if c.config.CreateNetNS && c.state.NetNS == nil && !c.config.PostConfigureNetNS { + noNetNS := c.state.NetNS == nil + if c.config.CreateNetNS && noNetNS && !c.config.PostConfigureNetNS { netNS, networkStatus, createNetNSErr = c.runtime.createNetNS(c) if createNetNSErr != nil { return @@ -94,7 +95,7 @@ func (c *Container) prepare() error { } // handle rootless network namespace setup - if c.state.NetNS != nil && c.config.NetMode.IsSlirp4netns() && !c.config.PostConfigureNetNS { + if noNetNS && c.config.NetMode.IsSlirp4netns() && !c.config.PostConfigureNetNS { createNetNSErr = c.runtime.setupRootlessNetNS(c) } }() -- cgit v1.2.3-54-g00ecf