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

Start tracking k8s liveness probes #1320

Merged
merged 2 commits into from
Jun 14, 2019
Merged

Start tracking k8s liveness probes #1320

merged 2 commits into from
Jun 14, 2019

Conversation

mstemm
Copy link
Contributor

@mstemm mstemm commented Feb 21, 2019

K8s has a similar but not identical method as docker for container
health checks. They are called liveness probes and are a different
property of the container json.

Read both out of the container json, load/save them both when writing
to/from container json information events, and use a shared "health
probe" command and args.

For threads, now that we have two properties, switch everything to use a
threadinfo category instead of a simple bool for has healthcheck. The
possible values for the category are CAT_NONE, CAT_HEALTHCHECK, and
CAT_LIVENESS_PROBE. identify_healthcheck becomes identify_category() but
otherwise behaves the same (passing categories down and checking the
args list otherwise).

The filterchecks aren't quite as generic as the threadinfo categories to
keep the filtering simple. A new field proc.is_container_liveness_probe
checks for k8s liveness probes, and container.liveness_probe prints the
exe + args.

@mstemm mstemm force-pushed the identify-k8s-liveness-probes branch from a657c17 to 83af9e0 Compare February 21, 2019 17:44
@mstemm mstemm requested a review from gnosek February 21, 2019 17:45
@mstemm mstemm force-pushed the identify-k8s-liveness-probes branch from 8aa0a4a to 5a79c19 Compare February 25, 2019 23:36
@mstemm mstemm force-pushed the identify-k8s-liveness-probes branch from db912c3 to 34b74bd Compare February 27, 2019 20:42
@mstemm
Copy link
Contributor Author

mstemm commented Feb 27, 2019

@gnosek I think this is ready to merge now, can you take a look?

Copy link
Contributor

@gnosek gnosek left a comment

Choose a reason for hiding this comment

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

Not too thrilled about handling Docker/k8s internals in container_info.cpp but otherwise LGTM

userspace/libsinsp/container_info.h Outdated Show resolved Hide resolved
userspace/libsinsp/container.cpp Outdated Show resolved Hide resolved
userspace/libsinsp/container_info.cpp Outdated Show resolved Hide resolved
userspace/libsinsp/container_info.cpp Show resolved Hide resolved
userspace/libsinsp/container_info.cpp Outdated Show resolved Hide resolved
@mstemm
Copy link
Contributor Author

mstemm commented Mar 15, 2019

I'm going to wait for #1326 to be merged before making additional updates. They both change the same container_engine code and I'd rather resolve conflicts just once.

@mstemm mstemm force-pushed the identify-k8s-liveness-probes branch 4 times, most recently from ffd575a to 800ebd8 Compare May 2, 2019 21:11
@mstemm
Copy link
Contributor Author

mstemm commented May 2, 2019

This is ready for review again @gnosek could you take another look?

@mstemm mstemm force-pushed the identify-k8s-liveness-probes branch from 800ebd8 to 1cc9055 Compare May 3, 2019 16:53
userspace/libsinsp/container.cpp Show resolved Hide resolved
@mstemm mstemm force-pushed the identify-k8s-liveness-probes branch 2 times, most recently from 1a30b95 to 555deed Compare June 10, 2019 22:19
mstemm added 2 commits June 11, 2019 13:49
Not used in this header file, so shouldn't be needed.
K8s has a similar but not identical method as docker for container
health checks. They are called liveness/readiness probes and are a part
of the pod specification, and not a part of the image.

Luckily, the pod configuration *is* a part of the container metadata as
stringified json, with a label
"annotation.kubectl.kubernetes.io/last-applied-configuration", so we can
use that label to identify liveness/readiness probes.

New methods in the docker container resolver handle parsing the pod
specification (and healthcheck info) out of the container json and
creating health probes from them.

A new class sinsp_container_info::container_health_probe represents one
of these health probes. It has a probe
type (healthcheck/liveness/readiness), the executable and arguments, and
methods to serialize/unserialize from json. The serialization doesn't
preserve the original container json--they only keep the exe + args.

The container info now has a list of possible health probe objects and
iterates over them when dumping the container to json.

For threads, switch everything to use a threadinfo category instead of a
simple bool for has healthcheck. The possible values for the category
are:

  - CAT_NONE: no specific category
  - CAT_CONTAINER: a process run in a container and *not* any
    of the following more specific categories.
  - CAT_HEALTHCHECK: part of a container healthcheck
  - CAT_LIVENESS_PROBE: part of a k8s liveness probe
  - CAT_READINESS_PROBE: part of a k8s readiness probe

Identify_healthcheck becomes identify_category() but
otherwise behaves the same (passing categories down and checking the
args list otherwise).

The logic in indentify_healthcheck tries to handle the common cases
first:

 - not running in a container or container info not present: CAT_NONE
 - vpid=1: CAT_CONTAINER
 - inherit categories other than CAT_NONE directly from parent

If those fail, the more expensive steps of matching against the health
check args and possibly traversing the parent state are done.

The filterchecks aren't quite as generic as the threadinfo categories to
keep the filtering simple. A new field
proc.is_container_{liveness,readiness}_probe checks for k8s
liveness/readiness probes, and container.{liveness,readiness}_probe
prints the exe + args.
@mstemm mstemm force-pushed the identify-k8s-liveness-probes branch from 555deed to 21f1139 Compare June 11, 2019 20:49
@mstemm mstemm merged commit ed782e8 into dev Jun 14, 2019
@mstemm mstemm deleted the identify-k8s-liveness-probes branch June 14, 2019 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants