Skip to content

Commit

Permalink
use cached containers.conf
Browse files Browse the repository at this point in the history
Use `Default()` instead of re-loading containers.conf.

Also rework how the containers.conf objects are handled for parsing the
CLI.  Previously, we were conflating "loading the defaults" with
"storing values from the CLI" with "libpod may further change fields"
which ultimately led to various bugs and test failues.

To address the issue, separate the defaults from the values from the CLI
and properly name the fields to make the semantics less ambiguous.

[NO NEW TESTS NEEDED] as it's not a functional change.

Fixes: containers/common/issues/1200
Signed-off-by: Valentin Rothberg <[email protected]>
  • Loading branch information
vrothberg committed Oct 21, 2022
1 parent a77ac5b commit 4e29ce2
Show file tree
Hide file tree
Showing 15 changed files with 122 additions and 126 deletions.
4 changes: 2 additions & 2 deletions cmd/podman/common/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (

const sizeWithUnitFormat = "(format: `<number>[<unit>]`, where unit = b (bytes), k (kibibytes), m (mebibytes), or g (gibibytes))"

var containerConfig = registry.PodmanConfig()
var podmanConfig = registry.PodmanConfig()

// ContainerToPodOptions takes the Container and Pod Create options, assigning the matching values back to podCreate for the purpose of the libpod API
// For this function to succeed, the JSON tags in PodCreateOptions and ContainerCreateOptions need to match due to the Marshaling and Unmarshaling done.
Expand Down Expand Up @@ -218,7 +218,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,

createFlags.BoolVar(
&cf.HTTPProxy,
"http-proxy", containerConfig.Containers.HTTPProxy,
"http-proxy", podmanConfig.ContainersConfDefaultsRO.Containers.HTTPProxy,
"Set proxy environment variables in the container based on the host proxy vars",
)

Expand Down
28 changes: 14 additions & 14 deletions cmd/podman/common/create_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,70 +8,70 @@ import (

func ulimits() []string {
if !registry.IsRemote() {
return containerConfig.Ulimits()
return podmanConfig.ContainersConfDefaultsRO.Ulimits()
}
return nil
}

func cgroupConfig() string {
if !registry.IsRemote() {
return containerConfig.Cgroups()
return podmanConfig.ContainersConfDefaultsRO.Cgroups()
}
return ""
}

func devices() []string {
if !registry.IsRemote() {
return containerConfig.Devices()
return podmanConfig.ContainersConfDefaultsRO.Devices()
}
return nil
}

func Env() []string {
if !registry.IsRemote() {
return containerConfig.Env()
return podmanConfig.ContainersConfDefaultsRO.Env()
}
return nil
}

func initPath() string {
if !registry.IsRemote() {
return containerConfig.InitPath()
return podmanConfig.ContainersConfDefaultsRO.InitPath()
}
return ""
}

func pidsLimit() int64 {
if !registry.IsRemote() {
return containerConfig.PidsLimit()
return podmanConfig.ContainersConfDefaultsRO.PidsLimit()
}
return -1
}

func policy() string {
if !registry.IsRemote() {
return containerConfig.Engine.PullPolicy
return podmanConfig.ContainersConfDefaultsRO.Engine.PullPolicy
}
return ""
}

func shmSize() string {
if !registry.IsRemote() {
return containerConfig.ShmSize()
return podmanConfig.ContainersConfDefaultsRO.ShmSize()
}
return ""
}

func volumes() []string {
if !registry.IsRemote() {
return containerConfig.Volumes()
return podmanConfig.ContainersConfDefaultsRO.Volumes()
}
return nil
}

func LogDriver() string {
if !registry.IsRemote() {
return containerConfig.Containers.LogDriver
return podmanConfig.ContainersConfDefaultsRO.Containers.LogDriver
}
return ""
}
Expand All @@ -81,14 +81,14 @@ func DefineCreateDefaults(opts *entities.ContainerCreateOptions) {
opts.LogDriver = LogDriver()
opts.CgroupsMode = cgroupConfig()
opts.MemorySwappiness = -1
opts.ImageVolume = containerConfig.Engine.ImageVolumeMode
opts.ImageVolume = podmanConfig.ContainersConfDefaultsRO.Engine.ImageVolumeMode
opts.Pull = policy()
opts.ReadOnlyTmpFS = true
opts.SdNotifyMode = define.SdNotifyModeContainer
opts.StopTimeout = containerConfig.Engine.StopTimeout
opts.StopTimeout = podmanConfig.ContainersConfDefaultsRO.Engine.StopTimeout
opts.Systemd = "true"
opts.Timezone = containerConfig.TZ()
opts.Umask = containerConfig.Umask()
opts.Timezone = podmanConfig.ContainersConfDefaultsRO.TZ()
opts.Umask = podmanConfig.ContainersConfDefaultsRO.Umask()
opts.Ulimit = ulimits()
opts.SeccompPolicy = "default"
opts.Volume = volumes()
Expand Down
8 changes: 4 additions & 4 deletions cmd/podman/common/netflags.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,20 @@ func DefineNetFlags(cmd *cobra.Command) {

dnsFlagName := "dns"
netFlags.StringSlice(
dnsFlagName, containerConfig.DNSServers(),
dnsFlagName, podmanConfig.ContainersConf.DNSServers(),
"Set custom DNS servers",
)
_ = cmd.RegisterFlagCompletionFunc(dnsFlagName, completion.AutocompleteNone)

dnsOptFlagName := "dns-option"
netFlags.StringSlice(
dnsOptFlagName, containerConfig.DNSOptions(),
dnsOptFlagName, podmanConfig.ContainersConf.DNSOptions(),
"Set custom DNS options",
)
_ = cmd.RegisterFlagCompletionFunc(dnsOptFlagName, completion.AutocompleteNone)
dnsSearchFlagName := "dns-search"
netFlags.StringSlice(
dnsSearchFlagName, containerConfig.DNSSearches(),
dnsSearchFlagName, podmanConfig.ContainersConf.DNSSearches(),
"Set custom DNS search domains",
)
_ = cmd.RegisterFlagCompletionFunc(dnsSearchFlagName, completion.AutocompleteNone)
Expand Down Expand Up @@ -89,7 +89,7 @@ func DefineNetFlags(cmd *cobra.Command) {
_ = cmd.RegisterFlagCompletionFunc(publishFlagName, completion.AutocompleteNone)

netFlags.Bool(
"no-hosts", containerConfig.Containers.NoHosts,
"no-hosts", podmanConfig.ContainersConfDefaultsRO.Containers.NoHosts,
"Do not create /etc/hosts within the container, instead use the version from the image",
)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/podman/containers/ps.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ func checkFlags(c *cobra.Command) error {
if listOpts.Watch > 0 && listOpts.Latest {
return errors.New("the watch and latest flags cannot be used together")
}
cfg := registry.PodmanConfig()
if cfg.Engine.Namespace != "" {
podmanConfig := registry.PodmanConfig()
if podmanConfig.ContainersConf.Engine.Namespace != "" {
if c.Flag("storage").Changed && listOpts.External {
return errors.New("--namespace and --external flags can not both be set")
}
Expand Down
10 changes: 5 additions & 5 deletions cmd/podman/images/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,11 +504,11 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
runtimeFlags = append(runtimeFlags, "--"+arg)
}

containerConfig := registry.PodmanConfig()
for _, arg := range containerConfig.RuntimeFlags {
podmanConfig := registry.PodmanConfig()
for _, arg := range podmanConfig.RuntimeFlags {
runtimeFlags = append(runtimeFlags, "--"+arg)
}
if containerConfig.Engine.CgroupManager == config.SystemdCgroupsManager {
if podmanConfig.ContainersConf.Engine.CgroupManager == config.SystemdCgroupsManager {
runtimeFlags = append(runtimeFlags, "--systemd-cgroup")
}

Expand Down Expand Up @@ -576,7 +576,7 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
ConfigureNetwork: networkPolicy,
ContextDirectory: contextDir,
CPPFlags: flags.CPPFlags,
DefaultMountsFilePath: containerConfig.Containers.DefaultMountsFile,
DefaultMountsFilePath: podmanConfig.ContainersConfDefaultsRO.Containers.DefaultMountsFile,
Devices: flags.Devices,
DropCapabilities: flags.CapDrop,
Envs: flags.Envs,
Expand Down Expand Up @@ -608,7 +608,7 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
Quiet: flags.Quiet,
RemoveIntermediateCtrs: flags.Rm,
ReportWriter: reporter,
Runtime: containerConfig.RuntimePath,
Runtime: podmanConfig.RuntimePath,
RuntimeArgs: runtimeFlags,
RusageLogFile: flags.RusageLogFile,
SignBy: flags.SignBy,
Expand Down
2 changes: 1 addition & 1 deletion cmd/podman/images/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func saveFlags(cmd *cobra.Command) {
_ = cmd.RegisterFlagCompletionFunc(outputFlagName, completion.AutocompleteDefault)

flags.BoolVarP(&saveOpts.Quiet, "quiet", "q", false, "Suppress the output")
flags.BoolVarP(&saveOpts.MultiImageArchive, "multi-image-archive", "m", containerConfig.Engine.MultiImageArchive, "Interpret additional arguments as images not tags and create a multi-image-archive (only for docker-archive)")
flags.BoolVarP(&saveOpts.MultiImageArchive, "multi-image-archive", "m", containerConfig.ContainersConfDefaultsRO.Engine.MultiImageArchive, "Interpret additional arguments as images not tags and create a multi-image-archive (only for docker-archive)")

if !registry.IsRemote() {
flags.StringVar(&saveOpts.SignaturePolicy, "signature-policy", "", "Path to a signature-policy file")
Expand Down
14 changes: 7 additions & 7 deletions cmd/podman/machine/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ func init() {
cpusFlagName := "cpus"
flags.Uint64Var(
&initOpts.CPUS,
cpusFlagName, cfg.Machine.CPUs,
cpusFlagName, cfg.ContainersConfDefaultsRO.Machine.CPUs,
"Number of CPUs",
)
_ = initCmd.RegisterFlagCompletionFunc(cpusFlagName, completion.AutocompleteNone)

diskSizeFlagName := "disk-size"
flags.Uint64Var(
&initOpts.DiskSize,
diskSizeFlagName, cfg.Machine.DiskSize,
diskSizeFlagName, cfg.ContainersConfDefaultsRO.Machine.DiskSize,
"Disk size in GB",
)

Expand All @@ -63,7 +63,7 @@ func init() {
memoryFlagName := "memory"
flags.Uint64VarP(
&initOpts.Memory,
memoryFlagName, "m", cfg.Machine.Memory,
memoryFlagName, "m", cfg.ContainersConfDefaultsRO.Machine.Memory,
"Memory in MB",
)
_ = initCmd.RegisterFlagCompletionFunc(memoryFlagName, completion.AutocompleteNone)
Expand All @@ -74,7 +74,7 @@ func init() {
"Start machine now",
)
timezoneFlagName := "timezone"
defaultTz := cfg.TZ()
defaultTz := cfg.ContainersConfDefaultsRO.TZ()
if len(defaultTz) < 1 {
defaultTz = "local"
}
Expand All @@ -89,15 +89,15 @@ func init() {
_ = flags.MarkHidden("reexec")

UsernameFlagName := "username"
flags.StringVar(&initOpts.Username, UsernameFlagName, cfg.Machine.User, "Username used in qcow image")
flags.StringVar(&initOpts.Username, UsernameFlagName, cfg.ContainersConfDefaultsRO.Machine.User, "Username used in qcow image")
_ = initCmd.RegisterFlagCompletionFunc(UsernameFlagName, completion.AutocompleteDefault)

ImagePathFlagName := "image-path"
flags.StringVar(&initOpts.ImagePath, ImagePathFlagName, cfg.Machine.Image, "Path to qcow image")
flags.StringVar(&initOpts.ImagePath, ImagePathFlagName, cfg.ContainersConfDefaultsRO.Machine.Image, "Path to qcow image")
_ = initCmd.RegisterFlagCompletionFunc(ImagePathFlagName, completion.AutocompleteDefault)

VolumeFlagName := "volume"
flags.StringArrayVarP(&initOpts.Volumes, VolumeFlagName, "v", cfg.Machine.Volumes, "Volumes to mount, source:target")
flags.StringArrayVarP(&initOpts.Volumes, VolumeFlagName, "v", cfg.ContainersConfDefaultsRO.Machine.Volumes, "Volumes to mount, source:target")
_ = initCmd.RegisterFlagCompletionFunc(VolumeFlagName, completion.AutocompleteDefault)

VolumeDriverFlagName := "volume-driver"
Expand Down
6 changes: 3 additions & 3 deletions cmd/podman/registry/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func newPodmanConfig() {
os.Exit(1)
}

cfg, err := config.NewConfig("")
defaultConfig, err := config.Default()
if err != nil {
fmt.Fprint(os.Stderr, "Failed to obtain podman configuration: "+err.Error())
os.Exit(1)
Expand All @@ -76,11 +76,11 @@ func newPodmanConfig() {

// If EngineMode==Tunnel has not been set on the command line or environment
// but has been set in containers.conf...
if mode == entities.ABIMode && cfg.Engine.Remote {
if mode == entities.ABIMode && defaultConfig.Engine.Remote {
mode = entities.TunnelMode
}

podmanOptions = entities.PodmanConfig{Config: cfg, EngineMode: mode}
podmanOptions = entities.PodmanConfig{ContainersConf: &config.Config{}, ContainersConfDefaultsRO: defaultConfig, EngineMode: mode}
}

// setXdgDirs ensures the XDG_RUNTIME_DIR env and XDG_CONFIG_HOME variables are set.
Expand Down
Loading

0 comments on commit 4e29ce2

Please sign in to comment.