Add ssn and txn tests for HTTP2. Depends on Issue #3143. Depends on PR #3301 #3047

xf6wang · 2018-01-26T01:52:07Z

Existing tests ensure that the open and close hooks for transactions and sessions match up. Modified existing tests to use curl instead of ab to remove dependancy. New tests use "curl --http2" to verify functionality with HTTP/2 requests.

openclose.test.py & openclose_h2.test.py use ssntxnorder_verify.cc plugin
double.test.py & double_h2.test.py use continuation_verify.cc plugin

persiaAziz-zz · 2018-01-26T20:00:41Z

Please check the return code error

Test : Checking that ReturnCode == None - Failed Reason: Returned Value 0 != None

persiaAziz-zz · 2018-01-26T21:46:35Z

[approve ci]

xf6wang · 2018-01-26T22:22:07Z

Test errors cannot be reproduced locally. What should I do in this situation?
openclose_h2.test.py makes 1159 requests but only logs 1134 started transactions.
remap_http.test.py has a mismatch in upper/lowercase
> Proxy-Connection: Keep-Alive > Proxy-Connection: keep-alive

https://ci.trafficserver.apache.org/job/autest-github/3608/console

bryancall · 2018-01-27T00:39:17Z

[approve ci]

bryancall · 2018-01-29T18:20:51Z

[approve ci autest]

d2r · 2018-02-08T01:14:17Z

tests/gold_tests/continuations/double.test.py

-from random import randint
 Test.Summary = '''
-Test transactions and sessions, making sure they open and close in the proper order.
+Test transactions and sessions, making sure they two continuations catch the same number of hooks.


remove they?

d2r · 2018-02-08T01:14:41Z

tests/gold_tests/continuations/double.test.py

+Test transactions and sessions, making sure they two continuations catch the same number of hooks.
 '''
-# need Apache Benchmark. For RHEL7, this is httpd-tools
+# needs curl


We can remove this comment.

d2r · 2018-02-08T01:15:00Z

tests/gold_tests/continuations/double.test.py

 )

-numberOfRequests = randint(1000, 1500)
+numberOfRequests = 10


Do we need this variable?

I think it makes the intention of the loop more clear. or for ((i=0; i<10; i++)) might be missed that 10 is the number of requests

d2r · 2018-02-08T01:16:42Z

tests/gold_tests/continuations/double.test.py

+# Make calls to the proxy!
 tr = Test.AddTestRun()
-tr.Processes.Default.Command = 'ab -n {0} -c 10 http://127.0.0.1:{1}/;sleep 5'.format(numberOfRequests, ts.Variables.port)
+tr.Processes.Default.Command = 'for ((i=0; i<{0}; i++)); do curl -k https://127.0.0.1:{1}; done; sleep 5'.format(numberOfRequests, ts.Variables.port)


Do we still need the sleep here?

Does it matter that the client requests will be made serially instead of in parallel?

The sleep is necessary because traffic_ctl needs to process and output the metrics. Without it, TSStatIntIncrement may not register.
Requests should be made in parallel. Good point. Changed

d2r · 2018-02-08T01:21:12Z

tests/gold_tests/continuations/double_h2.test.py

+# curl with http2
+tr.Processes.Default.Command = 'for ((i=0; i<{0}; i++)); do curl --http2 -k https://127.0.0.1:{1}; done; sleep 5'.format(numberOfRequests, ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode = 0
+# time delay as proxy.config.http.wait_for_cache could be broken


Does this mean this config's functionality is, in fact, broken? We should open an issue and fix it rather than working around it in multiple tests.

I think the delay is to prevent it from breaking in autest. This appears across multiple tests.

d2r · 2018-02-08T01:24:23Z

tests/gold_tests/continuations/double_h2.test.py

+Test transactions and sessions for http2, making sure the two continuations catch the same number of hooks.
+'''
+Test.SkipUnless(
+    Condition.HasProgram("curl", "Curl need to be installed on system for this test to work"),


d2r · 2018-02-08T01:24:35Z

tests/gold_tests/continuations/openclose.test.py

 Test.SkipUnless(
-    Condition.HasProgram(
-        "ab", "apache benchmark (httpd-tools) needs to be installed on system for this test to work")
+    Condition.HasProgram("curl", "Curl need to be installed on system for this test to work")


d2r · 2018-02-08T01:25:02Z

tests/gold_tests/continuations/openclose_h2.test.py

+Test transactions and sessions for http2, making sure they open and close in the proper order.
+'''
+Test.SkipUnless(
+    Condition.HasProgram("curl", "Curl need to be installed on system for this test to work"),


d2r · 2018-02-08T01:29:43Z

tests/gold_tests/continuations/openclose_h2.test.py

+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Env = ts.Env
+tr.Processes.Default.Streams.stdout = Testers.ExcludesExpression(
+    " 0", 'should be nonzero')


This is testing that the output is 0, but if it is not then we fail the test saying it should be nonzero?

This is to see how many sessions have been closed. It should be non zero. If it is 0 this is an error. Note, it's ExcludesExpression.

Ah, you're right. I misread it as another ContainsExpression.

d2r · 2018-02-08T01:30:55Z

tests/gold_tests/continuations/ssl/server.pem

+yS7FDj5JgCU15hZgFk1iPx3HCt44jZM2HaL+UUHAzRQjKxTLAl3G1rWVAWLMyQML
+lORoveLvotl4HOruSsMCQQCAx9dV9JUSFoyc1CWILp/FgUH/se4cjQCThGO0DoQQ
+vGTYmntY7j9WRJ9esQrjdD6Clw8zM/45GIBNwnXzqo7Z
+-----END RSA PRIVATE KEY-----


We are duplicating the private key here. Do we need two copies?

ya multiple test folders have copies of this key. if we want to move to all of the keys to a single folder that might be a seperate pr.

Let's leave it.

bryancall · 2018-02-13T19:43:18Z

[approve ci autest]

xf6wang · 2018-02-13T20:04:41Z

TS is seg faulting on these tests. Investigating.

d2r · 2018-02-20T20:47:32Z

TS is seg faulting on these tests. Investigating.

Yeah it is good that you found the problem with this test. Probably we need to track a solution to that problem in a new GitHub issue and block this PR on the new issue. And this test should have a good number of requests, not a lower number of requests so as to avoid the segfault.

bryancall · 2018-03-05T22:52:38Z

[approve ci debian]

dragon512 · 2018-03-07T16:07:30Z

tests/gold_tests/continuations/openclose_h2.test.py

+
+tr = Test.AddTestRun()
+# curl with http2
+tr.Processes.Default.Command = 'for ((i=0; i<{0}; i++)); do curl -vs -k --http2 https://127.0.0.1:{1} & done; wait; sleep 10'.format(numberOfRequests, ts.Variables.ssl_port)


This can be done without the shell.
Example:

def make_curl_chain(tr,num): ret=[] for cnt in range(0,num): ret.append( tr.Processes.Process( name="curl-{num}".format(num=cnt), cmdstr="curl -vs -k --http2 http://127.0.0.1:{port}".format(port = ts.Variables.port), returncode = 0 ) ) return ret tr = Test.AddTestRun() tr.Processes.Default.Command = 'curl -vs -k --http2 http://127.0.0.1:{1}'.format(numberOfRequests, ts.Variables.port) tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.StartAfter( *make_curl_chain(tr,numberOfRequests) )

I think the idea is to have all the curl commands run in parallel. From my understanding, if you chain test runs like this, even if you use & in the cmdstr the test runs are sequential rather than in parallel because you have finish each test run and get a return code before moving on to the next.

That is what this does

bryancall · 2018-04-12T17:11:08Z

[approve ci autest]

d2r · 2018-04-12T17:39:20Z

@xf6wang in addition to the test failures for double and http2, there are a lot of the following:

Ignoring adding ordered_item_t(object=<autest.testenities.process.Process object at 0x7f20842e11b0>, readyfunc=<bound method Order._isReady of <autest.testenities.process.Process object at 0x7f20842e11b0>>, args={}) to start order as it is already exist, breaking loop.

This doesn't look intentional, and we should try to silence these warnings if possible.

Not sure about the openclose_h2 test. I've seen some strange things happen when the program used in the test has trouble linking. This failure may be unrelated.

xf6wang · 2018-04-12T19:04:26Z

@d2r I think those messages are related to a bug in autest.

If you look at the SpawnCommands function we are creating a bunch of Processes to execute and then in the tests (ie double.test.py, ...) we structure the ordering of execution based on the list of created Processes. From what I understand, there is a bug that doesn't like the ordering created and as the message implies, ignores the ordering. So, then the curl commands can start before ts and microserver are fully up which causes the test to fail. badly.

This test needs a new drop of autest from @dragon512.

d2r · 2018-04-12T19:28:23Z

This test needs a new drop of autest from @dragon512.

OK is there a related pull request to upgrade our version of autest?

It might be simpler to use the shell:

bash -c 'for i in {0..$N}; do (curl $args || kill 0) & done; wait'

xf6wang · 2018-04-12T20:57:29Z

I'm not sure about the PR for autest. I don't think that would be part of this repo.

I had a loop for the curl commands in bash previously but @dragon512 preferred doing it through python.

d2r · 2018-04-12T21:01:05Z

I'm not sure about the PR for autest. I don't think that would be part of this repo.

I mean, if trafficserver is going to upgrade the version of autest it uses, is there a pull request for that change that we should point to as a dependency? How do we upgrade the autest version?

xf6wang · 2018-04-12T21:07:24Z

Oh, I misunderstood. I don't think there has been a PR made on autest yet. But, when that happens, I think: https://github.com/apache/trafficserver/blob/master/tests/gold_tests/autest-site/init.cli.ext#L26 needs to be updated.

zwoop · 2018-04-25T22:58:50Z

[approve ci clang-format]

scw00 assigned xf6wang Jan 26, 2018

scw00 added HTTP/2 Tests labels Jan 26, 2018

scw00 added this to the 8.0.0 milestone Jan 26, 2018

xf6wang force-pushed the ssn_txn_http2_tests branch from b78ed01 to e5df75b Compare January 26, 2018 18:27

xf6wang force-pushed the ssn_txn_http2_tests branch from e5df75b to bf7da85 Compare January 26, 2018 21:05

xf6wang closed this Jan 26, 2018

xf6wang reopened this Jan 26, 2018

xf6wang force-pushed the ssn_txn_http2_tests branch 4 times, most recently from 46c4819 to adc02f2 Compare January 29, 2018 23:21

xf6wang changed the title ~~Add ssn and txn tests for HTTP2~~ Add ssn and txn tests for HTTP2 - WIP Jan 31, 2018

xf6wang force-pushed the ssn_txn_http2_tests branch from b4e00ec to 66cfebc Compare January 31, 2018 19:17

xf6wang changed the title ~~Add ssn and txn tests for HTTP2 - WIP~~ Add ssn and txn tests for HTTP2 Jan 31, 2018

d2r reviewed Feb 8, 2018

View reviewed changes

xf6wang force-pushed the ssn_txn_http2_tests branch from 66cfebc to 7f4bcec Compare February 9, 2018 21:58

xf6wang force-pushed the ssn_txn_http2_tests branch from 7f4bcec to 7d06e6e Compare February 13, 2018 22:16

xf6wang mentioned this pull request Feb 20, 2018

TS Seg Faulting on HTTP_EVENT_TUNNEL_CLOSE #3143

Closed

xf6wang changed the title ~~Add ssn and txn tests for HTTP2~~ Add ssn and txn tests for HTTP2. Depends on Issue #3143 Feb 20, 2018

dragon512 reviewed Mar 7, 2018

View reviewed changes

xf6wang force-pushed the ssn_txn_http2_tests branch from 65d4ec2 to dae951d Compare April 10, 2018 01:20

xf6wang force-pushed the ssn_txn_http2_tests branch 15 times, most recently from 28bec5b to 0995dff Compare April 18, 2018 17:34

xf6wang mentioned this pull request Apr 18, 2018

TS Seg Fault on reschedule. #3497

Closed

xf6wang force-pushed the ssn_txn_http2_tests branch from 0995dff to 94de5ea Compare April 18, 2018 22:24

Add ssn and txn tests for HTTP2

8431783

xf6wang force-pushed the ssn_txn_http2_tests branch from 94de5ea to 8431783 Compare April 24, 2018 16:34

dragon512 approved these changes Apr 27, 2018

View reviewed changes

dragon512 merged commit e3d3c1a into apache:master Apr 27, 2018

Add ssn and txn tests for HTTP2. Depends on Issue #3143. Depends on PR #3301 #3047

Add ssn and txn tests for HTTP2. Depends on Issue #3143. Depends on PR #3301 #3047

Uh oh!

Conversation

xf6wang commented Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

persiaAziz-zz commented Jan 26, 2018

Uh oh!

persiaAziz-zz commented Jan 26, 2018

Uh oh!

xf6wang commented Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bryancall commented Jan 27, 2018

Uh oh!

bryancall commented Jan 29, 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!

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!

Choose a reason for hiding this comment

Uh oh!

bryancall commented Feb 13, 2018

Uh oh!

xf6wang commented Feb 13, 2018

Uh oh!

d2r commented Feb 20, 2018

Uh oh!

bryancall commented Mar 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xf6wang Mar 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

xf6wang commented Jan 26, 2018 •

edited

Loading

xf6wang commented Jan 26, 2018 •

edited

Loading

xf6wang Mar 8, 2018 •

edited

Loading

d2r commented Apr 12, 2018 •

edited

Loading