Skip to content

Use the standard resource creation libs for upgrade tests.#3539

Merged
knative-prow-robot merged 3 commits into
knative:masterfrom
vaikas:issue-3538-testing
Jul 8, 2020
Merged

Use the standard resource creation libs for upgrade tests.#3539
knative-prow-robot merged 3 commits into
knative:masterfrom
vaikas:issue-3538-testing

Conversation

@vaikas
Copy link
Copy Markdown
Contributor

@vaikas vaikas commented Jul 7, 2020 •
edited
Loading

Address #3538

Proposed Changes

  • Use the creation.*OrFail methods and dump resources via tracker.
  • Create the Broker directly without using the sugar annotations.

Release Note

Docs

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jul 7, 2020
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 7, 2020
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Jul 7, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vaikas

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:

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

@knative-prow-robot knative-prow-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release labels Jul 7, 2020
@knative-test-reporter-robot
Copy link
Copy Markdown

knative-test-reporter-robot commented Jul 8, 2020

The following jobs failed:

Test name Triggers Retries
pull-knative-eventing-upgrade-tests pull-knative-eventing-upgrade-tests
pull-knative-eventing-upgrade-tests
pull-knative-eventing-upgrade-tests
pull-knative-eventing-upgrade-tests		 3/3

Job pull-knative-eventing-upgrade-tests expended all 3 retries without success.

@cardil
Copy link
Copy Markdown
Contributor

cardil commented Jul 8, 2020

I said I can take a look at this shortly, but I've meant in a couple of days time. I have other tasks, higher profile ones, assigned currently. You've started working on this. So, if you like to work on it, feel free. Otherwise, wait couple days, until I can go back to it.

About this PR. General direction is good. I would like to say that I written the waits, as it is, to execute as much resource setup steps as possible in parallel. This parallel execution is there to speedup the whole setup process. It would be good to keep that behavior.

@vaikas
Copy link
Copy Markdown
Contributor Author

vaikas commented Jul 8, 2020

regarding the parallel creation, it seems we create 4 resources, and overall, we're talking about 15 seconds overall for tests that take 20+ minutes to run, so to me the benefit of getting:

  • standard way to stand up the resources, from coding perspective (so if I switch from e2e to upgrade, things look the same)
  • clear logs of the resources, as well as k8s events

outweigh the complexity and loss of debuggability when things go wrong.

Interesting bit of the latest run too, btw:

2020-07-08T18:13:38.448Z	INFO	prober/verify.go:39	Fetched receiver report. Events propagated: 1416. State: success
    TestContinuousEventsPropagationWithProber: prober.go:93: All 1416 events propagated well

@vaikas vaikas changed the title try to dump some state Use the standard resource creation libs for upgrade tests. Jul 8, 2020
@vaikas
Copy link
Copy Markdown
Contributor Author

vaikas commented Jul 8, 2020

@googlebot

@chaodaiG
Copy link
Copy Markdown
Contributor

chaodaiG commented Jul 8, 2020

/test pull-knative-eventing-build-tests pull-knative-eventing-upgrade-tests

@chaodaiG
Copy link
Copy Markdown
Contributor

chaodaiG commented Jul 8, 2020

/test pull-knative-eventing-upgrade-tests

@lionelvillard
Copy link
Copy Markdown
Contributor

lionelvillard commented Jul 8, 2020

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2020
@knative-prow-robot knative-prow-robot merged commit c8aa486 into knative:master Jul 8, 2020
@vaikas vaikas deleted the issue-3538-testing branch July 8, 2020 20:31
This was referenced Jul 9, 2020
Closed
Merged
knative-prow-robot pushed a commit that referenced this pull request Jul 12, 2020
@vayyappaneni @mattmoor
adding istion annotation (#3534) (#3557)
acdd118 
* adding istion annotation (#3534)

* backporting #3543

* Update test/lib/duck/resource_inspectors.go

Co-authored-by: Matt Moore <mattmoor@vmware.com>

* Use the standard resource creation libs for upgrade tests. (#3539)

* try to dump some state

* use client wait or fail

* create broker directly

* removed the unused import

Co-authored-by: Matt Moore <mattmoor@vmware.com>
matzew pushed a commit to matzew/eventing that referenced this pull request Sep 30, 2020
@vayyappaneni @mattmoor @matzew
adding istion annotation (knative#3534) (knative#3557)
555f645 
* adding istion annotation (knative#3534)

* backporting knative#3543

* Update test/lib/duck/resource_inspectors.go

Co-authored-by: Matt Moore <mattmoor@vmware.com>

* Use the standard resource creation libs for upgrade tests. (knative#3539)

* try to dump some state

* use client wait or fail

* create broker directly

* removed the unused import

Co-authored-by: Matt Moore <mattmoor@vmware.com>
matzew pushed a commit to matzew/eventing that referenced this pull request Oct 1, 2020
@vayyappaneni @mattmoor @matzew
adding istion annotation (knative#3534) (knative#3557)
6c2808e 
* adding istion annotation (knative#3534)

* backporting knative#3543

* Update test/lib/duck/resource_inspectors.go

Co-authored-by: Matt Moore <mattmoor@vmware.com>

* Use the standard resource creation libs for upgrade tests. (knative#3539)

* try to dump some state

* use client wait or fail

* create broker directly

* removed the unused import

Co-authored-by: Matt Moore <mattmoor@vmware.com>
openshift-merge-robot pushed a commit to openshift/knative-eventing that referenced this pull request Oct 7, 2020
@matzew @vayyappaneni
@mattmoor @vaikas
Add backports form 0.16.x releases (#864)
35d2c08 
* adding istion annotation (knative#3534) (knative#3557)

* adding istion annotation (knative#3534)

* backporting knative#3543

* Update test/lib/duck/resource_inspectors.go

Co-authored-by: Matt Moore <mattmoor@vmware.com>

* Use the standard resource creation libs for upgrade tests. (knative#3539)

* try to dump some state

* use client wait or fail

* create broker directly

* removed the unused import

Co-authored-by: Matt Moore <mattmoor@vmware.com>

* cp 3897 (knative#3902)

* cp 3897

* backport 3719

* x bit

* apiserversource v1beta1->v1alpha2

* install broker / sugar

* backport 3945 to .16 (knative#3948)

Co-authored-by: VijayaSenaReddy Ayyappaneni <48216363+vayyappaneni@users.noreply.github.com>
Co-authored-by: Matt Moore <mattmoor@vmware.com>
Co-authored-by: Ville Aikas <11279988+vaikas@users.noreply.github.com>
@cardil cardil mentioned this pull request Oct 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chaodaiG chaodaiG Awaiting requested review from chaodaiG

@steuhs steuhs Awaiting requested review from steuhs

Assignees

@lionelvillard lionelvillard

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@vaikas @knative-prow-robot @knative-test-reporter-robot @cardil @chaodaiG @lionelvillard @googlebot