diff options
-rw-r--r-- | cmd/podman/containers/mount.go | 21 | ||||
-rw-r--r-- | cmd/podman/images/history.go | 57 | ||||
-rw-r--r-- | cmd/podman/images/mount.go | 27 | ||||
-rw-r--r-- | cmd/podman/images/search.go | 51 | ||||
-rw-r--r-- | cmd/podman/networks/create.go | 4 | ||||
-rw-r--r-- | cmd/podman/networks/list.go | 2 | ||||
-rw-r--r-- | libpod/network/config.go (renamed from pkg/network/config.go) | 11 | ||||
-rw-r--r-- | libpod/network/create.go | 195 | ||||
-rw-r--r-- | libpod/network/devices.go (renamed from pkg/network/devices.go) | 0 | ||||
-rw-r--r-- | libpod/network/files.go (renamed from pkg/network/files.go) | 0 | ||||
-rw-r--r-- | libpod/network/ip.go (renamed from pkg/network/ip.go) | 0 | ||||
-rw-r--r-- | libpod/network/lock.go | 26 | ||||
-rw-r--r-- | libpod/network/netconflist.go (renamed from pkg/network/netconflist.go) | 0 | ||||
-rw-r--r-- | libpod/network/netconflist_test.go (renamed from pkg/network/netconflist_test.go) | 0 | ||||
-rw-r--r-- | libpod/network/network.go (renamed from pkg/network/network.go) | 10 | ||||
-rw-r--r-- | libpod/network/network_test.go (renamed from pkg/network/network_test.go) | 0 | ||||
-rw-r--r-- | libpod/network/subnet.go (renamed from pkg/network/subnet.go) | 0 | ||||
-rw-r--r-- | libpod/network/subnet_test.go (renamed from pkg/network/subnet_test.go) | 0 | ||||
-rw-r--r-- | libpod/pod_api.go | 241 | ||||
-rw-r--r-- | pkg/api/handlers/compat/networks.go | 8 | ||||
-rw-r--r-- | pkg/api/handlers/libpod/pods.go | 6 | ||||
-rw-r--r-- | pkg/domain/infra/abi/containers.go | 6 | ||||
-rw-r--r-- | pkg/domain/infra/abi/network.go | 176 | ||||
-rw-r--r-- | pkg/domain/infra/abi/pods.go | 6 | ||||
-rw-r--r-- | pkg/parallel/ctr/ctr.go (renamed from pkg/parallel/parallel_linux.go) | 45 | ||||
-rw-r--r-- | pkg/parallel/parallel.go | 30 | ||||
-rw-r--r-- | pkg/varlinkapi/pods.go | 7 | ||||
-rw-r--r-- | test/e2e/common_test.go | 4 | ||||
-rw-r--r-- | test/e2e/network_create_test.go | 2 | ||||
-rw-r--r-- | test/e2e/search_test.go | 6 | ||||
-rw-r--r-- | test/system/060-mount.bats | 2 |
31 files changed, 492 insertions, 451 deletions
diff --git a/cmd/podman/containers/mount.go b/cmd/podman/containers/mount.go index f2df5e99e..c4dfb513f 100644 --- a/cmd/podman/containers/mount.go +++ b/cmd/podman/containers/mount.go @@ -6,6 +6,7 @@ import ( "text/tabwriter" "text/template" + "github.com/containers/podman/v2/cmd/podman/parse" "github.com/containers/podman/v2/cmd/podman/registry" "github.com/containers/podman/v2/cmd/podman/utils" "github.com/containers/podman/v2/cmd/podman/validate" @@ -75,9 +76,6 @@ func init() { } func mount(_ *cobra.Command, args []string) error { - var ( - errs utils.OutputErrors - ) if len(args) > 0 && mountOpts.Latest { return errors.Errorf("--latest and containers cannot be used together") } @@ -85,7 +83,9 @@ func mount(_ *cobra.Command, args []string) error { if err != nil { return err } + if len(args) > 0 || mountOpts.Latest || mountOpts.All { + var errs utils.OutputErrors for _, r := range reports { if r.Err == nil { fmt.Println(r.Path) @@ -96,21 +96,21 @@ func mount(_ *cobra.Command, args []string) error { return errs.PrintErrors() } - switch mountOpts.Format { - case "json": + switch { + case parse.MatchesJSONFormat(mountOpts.Format): return printJSON(reports) - case "": - // do nothing + case mountOpts.Format == "": + break // print defaults default: - return errors.Errorf("unknown --format argument: %s", mountOpts.Format) + return errors.Errorf("unknown --format argument: %q", mountOpts.Format) } mrs := make([]mountReporter, 0, len(reports)) for _, r := range reports { mrs = append(mrs, mountReporter{r}) } - row := "{{.ID}} {{.Path}}\n" - format := "{{range . }}" + row + "{{end}}" + + format := "{{range . }}{{.ID}}\t{{.Path}}\n{{end}}" tmpl, err := template.New("mounts").Parse(format) if err != nil { return err @@ -139,6 +139,7 @@ func printJSON(reports []*entities.ContainerMountReport) error { if err != nil { return err } + fmt.Println(string(b)) return nil } diff --git a/cmd/podman/images/history.go b/cmd/podman/images/history.go index 30abf0ada..fa4b368c6 100644 --- a/cmd/podman/images/history.go +++ b/cmd/podman/images/history.go @@ -10,7 +10,9 @@ import ( "time" "unicode" + "github.com/containers/podman/v2/cmd/podman/parse" "github.com/containers/podman/v2/cmd/podman/registry" + "github.com/containers/podman/v2/cmd/podman/report" "github.com/containers/podman/v2/pkg/domain/entities" "github.com/docker/go-units" "github.com/pkg/errors" @@ -28,9 +30,9 @@ var ( Use: "history [flags] IMAGE", Short: "Show history of a specified image", Long: long, - Example: "podman history quay.io/fedora/fedora", Args: cobra.ExactArgs(1), RunE: history, + Example: "podman history quay.io/fedora/fedora", } imageHistoryCmd = &cobra.Command{ @@ -39,7 +41,7 @@ var ( Short: historyCmd.Short, Long: historyCmd.Long, RunE: historyCmd.RunE, - Example: `podman image history imageID`, + Example: `podman image history quay.io/fedora/fedora`, } opts = struct { @@ -79,7 +81,7 @@ func history(cmd *cobra.Command, args []string) error { return err } - if opts.format == "json" { + if parse.MatchesJSONFormat(opts.format) { var err error if len(results.Layers) == 0 { _, err = fmt.Fprintf(os.Stdout, "[]\n") @@ -100,69 +102,66 @@ func history(cmd *cobra.Command, args []string) error { } return err } - hr := make([]historyreporter, 0, len(results.Layers)) + + hr := make([]historyReporter, 0, len(results.Layers)) for _, l := range results.Layers { - hr = append(hr, historyreporter{l}) + hr = append(hr, historyReporter{l}) } + + hdrs := report.Headers(historyReporter{}, map[string]string{ + "CreatedBy": "CREATED BY", + }) + // Defaults - hdr := "ID\tCREATED\tCREATED BY\tSIZE\tCOMMENT\n" row := "{{.ID}}\t{{.Created}}\t{{.CreatedBy}}\t{{.Size}}\t{{.Comment}}\n" - switch { - case len(opts.format) > 0: - hdr = "" - row = opts.format - if !strings.HasSuffix(opts.format, "\n") { - row += "\n" - } + case cmd.Flags().Changed("format"): + row = report.NormalizeFormat(opts.format) case opts.quiet: - hdr = "" row = "{{.ID}}\n" - case opts.human: - row = "{{.ID}}\t{{.Created}}\t{{.CreatedBy}}\t{{.Size}}\t{{.Comment}}\n" - case opts.noTrunc: - row = "{{.ID}}\t{{.Created}}\t{{.CreatedBy}}\t{{.Size}}\t{{.Comment}}\n" } - format := hdr + "{{range . }}" + row + "{{end}}" + format := "{{range . }}" + row + "{{end}}" tmpl, err := template.New("report").Parse(format) if err != nil { return err } w := tabwriter.NewWriter(os.Stdout, 8, 2, 2, ' ', 0) - err = tmpl.Execute(w, hr) - if err != nil { - fmt.Fprintln(os.Stderr, errors.Wrapf(err, "failed to print report")) + defer w.Flush() + + if !opts.quiet && !cmd.Flags().Changed("format") { + if err := tmpl.Execute(w, hdrs); err != nil { + return errors.Wrapf(err, "failed to write report column headers") + } } - w.Flush() - return nil + return tmpl.Execute(w, hr) } -type historyreporter struct { +type historyReporter struct { entities.ImageHistoryLayer } -func (h historyreporter) Created() string { +func (h historyReporter) Created() string { if opts.human { return units.HumanDuration(time.Since(h.ImageHistoryLayer.Created)) + " ago" } return h.ImageHistoryLayer.Created.Format(time.RFC3339) } -func (h historyreporter) Size() string { +func (h historyReporter) Size() string { s := units.HumanSizeWithPrecision(float64(h.ImageHistoryLayer.Size), 3) i := strings.LastIndexFunc(s, unicode.IsNumber) return s[:i+1] + " " + s[i+1:] } -func (h historyreporter) CreatedBy() string { +func (h historyReporter) CreatedBy() string { if len(h.ImageHistoryLayer.CreatedBy) > 45 { return h.ImageHistoryLayer.CreatedBy[:45-3] + "..." } return h.ImageHistoryLayer.CreatedBy } -func (h historyreporter) ID() string { +func (h historyReporter) ID() string { if !opts.noTrunc && len(h.ImageHistoryLayer.ID) >= 12 { return h.ImageHistoryLayer.ID[0:12] } diff --git a/cmd/podman/images/mount.go b/cmd/podman/images/mount.go index fac06e324..0a972ea81 100644 --- a/cmd/podman/images/mount.go +++ b/cmd/podman/images/mount.go @@ -6,6 +6,7 @@ import ( "text/tabwriter" "text/template" + "github.com/containers/podman/v2/cmd/podman/parse" "github.com/containers/podman/v2/cmd/podman/registry" "github.com/containers/podman/v2/cmd/podman/utils" "github.com/containers/podman/v2/pkg/domain/entities" @@ -24,7 +25,7 @@ var ( mountCommand = &cobra.Command{ Use: "mount [flags] [IMAGE...]", - Short: "Mount an images's root filesystem", + Short: "Mount an image's root filesystem", Long: mountDescription, RunE: mount, Example: `podman image mount imgID @@ -56,18 +57,18 @@ func init() { mountFlags(mountCommand.Flags()) } -func mount(_ *cobra.Command, args []string) error { - var ( - errs utils.OutputErrors - ) +func mount(cmd *cobra.Command, args []string) error { if len(args) > 0 && mountOpts.All { return errors.New("when using the --all switch, you may not pass any image names or IDs") } + reports, err := registry.ImageEngine().Mount(registry.GetContext(), args, mountOpts) if err != nil { return err } + if len(args) > 0 || mountOpts.All { + var errs utils.OutputErrors for _, r := range reports { if r.Err == nil { fmt.Println(r.Path) @@ -78,22 +79,22 @@ func mount(_ *cobra.Command, args []string) error { return errs.PrintErrors() } - switch mountOpts.Format { - case "json": + switch { + case parse.MatchesJSONFormat(mountOpts.Format): return printJSON(reports) - case "": - // do nothing + case mountOpts.Format == "": + break // default format default: - return errors.Errorf("unknown --format argument: %s", mountOpts.Format) + return errors.Errorf("unknown --format argument: %q", mountOpts.Format) } mrs := make([]mountReporter, 0, len(reports)) for _, r := range reports { mrs = append(mrs, mountReporter{r}) } - row := "{{.ID}} {{.Path}}\n" - format := "{{range . }}" + row + "{{end}}" - tmpl, err := template.New("mounts").Parse(format) + + row := "{{range . }}{{.ID}}\t{{.Path}}\n{{end}}" + tmpl, err := template.New("mounts").Parse(row) if err != nil { return err } diff --git a/cmd/podman/images/search.go b/cmd/podman/images/search.go index b8d989d65..b8f590585 100644 --- a/cmd/podman/images/search.go +++ b/cmd/podman/images/search.go @@ -2,15 +2,14 @@ package images import ( "os" - "reflect" - "strings" + "text/tabwriter" + "text/template" - "github.com/containers/buildah/pkg/formats" "github.com/containers/common/pkg/auth" "github.com/containers/image/v5/types" "github.com/containers/podman/v2/cmd/podman/registry" + "github.com/containers/podman/v2/cmd/podman/report" "github.com/containers/podman/v2/pkg/domain/entities" - "github.com/containers/podman/v2/pkg/util/camelcase" "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -120,41 +119,29 @@ func imageSearch(cmd *cobra.Command, args []string) error { if err != nil { return err } - - format := genSearchFormat(searchOptions.Format) if len(searchReport) == 0 { return nil } - out := formats.StdoutTemplateArray{Output: searchToGeneric(searchReport), Template: format, Fields: searchHeaderMap()} - return out.Out() -} - -// searchHeaderMap returns the headers of a SearchResult. -func searchHeaderMap() map[string]string { - s := new(entities.ImageSearchReport) - v := reflect.Indirect(reflect.ValueOf(s)) - values := make(map[string]string, v.NumField()) - for i := 0; i < v.NumField(); i++ { - key := v.Type().Field(i).Name - value := key - values[key] = strings.ToUpper(strings.Join(camelcase.Split(value), " ")) + hdrs := report.Headers(entities.ImageSearchReport{}, nil) + row := "{{.Index}}\t{{.Name}}\t{{.Description}}\t{{.Stars}}\t{{.Official}}\t{{.Automated}}\n" + if cmd.Flags().Changed("format") { + row = report.NormalizeFormat(searchOptions.Format) } - return values -} + row = "{{range .}}" + row + "{{end}}" -func genSearchFormat(format string) string { - if format != "" { - // "\t" from the command line is not being recognized as a tab - // replacing the string "\t" to a tab character if the user passes in "\t" - return strings.Replace(format, `\t`, "\t", -1) + tmpl, err := template.New("search").Parse(row) + if err != nil { + return err } - return "table {{.Index}}\t{{.Name}}\t{{.Description}}\t{{.Stars}}\t{{.Official}}\t{{.Automated}}\t" -} + w := tabwriter.NewWriter(os.Stdout, 8, 2, 2, ' ', 0) + defer w.Flush() -func searchToGeneric(params []entities.ImageSearchReport) (genericParams []interface{}) { - for _, v := range params { - genericParams = append(genericParams, interface{}(v)) + if !cmd.Flags().Changed("format") { + if err := tmpl.Execute(w, hdrs); err != nil { + return errors.Wrapf(err, "failed to write search column headers") + } } - return genericParams + + return tmpl.Execute(w, searchReport) } diff --git a/cmd/podman/networks/create.go b/cmd/podman/networks/create.go index 68a577ae1..17f39bd8b 100644 --- a/cmd/podman/networks/create.go +++ b/cmd/podman/networks/create.go @@ -7,7 +7,6 @@ import ( "github.com/containers/podman/v2/cmd/podman/registry" "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/pkg/domain/entities" - "github.com/containers/podman/v2/pkg/network" "github.com/spf13/cobra" "github.com/spf13/pflag" ) @@ -56,9 +55,6 @@ func networkCreate(cmd *cobra.Command, args []string) error { var ( name string ) - if err := network.IsSupportedDriver(networkCreateOptions.Driver); err != nil { - return err - } if len(args) > 0 { if !define.NameRegex.MatchString(args[0]) { return define.RegexError diff --git a/cmd/podman/networks/list.go b/cmd/podman/networks/list.go index b6fb2bb80..c53f50c9f 100644 --- a/cmd/podman/networks/list.go +++ b/cmd/podman/networks/list.go @@ -10,8 +10,8 @@ import ( "github.com/containers/podman/v2/cmd/podman/registry" "github.com/containers/podman/v2/cmd/podman/validate" + "github.com/containers/podman/v2/libpod/network" "github.com/containers/podman/v2/pkg/domain/entities" - "github.com/containers/podman/v2/pkg/network" "github.com/spf13/cobra" "github.com/spf13/pflag" ) diff --git a/pkg/network/config.go b/libpod/network/config.go index 0115433e1..a08e684d8 100644 --- a/pkg/network/config.go +++ b/libpod/network/config.go @@ -3,6 +3,8 @@ package network import ( "encoding/json" "net" + + "github.com/containers/storage/pkg/lockfile" ) // TODO once the containers.conf file stuff is worked out, this should be modified @@ -17,8 +19,17 @@ const ( // DefaultPodmanDomainName is used for the dnsname plugin to define // a localized domain name for a created network DefaultPodmanDomainName = "dns.podman" + // LockFileName is used for obtaining a lock and is appended + // to libpod's tmpdir in practice + LockFileName = "cni.lock" ) +// CNILock is for preventing name collision and +// unpredictable results when doing some CNI operations. +type CNILock struct { + lockfile.Locker +} + // GetDefaultPodmanNetwork outputs the default network for podman func GetDefaultPodmanNetwork() (*net.IPNet, error) { _, n, err := net.ParseCIDR("10.88.1.0/24") diff --git a/libpod/network/create.go b/libpod/network/create.go new file mode 100644 index 000000000..a9ed4c4ef --- /dev/null +++ b/libpod/network/create.go @@ -0,0 +1,195 @@ +package network + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "os" + "path/filepath" + + "github.com/containernetworking/cni/pkg/version" + "github.com/containers/podman/v2/libpod" + "github.com/containers/podman/v2/pkg/domain/entities" + "github.com/containers/podman/v2/pkg/util" + "github.com/pkg/errors" +) + +func Create(name string, options entities.NetworkCreateOptions, r *libpod.Runtime) (*entities.NetworkCreateReport, error) { + var fileName string + if err := isSupportedDriver(options.Driver); err != nil { + return nil, err + } + config, err := r.GetConfig() + if err != nil { + return nil, err + } + // Acquire a lock for CNI + l, err := acquireCNILock(filepath.Join(config.Engine.TmpDir, LockFileName)) + if err != nil { + return nil, err + } + defer l.releaseCNILock() + if len(options.MacVLAN) > 0 { + fileName, err = createMacVLAN(r, name, options) + } else { + fileName, err = createBridge(r, name, options) + } + if err != nil { + return nil, err + } + return &entities.NetworkCreateReport{Filename: fileName}, nil +} + +// createBridge creates a CNI network +func createBridge(r *libpod.Runtime, name string, options entities.NetworkCreateOptions) (string, error) { + isGateway := true + ipMasq := true + subnet := &options.Subnet + ipRange := options.Range + runtimeConfig, err := r.GetConfig() + if err != nil { + return "", err + } + // if range is provided, make sure it is "in" network + if subnet.IP != nil { + // if network is provided, does it conflict with existing CNI or live networks + err = ValidateUserNetworkIsAvailable(runtimeConfig, subnet) + } else { + // if no network is provided, figure out network + subnet, err = GetFreeNetwork(runtimeConfig) + } + if err != nil { + return "", err + } + gateway := options.Gateway + if gateway == nil { + // if no gateway is provided, provide it as first ip of network + gateway = CalcGatewayIP(subnet) + } + // if network is provided and if gateway is provided, make sure it is "in" network + if options.Subnet.IP != nil && options.Gateway != nil { + if !subnet.Contains(gateway) { + return "", errors.Errorf("gateway %s is not in valid for subnet %s", gateway.String(), subnet.String()) + } + } + if options.Internal { + isGateway = false + ipMasq = false + } + + // if a range is given, we need to ensure it is "in" the network range. + if options.Range.IP != nil { + if options.Subnet.IP == nil { + return "", errors.New("you must define a subnet range to define an ip-range") + } + firstIP, err := FirstIPInSubnet(&options.Range) + if err != nil { + return "", err + } + lastIP, err := LastIPInSubnet(&options.Range) + if err != nil { + return "", err + } + if !subnet.Contains(firstIP) || !subnet.Contains(lastIP) { + return "", errors.Errorf("the ip range %s does not fall within the subnet range %s", options.Range.String(), subnet.String()) + } + } + bridgeDeviceName, err := GetFreeDeviceName(runtimeConfig) + if err != nil { + return "", err + } + + if len(name) > 0 { + netNames, err := GetNetworkNamesFromFileSystem(runtimeConfig) + if err != nil { + return "", err + } + if util.StringInSlice(name, netNames) { + return "", errors.Errorf("the network name %s is already used", name) + } + } else { + // If no name is given, we give the name of the bridge device + name = bridgeDeviceName + } + + ncList := NewNcList(name, version.Current()) + var plugins []CNIPlugins + var routes []IPAMRoute + + defaultRoute, err := NewIPAMDefaultRoute(IsIPv6(subnet.IP)) + if err != nil { + return "", err + } + routes = append(routes, defaultRoute) + ipamConfig, err := NewIPAMHostLocalConf(subnet, routes, ipRange, gateway) + if err != nil { + return "", err + } + + // TODO need to iron out the role of isDefaultGW and IPMasq + bridge := NewHostLocalBridge(bridgeDeviceName, isGateway, false, ipMasq, ipamConfig) + plugins = append(plugins, bridge) + plugins = append(plugins, NewPortMapPlugin()) + plugins = append(plugins, NewFirewallPlugin()) + // if we find the dnsname plugin, we add configuration for it + if HasDNSNamePlugin(runtimeConfig.Network.CNIPluginDirs) && !options.DisableDNS { + // Note: in the future we might like to allow for dynamic domain names + plugins = append(plugins, NewDNSNamePlugin(DefaultPodmanDomainName)) + } + ncList["plugins"] = plugins + b, err := json.MarshalIndent(ncList, "", " ") + if err != nil { + return "", err + } + if err := os.MkdirAll(GetCNIConfDir(runtimeConfig), 0755); err != nil { + return "", err + } + cniPathName := filepath.Join(GetCNIConfDir(runtimeConfig), fmt.Sprintf("%s.conflist", name)) + err = ioutil.WriteFile(cniPathName, b, 0644) + return cniPathName, err +} + +func createMacVLAN(r *libpod.Runtime, name string, options entities.NetworkCreateOptions) (string, error) { + var ( + plugins []CNIPlugins + ) + liveNetNames, err := GetLiveNetworkNames() + if err != nil { + return "", err + } + + config, err := r.GetConfig() + if err != nil { + return "", err + } + + // Make sure the host-device exists + if !util.StringInSlice(options.MacVLAN, liveNetNames) { + return "", errors.Errorf("failed to find network interface %q", options.MacVLAN) + } + if len(name) > 0 { + netNames, err := GetNetworkNamesFromFileSystem(config) + if err != nil { + return "", err + } + if util.StringInSlice(name, netNames) { + return "", errors.Errorf("the network name %s is already used", name) + } + } else { + name, err = GetFreeDeviceName(config) + if err != nil { + return "", err + } + } + ncList := NewNcList(name, version.Current()) + macvlan := NewMacVLANPlugin(options.MacVLAN) + plugins = append(plugins, macvlan) + ncList["plugins"] = plugins + b, err := json.MarshalIndent(ncList, "", " ") + if err != nil { + return "", err + } + cniPathName := filepath.Join(GetCNIConfDir(config), fmt.Sprintf("%s.conflist", name)) + err = ioutil.WriteFile(cniPathName, b, 0644) + return cniPathName, err +} diff --git a/pkg/network/devices.go b/libpod/network/devices.go index a5d23fae4..a5d23fae4 100644 --- a/pkg/network/devices.go +++ b/libpod/network/devices.go diff --git a/pkg/network/files.go b/libpod/network/files.go index a2090491f..a2090491f 100644 --- a/pkg/network/files.go +++ b/libpod/network/files.go diff --git a/pkg/network/ip.go b/libpod/network/ip.go index ba93a0d05..ba93a0d05 100644 --- a/pkg/network/ip.go +++ b/libpod/network/ip.go diff --git a/libpod/network/lock.go b/libpod/network/lock.go new file mode 100644 index 000000000..0395359eb --- /dev/null +++ b/libpod/network/lock.go @@ -0,0 +1,26 @@ +package network + +import ( + "github.com/containers/storage" +) + +// acquireCNILock gets a lock that should be used in create and +// delete cases to avoid unwanted collisions in network names. +// TODO this uses a file lock and should be converted to shared memory +// when we have a more general shared memory lock in libpod +func acquireCNILock(lockPath string) (*CNILock, error) { + l, err := storage.GetLockfile(lockPath) + if err != nil { + return nil, err + } + l.Lock() + cnilock := CNILock{ + Locker: l, + } + return &cnilock, nil +} + +// ReleaseCNILock unlocks the previously held lock +func (l *CNILock) releaseCNILock() { + l.Unlock() +} diff --git a/pkg/network/netconflist.go b/libpod/network/netconflist.go index 8187fdb39..8187fdb39 100644 --- a/pkg/network/netconflist.go +++ b/libpod/network/netconflist.go diff --git a/pkg/network/netconflist_test.go b/libpod/network/netconflist_test.go index 5893bf985..5893bf985 100644 --- a/pkg/network/netconflist_test.go +++ b/libpod/network/netconflist_test.go diff --git a/pkg/network/network.go b/libpod/network/network.go index c4c1ff67f..7327a1a7d 100644 --- a/pkg/network/network.go +++ b/libpod/network/network.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net" "os" + "path/filepath" "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/plugins/plugins/ipam/host-local/backend/allocator" @@ -20,8 +21,8 @@ var DefaultNetworkDriver = "bridge" // SupportedNetworkDrivers describes the list of supported drivers var SupportedNetworkDrivers = []string{DefaultNetworkDriver} -// IsSupportedDriver checks if the user provided driver is supported -func IsSupportedDriver(driver string) error { +// isSupportedDriver checks if the user provided driver is supported +func isSupportedDriver(driver string) error { if util.StringInSlice(driver, SupportedNetworkDrivers) { return nil } @@ -168,6 +169,11 @@ func ValidateUserNetworkIsAvailable(config *config.Config, userNet *net.IPNet) e // RemoveNetwork removes a given network by name. If the network has container associated with it, that // must be handled outside the context of this. func RemoveNetwork(config *config.Config, name string) error { + l, err := acquireCNILock(filepath.Join(config.Engine.TmpDir, LockFileName)) + if err != nil { + return err + } + defer l.releaseCNILock() cniPath, err := GetCNIConfigPathByName(config, name) if err != nil { return err diff --git a/pkg/network/network_test.go b/libpod/network/network_test.go index 1969e792c..1969e792c 100644 --- a/pkg/network/network_test.go +++ b/libpod/network/network_test.go diff --git a/pkg/network/subnet.go b/libpod/network/subnet.go index 90f0cdfce..90f0cdfce 100644 --- a/pkg/network/subnet.go +++ b/libpod/network/subnet.go diff --git a/pkg/network/subnet_test.go b/libpod/network/subnet_test.go index 917c3be88..917c3be88 100644 --- a/pkg/network/subnet_test.go +++ b/libpod/network/subnet_test.go diff --git a/libpod/pod_api.go b/libpod/pod_api.go index 0ae180356..f2ddba9c9 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -6,6 +6,7 @@ import ( "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/libpod/events" "github.com/containers/podman/v2/pkg/cgroups" + "github.com/containers/podman/v2/pkg/parallel" "github.com/containers/podman/v2/pkg/rootless" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -99,47 +100,52 @@ func (p *Pod) StopWithTimeout(ctx context.Context, cleanup bool, timeout int) (m return nil, err } - ctrErrors := make(map[string]error) - // TODO: There may be cases where it makes sense to order stops based on // dependencies. Should we bother with this? - // Stop to all containers - for _, ctr := range allCtrs { - ctr.lock.Lock() + ctrErrChan := make(map[string]<-chan error) - if err := ctr.syncContainer(); err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } - - // Ignore containers that are not running - if ctr.state.State != define.ContainerStateRunning { - ctr.lock.Unlock() - continue - } - stopTimeout := ctr.config.StopTimeout - if timeout > -1 { - stopTimeout = uint(timeout) - } - if err := ctr.stop(stopTimeout); err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } + // Enqueue a function for each container with the parallel executor. + for _, ctr := range allCtrs { + c := ctr + logrus.Debugf("Adding parallel job to stop container %s", c.ID()) + retChan := parallel.Enqueue(ctx, func() error { + // TODO: Might be better to batch stop and cleanup + // together? + if timeout > -1 { + if err := c.StopWithTimeout(uint(timeout)); err != nil { + return err + } + } else { + if err := c.Stop(); err != nil { + return err + } + } - if cleanup { - if err := ctr.cleanup(ctx); err != nil { - ctrErrors[ctr.ID()] = err + if cleanup { + return c.Cleanup(ctx) } - } - ctr.lock.Unlock() + return nil + }) + + ctrErrChan[c.ID()] = retChan } p.newPodEvent(events.Stop) + ctrErrors := make(map[string]error) + + // Get returned error for every container we worked on + for id, channel := range ctrErrChan { + if err := <-channel; err != nil { + if errors.Cause(err) == define.ErrCtrStateInvalid || errors.Cause(err) == define.ErrCtrStopped { + continue + } + ctrErrors[id] = err + } + } + if len(ctrErrors) > 0 { return ctrErrors, errors.Wrapf(define.ErrPodPartialFail, "error stopping some containers") } @@ -169,45 +175,29 @@ func (p *Pod) Cleanup(ctx context.Context) (map[string]error, error) { return nil, err } - ctrErrors := make(map[string]error) + ctrErrChan := make(map[string]<-chan error) - // Clean up all containers + // Enqueue a function for each container with the parallel executor. for _, ctr := range allCtrs { - ctr.lock.Lock() - - if err := ctr.syncContainer(); err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } - - // Ignore containers that are running/paused - if !ctr.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateStopped, define.ContainerStateExited) { - ctr.lock.Unlock() - continue - } - - // Check for running exec sessions, ignore containers with them. - sessions, err := ctr.getActiveExecSessions() - if err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } - if len(sessions) > 0 { - ctr.lock.Unlock() - continue - } + c := ctr + logrus.Debugf("Adding parallel job to clean up container %s", c.ID()) + retChan := parallel.Enqueue(ctx, func() error { + return c.Cleanup(ctx) + }) - // TODO: Should we handle restart policy here? + ctrErrChan[c.ID()] = retChan + } - ctr.newContainerEvent(events.Cleanup) + ctrErrors := make(map[string]error) - if err := ctr.cleanup(ctx); err != nil { - ctrErrors[ctr.ID()] = err + // Get returned error for every container we worked on + for id, channel := range ctrErrChan { + if err := <-channel; err != nil { + if errors.Cause(err) == define.ErrCtrStateInvalid || errors.Cause(err) == define.ErrCtrStopped { + continue + } + ctrErrors[id] = err } - - ctr.lock.Unlock() } if len(ctrErrors) > 0 { @@ -229,7 +219,7 @@ func (p *Pod) Cleanup(ctx context.Context) (map[string]error, error) { // containers. The container ID is mapped to the error encountered. The error is // set to ErrPodPartialFail. // If both error and the map are nil, all containers were paused without error -func (p *Pod) Pause() (map[string]error, error) { +func (p *Pod) Pause(ctx context.Context) (map[string]error, error) { p.lock.Lock() defer p.lock.Unlock() @@ -252,37 +242,34 @@ func (p *Pod) Pause() (map[string]error, error) { return nil, err } - ctrErrors := make(map[string]error) + ctrErrChan := make(map[string]<-chan error) - // Pause to all containers + // Enqueue a function for each container with the parallel executor. for _, ctr := range allCtrs { - ctr.lock.Lock() + c := ctr + logrus.Debugf("Adding parallel job to pause container %s", c.ID()) + retChan := parallel.Enqueue(ctx, c.Pause) - if err := ctr.syncContainer(); err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } + ctrErrChan[c.ID()] = retChan + } - // Ignore containers that are not running - if ctr.state.State != define.ContainerStateRunning { - ctr.lock.Unlock() - continue - } + p.newPodEvent(events.Pause) - if err := ctr.pause(); err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } + ctrErrors := make(map[string]error) - ctr.lock.Unlock() + // Get returned error for every container we worked on + for id, channel := range ctrErrChan { + if err := <-channel; err != nil { + if errors.Cause(err) == define.ErrCtrStateInvalid || errors.Cause(err) == define.ErrCtrStopped { + continue + } + ctrErrors[id] = err + } } if len(ctrErrors) > 0 { return ctrErrors, errors.Wrapf(define.ErrPodPartialFail, "error pausing some containers") } - defer p.newPodEvent(events.Pause) return nil, nil } @@ -298,7 +285,7 @@ func (p *Pod) Pause() (map[string]error, error) { // containers. The container ID is mapped to the error encountered. The error is // set to ErrPodPartialFail. // If both error and the map are nil, all containers were unpaused without error. -func (p *Pod) Unpause() (map[string]error, error) { +func (p *Pod) Unpause(ctx context.Context) (map[string]error, error) { p.lock.Lock() defer p.lock.Unlock() @@ -311,38 +298,34 @@ func (p *Pod) Unpause() (map[string]error, error) { return nil, err } - ctrErrors := make(map[string]error) + ctrErrChan := make(map[string]<-chan error) - // Pause to all containers + // Enqueue a function for each container with the parallel executor. for _, ctr := range allCtrs { - ctr.lock.Lock() + c := ctr + logrus.Debugf("Adding parallel job to unpause container %s", c.ID()) + retChan := parallel.Enqueue(ctx, c.Unpause) - if err := ctr.syncContainer(); err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } + ctrErrChan[c.ID()] = retChan + } - // Ignore containers that are not paused - if ctr.state.State != define.ContainerStatePaused { - ctr.lock.Unlock() - continue - } + p.newPodEvent(events.Unpause) - if err := ctr.unpause(); err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } + ctrErrors := make(map[string]error) - ctr.lock.Unlock() + // Get returned error for every container we worked on + for id, channel := range ctrErrChan { + if err := <-channel; err != nil { + if errors.Cause(err) == define.ErrCtrStateInvalid || errors.Cause(err) == define.ErrCtrStopped { + continue + } + ctrErrors[id] = err + } } if len(ctrErrors) > 0 { return ctrErrors, errors.Wrapf(define.ErrPodPartialFail, "error unpausing some containers") } - - defer p.newPodEvent(events.Unpause) return nil, nil } @@ -411,7 +394,7 @@ func (p *Pod) Restart(ctx context.Context) (map[string]error, error) { // containers. The container ID is mapped to the error encountered. The error is // set to ErrPodPartialFail. // If both error and the map are nil, all containers were signalled successfully. -func (p *Pod) Kill(signal uint) (map[string]error, error) { +func (p *Pod) Kill(ctx context.Context, signal uint) (map[string]error, error) { p.lock.Lock() defer p.lock.Unlock() @@ -424,44 +407,36 @@ func (p *Pod) Kill(signal uint) (map[string]error, error) { return nil, err } - ctrErrors := make(map[string]error) + ctrErrChan := make(map[string]<-chan error) - // Send a signal to all containers + // Enqueue a function for each container with the parallel executor. for _, ctr := range allCtrs { - ctr.lock.Lock() - - if err := ctr.syncContainer(); err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } + c := ctr + logrus.Debugf("Adding parallel job to kill container %s", c.ID()) + retChan := parallel.Enqueue(ctx, func() error { + return c.Kill(signal) + }) - // Ignore containers that are not running - if ctr.state.State != define.ContainerStateRunning { - ctr.lock.Unlock() - continue - } + ctrErrChan[c.ID()] = retChan + } - if err := ctr.ociRuntime.KillContainer(ctr, signal, false); err != nil { - ctr.lock.Unlock() - ctrErrors[ctr.ID()] = err - continue - } + p.newPodEvent(events.Kill) - logrus.Debugf("Killed container %s with signal %d", ctr.ID(), signal) + ctrErrors := make(map[string]error) - ctr.state.StoppedByUser = true - if err := ctr.save(); err != nil { - ctrErrors[ctr.ID()] = err + // Get returned error for every container we worked on + for id, channel := range ctrErrChan { + if err := <-channel; err != nil { + if errors.Cause(err) == define.ErrCtrStateInvalid || errors.Cause(err) == define.ErrCtrStopped { + continue + } + ctrErrors[id] = err } - - ctr.lock.Unlock() } if len(ctrErrors) > 0 { return ctrErrors, errors.Wrapf(define.ErrPodPartialFail, "error killing some containers") } - defer p.newPodEvent(events.Kill) return nil, nil } diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index c5387b1e9..a46784a6c 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -12,10 +12,10 @@ import ( "github.com/containernetworking/cni/libcni" "github.com/containers/podman/v2/libpod" "github.com/containers/podman/v2/libpod/define" + "github.com/containers/podman/v2/libpod/network" "github.com/containers/podman/v2/pkg/api/handlers/utils" "github.com/containers/podman/v2/pkg/domain/entities" "github.com/containers/podman/v2/pkg/domain/infra/abi" - "github.com/containers/podman/v2/pkg/network" "github.com/docker/docker/api/types" dockerNetwork "github.com/docker/docker/api/types/network" "github.com/gorilla/schema" @@ -210,6 +210,7 @@ func ListNetworks(w http.ResponseWriter, r *http.Request) { report, err := getNetworkResourceByName(name, runtime) if err != nil { utils.InternalServerError(w, err) + return } reports = append(reports, report) } @@ -267,9 +268,9 @@ func CreateNetwork(w http.ResponseWriter, r *http.Request) { } } ce := abi.ContainerEngine{Libpod: runtime} - _, err := ce.NetworkCreate(r.Context(), name, ncOptions) - if err != nil { + if _, err := ce.NetworkCreate(r.Context(), name, ncOptions); err != nil { utils.InternalServerError(w, err) + return } report := types.NetworkCreate{ CheckDuplicate: networkCreate.CheckDuplicate, @@ -307,6 +308,7 @@ func RemoveNetwork(w http.ResponseWriter, r *http.Request) { } if err := network.RemoveNetwork(config, name); err != nil { utils.InternalServerError(w, err) + return } utils.WriteResponse(w, http.StatusNoContent, "") } diff --git a/pkg/api/handlers/libpod/pods.go b/pkg/api/handlers/libpod/pods.go index 3aa554171..5422411cf 100644 --- a/pkg/api/handlers/libpod/pods.go +++ b/pkg/api/handlers/libpod/pods.go @@ -270,7 +270,7 @@ func PodPause(w http.ResponseWriter, r *http.Request) { utils.PodNotFound(w, name, err) return } - responses, err := pod.Pause() + responses, err := pod.Pause(r.Context()) if err != nil && errors.Cause(err) != define.ErrPodPartialFail { utils.Error(w, "Something went wrong", http.StatusInternalServerError, err) return @@ -294,7 +294,7 @@ func PodUnpause(w http.ResponseWriter, r *http.Request) { utils.PodNotFound(w, name, err) return } - responses, err := pod.Unpause() + responses, err := pod.Unpause(r.Context()) if err != nil && errors.Cause(err) != define.ErrPodPartialFail { utils.Error(w, "failed to pause pod", http.StatusInternalServerError, err) return @@ -402,7 +402,7 @@ func PodKill(w http.ResponseWriter, r *http.Request) { return } - responses, err := pod.Kill(uint(sig)) + responses, err := pod.Kill(r.Context(), uint(sig)) if err != nil && errors.Cause(err) != define.ErrPodPartialFail { utils.Error(w, "failed to kill pod", http.StatusInternalServerError, err) return diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index d92911e0c..0107e18c4 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -23,7 +23,7 @@ import ( "github.com/containers/podman/v2/pkg/checkpoint" "github.com/containers/podman/v2/pkg/domain/entities" "github.com/containers/podman/v2/pkg/domain/infra/abi/terminal" - "github.com/containers/podman/v2/pkg/parallel" + parallelctr "github.com/containers/podman/v2/pkg/parallel/ctr" "github.com/containers/podman/v2/pkg/ps" "github.com/containers/podman/v2/pkg/rootless" "github.com/containers/podman/v2/pkg/signal" @@ -157,7 +157,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.ContainerOp(ctx, ctrs, func(c *libpod.Container) error { + errMap, err := parallelctr.ContainerOp(ctx, ctrs, func(c *libpod.Container) error { var err error if options.Timeout != nil { err = c.StopWithTimeout(*options.Timeout) @@ -321,7 +321,7 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, return reports, nil } - errMap, err := parallel.ContainerOp(ctx, ctrs, func(c *libpod.Container) error { + errMap, err := parallelctr.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/network.go b/pkg/domain/infra/abi/network.go index 5acfea853..f40df828a 100644 --- a/pkg/domain/infra/abi/network.go +++ b/pkg/domain/infra/abi/network.go @@ -2,19 +2,13 @@ package abi import ( "context" - "encoding/json" "fmt" - "io/ioutil" - "os" - "path/filepath" "strings" "github.com/containernetworking/cni/libcni" - cniversion "github.com/containernetworking/cni/pkg/version" - "github.com/containers/podman/v2/libpod" "github.com/containers/podman/v2/libpod/define" + "github.com/containers/podman/v2/libpod/network" "github.com/containers/podman/v2/pkg/domain/entities" - "github.com/containers/podman/v2/pkg/network" "github.com/containers/podman/v2/pkg/util" "github.com/pkg/errors" ) @@ -111,173 +105,7 @@ func (ic *ContainerEngine) NetworkRm(ctx context.Context, namesOrIds []string, o } func (ic *ContainerEngine) NetworkCreate(ctx context.Context, name string, options entities.NetworkCreateOptions) (*entities.NetworkCreateReport, error) { - var ( - err error - fileName string - ) - if len(options.MacVLAN) > 0 { - fileName, err = createMacVLAN(ic.Libpod, name, options) - } else { - fileName, err = createBridge(ic.Libpod, name, options) - } - if err != nil { - return nil, err - } - return &entities.NetworkCreateReport{Filename: fileName}, nil -} - -// createBridge creates a CNI network -func createBridge(r *libpod.Runtime, name string, options entities.NetworkCreateOptions) (string, error) { - isGateway := true - ipMasq := true - subnet := &options.Subnet - ipRange := options.Range - runtimeConfig, err := r.GetConfig() - if err != nil { - return "", err - } - // if range is provided, make sure it is "in" network - if subnet.IP != nil { - // if network is provided, does it conflict with existing CNI or live networks - err = network.ValidateUserNetworkIsAvailable(runtimeConfig, subnet) - } else { - // if no network is provided, figure out network - subnet, err = network.GetFreeNetwork(runtimeConfig) - } - if err != nil { - return "", err - } - gateway := options.Gateway - if gateway == nil { - // if no gateway is provided, provide it as first ip of network - gateway = network.CalcGatewayIP(subnet) - } - // if network is provided and if gateway is provided, make sure it is "in" network - if options.Subnet.IP != nil && options.Gateway != nil { - if !subnet.Contains(gateway) { - return "", errors.Errorf("gateway %s is not in valid for subnet %s", gateway.String(), subnet.String()) - } - } - if options.Internal { - isGateway = false - ipMasq = false - } - - // if a range is given, we need to ensure it is "in" the network range. - if options.Range.IP != nil { - if options.Subnet.IP == nil { - return "", errors.New("you must define a subnet range to define an ip-range") - } - firstIP, err := network.FirstIPInSubnet(&options.Range) - if err != nil { - return "", err - } - lastIP, err := network.LastIPInSubnet(&options.Range) - if err != nil { - return "", err - } - if !subnet.Contains(firstIP) || !subnet.Contains(lastIP) { - return "", errors.Errorf("the ip range %s does not fall within the subnet range %s", options.Range.String(), subnet.String()) - } - } - bridgeDeviceName, err := network.GetFreeDeviceName(runtimeConfig) - if err != nil { - return "", err - } - - if len(name) > 0 { - netNames, err := network.GetNetworkNamesFromFileSystem(runtimeConfig) - if err != nil { - return "", err - } - if util.StringInSlice(name, netNames) { - return "", errors.Errorf("the network name %s is already used", name) - } - } else { - // If no name is given, we give the name of the bridge device - name = bridgeDeviceName - } - - ncList := network.NewNcList(name, cniversion.Current()) - var plugins []network.CNIPlugins - var routes []network.IPAMRoute - - defaultRoute, err := network.NewIPAMDefaultRoute(network.IsIPv6(subnet.IP)) - if err != nil { - return "", err - } - routes = append(routes, defaultRoute) - ipamConfig, err := network.NewIPAMHostLocalConf(subnet, routes, ipRange, gateway) - if err != nil { - return "", err - } - - // TODO need to iron out the role of isDefaultGW and IPMasq - bridge := network.NewHostLocalBridge(bridgeDeviceName, isGateway, false, ipMasq, ipamConfig) - plugins = append(plugins, bridge) - plugins = append(plugins, network.NewPortMapPlugin()) - plugins = append(plugins, network.NewFirewallPlugin()) - // if we find the dnsname plugin, we add configuration for it - if network.HasDNSNamePlugin(runtimeConfig.Network.CNIPluginDirs) && !options.DisableDNS { - // Note: in the future we might like to allow for dynamic domain names - plugins = append(plugins, network.NewDNSNamePlugin(network.DefaultPodmanDomainName)) - } - ncList["plugins"] = plugins - b, err := json.MarshalIndent(ncList, "", " ") - if err != nil { - return "", err - } - if err := os.MkdirAll(network.GetCNIConfDir(runtimeConfig), 0755); err != nil { - return "", err - } - cniPathName := filepath.Join(network.GetCNIConfDir(runtimeConfig), fmt.Sprintf("%s.conflist", name)) - err = ioutil.WriteFile(cniPathName, b, 0644) - return cniPathName, err -} - -func createMacVLAN(r *libpod.Runtime, name string, options entities.NetworkCreateOptions) (string, error) { - var ( - plugins []network.CNIPlugins - ) - liveNetNames, err := network.GetLiveNetworkNames() - if err != nil { - return "", err - } - - config, err := r.GetConfig() - if err != nil { - return "", err - } - - // Make sure the host-device exists - if !util.StringInSlice(options.MacVLAN, liveNetNames) { - return "", errors.Errorf("failed to find network interface %q", options.MacVLAN) - } - if len(name) > 0 { - netNames, err := network.GetNetworkNamesFromFileSystem(config) - if err != nil { - return "", err - } - if util.StringInSlice(name, netNames) { - return "", errors.Errorf("the network name %s is already used", name) - } - } else { - name, err = network.GetFreeDeviceName(config) - if err != nil { - return "", err - } - } - ncList := network.NewNcList(name, cniversion.Current()) - macvlan := network.NewMacVLANPlugin(options.MacVLAN) - plugins = append(plugins, macvlan) - ncList["plugins"] = plugins - b, err := json.MarshalIndent(ncList, "", " ") - if err != nil { - return "", err - } - cniPathName := filepath.Join(network.GetCNIConfDir(config), fmt.Sprintf("%s.conflist", name)) - err = ioutil.WriteFile(cniPathName, b, 0644) - return cniPathName, err + return network.Create(name, options, ic.Libpod) } func ifPassesFilterTest(netconf *libcni.NetworkConfigList, filter []string) bool { diff --git a/pkg/domain/infra/abi/pods.go b/pkg/domain/infra/abi/pods.go index 747da9fd4..258640a81 100644 --- a/pkg/domain/infra/abi/pods.go +++ b/pkg/domain/infra/abi/pods.go @@ -66,7 +66,7 @@ func (ic *ContainerEngine) PodKill(ctx context.Context, namesOrIds []string, opt for _, p := range pods { report := entities.PodKillReport{Id: p.ID()} - conErrs, err := p.Kill(uint(sig)) + conErrs, err := p.Kill(ctx, uint(sig)) if err != nil && errors.Cause(err) != define.ErrPodPartialFail { report.Errs = []error{err} reports = append(reports, &report) @@ -92,7 +92,7 @@ func (ic *ContainerEngine) PodPause(ctx context.Context, namesOrIds []string, op } for _, p := range pods { report := entities.PodPauseReport{Id: p.ID()} - errs, err := p.Pause() + errs, err := p.Pause(ctx) if err != nil && errors.Cause(err) != define.ErrPodPartialFail { report.Errs = []error{err} continue @@ -117,7 +117,7 @@ func (ic *ContainerEngine) PodUnpause(ctx context.Context, namesOrIds []string, } for _, p := range pods { report := entities.PodUnpauseReport{Id: p.ID()} - errs, err := p.Unpause() + errs, err := p.Unpause(ctx) if err != nil && errors.Cause(err) != define.ErrPodPartialFail { report.Errs = []error{err} continue diff --git a/pkg/parallel/parallel_linux.go b/pkg/parallel/ctr/ctr.go index 442db1502..e8c1292b8 100644 --- a/pkg/parallel/parallel_linux.go +++ b/pkg/parallel/ctr/ctr.go @@ -1,11 +1,10 @@ -package parallel +package ctr import ( "context" - "sync" "github.com/containers/podman/v2/libpod" - "github.com/pkg/errors" + "github.com/containers/podman/v2/pkg/parallel" "github.com/sirupsen/logrus" ) @@ -14,44 +13,28 @@ import ( // 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 ContainerOp(ctx context.Context, ctrs []*libpod.Container, applyFunc func(*libpod.Container) error) (map[*libpod.Container]error, error) { - jobControlLock.RLock() - defer jobControlLock.RUnlock() - // We could use a sync.Map but given Go's lack of generic I'd rather // just use a lock on a normal map... // The expectation is that most of the time is spent in applyFunc // anyways. var ( - errMap = make(map[*libpod.Container]error) - errLock sync.Mutex - allDone sync.WaitGroup + errMap = make(map[*libpod.Container]<-chan error) ) for _, ctr := range ctrs { - // Block until a thread is available - if err := jobControl.Acquire(ctx, 1); err != nil { - return nil, errors.Wrapf(err, "error acquiring job control semaphore") - } - - allDone.Add(1) - c := ctr - go func() { - logrus.Debugf("Launching job on container %s", c.ID()) - - err := applyFunc(c) - errLock.Lock() - errMap[c] = err - errLock.Unlock() - - allDone.Done() - jobControl.Release(1) - }() + logrus.Debugf("Starting parallel job on container %s", c.ID()) + errChan := parallel.Enqueue(ctx, func() error { + return applyFunc(c) + }) + errMap[c] = errChan } - allDone.Wait() + finalErr := make(map[*libpod.Container]error) + for ctr, errChan := range errMap { + err := <-errChan + finalErr[ctr] = err + } - return errMap, nil + return finalErr, nil } - -// TODO: Add an Enqueue() function that returns a promise diff --git a/pkg/parallel/parallel.go b/pkg/parallel/parallel.go index c9e4da50d..4da7e0f89 100644 --- a/pkg/parallel/parallel.go +++ b/pkg/parallel/parallel.go @@ -1,6 +1,7 @@ package parallel import ( + "context" "sync" "github.com/pkg/errors" @@ -42,3 +43,32 @@ func SetMaxThreads(threads uint) error { func GetMaxThreads() uint { return numThreads } + +// Enqueue adds a single function to the parallel jobs queue. This function will +// be run when an unused thread is available. +// Returns a receive-only error channel that will return the error (if any) from +// the provided function fn when fn has finished executing. The channel will be +// closed after this. +func Enqueue(ctx context.Context, fn func() error) <-chan error { + retChan := make(chan error) + + go func() { + jobControlLock.RLock() + defer jobControlLock.RUnlock() + + defer close(retChan) + + if err := jobControl.Acquire(ctx, 1); err != nil { + retChan <- errors.Wrapf(err, "error acquiring job control semaphore") + return + } + + err := fn() + + jobControl.Release(1) + + retChan <- err + }() + + return retChan +} diff --git a/pkg/varlinkapi/pods.go b/pkg/varlinkapi/pods.go index 189434780..6d03afb7a 100644 --- a/pkg/varlinkapi/pods.go +++ b/pkg/varlinkapi/pods.go @@ -3,6 +3,7 @@ package varlinkapi import ( + "context" "encoding/json" "fmt" "strconv" @@ -207,7 +208,7 @@ func (i *VarlinkAPI) KillPod(call iopodman.VarlinkCall, name string, signal int6 if err != nil { return call.ReplyPodNotFound(name, err.Error()) } - ctrErrs, err := pod.Kill(killSignal) + ctrErrs, err := pod.Kill(context.TODO(), killSignal) callErr := handlePodCall(call, pod, ctrErrs, err) if callErr != nil { return err @@ -221,7 +222,7 @@ func (i *VarlinkAPI) PausePod(call iopodman.VarlinkCall, name string) error { if err != nil { return call.ReplyPodNotFound(name, err.Error()) } - ctrErrs, err := pod.Pause() + ctrErrs, err := pod.Pause(context.TODO()) callErr := handlePodCall(call, pod, ctrErrs, err) if callErr != nil { return err @@ -235,7 +236,7 @@ func (i *VarlinkAPI) UnpausePod(call iopodman.VarlinkCall, name string) error { if err != nil { return call.ReplyPodNotFound(name, err.Error()) } - ctrErrs, err := pod.Unpause() + ctrErrs, err := pod.Unpause(context.TODO()) callErr := handlePodCall(call, pod, ctrErrs, err) if callErr != nil { return err diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index c663a4dca..ec910109b 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -453,7 +453,7 @@ func (p *PodmanTestIntegration) PodmanPID(args []string) (*PodmanSessionIntegrat func (p *PodmanTestIntegration) Cleanup() { // Remove all containers stopall := p.Podman([]string{"stop", "-a", "--time", "0"}) - stopall.Wait(90) + stopall.WaitWithDefaultTimeout() podstop := p.Podman([]string{"pod", "stop", "-a", "-t", "0"}) podstop.WaitWithDefaultTimeout() @@ -461,7 +461,7 @@ func (p *PodmanTestIntegration) Cleanup() { podrm.WaitWithDefaultTimeout() session := p.Podman([]string{"rm", "-fa"}) - session.Wait(90) + session.WaitWithDefaultTimeout() p.StopRemoteService() // Nuke tempdir diff --git a/test/e2e/network_create_test.go b/test/e2e/network_create_test.go index e476b5ae8..21f03901b 100644 --- a/test/e2e/network_create_test.go +++ b/test/e2e/network_create_test.go @@ -8,7 +8,7 @@ import ( "strings" cniversion "github.com/containernetworking/cni/pkg/version" - "github.com/containers/podman/v2/pkg/network" + "github.com/containers/podman/v2/libpod/network" . "github.com/containers/podman/v2/test/utils" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" diff --git a/test/e2e/search_test.go b/test/e2e/search_test.go index 497949bbc..043da9059 100644 --- a/test/e2e/search_test.go +++ b/test/e2e/search_test.go @@ -237,7 +237,7 @@ registries = ['{{.Host}}:{{.Port}}']` }) It("podman search attempts HTTP if registry is in registries.insecure and force secure is false", func() { - SkipIfRemote("--tls-verify is not supportedon podman-remote search") + SkipIfRemote("--tls-verify is not supported on podman-remote search") if podmanTest.Host.Arch == "ppc64le" { Skip("No registry image for ppc64le") } @@ -278,7 +278,7 @@ registries = ['{{.Host}}:{{.Port}}']` }) It("podman search doesn't attempt HTTP if force secure is true", func() { - SkipIfRemote("--tls-verify is not supportedon podman-remote search") + SkipIfRemote("--tls-verify is not supported on podman-remote search") if podmanTest.Host.Arch == "ppc64le" { Skip("No registry image for ppc64le") } @@ -317,7 +317,7 @@ registries = ['{{.Host}}:{{.Port}}']` }) It("podman search doesn't attempt HTTP if registry is not listed as insecure", func() { - SkipIfRemote("--tls-verify is not supportedon podman-remote search") + SkipIfRemote("--tls-verify is not supported on podman-remote search") if podmanTest.Host.Arch == "ppc64le" { Skip("No registry image for ppc64le") } diff --git a/test/system/060-mount.bats b/test/system/060-mount.bats index 75c88e4ad..f11aff773 100644 --- a/test/system/060-mount.bats +++ b/test/system/060-mount.bats @@ -56,7 +56,7 @@ load helpers # 'image mount', no args, tells us what's mounted run_podman image mount - is "$output" "$IMAGE $mount_path" "podman image mount with no args" + is "$output" "$IMAGE *$mount_path" "podman image mount with no args" # Clean up run_podman image umount $IMAGE |