Skip to content

Expose concurrency flag of envoy #3065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Feb 7, 2018
Merged

Conversation

mcieplak
Copy link
Member

@mcieplak mcieplak commented Feb 1, 2018

This is to address the following github issue: #3031

@mcieplak mcieplak requested a review from a team February 1, 2018 19:02
@istio-testing
Copy link
Collaborator

Hi @mcieplak. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

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.

@googlebot
Copy link
Collaborator

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@googlebot googlebot added the cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. label Feb 1, 2018
@andraxylia
Copy link
Contributor

/lgtm

@andraxylia
Copy link
Contributor

@mcieplak Thanks for your PR! Can you please sign the CLA?

@mcieplak
Copy link
Member Author

mcieplak commented Feb 1, 2018

@andraxylia No problem! I just signed the CLA now. This PR is also dependent on istio/api#341 being merged. I will send another PR to handle the kube-inject side as well.

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels Feb 1, 2018
@istio-merge-robot
Copy link

/lgtm cancel //PR changed after LGTM, removing LGTM. @andraxylia @mcieplak

@andraxylia
Copy link
Contributor

Test fail because the corresponding istio/api change has not been merged yet.

@andraxylia
Copy link
Contributor

Please update the istio/api sha to 8d86ca7 in the dependency file here and then to /restest

https://github.com/istio/istio/blob/master/Gopkg.lock#L1058

@mcieplak mcieplak requested a review from a team February 1, 2018 23:37
@andraxylia
Copy link
Contributor

/ok-to-test

@istio-merge-robot istio-merge-robot removed the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 2, 2018
@mcieplak
Copy link
Member Author

mcieplak commented Feb 5, 2018

@andraxylia Thank you for the updates!

@costinm I have updated the PR to only add the concurrency flag if it's specified. Please review.

@mcieplak
Copy link
Member Author

mcieplak commented Feb 5, 2018

@andraxylia The build is now passing. Can you please review?

@andraxylia
Copy link
Contributor

andraxylia commented Feb 6, 2018

e2e-smoke and istio-pilot-e2e failure caused by #3139

@andraxylia
Copy link
Contributor

@costinm tests are passing, please check the code changes, remove the hold label and merge if everything is fine.

@mcieplak
Copy link
Member Author

mcieplak commented Feb 6, 2018

@andraxylia I believe the e2e-pilot ci/circleci test is also failing because of #3139.

@costinm costinm removed the do-not-merge/hold Block automatic merging of a PR. label Feb 6, 2018
Copy link
Contributor

@costinm costinm left a comment

Choose a reason for hiding this comment

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

/lgtm

@istio-merge-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andraxylia, costinm

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

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

@mcieplak
Copy link
Member Author

mcieplak commented Feb 6, 2018

@andraxylia @costinm Thank you for the reviews! Can we please merge?

@istio-merge-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@rkpagadala
Copy link
Contributor

/test istio-unit-tests

@istio-merge-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@istio-merge-robot
Copy link

Automatic merge from submit-queue.

@istio-merge-robot istio-merge-robot merged commit 2e916ee into istio:master Feb 7, 2018
@mcieplak mcieplak deleted the concurrency branch February 7, 2018 04:40
@sureshvis sureshvis mentioned this pull request Feb 7, 2018
@rkpagadala rkpagadala mentioned this pull request Feb 8, 2018
hklai pushed a commit that referenced this pull request Feb 9, 2018
Automatic merge from submit-queue.

Expose concurrency flag of envoy

This is to address the following github issue: #3031
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants