Upgrade tests pluggable test suite

[cardil]

Problem

Upgrade tests are located in Serving, Eventing, and also a merger of those in Operator. All those utilize Bash scripts that perform upgrade & downgrade, invoke smoke tests, and orchestrate continues assertion on requests or events in background. This follow up on knative/serving#9137.

Possible solution

I propose to create a pluggable test suite for upgrade tests. That upgrade suite might be located in knative.dev/pkg/test/upgrade package. That test suite should be configurable, so that can be used for upgrade testing in Knative Serving, and Eventing, and Operator. Different configuration can be used by vendors that needs this to be slightly different.

Consider the example of pseudo code below for Serving upgrades:

// +build e2e
// Serving upgrade test

package e2e

import (
  "knative.dev/pkg/test/upgrade"
  serving "knative.dev/serving/test/upgrade"
)

func TestServingUpgrades(t *testing.T) {
  suite := upgrade.NewBuilder(t).
    PreUpgradeTest(serving.PreUpgradeTest).
    PostUpgradeTest(serving.PostUpgradeTest).
    PostDowngradeTest(serving.PostDowngradeTest).
    ProbeTest(serving.ProbeTest).
    BaseInstall(serving.LatestReleaseInstall).
    UpgradeWith(serving.HeadInstall).
    DowngradeWith(serving.LatestReleaseInstall).
    Build()
  suite.Execute()
}

And Eventing upgrades:

// +build e2e
// Eventing upgrade test

package e2e

import (
  "knative.dev/pkg/test/upgrade"
  eventing "knative.dev/eventing/test/upgrade"
)

func TestEventingUpgrades(t *testing.T) {
  suite := upgrade.NewBuilder(t).
    PreUpgradeTest(eventing.PreUpgradeTest).
    PostUpgradeTest(eventing.PostUpgradeTest).
    PostDowngradeTest(eventing.PostDowngradeTest).
    ProbeTest(eventing.ProbeTest).
    BaseInstall(eventing.LatestReleaseInstall).
    UpgradeWith(eventing.HeadInstall).
    DowngradeWith(eventing.LatestReleaseInstall).
    Build()
  suite.Execute()
}

And Knative Operator upgrades test might then look like:

// +build e2e
// Operator upgrade test

package e2e

import (
  "knative.dev/pkg/test/upgrade"
  serving "knative.dev/serving/test/upgrade"
  eventing "knative.dev/eventing/test/upgrade"
)

func TestOperatorUpgrades(t *testing.T) {
  suite := upgrade.NewBuilder(t).
    PreUpgradeTest(serving.PreUpgradeTest).
    PreUpgradeTest(eventing.PreUpgradeTest).
    PostUpgradeTest(serving.PostUpgradeTest).
    PostUpgradeTest(eventing.PostUpgradeTest).
    PostDowngradeTest(serving.PostDowngradeTest).
    PostDowngradeTest(eventing.PostDowngradeTest).
    ProbeTest(serving.ProbeTest).
    ProbeTest(eventing.ProbeTest).
    BaseInstall(serving.LatestReleaseInstall).
    BaseInstall(eventing.LatestReleaseInstall).
    UpgradeWith(serving.HeadInstall).
    UpgradeWith(eventing.HeadInstall).
    DowngradeWith(serving.LatestReleaseInstall).
    DowngradeWith(eventing.LatestReleaseInstall).
    Build()
  suite.Execute()
}

As a final example Openshift Serverless example:

// +build e2e
// Serverless upgrade test

package e2e

import (
  "knative.dev/pkg/test/upgrade"
  serving "knative.dev/serving/test/upgrade"
  eventing "knative.dev/eventing/test/upgrade"
  serverless "github.com/openshift-knative/serverless-operator/test/upgrade"
  "github.com/openshift-knative/serverless-operator/test/upgrade/ocp"
)

func TestServerlessOperatorUpgrades(t *testing.T) {
  suite := upgrade.NewBuilder(t).
    PreUpgradeTest(serving.PreUpgradeTest).
    PreUpgradeTest(eventing.PreUpgradeTest).
    PostUpgradeTest(serving.PostUpgradeTest).
    PostUpgradeTest(eventing.PostUpgradeTest).
    PostDowngradeTest(serving.PostDowngradeTest).
    PostDowngradeTest(eventing.PostDowngradeTest).
    ProbeTest(serving.ProbeTest).
    ProbeTest(eventing.ProbeTest).
    BaseInstall(serverless.LatestReleaseInstall).
    UpgradeWith(ocp.Latest).
    UpgradeWith(serverless.HeadInstall).
    // No downgrade in this test...
    Build()
  suite.Execute()
}

[chizhg]

This code structure looks good to me - we have the general upgrade testing framework defined in pkg, and each project is responsible for defining their own PreUpgrade, PostUpgrade and PostDowngrade logic and inject into the testing framework. We can also automatically get proper versioning as the code is shipped as part of each project.

[coryrc]

You'll need to pull all the tests out of test into modules others can include. Will go test still work? How are you going to provide junit xml for TestGrid/Prow to report results? what if you want to change the test flow -- either all of those features are baked in or you have to start from scratch rewriting?

And does this only help with upgrade tests and not the others? Is it worth having upgrades be different from the other tests too?

[cardil]

Rply @coryrc:

You'll need to pull all the tests out of test into modules others can include. Will go test still work?

In _test.go files only the main method would reside (like TestOperatorUpgrades(t *testing.T)). Other methods like PreUpgradeTest would be located in regular code in package like knative.dev/eventing/test/upgrade. That would let us reuse them easily.

For clarity here is PreUpgradeTest interface:

type UpgradeTestsElement interface {
  Name()    string
  Handler() func(t *testing.T)
}

How are you going to provide junit xml for TestGrid/Prow to report results?

That is easy as well. The main test method will be recorded, just as all others do. I mean, for example TestOperatorUpgrades(t *testing.T) will be reported. I would also add, that all registered plugins will run in itself block of t.Run("<plugin-name>", func (t *testing.T) { }) and that will give us even greater know how on what is happening. To rephrase that, the jUnit XML reports will be reported for all of test's components.

what if you want to change the test flow -- either all of those features are baked in or you have to start from scratch rewriting?

In scenario that you would like to change the upgrade test flow, you would just change this testing framework. Then all repos that use it will need to adjust to the changes. That must happen when they decide to consume new version of knative.dev/pkg.

And does this only help with upgrade tests and not the others? Is it worth having upgrades be different from the other tests too?

Originally, I thought only about upgrades. But, after your comment I realized that this can be generalized, in a way that user may like to provide a flow, and register plugins for it. It may look like that:

// +build e2e
// Operator upgrade test

package e2e

import (
  "knative.dev/pkg/test/flow"
  "knative.dev/pkg/test/upgrade"
  serving "knative.dev/serving/test/upgrade"
  eventing "knative.dev/eventing/test/upgrade"
)

func TestOperatorUpgrades(t *testing.T) {
  suite := flow.NewBuilder(t).
    WithFlow(upgrade.Flow).
    WithPlugins(
      serving.PreUpgradeTest,
      eventing.PreUpgradeTest,
      serving.PostUpgradeTest,
      eventing.PostUpgradeTest,
      serving.PostDowngradeTest,
      eventing.PostDowngradeTest,
      serving.ProbeTest,
      eventing.ProbeTest,
      serving.LatestReleaseInstall,
      eventing.LatestReleaseInstall,
      serving.HeadInstall,
      eventing.HeadInstall,
      serving.LatestReleaseInstall,
      eventing.LatestReleaseInstall,
    ).Build()
  suite.Execute()
}

That may be handy in other situations too. On the other hand it's less readable. This kind of generalization can be implemented in the future, and my original proposal (upgrade.NewBuilder(t)) can be written then to use that generalized flow.

[mgencur]

I like the first solution from @cardil which is specific for upgrades/downgrades. The skeleton of the upgrade/downgrade tests has evolved over time and doesn't seem to be changing that much.

It's important that users can debug/step through the source code of tests and the plugins so I'm not sure we can orchestrate the flow using Tekton.

Regarding kntest, it sounds like kntest could implement plugins for "base install", "upgrade", "downgrade" which could be used in this flow as plugins (as long as kntest provide Go functions to the outside world). I'm not sure what's the current shape of kntest but I haven't seen much code in https://github.com/knative/test-infra/blob/master/kntest . Perhaps it would make sense to implement this flow in knative.dev/pkg and later possibly move to kntest? With the current shape of kntest it's not clear how these could be integrated together at this point.

Side note for the pseudo-code above: In order to simplify the API one could also use functions with variable length parameters and define multiple plugins for the given phase at once.

func TestOperatorUpgrades(t *testing.T) {
  suite := upgrade.NewBuilder(t).
    PreUpgradeTests(serving.PreUpgradeTest, eventing.PreUpgradeTest).
    PostUpgradeTests(serving.PostUpgradeTest, eventing.PostUpgradeTest).
    PostDowngradeTests(serving.PostDowngradeTest, eventing.PostDowngradeTest).
    ContinualTests(serving.ProbeTest, eventing.ProbeTest).   //call this phase other than "prober" to make it more general
    BaseInstalls(serving.LatestReleaseInstall, eventing.LatestReleaseInstall).
    UpgradeWith(serving.HeadInstall, eventing.HeadInstall).
    DowngradeWith(serving.LatestReleaseInstall, eventing.LatestReleaseInstall).
    Build()
  suite.Execute()
}

Edit: This might be closer to idiomatic Golang code:

suite := UpgradeSuite{
  PreUpgradeTests: []UpgradeTestElement{ serving.PreUpgradeTest, eventing.PreUpgradeTest },
  PostUpgradeTests: []UpgradeTestElement{ serving.PostUpgradeTest, eventing.PostUpgradeTest },
  PostDowngradeTests: []UpgradeTestElement{ serving.PostDowngradeTest, 
eventing.PostDowngradeTest},
  ContinualTests: []UpgradeTestElement{ serving.ProbeTest, eventing.ProbeTest},   //call this phase other than "prober" to make it more general
  BaseInstalls: []UpgradeTestElement{ serving.LatestReleaseInstall, eventing.LatestReleaseInstall},
  UpgradeWith: []UpgradeTestElement{ serving.HeadInstall, eventing.HeadInstall},
  DowngradeWith: []UpgradeTestElement{ serving.LatestReleaseInstall, eventing.LatestReleaseInstall},
}
suite.Execute()

[cardil]

/assign

[coryrc]

Sorry I missed your response.

You'll need to pull all the tests out of test into modules others can include. Will go test still work?

In _test.go files only the main method would reside (like TestOperatorUpgrades(t *testing.T)).

So the contents of all existing Test*(t) functions will be brought into non _test packages, then the Test*() will be one line calling the new function. You could miss coverage but it'd be easy to do and verbose to write but that sounds like golang.

what if you want to change the test flow

In scenario that you would like to change the upgrade test flow, you would just change this testing framework. Then all repos that use it will need to adjust to the changes. That must happen when they decide to consume new version of knative.dev/pkg.

That is my worry; today when serving needs to customize flow they can hack a bunch of scripts in /test. (Not ideal, but it works). I worry this will prove to be too inflexible.

I assume the Continual tests are designed to run continuously during upgrades -- once you have more than the parallel limit (which is under the control of the user/machine) then won't you get the wrong behavior?

[mgencur]

what if you want to change the test flow

In scenario that you would like to change the upgrade test flow, you would just change this testing framework. Then all repos that use it will need to adjust to the changes. That must happen when they decide to consume new version of knative.dev/pkg.

That is my worry; today when serving needs to customize flow they can hack a bunch of scripts in /test. (Not ideal, but it works). I worry this will prove to be too inflexible.

IMO, the framework has a great deal of flexibility in itself: Individual stages with tests are implemented in the repository under test (e.g. Serving) - they can customize which tests are running, or add more tests. I believe they could also customize the upgrade/downgrade by implementing their own version of the procedure rather than re-using a common impl.
OTOH, if there are more drastic changes required to the framework like adding stages - they're likely to be common for all repos using this framework (Serving, Eventing, operators). But, the basic stages of upgrade tests seem to be stable for some time.