Skip to content

describe e2e observer pods #425

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 4 commits into from
Oct 28, 2020
Merged

describe e2e observer pods #425

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8bbcd82
describe e2e observer pods
deads2k Aug 7, 2020
a85cfcd
comments
deads2k Aug 11, 2020
56a33a3
more
deads2k Aug 12, 2020
44c687b
update e2e-observers
deads2k Oct 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions enhancements/test-platform/e2e-observer-pods.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
---
title: e2e-observer-pods
authors:
- "@deads2k"
reviewers:
approvers:
- "@stevek"
creation-date: yyyy-mm-dd
last-updated: yyyy-mm-dd
status: provisional|implementable|implemented|deferred|rejected|withdrawn|replaced
see-also:
replaces:
superseded-by:
---

# e2e Observer Pods

## Release Signoff Checklist

- [ ] Enhancement is `implementable`
- [ ] Design details are appropriately documented from clear requirements
- [ ] Test plan is defined
- [ ] Graduation criteria for dev preview, tech preview, GA
- [ ] User-facing documentation is created in [openshift-docs](https://github.com/openshift/openshift-docs/)

## Summary

e2e tests have multiple dimensions:
1. which tests are running (parallel, serial, conformance, operator-specific, etc)
2. which platform (gcp, aws, azure, etc)
3. which configuration of that platform (proxy, fips, ovs, etc)
4. which version is running (4.4, 4.5, 4.6, etc)
5. which version change is running (single upgrade, double upgrade, minor upgrade, etc)
Our individual jobs are an intersection of these dimensions and that intersectionality drove the development of steps
to help handle the complexity.

There is a kind of CI test observer that wants to run across all of these intersections by leveraging the uniformity of
OCP across all the different dimensions above.
It wants to do something like start an observer agent of some kind for every CI cluster ever created and provide o(100M)
of data back to include in the CI job overall.
Working with steps would require integrating in multiple dimensions and lead to gaps in coverage.
Comment thread
stevekuznetsov marked this conversation as resolved.

## Motivation

We need to run tools like
1. e2e monitor - doesn't cover installation
2. resourcewatcher - doesn't cover installation today if we wire it directly into tests.
3. loki log collection (as created by group-b) - hasn't been able to work in upgrades at all.
4. loki as provided by the logging team (future)
To debug our existing install and upgrade failures.

### Goals

1. allow a cluster observing tool to run in the CI cluster (this avoid restarts during upgrades)
2. allow a cluster observer to provide data (including junit) to be collected by the CI job
3. collect data from cluster observer regardless of whether the job succeeded or failed
4. allow multiple observers per CI cluster

Comment thread
stevekuznetsov marked this conversation as resolved.
### Non-Goals

1. allow a cluster observer to impact success or failure of a job
2. allow a cluster observer to impact the running test. This is an observer author failure.
3. provide ANY dimension specific data. If an observer needs this, they need to integrate differently.
4. allow multiple instances of a single cluster observer to run against one CI cluster
5. allow providing the cluster observer from a repo being built. The images are provided separately.

@stevekuznetsov stevekuznetsov Aug 12, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is trivial to allow, we can either remove it or move it to goals

@stevekuznetsov stevekuznetsov Aug 12, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(like, the mechanism we use to ensure that the Pod can only start when the image is imported is identical to that which is used if we're waiting for it to be built)


## Proposal

The overall goal:
1. have an e2e-observer process is expected to be running before a kubeconfig exists

@stevekuznetsov stevekuznetsov Aug 12, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: s/is/that is/

2. the kubeconfig should be provided to the e2e-observer at the earliest possible time. Even before it can be used.
3. the e2e-observer process is expected to detect the presence of the kubeconfig itself
4. the e2e-observer process will accept a signal indicating that teardown begins

### Changes to CI
Comment thread
stevekuznetsov marked this conversation as resolved.
This is a sketch of a possible path.
1. Allow a non-dptp-developer to produces a pod template that mounts a `secret/<job>-kubeconfig` that will later contain a kubeconfig.

@stevekuznetsov stevekuznetsov Aug 7, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why does it need to be a Pod template? Our current interface for developers looks like a (container image, commands) tuple. Is that sufficient? What do we need users to be able to configure?

@deads2k deads2k Aug 7, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why does it need to be a Pod template? Our current interface for developers looks like a (container image, commands) tuple. Is that sufficient? What do we need users to be able to configure?

I'm not sure. But I do know that podtemplates are standard kube structs, with their existing documentation and clear semantics. Resource requirements come to mind. Env vars probably.

I think it's actually easier for me to provide a podtemplate since my development is likely to be done using a pod.

@stevekuznetsov stevekuznetsov Aug 11, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Most if not all of those points of variance are already handled by existing types in the CI system. I don't want to expose e.g. CAP_SYS_ADMIN or host mount ...

@stevekuznetsov stevekuznetsov Aug 12, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is important. s/pod template/step reference/g

2. Before a kubeconfig is present, create an instance of each pod template,
Let's say `pod/<job>-<e2e-observer-name>` and an empty `secret/<job>-kubeconfig`.
3. As soon as a kubeconfig is available (these go into a known location in setup container today), write that
Comment thread
stevekuznetsov marked this conversation as resolved.
Outdated
`kubeconfig` into every `secret/<job>-kubeconfig` (you probably want to label them).
4. When it is time for collection, the existing pod (I think it's teardown container), writes a new data entry at `.data['teardown']` into every

@stevekuznetsov stevekuznetsov Aug 7, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What is important about this mechanism?

@deads2k deads2k Aug 7, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What is important about this mechanism?

It's a simple notification that meets the min of, "easy to provide and easy to observe in an e2e-observer pod". I don't really how, but this way is easy.

@wking wking Aug 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why do the monitors need to know the teardown time? Won't it be enough for them to record "cluster seems to have disappeared" and rely on test-suite failures to get folks to look at jobs where the cluster expoded before teardown.

@deads2k deads2k Aug 10, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

why do the monitors need to know the teardown time? Won't it be enough for them to record "cluster seems to have disappeared" and rely on test-suite failures to get folks to look at jobs where the cluster expoded before teardown.

collection can be expensive. You may want to defer until the end. Imagine using this for must-gather so we can actually get it on all CI runs. Amazingly, we have failed to achieve even that collection in a consistent way so far.

@wking wking Aug 10, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Imagine using this for must-gather so we can actually get it on all CI runs.

When clusters die hard enough for must-gather to fail, I doubt ci-operator will be able to reliably guess the right window before to trigger the toppling-cluster must-gather. I think log-streaming and other continuous monitoring are probably the best we can deliver in those cases.

@stevekuznetsov stevekuznetsov Aug 12, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is outstanding -- if it's important that we signal. I'd prefer sending SIGTERM rather than messing with files.

@stevekuznetsov stevekuznetsov Aug 12, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggest:

4. When the test workflow enters the post-step phase, SIGTERM is sent to each observer process.

`secret/<job>-kubeconfig`.
The value should be a timestamp.
5. Ten minutes after `.data['teardown']` is written, the teardown container rsyncs a well known directory,

@stevekuznetsov stevekuznetsov Aug 7, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is this necessary? ci-operator has oversight on the control flow and handles cancellation and failure gracefully, with timeouts and grace periods as necessary.

What do you need from the feature?

  • A signal sent to the observer when the teardown phase of a job begins
  • A (configurable) grace period, starting at that signal, after which artifacts are gathered

@deads2k deads2k Aug 7, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • A (configurable) grace period, starting at that signal, after which artifacts are gathered

I wouldn't let someone configure it.

@wking wking Aug 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why is this not just "ci-operator TERMs the monitor containers once the workflow wraps up entirely, or after the test steps complete (which would also work as an "I'll reap this cluster soon, you can stop watching" signal), or some such? And then uploads anything from the monitor container's mounted artifacts volume (just like the existing step logic).

@stevekuznetsov stevekuznetsov Aug 12, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggest:

5. Observer pods have their artifacts gathered after a grace period, or after the process exists, whichever comes first.

`/var/e2e-observer`, which may contain `/var/e2e-observer/junit` and `/var/e2e-observer/artifacts`. These contents
are placed in some reasonable spot.

This could be optimized with a file write in the other direction, but the naive approach doesn't require it.
6. All `pod/<job>-<e2e-observer-name>` are deleted.

@stevekuznetsov stevekuznetsov Aug 7, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is not germane to this proposal

@deads2k deads2k Aug 7, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is not germane to this proposal

it's very germane to the example and concrete bits. I'm happy to not use this sketch if you wish to provide a different one, but for a concrete example of how this could be built, this is pretty straightforward

@stevekuznetsov stevekuznetsov Aug 11, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This does not fit into the general model of lifecycle in the CI Operator, and it's not germane in that it does not matter to a functional and correct implementation of an observer Pod.

@stevekuznetsov stevekuznetsov Aug 12, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Outstanding, please remove.


### Requirements on e2e-observer authors
1. Your pod must be able to run against *every* dimension. No exceptions. If your pod needs to quietly no-op, it can do that.
2. Your pod must handle a case of a kubeconfig file that isn't present when the pod starts and appears afterward.
Or even doesn't appear at all.
3. Your pod must be able to tolerate a kubeconfig that doesn't work.
The kubeconfig may point to a cluster than never comes up or that hasn't come up yet. Your pod must not fail.
4. If your pod fails, it will not fail the e2e job. If you need to fail an e2e job reliably, you need something else.
5. Your pod must terminate when asked.

## Alternatives

### Modify each type of job
Given the matrix of dimensions, this seems impractical.
Comment thread
stevekuznetsov marked this conversation as resolved.
Even today, we have a wide variance in the quality of artifacts from different jobs.

### Modify the openshift-tests command
This is easy for some developers, BUT as e2e monitor shows us, it doesn't provide enough information.
We need information from before the test command is run to find many of our problems.
Missing logs and missing intermediate resource states (intermediate operator status) are the most egregious so far.

@wking wking Aug 7, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The vanilla e2e suite already records intermediate operator status in Monitor cluster while tests execute. It does not seem like that big a reach to me to ask the installer to also monitor and log operator status, instead of its current "just collect this at the end if the install failed" behavior. And that would also be useful information to have in the log for installs that fail in the wild, which will not benefit from CI-specific monitoring suites.

Log preservation is unlikely to be covered by either the installer or the Monitor cluster while tests execute "test-case". I think that's a valid use-case for this enhancement. But being able to configure all clusters, CI and otherwise, to be able to stream logs to an external logging aggregator would be nice too, and would cover both CI and real-world use-cases.