Add blue/green conformance test by jonjohnsonjr · Pull Request #1460 · knative/serving

jonjohnsonjr · 2018-07-03T06:26:04Z

Fixes #1416

Based on #1458

The test:

Create config pointing first image.
Get the revision name, this is the blue revision.
Update the config, pointing to second image.
Get the revision name, this is the green revision.
Create a Route that points 50/50 to blue and green revision.
Spin up goroutines to make 100 requests against:
1. blue revision
2. green revision
3. route
For 1/2, assert that each request is handled by the appropriate image.
For 3, assert that we get a reasonable distribution of responses (at worst 25/75 split).

This has exposed some less-than-perfect performance. Many requests return 404s :(

jonjohnsonjr · 2018-07-03T07:02:25Z

/assign @mattmoor

jonjohnsonjr · 2018-07-03T16:14:08Z

Sample output.

$ go test -v -tags=e2e -count=1 ./test/conformance/ -run TestBlueGreenRoute
info	logging/config.go:80	Successfully created the logger.	{"knative.dev/jsonconfig": "{\n\t  \"level\": \"info\",\n\t  \"encoding\": \"console\",\n\t  \"outputPaths\": [\"stdout\"],\n\t  \"errorOutputPaths\": [\"stderr\"],\n\t  \"encoderConfig\": {\n\t    \"messageKey\": \"message\",\n\t\t\t\"levelKey\": \"level\",\n\t\t\t\"nameKey\": \"logger\",\n\t\t\t\"callerKey\": \"caller\",\n\t\t\t\"messageKey\": \"msg\",\n      \"stacktraceKey\": \"stacktrace\",\n      \"lineEnding\": \"\",\n      \"levelEncoder\": \"\",\n      \"timeEncoder\": \"\",\n      \"durationEncoder\": \"\",\n      \"callerEncoder\": \"\"\n\t  }\n\t}"}
info	logging/config.go:81	Logging level set to info
=== RUN   TestBlueGreenRoute
info	TestBlueGreenRoute	test/crd.go:141	Seeding rand.Rand with 1530634217046958506
info	TestBlueGreenRoute	conformance/blue_green_test.go:130	Creating a Configuration
info	TestBlueGreenRoute	conformance/blue_green_test.go:137	The Configuration will be updated with the name of the Revision once it is created
info	TestBlueGreenRoute	conformance/blue_green_test.go:143	Updating the Configuration to use a different image
info	TestBlueGreenRoute	conformance/blue_green_test.go:152	Since the Configuration was updated a new Revision will be created and the Configuration will be updated
info	TestBlueGreenRoute	conformance/blue_green_test.go:159	Waiting for revision "prodcmkcssxv-00001" to be ready
info	TestBlueGreenRoute	conformance/blue_green_test.go:163	Waiting for revision "prodcmkcssxv-00002" to be ready
info	TestBlueGreenRoute	conformance/blue_green_test.go:172	Creating a Route
info	TestBlueGreenRoute	conformance/blue_green_test.go:177	When the Route reports as Ready, everything should be ready.
info	TestBlueGreenRoute	conformance/blue_green_test.go:61	Polling pizzaplanetewmqkvyp.pizzaplanet.example.com until we get a 200
info	TestBlueGreenRoute	conformance/blue_green_test.go:61	Polling green.pizzaplanetewmqkvyp.pizzaplanet.example.com until we get a 200
info	TestBlueGreenRoute	conformance/blue_green_test.go:61	Polling blue.pizzaplanetewmqkvyp.pizzaplanet.example.com until we get a 200
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	conformance/blue_green_test.go:66	Performing 100 concurrent requests to green.pizzaplanetewmqkvyp.pizzaplanet.example.com
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	test/request.go:170	Retrying for code 404
info	TestBlueGreenRoute	conformance/blue_green_test.go:66	Performing 100 concurrent requests to pizzaplanetewmqkvyp.pizzaplanet.example.com
info	TestBlueGreenRoute	conformance/blue_green_test.go:66	Performing 100 concurrent requests to blue.pizzaplanetewmqkvyp.pizzaplanet.example.com
info	TestBlueGreenRoute	conformance/blue_green_test.go:106	wanted at least 100, got 100 requests for green.pizzaplanetewmqkvyp.pizzaplanet.example.com
info	TestBlueGreenRoute	conformance/blue_green_test.go:106	wanted at least 25, got 42 requests for pizzaplanetewmqkvyp.pizzaplanet.example.com
info	TestBlueGreenRoute	conformance/blue_green_test.go:106	wanted at least 25, got 58 requests for pizzaplanetewmqkvyp.pizzaplanet.example.com
info	TestBlueGreenRoute	conformance/blue_green_test.go:106	wanted at least 100, got 100 requests for blue.pizzaplanetewmqkvyp.pizzaplanet.example.com
--- PASS: TestBlueGreenRoute (13.40s)
PASS
ok  	github.com/knative/serving/test/conformance	13.409s

mattmoor

This is super cool, thanks for putting this together.
🎉

mattmoor · 2018-07-03T16:23:54Z

If this is expected to implement a particular interface, I'd recommend using this pattern to assert that:

var _ Interface = (*SpoofingClient)(nil)

The closest I found is http.Roundtripper, but I'm not sure if implementing that would make this easier or harder to use. WDYT?

mattmoor · 2018-07-03T16:25:20Z

Please use http.StatusNotFound and other http constants in place of hardcoding.

mattmoor · 2018-07-03T16:25:38Z

Same not for http.MethodGet

mattmoor · 2018-07-03T16:26:17Z

I think this encapsulation (which is good) loses something by being in the same file as other stuff.

mattmoor · 2018-07-03T16:34:43Z

see comment about using http constants throughout this file.

mattmoor · 2018-07-03T16:35:49Z

check error?

_ is a context.Context

mattmoor · 2018-07-03T16:38:47Z

_ is a context.Context

mattmoor · 2018-07-03T16:45:01Z

Can we hoist 100 and 25% to constants to make them easier to vary?

bobcatfish

I think this is pretty cool - most of my confusion with this initially was about #1458 - generally i am excited about these changes :D

i really think we should start some docs on what each conformance test is for - and/or use an elaborate docstring for each test explaining the test case it is covering - if we don't start now this will be painful to add later

bobcatfish · 2018-07-03T17:52:53Z

i like this interface a lot, i think it's really slick and pretty easy to understand what's going on!

bobcatfish · 2018-07-03T17:53:18Z

plz add name to TODO or issue

@tcnghia help

bobcatfish · 2018-07-03T17:54:02Z

i am nervous about adding middleware - i think middleware tends to be hard to reason about and make debugging more difficult

Acknowledged. I will leave this as a TODO for now, but I think it's possible to do this in an acceptable way by just function wrapping.

We'll need to do something like it for #1298

bobcatfish · 2018-07-03T17:55:25Z

there's a lot going on in this function, I think we could use some docs explaining what's going on at least

bobcatfish · 2018-07-03T17:55:48Z

can has more comments explaining the details here

jonjohnsonjr · 2018-07-03T22:46:32Z

/assign @tcnghia

I TODO(you)'d you a few places, can you take a look?

bobcatfish · 2018-07-03T23:06:04Z

oooo nice update, i like the percentage

bobcatfish · 2018-07-03T23:07:10Z

it's not quite clear to me why this is called "recording" checker?

It was a bad abstraction, I have eradicated it. Thanks.

That is: * Create config pointing first image. * Get the revision name, this is the blue revision. * Update the config, pointing to second image. * Get the revision name, this is the green revision. * Create a Route that points 50/50 to blue and green revision. * Spin up goroutines to make 100 requests against: 1. blue revision 2. green revision 3. route * For 1/2, assert that each request is handled by the appropriate image. * For 3, assert that we get a reasonable distribution of responses (at worst 25/75 split).

jonjohnsonjr · 2018-07-04T00:53:27Z

@bobcatfish up for another round? 😀

I refactored some things such that I find them much more legible, LMK if you think it could be improved any further.

i really think we should start some docs on what each conformance test is for - and/or use an elaborate docstring for each test explaining the test case it is covering - if we don't start now this will be painful to add later

I've added a small comment to the blue/green test. Happy to change it if you had something else in mind.

mattmoor · 2018-07-04T02:12:34Z

TIL @jonjohnsonjr is evil

jonjohnsonjr · 2018-07-04T03:19:11Z

@mattmoor @bobcatfish this is now 100% free of greek alphabet and evil.

RFAL 😅

mattmoor

/lgtm
/approve

google-prow-robot · 2018-07-04T15:38:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jonjohnsonjr, mattmoor

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:

~~test/OWNERS~~ [mattmoor]

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

…e-v1.18 [release-v1.18] Update OWNERS file

google-prow-robot requested review from bobcatfish and srinivashegde86 July 3, 2018 06:26

google-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 3, 2018

jonjohnsonjr force-pushed the bluegreen branch from 2c608fc to e658e59 Compare July 3, 2018 06:27

google-prow-robot assigned mattmoor Jul 3, 2018

jonjohnsonjr force-pushed the bluegreen branch from e658e59 to f863663 Compare July 3, 2018 16:13

mattmoor reviewed Jul 3, 2018

View reviewed changes

jonjohnsonjr mentioned this pull request Jul 3, 2018

Pull host spoofing logic into an http client #1458

Merged

bobcatfish reviewed Jul 3, 2018

View reviewed changes

jonjohnsonjr force-pushed the bluegreen branch from f863663 to 85bfcd5 Compare July 3, 2018 22:25

google-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 3, 2018

google-prow-robot assigned tcnghia Jul 3, 2018

bobcatfish reviewed Jul 3, 2018

View reviewed changes

jonjohnsonjr force-pushed the bluegreen branch from 85bfcd5 to 9a484d5 Compare July 3, 2018 23:21

google-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 3, 2018

jonjohnsonjr force-pushed the bluegreen branch from a814b34 to b8d0bae Compare July 4, 2018 00:54

Address PR feedback

1ab05d6

jonjohnsonjr force-pushed the bluegreen branch from b8d0bae to 1ab05d6 Compare July 4, 2018 00:57

Clean up some small nits

3af3dd3

jonjohnsonjr force-pushed the bluegreen branch from 01aaa6d to 3af3dd3 Compare July 4, 2018 01:11

mattmoor reviewed Jul 4, 2018

View reviewed changes

No fun allowed

5332eef

mattmoor approved these changes Jul 4, 2018

View reviewed changes

google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 4, 2018

google-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 4, 2018

google-prow-robot merged commit a2a85e6 into knative:master Jul 4, 2018

jonjohnsonjr deleted the bluegreen branch July 4, 2018 16:38

tcnghia mentioned this pull request Jul 9, 2018

End to end tests for networking related items (routing, traffic split, timeouts, dns, etc) #646

Closed

bobcatfish mentioned this pull request Oct 11, 2018

Get simple PipelineRun implementation working tektoncd/pipeline#128

Merged

openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/knative-serving that referenced this pull request Oct 3, 2025

Merge pull request knative#1460 from serverless-qe/sync-owners-releas…

bc99e99

…e-v1.18 [release-v1.18] Update OWNERS file

Conversation

jonjohnsonjr commented Jul 3, 2018

Uh oh!

jonjohnsonjr commented Jul 3, 2018

Uh oh!

jonjohnsonjr commented Jul 3, 2018

Uh oh!

mattmoor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobcatfish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonjohnsonjr commented Jul 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonjohnsonjr commented Jul 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment