Commit graph

1184 commits

Author SHA1 Message Date
Mrunal Patel
6ff71d0f84 Merge pull request #552 from alexlarsson/conmon-fixes
Various fixes for conmon
2017-06-02 10:10:16 -07:00
Alexander Larsson
2507ba6453 Remove json-glib in the remaining places
Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:18:27 +02:00
Alexander Larsson
f4b3e90141 conmon: Make console socket mode 0700
It doesn't make sense for other users to connect to this, so
lets make sure of this.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:11:26 +02:00
Alexander Larsson
f1b0f542e1 conmon: Silence uninitialized read compiler warning
This is not actually read uninitialized, its just that the compiler
can't detect this, but we initilize it anyway to silence the compiler.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:11:21 +02:00
Alexander Larsson
fe6f1f4786 conmon: Add -Os flag
This is what the other C code uses, and its nice to have as adding
any optimization flags enables a bunch of more warnings.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:11:15 +02:00
Alexander Larsson
1a168cb196 conmon: Drop json-glib dependency
json-glib is a fine library for parsing json. However, all we need
to do is generate some trivial json output, so it is not needed.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:11:10 +02:00
Alexander Larsson
f3408cbb5c conmon: Make all file descriptors CLOEXEC
We want to avoid inheriting these into the child. Doing so is both
confusing for the child, and a potential security issue if the
container has access to FDs that are from the outside of the
container.

Some of these are created after we fork for the child, so they
are not technically necessary. However, its best to do this as
we may change the code in the future and forget about this.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:11:06 +02:00
Alexander Larsson
829ec7f351 conmon: Build argv instead of commandline to spawn runtime
This means we don't have to spawn via a shell, but it also
means we do the right thing for any input that would have
needed to be escaped. For instance if the container name had
a $ in i, or even worse, a back-quote!

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:11:01 +02:00
Alexander Larsson
d2f09ef483 conmon: Increase buffer size
The buffer is used to read from the stderr/stdout stream, which
can easily be larger than 256 bytes. With a larger buffer we will
do fewer, larger reads, which is more efficient. And 8k more stack
size use is not really a problem.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:10:56 +02:00
Alexander Larsson
fe80f857ca conmon: Fix cgroup subsystem parsing
The code as is doesn't handle merged controllers.
For instance, I have this in my /proc/self/cgrous:

4:cpu,cpuacct:/user.slice/user-0.slice/session-4.scope

The current code fails to match "cpuacct" wit this line, and
additionally it just does a prefix match so if you were looking
for say "cpu", it would match this:

2:cpuset:/

I also removed some ninfo spew that didn't seem very useful.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:10:36 +02:00
Alexander Larsson
d34c5829f8 conmon: Write log in larger chunks
Rather than writing the logs with one write per line, use writev()
to write multiple lines in one call. Additionally, this avoids
using dprintf() when writing to the log, which is nice because that
doesn't correctly handle partial writes or ENOINTR.

This also changes set_k8s_timestamp to add the pipe to the reused
buffer so that we don't have to append it on each line.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:10:30 +02:00
Alexander Larsson
ae933d0d03 conmon: Handle EINTR and partial writes when writing
Any write could be interupted by EINTR if we get some kind of signal,
which means we could be either reporting a EINTR error or a partial
write (if some data was written). Its also generally good to handle
partial writes correctly, as they can happen e.g. when writing to
full pipes.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
2017-06-02 16:09:48 +02:00
Samuel Ortiz
f15859c79f pkg/annotations: Export CRI-O annotations namespace
Some runtimes like Clear Containers need to interpret the CRI-O
annotations, to distinguish the infra container from the regular one.
Here we export those annotations and use a more standard dotted
namespace for them.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
2017-06-01 23:45:44 +02:00
Mrunal Patel
36255b8663 Merge pull request #549 from runcom/stability-fixes
Stability fixes
2017-06-01 10:10:14 -07:00
Antonio Murdaca
f3650533f0
create src dir for bind mounts
match docker behavior for bind mounts

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-06-01 17:37:20 +02:00
Antonio Murdaca
a28ed75e12
sandbox_run: fix name releasing on error
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-06-01 17:37:20 +02:00
Antonio Murdaca
6fd1c8957c
RemovePodSandbox must be idempotent
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-06-01 17:37:20 +02:00
Antonio Murdaca
88fb9094d0
oci: do not error out on runtime state failure
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-06-01 17:37:17 +02:00
Antonio Murdaca
a37dd46654
*: stability fixes
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-06-01 15:42:01 +02:00
Mrunal Patel
b5153e08c5 Merge pull request #547 from sameo/topic/privileged-paths
container: Do not restrict path access for privileged containers
2017-05-30 21:38:57 -07:00
Samuel Ortiz
e23d986cf2 container: Do not restrict path access for privileged containers
Privileged containers should see and reach all host paths.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
2017-05-31 02:06:47 +02:00
Mrunal Patel
42d2fa6576 Merge pull request #545 from runcom/fix-hostspecific-spec
server: container_create: make the spec hostspecific
2017-05-30 11:49:22 -07:00
Antonio Murdaca
089cb88f17
server: container_create: make the spec hostspecific
node-e2e tests were failing in RHEL because, if running a privileged
container, we get all capability in the spec. The spec generator wasn't
filtering caps based on actual host caps, it was just adding _everything_.
This patch makes spec generator host specific.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-30 18:30:26 +02:00
Mrunal Patel
90e3e7a632 Merge pull request #529 from runcom/k8s-node-e2e-RHEL
enable RHEL k8s node-e2e tests
2017-05-30 07:42:09 -07:00
Antonio Murdaca
cf037ce947
contrib: test: fix and enable RHEL k8s node-e2e tests
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-30 11:52:59 +02:00
Mrunal Patel
fa8d5c035a Merge pull request #544 from runcom/fix-panics
[e2e fix] server: add nil checks to not panic
2017-05-29 12:20:22 -07:00
Antonio Murdaca
404194c1fd
server: add nil checks to not panic
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-29 13:16:27 +02:00
Mrunal Patel
5ed79fb5cd Merge pull request #543 from runcom/fix-ctr-status-reasons
[e2e fix] server: correctly fill ctr termination reason
2017-05-28 18:03:54 -07:00
Antonio Murdaca
ad3a3fcd5a
server: properly format error
`containerdID` is overridden in `s.ctrIDIndex.Get()`, if the ctr is not
found it's overridden by an empty string making the error return
totally unusable.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-28 20:43:01 +02:00
Mrunal Patel
988f29fae8 Merge pull request #542 from runcom/port-forward-panic
[e2e fix] server: fix PortForward panic
2017-05-28 11:40:17 -07:00
Antonio Murdaca
bc8570d1de
test: use nginx:alpine when testing pull-by-digest
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-28 20:33:55 +02:00
Antonio Murdaca
b9336c74a3
server: correctly fill ctr termination reason
This patch fixes all port forwarding e2e tests. Those tests were
specifically looking for a termination reason to say that a given
container has finished running. CRI-O wasn't actually returning any
reason field for an exited container.

-> https://github.com/kubernetes/kubernetes/blob/master/test/e2e/portforward.go#L116
   -> https://github.com/kubernetes/kubernetes/blob/master/test/utils/conditions.go#L97

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-28 20:14:45 +02:00
Antonio Murdaca
1e9ef65345
server: fix PortForward panic
During "Port forwarding" e2e tests, the following panic happened:

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x64981d]

goroutine 52788 [running]:
panic(0x1830ee0, 0xc4200100c0)
        /usr/lib/golang/src/runtime/panic.go:500 +0x1a1
github.com/kubernetes-incubator/cri-o/oci.(*Runtime).UpdateStatus(0xc4202afc00,
0x0, 0x0, 0x0)
        /home/amurdaca/go/src/github.com/kubernetes-incubator/cri-o/oci/oci.go:549
+0x7d
github.com/kubernetes-incubator/cri-o/server.streamService.PortForward(0xc42026e000,
0x0, 0x0, 0x0, 0x0, 0xc420d9af40, 0x40, 0xc400000050, 0x7fe660659a28,
0xc4201cd0e0, ...)
```

The issue is `streamService.PortForward` assumed the first argument to
be the sandbox's infra container ID, thus trying to get it from memory
store using `.state.containers.Get`. Since that ID is of the sandbox
itself, it fails to get the container object from memory and panics in
`UpdateStatus`.

Fix it by looking for the sandbox's infra container ID starting from a
sandbox ID.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-28 18:22:46 +02:00
Mrunal Patel
cf4e5ee903 Merge pull request #539 from runcom/stop-signal-cc
server: store and use image's stop signal to stop containers
2017-05-27 08:43:36 -07:00
Antonio Murdaca
b4f1cee2a2
server: store and use image's stop signal to stop containers
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-27 10:21:04 +02:00
Antonio Murdaca
7c43d34a1b
contrib: test: redirect stderr to stdout
I don't want to see stuff like this anymore
https://aos-ci.s3.amazonaws.com/kubernetes-incubator/cri-o/crio-integration-tests-prs/360/fullresults.txt

It's basically missing the actual go build error because stdout gets
eaten somewhere by ansible I guess.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-27 10:20:54 +02:00
Mrunal Patel
961edb2719 Merge pull request #538 from runcom/fixies-test
fixes for tests and cleanup
2017-05-26 09:37:23 -07:00
Antonio Murdaca
aa9abdfe40
test: pull just once in integration tests
w/o this patch we were always pulling redis:alpine by digest in each
test.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-26 16:01:31 +02:00
Antonio Murdaca
21d8c2544c
.gitignore: do not ignore *.rej files
also do some cleanup

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-26 15:46:32 +02:00
Mrunal Patel
5749da7f5d Merge pull request #537 from runcom/fix-execsync
execsync: rewrite to fix a bug in conmon
2017-05-25 17:25:38 -07:00
Antonio Murdaca
b4251aebd8
execsync: rewrite to fix a bug in conmon
conmon has many flags that are parsed when it's executed, one of them
is "-c". During PR #510 where we vendor latest kube master code,
upstream has changed a test to call a "ctr execsync" with a command of
"sh -c commmand ...".
Turns out:

a) conmon has a "-c" flag which refers to the container name/id
b) the exec command has a "-c" flags but it's for "sh"

That leads to conmon parsing the second "-c" flags from the exec
command causing an error. The executed command looks like:

conmon -c [..other flags..] CONTAINERID -e sh -c echo hello world

This patch rewrites the exec sync code to not pass down to conmon the
exec command via command line. Rather, we're now creating an OCI runtime
process spec in a temp file, pass _the path_ down to conmon, and have
runc exec the command using "runc exec --process
/path/to/process-spec.json CONTAINERID". This is far better in which we
don't need to bother anymore about conflicts with flags in conmon.

Added and fixed some tests also.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2017-05-25 22:36:33 +02:00
Antonio Murdaca
26e90190fc Merge pull request #535 from mrunalp/oom_handling
OOM handling
2017-05-25 22:33:44 +02:00
Mrunal Patel
d06b2ff20f test: Add a test for container OOM
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
2017-05-25 11:30:58 -07:00
Mrunal Patel
f64032483e test: Ensure image for testing oom is present
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
2017-05-25 11:30:58 -07:00
Mrunal Patel
ea9a90abce Set Container Status Reason when OOM Killed
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
2017-05-25 11:30:58 -07:00
Mrunal Patel
52b27da680 conmon: Disable OOM handling if cgroups not setup
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
2017-05-25 11:30:58 -07:00
Mrunal Patel
7700a62347 conmon: Create oom file for container on OOM notification
Signed-off-by: Mrunal Patel <mpatel@redhat.com>
2017-05-25 11:30:58 -07:00
Mrunal Patel
46f6248e42 conmon: Add OOM eventfd to epoll monitoring list
Signed-off-by: Mrunal Patel <mpatel@redhat.com>
2017-05-25 11:30:58 -07:00
Mrunal Patel
ddb54bf614 conmon: Setup cgroups for container pid OOM notification
Signed-off-by: Mrunal Patel <mpatel@redhat.com>
2017-05-25 11:30:58 -07:00
Mrunal Patel
04ddb57ed7 conmon: Add helper function to get pid cgroup subsystem path
Signed-off-by: Mrunal Patel <mpatel@redhat.com>
2017-05-25 11:30:58 -07:00