Some tests in sandbox.rs need root user to run, because they need create
directories under /run/agent directories, actually this is a limit
that shouldn't be there. By using a temp directory for test containers
will not need run tests as root user.
Fixes: #3122
Signed-off-by: bin <bin@hyper.sh>
Kata agent logs unknown system calls given by seccomp profiles
in advance before the log file descriptor closes.
Fixes: #2957
Signed-off-by: Manabu Sugimoto <Manabu.Sugimoto@sony.com>
Replace some `unwrap()` and `expect()` calls with code to return the
error to the caller.
Fixes: #3011.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Improved the `verify_cid()` function that validates container ID's by
removing the need for an `unwrap()`.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Change `baremount()` to accept `Path` values rather than string values
since:
- `Path` is more natural given the function deals with paths.
- This minimises the caller having to convert between string and `Path`
types, which simplifies the surrounding code.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
The unrecognized option: 'deny-warnings' args caused `make optimize` failed.
Fixed the Makefile of the agent project, make sure the `make optimize` command
execute correctly. This PR modify the rustc args from '--deny-warnings' to
'--deny warnings'.
Fixes: #3104
Signed-off-by: wangyongchao.bj <wangyongchao.bj@inspur.com>
Function parameters in test config is not used. This
commit will add under score before variable name
in test config.
Fixes: #3091
Signed-off-by: bin <bin@hyper.sh>
We noticed s390x test failures on several of the watcher unit tests.
Discovered that on s390 in particular, if we update a file in quick
sucecssion, the time stampe on the file would not be unique between the
writes. Through testing, we observe that a 20 millisecond delay is very
reliable for being able to observe the timestamp update. Let's ensure we
have this delay between writes for our tests so our tests are more
reliable.
In "the real world" we'll be polling for changes every 2 seconds, and
frequency of filesystem updates will be on order of minutes and days,
rather that microseconds.
Fixes: #2946
Signed-off-by: Eric Ernst <eric_ernst@apple.com>
The k8s SR-IOV plugin, when it assigns a VFIO device to a container, adds
an variable of the form PCIDEVICE_<identifier> to the container's
environment, so that the payload knows which device is which. The contents
of the variable gives the PCI address of the device to use.
Kata allows VFIO devices to be passed in to a Kata container, however it
runs within a VM which has a different PCI topology. In order for the
payload to find the right device, the environment variables therefore need
to be converted to list the guest PCI addresses instead of the host PCI
addresses.
fixes#2897
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
update_spec_devices() takes a bunch of updates for the device entries in
the OCI spec and applies them, adjusting things in both the linux.devices
and linux.resources.devices sections of the spec.
It's important that each entry in the spec only be updated once. Currently
we ensure this by first creating an index of where the entries are, then
consulting that as we apply each update, so that earlier updates don't
cause us to incorrectly detect an entry as being relevant to a later
update. This method works, but it's quite awkward.
This inverts the loop structure in update_spec_devices() to make this
clearer. Instead of stepping through each update and finding the relevant
entries in the spec to change, we step through each entry in the spec and
find the relevant update. This makes it structurally clear that we're only
updating each entry once.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
We have a test case commented as testing the case where linux.devices is
empty in the OCI spec. While it's true that linux.devices is empth in this
example, the reason it fails isn't specifically because it's empty but
because it doesn't contain a device for the update we're trying to apply.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
update_spec_devices() explicitly checks for being called with an empty
container path and fails. We have a unit test to verify this behaviour.
But while an empty container_path probably does mean something has gone
wrong elsewhere, that's also true of any number of other bad paths. Having
an empty string here doesn't prevent what we're doing in this function
making sense - we can compare it to the strings in the OCI spec perfectly
well (though more likely we simply won't find it there).
So, there's no real reason to check this one particular odd case.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The DevIndex data structure keeps track of devices in the OCI
specification. We used to carry it around to quite a lot of
functions, but it's now used only within update_spec_devices(). That
means we can simplify things a bit by just open coding the maps we
need, rather than declaring a special type.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
update_spec_device() adjusts the OCI spec for device differences
between the host and guest. It is called repeatedly for each device
we need to alter. These calls are now all in a single loop in
add_devices(), so it makes more sense to move the loop into a renamed
update_spec_devices() and process all the fixups in one call.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Currently the DevNumUpdate structure is created with a path to a
device node in the VM, which is then used by update_spec_device().
However the only piece of information that update_spec_device()
actually needs is the VM side major and minor numbers for the device.
We can determine those when we create the DevNumUpdate structure.
This means we detect errors earlier and as a bonus we don't need to
make a copy of the vm path string.
Since that change requires updating 2 of the log statements, we take the
opportunity to update all the log statements to structured style.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
For each device in the OCI spec we need to update it to reflect the guest
rather than the host. We do this with additional device information
provided by the runtime. There should only be one update for each device
though, if there are multiple, something has gone horribly wrong.
Detect and report this situation, for safety.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
As we process container devices in the agent, we repeatedly call
update_spec_device() to adjust the OCI spec as necessary for differences
between the host and the VM. This means that for the whole of a pretty
complex call graph, the spec is in a partially-updated state - neither
fully as it was on the host, not fully as it will be for the container
within the VM.
Worse, it's not discernable from the contents itself which parts of the
spec have already been updated and which have not. We used to have real
bugs because of this, until the DevIndex structure was introduced, but that
means a whole, fairly complex, parallel data structure needs to be passed
around this call graph just to keep track of the state we're in.
Start simplifying this by having the device handler functions not directly
update the spec, but instead return an update structure describing the
change they need. Once all the devices are added, add_devices() will
process all the updates as a batch.
Note that collecting the updates in a HashMap, rather than a simple Vec
doesn't make a lot of sense in the current code, but will reduce churn
in future changes which make use of it.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Currently update_spec_device() takes parameters 'vm_path' and 'final_path'
to give it the information it needs to update a single device in the OCI
spec for the guest. This bundles these parameters into a single structure
type describing the updates to a single device. This doesn't accomplish
much immediately, but will allow a number of further cleanups.
At the same time we change the representation of vm_path from a Unicode
string to a std::path::Path, which is a bit more natural since we are
performing file operations on it.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
virtio_blk_device_handler(), virtio_blk_ccw_device_handler() and
virtio_scsi_device_handler() all take a clone of their 'device' parameter.
They appear to do this in order to get a mutable copy in which they can
update the vm_path field.
However, the copy is dropped at the end of the function, so the only thing
that's used in it is the vm_path field passed to update_spec_device()
afterwards.
We can avoid the clone by just using a local variable for the vm_path.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
update_spec_device() takes a 'final_path' parameter which gives the
name the device should be given in the "inner" OCI spec. We need this
for VFIO devices where the name the payload sees needs to match the
VM's IOMMU groups. However, in all other cases (for now, and maybe
forever), this is the same as the original 'container_path' given in
the input OCI spec. To make this clearer and simplify callers, make
this parameter an Option, and only update the device name if it is
non-None.
Additionally, update_spec_device() needs to call to_string() on
update_path to get an owned version. Rust convention[0] is to let the
caller decide whether it should copy, or just give an existing owned
version to the function. Change from &str to String to allow that; it
doesn't buy us anything right now, but will make some things a little
nicer in future.
[0] https://rust-lang.github.io/api-guidelines/flexibility.html?highlight=clone#caller-decides-where-to-copy-and-place-data-c-caller-control
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
update_spec_device() takes a 'host_path' parameter which it uses to locate
the device to correct in the OCI spec. Although this will usually be the
path of the device on the host, it doesn't have to be - a traditional
runtime like runc would create a device node of that name in the container
with the given (host) major and minor numbers. To clarify that, rename it
to 'container_path'.
We also update the block comment to explain the distinctions more
carefully. Finally we update some variable names in tests to match.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This error is returned if we have information for a device from the
runtime, but a matching device does not appear in the OCI spec. However,
the name for the device we print is the name from the VM, rather than the
name from the container which is what we actually expect in the spec.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
update_spec_devices() includes an unsafe block, in order to call the libc
functions to get the major and minor numbers from a device ID. However,
the nix crate already has a safe wrapper for this function, which we use in
other places in the file.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
- Even a directory could be a symlink - check for this. This is very
common when using configmaps/secrets
- Add unit test to better mimic a configmap, configmap update
- We would never remove directories before. Let's ensure that these are
added to the watched_list, and verify in unit tests
- Update unit tests which exercise maximum number of files per entry. There's a change
in behavior now that we consider directories/symlinks watchable as well.
For these tests, it means we support one less file in a watchable mount.
Signed-off-by: Eric Ernst <eric_ernst@apple.com>
The current implementation just copies the file, dereferencing any
simlinks in the process. This results in symlinks no being preserved,
and a change in layout relative to the mount that we are making
watchable.
What we want is something like "cp -d"
This isn't available in a crate, so let's go ahead and introduce a copy
function which will create a symlink with same relative path if the
source file is a symlink. Regular files are handled with the standard
fs::copy.
Introduce a unit test to verify symlinks are now handled appropriately.
Fixes: #2950
Signed-off-by: Eric Ernst <eric_ernst@apple.com>
When the container didn't had a tty console, it would be in a same
process group with the kata-agent, which wasn't expected. Thus,
create a new session for the container process.
Fixes: #3063
Signed-off-by: Fupan Li <fupan.lfp@antgroup.com>
Update the agent README by removing the historical details about the
conversion from golang to rust which (occurred at the start of Kata 2.x
development) and replacing it with information that developers and
testers should find more useful.
Fixes: #3056.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Update the `agent-ctl` tool to allow API fields to be specified in JSON
format, either directly on the command-line, or via a file URI.
This feature is made possible by enabling `serde` support in the agent
`protocols` crate. Careful use of the `serde` macros allows the
`agent-ctl` tool to accept _partially_ specified API objects in JSON
format; fields that are not specified are set to the default value for
their respective types.
`build.rs` changes based on work by Fupan.
Fixes: #2978.
Contributions-by: Fupan Li <lifupan@gmail.com>
Contributions-by: Bin Liu <bin@hyper.sh>
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
If Kata agent cannot resolve the system calls given by seccomp profiles,
the agent ignores the system calls and continues to run without an error.
Fixes: #2957
Signed-off-by: Manabu Sugimoto <Manabu.Sugimoto@sony.com>
Ensure the tests in the local `logging` crate are run for all consumers
of it.
Additionally, add a new test which checks that output is generated by a
range of different log level `slog` macros. This is designed to ensure
debug level output is always available for the consumers of the
`logging` crate.
Fixes: #2969.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Raise the `slog` maximum log level feature for release code from `info`
to `debug` by changing the `slog` maximum level features in the shared
`logging` crate. This allows the consumers of the `logging` crate (the
agent, the `trace-forwarder` and the `agent-ctl` tool) to produce debug
output when their debug options are enabled. Currently, those options
will essentially be a NOP (unless using a debug version of the code).
Testing showed that setting the `slog` maximum level features in the
rust manifest files for the consumers of the `logging` crate has no
impact: those values are ignored, so they have been removed and replaced
with a comment stating the levels are set in the `logging` crate.
Fixes: #2966.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Use `dup3` system call instead of `dup2` in unit tests of seccomp
because `dup2` is obsolete on aarch64.
Fixes: #2939
Signed-off-by: Manabu Sugimoto <Manabu.Sugimoto@sony.com>
