Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: add omitzero for numeric types #795

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Oct 1, 2021

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

@openshift-ci openshift-ci bot added the approved label Oct 1, 2021
@Luap99
Copy link
Member Author

Luap99 commented Oct 1, 2021

@baude @ashley-cui @mheon @rhatdan @vrothberg PTAL
The podman machine config file changes are broken, we need to get this backported to podman 3.4 ASAP

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 1, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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>
@Luap99
Copy link
Member Author

Luap99 commented Oct 1, 2021

I just added a test to prevent regressions.

@ashley-cui
Copy link
Member

LGTM, good catch, thanks!!

@mheon
Copy link
Member

mheon commented Oct 1, 2021

/lgtm
/hold

Ack on backport to 3.4, guess we need to start planning a 3.4.1.

@Luap99
Copy link
Member Author

Luap99 commented Oct 1, 2021

/hold cancel

@openshift-merge-robot openshift-merge-robot merged commit 0882829 into containers:main Oct 1, 2021
@Luap99 Luap99 deleted the omitzero branch October 1, 2021 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman machine start fails due to zero machine config
5 participants