Upgrade tests for knative eventing

[akashrv]

Problem
Found basic issues when upgrading my cluster from v0.10.1 to current head (v0.11.0). This was fixed asap when brought up in slack channel. However it exposed a gap in E2E tests. Ideally any such change that breaks a running application in previous version should be rejected.
Refer to serving to figure out how many older versions we support for upgrade tests.

Exit Criteria
Set of E2E tests that fail if a change would break user app post upgrade.

Time Estimate (optional):
How many developer-days do you think this may take to resolve?

Additional context (optional)
Add any other context about the feature request here.

[mikehelmick]

cc @mayaspivak

[cardil]

Tests should:

• install the previous release,
• run some tests,
• install HEAD,
• run some tests,
• install the previous release (again),
• and run some final tests.

That will allows us to test that we don't break things between releases.

There should be only smoke tests executed. We shouldn't validate as many things, just that it works at all.

[cardil]

I'm currently working to prepare those upgrade & downgrade tests. The goal is to have same thing as in serving.

[cardil]

I have advanced work on this task. I have prepared a tool (https://github.com/cardil/wathola) that I want to use to send events continuously and make sure that all these events have been sent exactly once.

This tool uses 3 components: sender, forwarder, and receiver. Sender is the usual Kubernetes pod that publishes events to the default broker with given interval. When it closes (SIGTERM, SIGINT), an finished event is generated. Forwarder is a knative serving service that scales from zero to receive the requested traffic. Forwarders receive events and forwards them to given target. Receiver is an ordinary platform that collects events from multiple forwarders and has an endpoint /result that can be polled to get the status of sent events.

You can see example yamls in my scratchpad: https://github.com/cardil/eventing-upgrade-scratchpad/tree/master/tests

Below diagram describes this setup:

  (pod)              (ksvc)             (pod)
+--------+       +-----------+       +----------+
|        |       |           ++      |          |
| Sender |   +-->| Forwarder ||----->+ Receiver |
|        |   |   |           ||      |          |
+---+----+   |   +------------|      +----------+
    |        |    +-----------+
    |        |
    |        |
    |     +--+-----+       +---------+
    +----->        |       |         +-+
          | Broker | < - - | Trigger | |
          |        |       |         | |
          +--------+       +---------+ |
           (default)        +----------+

That all works, and I was able to use that to find a bug with that setup (#2357), but there is a small problem with that. Eventing e2e tests doesn't install or use knative serving.

So, I propose to install serving in upgrade tests, so I will be able to utilize whole setup. Without knative serving I will have to remove forwarders from these tests, but I think it would be better to have them.

[chizhg]

@cardil thanks for working on this!

I agree we should have tests using Serving, to ensure the compatibility with the same Knative version. Actually we already have such tests in project https://github.com/google/knative-gcp.

There are a few other thoughts in my mind:

1. In the existing e2e tests, we are creating all resources with code instead of YAML files. So I think it would be better to keep it consistent. For ksvcs, we can create them with the dynamic client like https://github.com/google/knative-gcp/pull/471/files#diff-2a90396ccd728edf8ecdd2c47bc261bdR48

2. I think this test can be written as an e2e test, and run in upgrade tests. In this way when an error happens, it would be easier to narrow down the scope. What do you think?

3. In the current e2e test framework, we are logging the events in the receiver Pod, and doing the verification by reading the logs. I know it is not elegant and I'm considering to improve it. It looks the receiver in your test can be a generic solution for the verification step :)

[cardil]

@cardil thanks for working on this!

I agree we should have tests using Serving, to ensure the compatibility with the same Knative version. Actually we already have such tests in project https://github.com/google/knative-gcp.

Awesome.

There are a few other thoughts in my mind:

1. In the existing e2e tests, we are creating all resources with code instead of YAML files. So I think it would be better to keep it consistent. For ksvcs, we can create them with the dynamic client like https://github.com/google/knative-gcp/pull/471/files#diff-2a90396ccd728edf8ecdd2c47bc261bdR48

Yes. I was about to do that with Go client. YAML files are just my PoC scratchbook.

2. I think this test can be written as an e2e test, and run in upgrade tests. In this way when an error happens, it would be easier to narrow down the scope. What do you think?

Hmm. Theoretically it can be done. I will write it as a probe, and that can be deployed and ran in single e2e test. In upgrade test the probe will be running in background while upgrade and downgrade.

3. In the current e2e test framework, we are logging the events in the receiver Pod, and doing the verification by reading the logs. I know it is not elegant and I'm considering to improve it. It looks the receiver in your test can be a generic solution for the verification step :)

Yes. I'm doing the same, but also I have /result REST endpoint that ease the verification process and makes it more reliable.

[houshengbo]

@cardil @chizhg I refer to this issue in the design doc about how operator can do upgrade: https://docs.google.com/document/d/1HEH9b4XWqFXtitAeHvCzymEG8-dtKH5y.

The upgrade tests for both of serving and eventing can be crucial for the whole upgrade process. When we upgrade to the new version of knative, by applying the YAML of the new version, operator can help with is to remove the obsolete resources in the old version. Operator needs the upgrade tests as well to verify the upgrade process is successful or not.

[cardil]

@houshengbo Prober will be in general code, not _test, so it will be easy to vendor it in operator and invoke prober directly in operator e2e test.

Sneak peak can be seen here: master...cardil:feature/upgrade-tests