Skip to content

fix wrong url when clearing cache stats#3985

Merged
mandarjog merged 2 commits intoistio:masterfrom
baodongli:istio-proxy-cfg
Mar 6, 2018
Merged

fix wrong url when clearing cache stats#3985
mandarjog merged 2 commits intoistio:masterfrom
baodongli:istio-proxy-cfg

Conversation

@baodongli
Copy link
Copy Markdown

@baodongli baodongli commented Mar 5, 2018 •
edited
Loading

Incorrect URL is used when sending a request to pilot to clear cache stats, and json() call should be removed when posting the request.

In addition, if user uses the --pilot_url and provides a url without prefix http:// or https://, prefix it with http://, to make it more user friendly

@baodongli baodongli requested review from a team and mandarjog March 5, 2018 23:12
@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Mar 5, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
We suggest the following additional approver: rshriram

Assign the PR to them by writing /assign @rshriram in a comment when ready.

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

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@mandarjog
Copy link
Copy Markdown
Contributor

mandarjog commented Mar 6, 2018

@baodongli while you are at it, can you update code to ensure that when --pilot_url is specified, we automatically prefix it with http:// if it is not present.

The issue right now is that simply specifying localhost:9080 does not work.

mandarjog
mandarjog approved these changes Mar 6, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@mandarjog mandarjog left a comment

Choose a reason for hiding this comment

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

lgtm

@mandarjog
Copy link
Copy Markdown
Contributor

mandarjog commented Mar 6, 2018

@baodongli please provide a description to the PR, thanks.

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Mar 6, 2018

@baodongli: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/e2e-bookInfoTests.sh 50f46a8 link /test e2e-bookInfo
prow/istio-pilot-e2e.sh 50f46a8 link /test istio-pilot-e2e
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.

@mandarjog mandarjog merged commit 16272d6 into istio:master Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@mandarjog mandarjog mandarjog approved these changes

@xiaolanz xiaolanz xiaolanz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@baodongli @istio-merge-robot @mandarjog @istio-testing @xiaolanz @googlebot