Skip to content

Commit

Permalink
config: add omitzero for numeric types
Browse files Browse the repository at this point in the history
When toml writes the config file it does not use `omitempty` for numeric
values instead it requires `omitzero`. [1]

The problem is that without this change, `config.Write()` writes
```
[machine]
  cpus = 0
  disk_size = 0
  memory = 0
```
to the user file. Because podman machine system connection add code will
do this the config file is broken afterwards. The first vm will be created
successfully but after this every other vm will be broken because the
cpu, memory and disk size are set to zero.

[1] BurntSushi/toml#81

Fixes containers/podman#11824

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
  • Loading branch information
Luap99 committed Oct 1, 2021
1 parent a288e2a commit 2f22f3d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 27 deletions.
38 changes: 11 additions & 27 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ type ContainersConfig struct {
// will be truncated. It can be expressed as a human-friendly string
// that is parsed to bytes.
// Negative values indicate that the log file won't be truncated.
LogSizeMax int64 `toml:"log_size_max,omitempty"`
LogSizeMax int64 `toml:"log_size_max,omitempty,omitzero"`

// Specifies default format tag for container log messages.
// This is useful for creating a specific tag for container log messages.
Expand All @@ -155,7 +155,7 @@ type ContainersConfig struct {

// PidsLimit is the number of processes each container is restricted to
// by the cgroup process number controller.
PidsLimit int64 `toml:"pids_limit,omitempty"`
PidsLimit int64 `toml:"pids_limit,omitempty,omitzero"`

// PidNS indicates how to create a pid namespace for the container
PidNS string `toml:"pidns,omitempty"`
Expand Down Expand Up @@ -192,7 +192,7 @@ type ContainersConfig struct {
UserNS string `toml:"userns,omitempty"`

// UserNSSize how many UIDs to allocate for automatically created UserNS
UserNSSize int `toml:"userns_size,omitempty"`
UserNSSize int `toml:"userns_size,omitempty,omitzero"`
}

// EngineConfig contains configuration options used to set up a engine runtime
Expand Down Expand Up @@ -259,7 +259,7 @@ type EngineConfig struct {
// ImageParallelCopies indicates the maximum number of image layers
// to be copied simultaneously. If this is zero, container engines
// will fall back to containers/image defaults.
ImageParallelCopies uint `toml:"image_parallel_copies,omitempty"`
ImageParallelCopies uint `toml:"image_parallel_copies,omitempty,omitzero"`

// ImageDefaultFormat specified the manifest Type (oci, v2s2, or v2s1)
// to use when pulling, pushing, building container images. By default
Expand Down Expand Up @@ -308,7 +308,7 @@ type EngineConfig struct {

// NumLocks is the number of locks to make available for containers and
// pods.
NumLocks uint32 `toml:"num_locks,omitempty"`
NumLocks uint32 `toml:"num_locks,omitempty,omitzero"`

// OCIRuntime is the OCI runtime to use.
OCIRuntime string `toml:"runtime,omitempty"`
Expand Down Expand Up @@ -380,15 +380,15 @@ type EngineConfig struct {

// ServiceTimeout is the number of seconds to wait without a connection
// before the `podman system service` times out and exits
ServiceTimeout uint `toml:"service_timeout,omitempty"`
ServiceTimeout uint `toml:"service_timeout,omitempty,omitzero"`

// StaticDir is the path to a persistent directory to store container
// files.
StaticDir string `toml:"static_dir,omitempty"`

// StopTimeout is the number of seconds to wait for container to exit
// before sending kill signal.
StopTimeout uint `toml:"stop_timeout,omitempty"`
StopTimeout uint `toml:"stop_timeout,omitempty,omitzero"`

// ImageCopyTmpDir is the default location for storing temporary
// container image content, Can be overridden with the TMPDIR
Expand All @@ -413,7 +413,7 @@ type EngineConfig struct {

// ChownCopiedFiles tells the container engine whether to chown files copied
// into a container to the container's primary uid/gid.
ChownCopiedFiles bool `toml:"chown_copied_files"`
ChownCopiedFiles bool `toml:"chown_copied_files,omitempty"`
}

// SetOptions contains a subset of options in a Config. It's used to indicate if
Expand Down Expand Up @@ -492,13 +492,13 @@ type SecretConfig struct {
// MachineConfig represents the "machine" TOML config table
type MachineConfig struct {
// Number of CPU's a machine is created with.
CPUs uint64 `toml:"cpus,omitempty"`
CPUs uint64 `toml:"cpus,omitempty,omitzero"`
// DiskSize is the size of the disk in GB created when init-ing a podman-machine VM
DiskSize uint64 `toml:"disk_size,omitempty"`
DiskSize uint64 `toml:"disk_size,omitempty,omitzero"`
// MachineImage is the image used when init-ing a podman-machine VM
Image string `toml:"image,omitempty"`
// Memory in MB a machine is created with.
Memory uint64 `toml:"memory,omitempty"`
Memory uint64 `toml:"memory,omitempty,omitzero"`
}

// Destination represents destination for remote service
Expand Down Expand Up @@ -1067,17 +1067,6 @@ func ReadCustomConfig() (*Config, error) {
if err != nil {
return nil, err
}
// hack since Ommitempty does not seem to work with Write
c, err := Default()
if err != nil {
if os.IsNotExist(errors.Cause(err)) {
c, err = DefaultConfig()
}
if err != nil {
return nil, err
}
}

newConfig := &Config{}
if _, err := os.Stat(path); err == nil {
if err := readConfigFromFile(path, newConfig); err != nil {
Expand All @@ -1088,11 +1077,6 @@ func ReadCustomConfig() (*Config, error) {
return nil, err
}
}
newConfig.Containers.LogSizeMax = c.Containers.LogSizeMax
newConfig.Containers.PidsLimit = c.Containers.PidsLimit
newConfig.Containers.UserNSSize = c.Containers.UserNSSize
newConfig.Engine.NumLocks = c.Engine.NumLocks
newConfig.Engine.StopTimeout = c.Engine.StopTimeout
return newConfig, nil
}

Expand Down
11 changes: 11 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,17 @@ image_copy_tmp_dir="storage"`
err = cfg.Write()
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())

// test that we do not write zero values to the file
path, err := customConfigFile()
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
f, err := os.Open(path)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
data, err := ioutil.ReadAll(f)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
gomega.Expect(string(data)).ShouldNot(gomega.ContainSubstring("cpus"))
gomega.Expect(string(data)).ShouldNot(gomega.ContainSubstring("disk_size"))
gomega.Expect(string(data)).ShouldNot(gomega.ContainSubstring("memory"))

cfg, err = ReadCustomConfig()
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())

Expand Down

0 comments on commit 2f22f3d

Please sign in to comment.