CI complains about cyclomatic complexity in sendReq.
warning: cyclomatic complexity 16 of function (*kataAgent).sendReq() is
high (> 15) (gocyclo)
Refactor it a bit to avoid such error. I'm not a big fan of the new code
but it is done so because golang does not support generics.
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Currently we sometimes pass it as a pointer and other times not. As
a result, the view of sandbox across virtcontainers may not be the same
and it costs extra memory copy each time we pass it by value. Fix it
by ensuring sandbox is always passed by pointers.
Fixes: #262
Signed-off-by: Peng Tao <bergwolf@gmail.com>
This commit will allow for better performance regarding the time spent
to retrieve the sandbox ID related to a container ID.
The way it works is by relying on a specific mapping between container
IDs and sanbox IDs, meaning it allows to retrieve directly the sandbox
ID related to a container ID from the CLI. This lowers complexity from
O(n²) to O(1), because we don't need to call into ListPod() which was
parsing all the pods and all the containers on the system everytime
the CLI need to retrieve this mapping.
This commit also updates the whole unit tests as a consequence. This
is involving most of them since they were all relying on ListPod()
before.
Fixes#212
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Here is an interesting case I have been debugging. I was trying to
understand why a "kubeadm reset" was not working for kata-runtime
compared to runc. In this case, the only pod started with Kata is
the kube-dns pod. For some reasons, when this pod is stopped and
removed, its containers receive some signals, 2 of them being SIGTERM
signals, which seems the way to properly stop them, but the third
container receives a SIGCONT. Obviously, nothing happens in this
case, but apparently CRI-O considers this should be the end of the
container and after a few seconds, it kills the container process
(being the shim in Kata case). Because it is using a SIGKILL, the
signal does not get forwarded to the agent because the shim itself
is killed right away. After this happened, CRI-O calls into
"kata-runtime state", we detect the shim is not running anymore
and we try to stop the container. The code will eventually call
into agent.RemoveContainer(), but this will fail and return an
error because inside the agent, the container is still running.
The approach to solve this issue here is to send a SIGKILL signal
to the container after the shim has been waited for. This call does
not check for the error returned because most of the cases, regular
use cases, will end up returning an error because the shim itself
not being there actually represents the container inside the VM has
already terminated.
And in case the shim has been killed without the possibility to
forward the signal (like described in first paragraph), the SIGKILL
will work and will allow the following call to agent.stopContainer()
to proceed to the removal of the container inside the agent.
Fixes#274
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
We currently just send the pid in the state. While OCI specifies
a few other fields as well, this commit just adds the bundle path
and the container id to the state. This should fix the errors seen
with hooks that rely on the bundle path.
Other fields like running "state" string have been left out. As this
would need sending the strings that OCI recognises. Hooks have been
implemented in virtcontainers and sending the state string would
require calling into OCI specific code in virtcontainers.
The bundle path again is OCI specific, but this can be accessed
using annotations. Hooks really need to be moved to the cli as they
are OCI specific. This however needs network hotplug to be implemented
first so that the hooks can be called from the cli after the
VM has been created.
Fixes#271
Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
Now that our CI has moved to Go 1.10, we need to update one file
that is not formatted as the new gofmt (1.10) expects it to be
formatted.
Fixes#249
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Rework the signal handling code so that if debug is enabled and a
`SIGUSR1` signal is received, backtrace to the system log but continue
to run.
Added some basic tests for the signal handling code.
Fixes#241.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
The same way a caller of "kata-runtime kill 12345" expects
the container 12345 to be killed, the same call to a container
representing a sandbox should actually kill the sandbox, meaning
it would be stopped after the container has been killed.
This way, the caller knows the VM is stopped after kill returns.
This is an issue raised by Openshift and Kubernetes tests. They
call into delete way after the call to kill has been submitted,
and in the meantime they kill all processes related to the container,
meaning they do kill the VM before we could do it ourselves. In this
case, the delete responsible of stopping the VM comes too late and it
returns an error when trying to destroy the sandbox while trying to
communicate with the agent since the VM is not here anymore.
This commit addresses this issue by letting "kill" call into
StopSandbox() if the command relates to a sandbox instead of
a simple container.
Fixes#246
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
The way a delete works, it was always trying to stop the sandbox, even
when the force flag was not enabled. Because we want to be able to stop
the sandbox from a kill command, this means a sandbox stop might be
called twice, and we don't want the second stop to fail, leading to the
failure of the delete command.
That's why this commit checks for the sandbox status before to try
stopping the sandbox.
Fixes#246
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
change from go1.10 to 1.9.2.
Our static checks and unit tests fail when using
go 1.10. Since we use go 1.9.2 to test in our CI,
reflect this version in versions.yaml
By doing this, we will be able to remove the hardcoded version
from the jenkins scripts and instead install golang using
`.ci/install_go.sh` from the tests repository. And when moving
to go1.10 using a PR, the CI will test that the static checks
and unit tests pass correctly.
Fixes: #254.
Signed-off-by: Salvador Fuentes <salvador.fuentes@intel.com>
It disconnects the agent connection and removes the sandbox
from global sandbox list.
A new option `LongLiveConn` is also added to kata
agent's configuration. When set, the API caller is expected
to call sandbox.Release() to drop the agent connection explicitly.
`proxyBuiltIn` is moved out of agent state because we can always
retrieve it from sandbox config instead.
Fixes: #217
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Don't Attempt to create file below `/dev` when running as non-`root`.
Move the logic into a new `TestIsHostDeviceCreateFile` test and skip
unless `root.`
Fixes#242.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
The collect script is now able to extract the osbuilder metadata
from an initrd image.
Fixes#237.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Changed the collect script to display the contents of the
osbuilder metadata file which provides details of the image.
Partially fixes#237.
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
The k8s test creates a log file in /dev under
/dev/termination-log, which is not the right place to create
logs, but we need to handle this. With this commit, we handle
regular files under /dev by passing them as 9p shares. All other
special files including device files and directories
are not passed as 9p shares as these are specific to the host.
Any operations on these in the guest would fail anyways.
Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
