Add upgrade tests into the e2e-tests-latest-eventing by houshengbo · Pull Request #93 · knative/eventing-operator

houshengbo · 2020-02-05T16:15:56Z

Issue to be fixed

Fixes #94

Proposed Changes

This PR adds the tests to verify the correct number and names of knative eventing
deployments.
The test tag postupgrade is added, marking the tests to run after upgrade to the
latest HEAD of operator, with the latest generated manifest of knative eventing.

Release Note

knative-prow-robot

@houshengbo: 0 warnings.

Details

In response to this:

This PR adds the tests to verify the correct number and names of knative eventing
deployments.

The test tag postupgrade is added, marking the tests to run after upgrade to the
latest HEAD of operator, with the latest generated manifest of knative eventing.

Issue to be fixed

Fixes #

Proposed Changes

Release Note

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

knative-prow-robot · 2020-02-05T16:16:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: houshengbo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [houshengbo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

houshengbo · 2020-02-05T16:20:06Z

/test pull-knative-eventing-operator-integration-tests-on-latest-eventing

houshengbo · 2020-02-05T16:41:36Z

/test pull-knative-eventing-operator-integration-tests-on-latest-eventing

aliok · 2020-02-06T07:57:45Z

cc @cardil you might want to have a look perhaps?

cardil · 2020-02-06T13:42:21Z

Doesn't this upgrade test should mirror the knative/eventing#2388 ? Now, it only runs some simple test after upgrade.

In knative/eventing#2388 I'm doing upgrade & downgrade, while validating that running knative services (probe), will still run, and we will not lose any events during upgrade & downgrade.

houshengbo · 2020-02-06T14:46:00Z

@cardil That's the goal as well, so far we add some tests to verify the deployments of the knatvie eventing in a macro scope from the operator's perspective. We will ultamately own the tests in eventing with the tag preupgrade, and postupgrade.
Also, we also move on with serving-operator to add the upgrade flow as well, so all the upgrade tests will live in operators in future.

aliok · 2020-02-07T12:44:03Z

test/e2e-tests-latest-eventing.sh

-# Run the integration tests
-go_test_e2e -timeout=20m ./test/e2e || failed=1
+# Run the postupgrade tests
+go_test_e2e -tags=postupgrade -timeout=${TIMEOUT} ./test/upgrade || failed=1


+1 on this. Also good that we're changing the name of this file with #96

aliok · 2020-02-07T12:52:54Z

@houshengbo while working on this, I found some errors in the .sh file and fixed them.
Here's the PR that is sent against your PR branch: houshengbo#1

Actually, those improvements are applicable to other scripts in this and other Knative repos, but let me send PRs for them separately. Please leave that to me.

aliok · 2020-02-07T13:13:57Z

test/upgrade/knativeeventingupgrade_test.go

+	// Knative Eventing has 4 deployments.
+	expectedNumDeployments := 4
+	deploys := []string{"eventing-controller", "eventing-webhook", "imc-controller", "imc-dispatcher"}


After knative/eventing#2519 this is changed

// Knative Eventing has 5 deployments. expectedNumDeployments := 5 deploys := []string{"eventing-controller", "broker-controller", "eventing-webhook", "imc-controller", "imc-dispatcher"}

aliok

Needs some adaption. Added a few comments.

aliok · 2020-02-07T13:29:44Z

@houshengbo

Added some comments.

IMO, let's merge #95 first. Then adapt this PR (see my reasons below)

My reasons:

I have some doubts about the purpose of this exercise.

e2e-tests.sh is using the release manifest in kodata. So:
- it starts the operator
- installs Knative eventing using the 0.12 manifest
- doing some tests (like deployment recreated when deleted etc.)
e2e-tests-latest-eventing.sh which uses the manifest from master of Knative eventing.
- it starts the operator
- installs Knative eventing using the manifest from master
- doing some tests like (checking number of deployments and expected deployments etc.)

As we're going test the upgrade of the operator, we need the resources created by a previous version of the operator. Simply pointing the operator to old manifest won't work because in #95 we're deleting obsolote resources by name.

So, IMO, the process should be:

Deploy the latest released version of operator
Create CR, so that old Knative eventing version is created (with obsolote resources)
Delete the operator and deploy the operator from local code
Check expected resources and new resources (obsolote things should be gone, new things should be there)

IMO, let's merge #95 first. Then adapt this PR.

About @cardil 's comments: let's do them later in sync with @cardil

houshengbo · 2020-02-07T20:02:06Z

I have renamed the script from e2e-tests-latest-eventing.sh to e2e-upgrade-tests.sh, which is dedicated to run the upgrade tests for operator, including the operator itself and the knative.

Prow job has been created to run the upgrade tests as well in test-infra.

k4leung4 · 2020-02-12T16:20:14Z

test/client/setup.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2019 The Knative Authors
+Copyright 2020 The Knative Authors


in the future, this doesnt need to changed
the copyright year is based on when the file is first added.

k4leung4 · 2020-02-12T16:20:34Z

test/e2e-upgrade-tests.sh

+  header "Installing Knative Eventing operator latest public release"
+  local full_url="https://github.com/knative/eventing-operator/releases/download/${LATEST_EVENTING_RELEASE_VERSION}/eventing-operator.yaml"
+
+  local RELEASE_YAML="$(mktemp)"


lower case
release_yaml

k4leung4 · 2020-02-12T16:21:27Z

test/e2e/knativeeventingdeployment_test.go

-	"errors"
-	"path/filepath"
-	"runtime"
+	"knative.dev/eventing-operator/test/client"


group with other knative.dev/eventing-operator imports

This PR adds the tests to verify the correct number and names of knative eventing deployments. The test tag postupgrade is added, marking the tests to run after upgrade to the latest HEAD of operator, with the latest generated manifest of knative eventing.

k4leung4 · 2020-02-13T21:37:35Z

/lgtm

aliok · 2020-02-14T12:59:01Z

I finally had time to review this PR after the changes.

Great work @houshengbo , thank you!

* Add upgrade tests into the e2e-tests-latest-eventing This PR adds the tests to verify the correct number and names of knative eventing deployments. The test tag postupgrade is added, marking the tests to run after upgrade to the latest HEAD of operator, with the latest generated manifest of knative eventing. * Add the installation of the previous release and upgrade to the latest * Fixes this PR based on the latest comments

* Build config consistently as per in serving (#105) * Add upgrade tests into the e2e-tests-latest-eventing (#93) * Add upgrade tests into the e2e-tests-latest-eventing This PR adds the tests to verify the correct number and names of knative eventing deployments. The test tag postupgrade is added, marking the tests to run after upgrade to the latest HEAD of operator, with the latest generated manifest of knative eventing. * Add the installation of the previous release and upgrade to the latest * Fixes this PR based on the latest comments * Add additionalPrinterColumns (#98) * Update the library from jcrossley3/manifestival to manifestival Co-authored-by: Ali Ok <[email protected]>

knative-prow-robot added the size/L label Feb 5, 2020

knative-prow-robot reviewed Feb 5, 2020

View reviewed changes

mattmoor reviewed Feb 5, 2020

View reviewed changes

knative-prow-robot requested review from bbrowning and matzew February 5, 2020 16:16

knative-prow-robot added the approved label Feb 5, 2020

houshengbo force-pushed the upgrade-test branch from f412603 to a3e0592 Compare February 5, 2020 16:16

mattmoor reviewed Feb 5, 2020

View reviewed changes

houshengbo mentioned this pull request Feb 5, 2020

Removal of sources-controller after 0.12.0 #90

Closed

houshengbo force-pushed the upgrade-test branch from a3e0592 to 4863a18 Compare February 5, 2020 16:40

aliok reviewed Feb 7, 2020

View reviewed changes

aliok suggested changes Feb 7, 2020

View reviewed changes

aliok mentioned this pull request Feb 7, 2020

Remove obsolete resources caused by merging controllers #95

Merged

houshengbo force-pushed the upgrade-test branch 5 times, most recently from 4d8bc1e to 2073e44 Compare February 11, 2020 16:55

k4leung4 reviewed Feb 12, 2020

View reviewed changes

Add the installation of the previous release and upgrade to the latest

51b5665

houshengbo force-pushed the upgrade-test branch from 2073e44 to 388f348 Compare February 12, 2020 22:39

Fixes this PR based on the latest comments

43d7852

houshengbo force-pushed the upgrade-test branch from 388f348 to 43d7852 Compare February 12, 2020 22:46

knative-prow-robot assigned k4leung4 Feb 13, 2020

knative-prow-robot added the lgtm label Feb 13, 2020

knative-prow-robot merged commit 723874e into knative:master Feb 13, 2020

Conversation

houshengbo commented Feb 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue to be fixed

Proposed Changes

Release Note

Uh oh!

knative-prow-robot left a comment

Choose a reason for hiding this comment

Issue to be fixed

Proposed Changes

Release Note

Uh oh!

knative-prow-robot commented Feb 5, 2020

Uh oh!

houshengbo commented Feb 5, 2020

Uh oh!

houshengbo commented Feb 5, 2020

Uh oh!

aliok commented Feb 6, 2020

Uh oh!

cardil commented Feb 6, 2020

Uh oh!

houshengbo commented Feb 6, 2020

Uh oh!

aliok Feb 7, 2020

Choose a reason for hiding this comment

Uh oh!

aliok commented Feb 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aliok Feb 7, 2020

Choose a reason for hiding this comment

Uh oh!

houshengbo Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

aliok left a comment

Choose a reason for hiding this comment

Uh oh!

aliok commented Feb 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

houshengbo commented Feb 7, 2020

Uh oh!

k4leung4 Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

k4leung4 Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

k4leung4 Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

k4leung4 commented Feb 13, 2020

Uh oh!

aliok commented Feb 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

houshengbo commented Feb 5, 2020 •

edited

Loading

aliok commented Feb 7, 2020 •

edited

Loading

aliok commented Feb 7, 2020 •

edited

Loading