Skip to content

Update session cookie in bookinfo e2e test#8052

Merged
mandarjog merged 1 commit intoistio:masterfrom
bianpengyuan:fix-bookinfo
Aug 18, 2018
Merged

Update session cookie in bookinfo e2e test#8052
mandarjog merged 1 commit intoistio:masterfrom
bianpengyuan:fix-bookinfo

Conversation

@bianpengyuan
Copy link
Copy Markdown
Contributor

@bianpengyuan bianpengyuan commented Aug 18, 2018

see if this fixes #8046...

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Aug 18, 2018

@bianpengyuan: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/istio-unit-tests.sh 25ac0fc link /test istio-unit-tests
Details

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. I understand the commands that are listed here.

sdake
sdake approved these changes Aug 18, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@sdake sdake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/assign @rshriram

It appears this change: 7e65cd9 introduced a regression. From looking at the change, sessionCookie was previously unspecified and unvalidated. I am not clear if this cookie changes.

Note, this PR or a followup should also update this file:

+diff --git a/tests/e2e/tests/upgrade/upgrade_test.go b/tests/e2e/tests/upgrade/upgrade_test.go
index b976f8b..1593950 100644
--- a/tests/e2e/tests/upgrade/upgrade_test.go
+++ b/tests/e2e/tests/upgrade/upgrade_test.go

@rshriram can you merge this? This is blocking all of the gates including release-1.0.

Cheers
-steve

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Aug 18, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sdake
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: baodongli

If they are not already assigned, you can assign the PR to them by writing /assign @baodongli in a comment when ready.

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

@mandarjog
Copy link
Copy Markdown
Contributor

mandarjog commented Aug 18, 2018

Thanks @bianpengyuan for the PR, this will unblock many others!

@mandarjog mandarjog merged commit bc53c13 into istio:master Aug 18, 2018
sdake pushed a commit to sdake/istio that referenced this pull request Aug 18, 2018
@bianpengyuan
update session cookie. (istio#8052)
596805e 
(cherry picked from commit bc53c13)
mandarjog pushed a commit that referenced this pull request Aug 18, 2018
@mandarjog
update session cookie. (#8052) (#8055)
22bc84e 
(cherry picked from commit bc53c13)
@douglas-reid
Copy link
Copy Markdown
Contributor

douglas-reid commented Aug 18, 2018

@mandarjog is the unit test failure here a flake? is anyone tracking fixing that?

@rshriram
Copy link
Copy Markdown
Member

rshriram commented Aug 19, 2018

@krancour what goes into this session cookie? Why does it need to be updated frequently? Is it expiring every few months? if so, is it possible to create a super long lived cookie?

@mandarjog
Copy link
Copy Markdown
Contributor

mandarjog commented Aug 19, 2018

@ajy
https://k8s-gubernator.appspot.com/build/istio-prow/pull/istio_istio/8052/istio-unit-tests/11797/ a galley test failure; config-map test; flake

@baodongli baodongli mentioned this pull request Aug 20, 2018
Merged
quanjielin pushed a commit to quanjielin/istio that referenced this pull request Aug 23, 2018
@bianpengyuan @quanjielin
update session cookie. (istio#8052)
887c929
@quanjielin quanjielin mentioned this pull request Aug 23, 2018
Merged
istio-testing pushed a commit that referenced this pull request Aug 23, 2018
@quanjielin @istio-testing
update proxy/api sha in collab-gcp-identity (#8137)
c1551ba 
* update proxy sha

* update proxy sha

* update api sha

* update api sha

* add unit test for dictionary race. (#8087)

git.

* update session cookie. (#8052)

* make pilotplugin_mtls_test.TestPilotPlugin test pass
hklai pushed a commit that referenced this pull request Sep 18, 2018
@hklai
[test-framework] Refactoring local application and agent (#8809)
6afefef 
Merging manually since test framework does not affect production code and the failing test is unrelated. The test failure is likely due to something like #8052?.
@yangminzhu yangminzhu mentioned this pull request Sep 18, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chxchx chxchx Awaiting requested review from chxchx

@yutongz yutongz Awaiting requested review from yutongz

1 more reviewer

@sdake sdake sdake approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@sdake sdake

@rshriram rshriram

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

e2e-bookInfoTests-v1alpha3 is consistently failing in both release-1.0 and master

7 participants

@bianpengyuan @istio-testing @mandarjog @douglas-reid @rshriram @sdake @googlebot