enable broker chaos, add debug logging for tests for MT Broker. #3599

vaikas · 2020-07-15T21:39:57Z

Fixes #3591

Proposed Changes

Enable logging to be configured for e2e-tests

Release Note

Docs

test/config/chaosduck.yaml

test/e2e-common.sh

vaikas · 2020-07-17T22:40:15Z

monitoring didn't come up.

vaikas · 2020-07-17T22:45:05Z

/test pull-knative-eventing-integration-tests

vaikas · 2020-07-26T15:34:17Z

worked, retesting
/test pull-knative-eventing-integration-tests

vaikas · 2020-07-27T05:44:22Z

worked, retesting
/test pull-knative-eventing-integration-tests

knative-metrics-robot · 2020-07-27T17:05:49Z

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/mtbroker/broker.go	86.4%	84.3%	-2.0
pkg/reconciler/mtbroker/controller.go	85.2%	90.9%	5.7
pkg/reconciler/mtbroker/trigger/controller.go	Do not exist	100.0%
pkg/reconciler/mtbroker/trigger/trigger.go	Do not exist	78.6%

vaikas · 2020-07-27T18:30:06Z

Latest run found an interesting failure (not related to chaos):

    broker_control_plane_test_helper.go:135: Error: Could not update trigger trigger-no-broker: Operation cannot be fulfilled on triggers.eventing.knative.dev "trigger-no-broker": the object has been modified; please apply your changes to the latest version and try again
        --- FAIL: TestBrokerV1Beta1ControlPlane/Channel-messaging.knative.dev/v1beta1/Ready_Trigger_V1Beta1_(no_Broker)_set_Broker_and_includes_status.subscriber_Uri (0.16s)

vaikas · 2020-07-27T18:59:24Z

/test pull-knative-eventing-integration-tests
Seemingly unrelated failures, filed issues for those

knative-test-reporter-robot · 2020-07-27T19:43:17Z

The following jobs failed:

Test name	Triggers	Retries
pull-knative-eventing-integration-tests		0/3

Failed non-flaky tests preventing automatic retry of pull-knative-eventing-integration-tests:

test/e2e.TestBrokerChannelFlowTriggerV1Beta1BrokerV1Beta1
test/e2e.TestBrokerChannelFlowTriggerV1Beta1BrokerV1Beta1/InMemoryChannel-messaging.knative.dev/v1
test/e2e.TestChannelClusterDefaulter/InMemoryChannel-messaging.knative.dev/v1

pkg/reconciler/mtbroker/broker.go

n3wscott · 2020-07-27T20:07:49Z

pkg/reconciler/mtbroker/trigger/controller.go

+	r.kresourceTracker = duck.NewListableTracker(ctx, conditions.Get, impl.EnqueueKey, controller.GetTrackerLease(ctx))
+	r.uriResolver = resolver.NewURIResolver(ctx, impl.EnqueueKey)
+
+	triggerInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))


I think you will also need to watch for broker changes and enqueue yourself if the class match.

if you make the trigger before you make the broker, you will have to wait an entire relist period for the trigger to go ready or even reconcile.

ah, you are tracking brokers in the reconciler now. That works for all cases except where you create triggers first and the broker is not around yet.

n3wscott · 2020-07-27T20:09:03Z

pkg/reconciler/mtbroker/trigger/trigger.go

 	t.Status.InitializeConditions()

 	if t.DeletionTimestamp != nil {
 		// Everything is cleaned up by the garbage collector.
 		return nil
 	}
+	// Start tracking the broker
+	r.trackBroker(ctx, t)


oh! I see, you are going to track the broker directly. That works too.

I think you have a race condition for late bound triggers without any brokers in the cluster yet.

pkg/reconciler/mtbroker/trigger/trigger.go

vaikas · 2020-07-28T11:23:38Z

Channel creation failed with:

    creation.go:110: Failed to create channel "e2e-defaulter-channel": Internal error occurred: failed calling webhook "webhook.eventing.knative.dev": Post https://eventing-webhook.knative-eventing-0xrp4u7way.svc:443/defaulting?timeout=2s: EOF

Fixed here:
#3734

knative-metrics-robot · 2020-07-28T14:55:05Z

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/mtbroker/broker.go	86.4%	84.3%	-2.0
pkg/reconciler/mtbroker/controller.go	85.2%	90.9%	5.7
pkg/reconciler/mtbroker/trigger/controller.go	Do not exist	100.0%
pkg/reconciler/mtbroker/trigger/trigger.go	Do not exist	78.6%

vaikas · 2020-07-28T15:37:43Z

worked, retesting
/test pull-knative-eventing-integration-tests

n3wscott · 2020-07-28T16:32:48Z

/lgtm
/approve

knative-prow-robot · 2020-07-28T16:32:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n3wscott, vaikas

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [n3wscott,vaikas]

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

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jul 15, 2020

knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 15, 2020

knative-prow-robot requested review from lionelvillard and yt3liu July 15, 2020 21:40

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

mattmoor reviewed Jul 15, 2020

View reviewed changes

test/config/chaosduck.yaml Outdated Show resolved Hide resolved

test/e2e-common.sh Show resolved Hide resolved

vaikas force-pushed the broker-chaos branch from e90c689 to 8143d95 Compare July 15, 2020 21:49

mattmoor reviewed Jul 15, 2020

View reviewed changes

test/e2e-common.sh Outdated Show resolved Hide resolved

vaikas mentioned this pull request Jul 15, 2020

use configmaplister #3600

Merged

knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 15, 2020

vaikas force-pushed the broker-chaos branch from 970d380 to de0ec02 Compare July 16, 2020 00:23

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 16, 2020

vaikas changed the title ~~enable broker, use cm lister, prune trigger status~~ enable broker chaos, add debug logging for tests for MT Broker. Jul 16, 2020

vaikas force-pushed the broker-chaos branch from de0ec02 to 9b29139 Compare July 16, 2020 19:22

vaikas force-pushed the broker-chaos branch from 3210888 to 921c32e Compare July 17, 2020 19:49

knative-prow-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 17, 2020

vaikas force-pushed the broker-chaos branch from 7d0650f to db2334c Compare July 27, 2020 17:00

vaikas mentioned this pull request Jul 27, 2020

Test fails without retrying on concurrent modifications. #3730

Closed

n3wscott reviewed Jul 27, 2020

View reviewed changes

pkg/reconciler/mtbroker/broker.go Show resolved Hide resolved

n3wscott reviewed Jul 27, 2020

View reviewed changes

vaikas added 10 commits July 28, 2020 07:49

rebased

b38b9f2

collapse commits

8d9098a

5/10 -> 10/20 for client qps

ceb0fe3

try with no event for finalizer

21b667d

bump values up

a9adc08

just testing, checkpoints...

9643222

use tracker instead

f47a5a0

use defaults

ffdfd28

Add unit tests

74856f3

revert the no finalizer event

f752fdc

vaikas force-pushed the broker-chaos branch from db2334c to f752fdc Compare July 28, 2020 14:49

knative-prow-robot assigned n3wscott Jul 28, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2020

knative-prow-robot merged commit 3a5164c into knative:master Jul 28, 2020

vaikas deleted the broker-chaos branch July 29, 2020 05:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable broker chaos, add debug logging for tests for MT Broker. #3599

enable broker chaos, add debug logging for tests for MT Broker. #3599

vaikas commented Jul 15, 2020 •

edited

Loading

vaikas commented Jul 17, 2020

vaikas commented Jul 17, 2020

vaikas commented Jul 26, 2020

vaikas commented Jul 27, 2020

knative-metrics-robot commented Jul 27, 2020

vaikas commented Jul 27, 2020

vaikas commented Jul 27, 2020

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

n3wscott Jul 27, 2020

n3wscott Jul 27, 2020

n3wscott Jul 27, 2020

n3wscott Jul 27, 2020

n3wscott Jul 27, 2020

vaikas commented Jul 28, 2020

knative-metrics-robot commented Jul 28, 2020

vaikas commented Jul 28, 2020

n3wscott commented Jul 28, 2020

knative-prow-robot commented Jul 28, 2020

enable broker chaos, add debug logging for tests for MT Broker. #3599

enable broker chaos, add debug logging for tests for MT Broker. #3599

Conversation

vaikas commented Jul 15, 2020 • edited Loading

Proposed Changes

vaikas commented Jul 17, 2020

vaikas commented Jul 17, 2020

vaikas commented Jul 26, 2020

vaikas commented Jul 27, 2020

knative-metrics-robot commented Jul 27, 2020

vaikas commented Jul 27, 2020

vaikas commented Jul 27, 2020

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

n3wscott Jul 27, 2020

Choose a reason for hiding this comment

n3wscott Jul 27, 2020

Choose a reason for hiding this comment

n3wscott Jul 27, 2020

Choose a reason for hiding this comment

n3wscott Jul 27, 2020

Choose a reason for hiding this comment

n3wscott Jul 27, 2020

Choose a reason for hiding this comment

vaikas commented Jul 28, 2020

knative-metrics-robot commented Jul 28, 2020

vaikas commented Jul 28, 2020

n3wscott commented Jul 28, 2020

knative-prow-robot commented Jul 28, 2020

vaikas commented Jul 15, 2020 •

edited

Loading