Skip to content

Copy start_envoy and envoy.conf.template into istio/istio from proxy #4358

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 23, 2018
Merged

Copy start_envoy and envoy.conf.template into istio/istio from proxy #4358

merged 2 commits into from
Mar 23, 2018

Conversation

yutongz
Copy link
Contributor

@yutongz yutongz commented Mar 19, 2018 •
edited
Loading

fixes #4317

@codecov
Copy link

codecov bot commented Mar 19, 2018 •
edited
Loading

Codecov Report

Merging #4358 into master will decrease coverage by 1%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #4358    +/-   ##
=======================================
- Coverage      71%     71%   -<1%     
=======================================
  Files         325     325            
  Lines       29651   29565    -86     
=======================================
- Hits        20812   20711   -101     
- Misses       7594    7607    +13     
- Partials     1245    1247     +2
Impacted Files Coverage Δ
...olarwinds/internal/papertrail/papertrail_logger.go 59% <0%> (-21%) ⬇️
mixer/adapter/solarwinds/log_handler.go 58% <0%> (-15%) ⬇️
...er/adapter/solarwinds/internal/appoptics/client.go 57% <0%> (-12%) ⬇️
pilot/pkg/config/memory/monitor.go 82% <0%> (-9%) ⬇️
pilot/pkg/serviceregistry/kube/queue.go 86% <0%> (-3%) ⬇️
mixer/adapter/denier/denier.go 93% <0%> (-3%) ⬇️
mixer/adapter/rbac/rbacStore.go 83% <0%> (-1%) ⬇️
mixer/adapter/servicecontrol/reportbuilder.go 89% <0%> (ø) ⬇️
mixer/adapter/servicecontrol/utils.go 89% <0%> (ø) ⬇️
mixer/template/sample/template.gen.go 55% <0%> (ø) ⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 615959e...7874862. Read the comment docs.

@yutongz
Copy link
Contributor Author

yutongz commented Mar 19, 2018

/cc @andraxylia @qiwzhang

@ldemailly
Copy link
Member

ldemailly commented Mar 20, 2018 •
edited
Loading

can you just put a header on both this file and the original one to reference each other
so someone knows to update both ?
(tricky part: what is a json comment?)

/lgtm
/hold

@kyessenov
Copy link
Contributor

kyessenov commented Mar 20, 2018 via email

@istio-merge-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kyessenov, ldemailly

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

@istio-merge-robot
Copy link

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

@ldemailly
Copy link
Member

looks like you got the broken version:

[2018-03-20 01:16:33.796][11235][debug][main] external/envoy/source/server/server.cc:164] Unable to initialize config as v2, will retry as v1: Unable to parse JSON as proto (INVALID_ARGUMENT:cluster_manager: Cannot find field.):

@ldemailly ldemailly added do-not-merge Block automatic merging of a PR. do-not-merge/hold Block automatic merging of a PR. labels Mar 20, 2018
@yutongz yutongz mentioned this pull request Mar 22, 2018
Merged
ldemailly
ldemailly approved these changes Mar 22, 2018
View reviewed changes
Copy link
Member

@ldemailly ldemailly left a comment

Choose a reason for hiding this comment

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

ty

@ldemailly
Copy link
Member

/lgtm

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kyessenov, ldemailly

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

The pull request process is described here

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

istio-merge-robot pushed a commit to istio/proxy that referenced this pull request Mar 22, 2018
@yutongz @istio-merge-robot
Add notification for duplication (#1234)
a62fc33 
Automatic merge from submit-queue.

Add notification for duplication

**What this PR does / why we need it**:

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
```

Add notification to match istio/istio#4358
@yutongz
Copy link
Contributor Author

yutongz commented Mar 23, 2018

/hold cancel
Reviewer approved, all required tests passed.

@istio-testing istio-testing removed the do-not-merge/hold Block automatic merging of a PR. label Mar 23, 2018
@yutongz yutongz removed the do-not-merge Block automatic merging of a PR. label Mar 23, 2018
@istio-merge-robot
Copy link

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

@istio-testing
Copy link
Collaborator

istio-testing commented Mar 23, 2018 •
edited
Loading

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

Test name Commit Details Rerun command
prow/istio-pilot-e2e-v1alpha3.sh 7874862 link /test istio-pilot-e2e-v1alpha3
prow/istio-pilot-e2e.sh 7874862 link /test istio-pilot-e2e

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.

@istio-merge-robot
Copy link

Automatic merge from submit-queue.

@istio-merge-robot istio-merge-robot merged commit 8f97c88 into istio:master Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andraxylia andraxylia Awaiting requested review from andraxylia

@qiwzhang qiwzhang Awaiting requested review from qiwzhang

1 more reviewer

@ldemailly ldemailly ldemailly approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@ldemailly ldemailly

@kyessenov kyessenov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve prow-presubmit test: ./tests/integration/example/integration.sh
6 participants
@yutongz @ldemailly @kyessenov @istio-merge-robot @istio-testing @googlebot