Commit Graph

148 Commits

Author SHA1 Message Date
Christophe Fergeau 8e56a5605f machine: Fix check which is always true
Before making / mutable/immutable, podman-machine checks if the mount is
being done in /home or /mnt. However the current check is always going
to be true:
```
!strings.HasPrefix(mount.Target, "/home") || !strings.HasPrefix(mount.Target, "/mnt")
```
is false when mount.Target starts with "/home" and mount.Target starts
with "/mnt", which cannot happen at the same time.

The correct check is:
```
!strings.HasPrefix(mount.Target, "/home") && !strings.HasPrefix(mount.Target, "/mnt")
```
which can also be written as:
```
!(strings.HasPrefix(mount.Target, "/home") || strings.HasPrefix(mount.Target, "/mnt"))
```

The impact is not too bad, it results in extra 'chattr -i' calls which
should be unneeded.

[NO NEW TESTS NEEDED]

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
2022-07-26 09:12:27 +02:00
Paul Holzinger 3ce0709f37
podman machine: do not commit proxies into config file
qemu fails when the same `fw_cfg` options is used more than once.
Since the current logic always adds a new option on each machine load
this will fail on the second start.

We can fix this by checking if the option is already set and replace but
I think it is easier to just not commit the option in the config and add
it dynamically on start. User that hit this bug have to recreate the
machine.

[NO NEW TESTS NEEDED]

Fixes #14636
Fixes #14837

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-07-11 17:40:37 +02:00
Paul Holzinger 61a67a07b1
pkg/machine/qemu: start VM check if qemu is alive
When trying to connect to the qemu ready socket we should check if the
qemu process is still running, if it is not we can just error out. There
is no point in retrying.
To do so we have to directly call wait with WNOHANG.

Also change StartProcess to os/exec package which is higher level and
allows us to use a buffer as qemu stderr fd.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-07-07 20:57:26 +02:00
Paul Holzinger a26cf638e0
machine: qemu fix chardev id starting with letter
qemu need the id to start with a letter for some reason.
If this is not the case qemu will fail:
```
qemu-system-x86_64: -device virtserialport,chardev=ad053e0bb519f_ready,name=org.fedoraproject.port.0: Property 'virtserialport.chardev' can't find value 'ad053e0bb519f_ready'
er
Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.
```

To fix this we just add an "a" in front of it.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-07-07 20:48:28 +02:00
openshift-ci[bot] d481fbe759
Merge pull request #14803 from bugfood/volumes
make 9p security model configurable; document
2022-07-07 18:21:55 +00:00
openshift-ci[bot] dd0418a5fe
Merge pull request #14762 from ashley-cui/machinfo
Podman machine info
2022-07-07 15:17:40 +00:00
Corey Hickey 03ee8204d3 podman machine: make 9p security model configurable; adjust docs
This addresses:
Symlinks don't work on podman machine on macOS Monterey when using volumes feature #13784

This change does NOT exactly fix the bug, but it does allow the user to
work around it via 'podman init' option, e.g.:
podman machine init -v "$HOME/git:$HOME/git:ro:security_model=none"

If the default security model were to be changed to 'none', then that
would fix the bug, at the possible cost of breaking any use cases that
depend on 'mapped-xattr'.

The documentation of the purpose and behavior of the different security
models seems to be rather light:
https://wiki.qemu.org/Documentation/9psetup#Starting_the_Guest_directly

From testing, it appears that the mapped-xattr security model intends to
manage symlinks such that the guest can see the symlinks but the host
only sees regular files (with extended attributes). As far as I can
tell, this behavior only makes sense when the guest is the only thing
that ever needs to create and read symlinks. Otherwise, symlinks created
on the host are unusable on the guest, and vice versa.

As per the original commit: 8e7eeaa4dd
[NO NEW TESTS NEEDED]

Also document existing ro and rw options.

Also remove misleading statement about /mnt. By my observation, this
line is incorrect. If the intended meaning is different, then I don't
understand.

The default volume is mounted read/write and is not within /mnt.

[core@localhost ~]$ mount | grep 9p
vol0 on /Users/chickey type 9p (rw,relatime,sync,dirsync,access=client,trans=virtio)

Signed-off-by: Corey Hickey <chickey@tagged.com>
2022-07-06 16:07:56 -07:00
Shane Smith a5898129cf
Fix qemu machine startHostNetworking always failing
Issue introduced in #14828

[NO NEW TESTS NEEDED]

Signed-off-by: Shane Smith <shane.smith@shopify.com>
2022-07-06 11:00:47 -04:00
Ashley Cui 9d6efb3442 Podman machine info
Add podman machine info command, which displays infor about the machine
host as well as version info.

Signed-off-by: Ashley Cui <acui@redhat.com>
2022-07-05 15:18:41 -04:00
Sascha Grunert 251d91699d
libpod: switch to golang native error wrapping
We now use the golang error wrapping format specifier `%w` instead of
the deprecated github.com/pkg/errors package.

[NO NEW TESTS NEEDED]

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
2022-07-05 16:06:32 +02:00
openshift-ci[bot] 1fb1cab21c
Merge pull request #14824 from shanesmith/silence-machine-ssh-locale-warning
Silence setlocale warnings from `podman machine ssh`
2022-07-05 13:29:38 +00:00
Shane Smith 8601ab6b06
Silence setlocale warnings from `podman machine ssh`
Connecting with `podman machine ssh` can results in the following
warning:

```
/usr/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_CA.UTF-8)
/usr/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_CA.UTF-8)
/usr/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_CA.UTF-8)
/usr/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_CA.UTF-8)
```

Best would probably be to remove `LC_ALL` (and other locale and lang
env vars) from `/etc/ssh/sshd_config.d/50-redhat.conf` in the CoreOS
image, but I'm not terribly sure how, so this is a quick alternative.

[NO NEW TESTS NEEDED]

Signed-off-by: Shane Smith <shane.smith@shopify.com>
2022-07-04 12:23:46 -04:00
Paul Holzinger 33a474286b
pkg/machine: add missing build tags to tests
Machine can only run on amd64 and arm64 platforms so we need to make
sure the test are only run on those platforms. We do not have CI checks
for this but it fails in debian build infra since debian supports many
other architectures as well.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-07-04 18:09:30 +02:00
openshift-ci[bot] 01beba3667
Merge pull request #14704 from baude/machinestopped
reveal machine error, ignore false state
2022-06-30 17:58:28 +00:00
openshift-ci[bot] d7121b000f
Merge pull request #14666 from shanesmith/machine-pidfile
Make `podman machine stop` wait for qemu to exit
2022-06-29 08:51:26 +00:00
Shane Smith 59a7ac210b
Make `podman machine stop` wait for qemu to exit
- New `VMPidFilePath` field in MachineVM config holds the path for the
  qemu PID file

- qemu is now started with the `-pidfile` argument set to `VMPidFilePath`

- Machines created before this won't have the VM PID file configured,
  stopping these VMs will revert back to waiting on the state to change
  away from `Running`, plus an added 2s sleep to give time for the VM to
  exit and to avoid potential issues

- Machines created after this will have a VM PID file configured and
  stopping the machine will wait indefinitely for the VM to exit

[NO NEW TESTS NEEDED]

Signed-off-by: Shane Smith <shane.smith@shopify.com>
2022-06-28 13:28:38 -04:00
Brent Baude 99f68898c0 reveal machine error, ignore false state
This PR covers two edge cases discovered by fiddling with machine
manually.  It is possible (like after a manual cleanup of a machine)
that a leftover qemu socket file can indicate the prescense of a machine
running.

Also, reveal the error of a Exec.Command by wrapping the generic error
around what was in stderr.

[NO NEW TESTS NEEDED]

Signed-off-by: Brent Baude <bbaude@redhat.com>
2022-06-27 12:40:15 -05:00
Daniel J Walsh 386ea49cf5
Show starting state when machine is starting
Currently podman machine list never shows the starting state.

Fixes: https://github.com/containers/podman/issues/14738

[NO NEW TESTS NEEDED]

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2022-06-27 09:27:46 -04:00
Erik Sjölund aa4279ae15 Fix spelling "setup" -> "set up" and similar
* Replace "setup", "lookup", "cleanup", "backup" with
  "set up", "look up", "clean up", "back up"
  when used as verbs. Replace also variations of those.

* Improve language in a few places.

Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>
2022-06-22 18:39:21 +02:00
Shane Smith e69691c277
Fix interrupting machine start leaves the machine unstartable
Interrupting a `podman machine start` (ex: with CTRL-C) would leave
`Starting: true` in the machine's config file. Due to #14469 any
subsequent starts would fail since Podman would think the machine is
still in the process of starting.

Fixed here by listening for the interrupt signal and setting `Starting:
false` in the event.

[NO NEW TESTS NEEDED]

Signed-off-by: Shane Smith <shane.smith@shopify.com>
2022-06-15 16:19:10 -04:00
openshift-ci[bot] e084f0ee1e
Merge pull request #14585 from Luap99/nolint
golangci-lint: enable nolintlint
2022-06-14 18:58:53 +00:00
openshift-ci[bot] a22e270ef6
Merge pull request #14324 from anjannath/qemu-test
[macos: podman-machine] look for firmware (edk2-code-fd) based on the path of qemu binary
2022-06-14 18:02:27 +00:00
Paul Holzinger 41528739ce
golangci-lint: enable nolintlint
The nolintlint linter does not deny the use of `//nolint`
Instead it allows us to enforce a common nolint style:
- force that a linter name must be specified
- do not add a space between `//` and `nolint`
- make sure nolint is only used when there is actually a problem

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-06-14 16:29:42 +02:00
openshift-ci[bot] 9fac1b335f
Merge pull request #14563 from ashley-cui/qemu
Fix M1 QEMU flags
2022-06-13 19:11:34 +00:00
Ashley Cui 8d3e6577ae Fix M1 QEMU flags
When calling QEMU, the CPU arch should be host, and highmem should be on, or else the VM start fails.

[NO NEW TESTS NEEDED]

Signed-off-by: Ashley Cui <acui@redhat.com>
2022-06-10 09:21:04 -04:00
OpenShift Merge Robot f808907d85
Merge pull request #14469 from shanesmith/prevent-simultaneous-machine-starts
Prevent simultaneous machine starts
2022-06-09 16:23:25 -04:00
Shane Smith 81153ffa21
Introduce 'Starting' status for machines
- The State() function now returns machine.Starting status instead of an
  empty string if the VM is in the process of starting.

- The `CheckExclusiveActiveVM()` function returns `true` to prevent
  starting a VM while another is in the process of starting.

- `podman machine ls` displays "Currently starting" under "Last Up" for
  the starting VM

- `podman machine ls` supports `{{.Starting}}` boolean field in the format

- `podman machine inspect` displays "starting" in the "State" field for
  the starting VM

Signed-off-by: Shane Smith <shane.smith@shopify.com>
2022-06-09 12:42:43 -04:00
Shane Smith 87b05b6a6f
Prevent simultaneous machine starts
Running `podman machine start` twice at the same time in different
terminals, for example, will make the second invocation fail and the
first one hang.

[NO NEW TESTS NEEDED]

Signed-off-by: Shane Smith <shane.smith@shopify.com>
2022-06-03 10:26:12 -04:00
Shane Smith b8de285a42
Stop machine before force removing files
In #13466 the ability to force remove a machine while it's running was
added but it did not first stop the machine, all files get deleted but
the qemu VM would essentially be orphaned.

[NO NEW TESTS NEEDED]

Signed-off-by: Shane Smith <shane.smith@shopify.com>
2022-06-02 16:58:43 -04:00
OpenShift Merge Robot 2958aee083
Merge pull request #14417 from Luap99/machine-ssh
podman machine ssh: set correct exit code
2022-06-02 04:24:01 -04:00
Brent Baude 8291b51ceb expose podman.sock in machine inspect
For consumers of the podman.sock who want a predictable way to find the
podman sock, we now include it under 'ConnectionConfig' in podman
machine inspect.

Fixes: #14231

Signed-off-by: Brent Baude <bbaude@redhat.com>
2022-06-01 10:48:17 -05:00
Paul Holzinger 4a83465511
podman machine ssh: do not print warning everytime
Currenlty this ssh warning is printed everytime:
`Warning: Permanently added '[localhost]:33915' (ED25519) to the list of known hosts.`

Since this is very anoying and makes it harder to capture the actual
command output we should silence this. With log level error we will only
see the important messages from ssh.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-05-30 16:21:11 +02:00
Daniel J Walsh 429b1f7685
Fix codespell errors
[NO NEW TESTS NEEDED]

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2022-05-25 05:56:29 -04:00
Anjan Nath 8fbb933f5a look for firmware based on the path of qemu binary
this allows users to use a qemu installation that is
not in the default /usr/local/bin location

a user can configure engine.helper_binaries_dir key
or update PATH to include the installation location
to find the qemu binary

[NO NEW TESTS NEEDED]

Signed-off-by: Anjan Nath <kaludios@gmail.com>
2022-05-23 22:04:52 +05:30
OpenShift Merge Robot 5d5cb402cb
Merge pull request #14129 from Juneezee/test/t.TempDir
test: use `T.TempDir` to create temporary test directory
2022-05-06 04:58:25 -04:00
Eng Zer Jun cf35168f0a
test: use `T.TempDir` to create temporary test directory
This commit replaces `ioutil.TempDir` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.

Prior to this commit, temporary directory created using `ioutil.TempDir`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
	defer func() {
		if err := os.RemoveAll(dir); err != nil {
			t.Fatal(err)
		}
	}
is also tedious, but `t.TempDir` handles this for us nicely.

Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
2022-05-05 21:09:41 +08:00
OpenShift Merge Robot ad93318370
Merge pull request #14066 from ashley-cui/sysres
podman system reset removed machines incorrectly
2022-05-04 13:20:09 -04:00
Ashley Cui 80744c6441 podman system reset removed machines incorrectly
podman system reset did not clean up machines fully, leaving some config
files, and breaking machines. Now it removes all machines files fully.

Signed-off-by: Ashley Cui <acui@redhat.com>
2022-05-04 10:31:42 -04:00
Jhon Honce 88015cf0d8 Implement --format for machine inspect
* Fix issue of nil pointer derefence

Signed-off-by: Jhon Honce <jhonce@redhat.com>
2022-05-03 16:15:59 -07:00
Paul Holzinger 69c479b16e
enable errcheck linter
The errcheck linter makes sure that errors are always check and not
ignored by accident. It spotted a lot of unchecked errors, mostly in the
tests but also some real problem in the code.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-04-29 14:06:38 +02:00
OpenShift Merge Robot 765c8818e4
Merge pull request #14033 from baude/inspectredo
Refactor machine inspect
2022-04-28 16:21:33 -04:00
Brent Baude 2902d32c49 Refactor machine inspect
I was asked to refactor machine inspect output to represent more common
and basic information.  machine inspect now has information that would
be appropriate for different machines.

[NO NEW TESTS NEEDED]

Signed-off-by: Brent Baude <bbaude@redhat.com>
2022-04-28 13:32:21 -05:00
OpenShift Merge Robot b2725024f8
Merge pull request #14024 from cdoern/machine
podman machine starting test
2022-04-28 13:59:23 -04:00
cdoern c721acf082 podman machine starting test
add a test to make sure machines are not running while still starting
in order to do this, I added a parameter to `run()` to delineate whether
or not the command should block or not. The non blocking run allows for tests
to get and use the `machineSession` pointer and check the exit code to see if it has finished.

also fix a bug (created by #13996) that before started, the machines would
always say "LastUp" and "Created" Less than one second ago

Signed-off-by: cdoern <cbdoer23@g.holycross.edu>
2022-04-27 20:12:43 -04:00
Ed Santiago 3b8fa515f8 Emergency fix for new CI linter
Signed-off-by: Ed Santiago <santiago@redhat.com>
2022-04-27 16:15:23 -06:00
OpenShift Merge Robot 60d6cc8e1e
Merge pull request #13953 from ashley-cui/mach
Allow changing of CPUs, Memory, and Disk Size
2022-04-27 16:02:57 -04:00
Ashley Cui e7390f30b9 Allow changing of CPUs, Memory, and Disk Size
Allow podman machine set to change CPUs, Memory and Disk size of a QEMU machine after its been created.
Disk size can only be increased.

If one setting fails to be changed, the other settings will still be applied.

Signed-off-by: Ashley Cui <acui@redhat.com>
2022-04-27 13:56:14 -04:00
Paul Holzinger 51fbf3da9e
enable gocritic linter
The linter ensures a common code style.
- use switch/case instead of else if
- use if instead of switch/case for single case statement
- add space between comment and text
- detect the use of defer with os.Exit()
- use short form var += "..." instead of var = var + "..."
- detect problems with append()
```
newSlice := append(orgSlice, val)
```
  This could lead to nasty bugs because the orgSlice will be changed in
  place if it has enough capacity too hold the new elements. Thus we
  newSlice might not be a copy.

Of course most of the changes are just cosmetic and do not cause any
logic errors but I think it is a good idea to enforce a common style.
This should help maintainability.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-04-26 18:12:22 +02:00
OpenShift Merge Robot e7c30d855f
Merge pull request #13996 from cdoern/machine
machine starting status
2022-04-26 09:08:31 -04:00
cdoern d441a711e5 machine starting status
podman machine was using the file modification time to get the running status
add three new config entries Starting (bool) Created (time) LastUp (time) to actually
keep track of when these events happened. This means we can use the config file
to actually store this data and not mess up the created/last-up time.

This fixes the issues where the machine would report running 15 seconds before it was up.
Also fixes the issue of modifying the file manually and saying the machine is "up"

[NO NEW TESTS NEEDED]

resolves #13711

Signed-off-by: cdoern <cbdoer23@g.holycross.edu>
2022-04-25 14:14:45 -04:00