Skip to content

Add e2e test for automatic sidecar injection#4926

Closed
yusuoh wants to merge 4 commits intoistio:masterfrom
yusuoh:test
Closed

Add e2e test for automatic sidecar injection#4926
yusuoh wants to merge 4 commits intoistio:masterfrom
yusuoh:test

Conversation

@yusuoh
Copy link
Copy Markdown

@yusuoh yusuoh commented Apr 13, 2018

No description provided.

@yusuoh yusuoh requested review from ayj and chxchx April 13, 2018 01:22
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2018 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #4926     +/-   ##
========================================
+ Coverage      73%     74%     +1%     
========================================
  Files         322     310     -12     
  Lines       27618   26104   -1514     
========================================
- Hits        20141   19123   -1018     
+ Misses       6689    6206    -483     
+ Partials      788     775     -13
Impacted Files Coverage Δ
pilot/pkg/model/jwks_resolver.go 56% <0%> (-12%) ⬇️
istioctl/cmd/istioctl/convert/cmd.go 40% <0%> (-6%) ⬇️
mixer/adapter/stackdriver/metric/metric.go 82% <0%> (-5%) ⬇️
mixer/adapter/stackdriver/helper/common.go 79% <0%> (-5%) ⬇️
pilot/pkg/serviceregistry/kube/queue.go 86% <0%> (-3%) ⬇️
pilot/pkg/proxy/envoy/v2/lds.go 59% <0%> (-3%) ⬇️
pilot/cmd/pilot-discovery/main.go 75% <0%> (-3%) ⬇️
pilot/pkg/proxy/envoy/v2/ads.go 75% <0%> (-3%) ⬇️
pilot/pkg/proxy/envoy/v2/eds.go 77% <0%> (-3%) ⬇️
pilot/pkg/proxy/envoy/v1/watcher.go 52% <0%> (-2%) ⬇️
... and 95 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 26262e8...896a456. Read the comment docs.

@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Apr 13, 2018

@yusuoh PR needs rebase

@istio-merge-robot istio-merge-robot added the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 13, 2018
@istio-testing istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 13, 2018
@istio-testing istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged and removed needs-rebase Indicates a PR needs to be rebased before being merged labels Apr 13, 2018
@chxchx
Copy link
Copy Markdown
Contributor

chxchx commented Apr 13, 2018

/lgtm

@chxchx
Copy link
Copy Markdown
Contributor

chxchx commented Apr 13, 2018

/hold
Would like to have Jason review this before it gets merged.

Also, you may want to update prow/e2e-cluster_wide-auth.sh in the daily-release repo as well. The tricky thing is after adding the flag the test will not pass until a new daily candidate that actually have this flag defined is cut.

@istio-testing istio-testing added the do-not-merge/hold Block automatic merging of a PR. label Apr 13, 2018
ayj
ayj reviewed Apr 13, 2018
View reviewed changes
tests/e2e/framework/kubernetes.go Outdated
Copy link
Copy Markdown
Contributor

@ayj ayj Apr 13, 2018

Choose a reason for hiding this comment

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

+1

ayj
ayj approved these changes Apr 13, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@ayj ayj left a comment

Choose a reason for hiding this comment

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

/lgtm

@chxchx
Copy link
Copy Markdown
Contributor

chxchx commented Apr 13, 2018

/hold cancel

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

istio-merge-robot commented Apr 13, 2018

@yusuoh PR needs rebase

@istio-merge-robot istio-merge-robot added the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 13, 2018
@istio-testing istio-testing removed lgtm needs-rebase Indicates a PR needs to be rebased before being merged labels Apr 13, 2018
@chxchx
Copy link
Copy Markdown
Contributor

chxchx commented Apr 16, 2018

ping

@istio-testing istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged and removed needs-rebase Indicates a PR needs to be rebased before being merged labels Apr 16, 2018
@yusuoh
Copy link
Copy Markdown
Author

yusuoh commented Apr 17, 2018

/test istio-unit-tests

Fix sidecar injector in e2e tests
1b5ef4c 
add yaml

enable webhook in cluser wide test

fix lint

fix lint

fix lint
@chxchx
Copy link
Copy Markdown
Contributor

chxchx commented Apr 18, 2018

/lgtm

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Apr 18, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

Assign the PR to them by writing /assign @geeknoid 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

@chxchx
Copy link
Copy Markdown
Contributor

chxchx commented Apr 18, 2018

/assign @geeknoid

@yusuoh yusuoh requested a review from costinm April 18, 2018 20:01
@ayj ayj mentioned this pull request Apr 19, 2018
Closed
@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 27, 2018
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented May 14, 2018

New changes are detected. LGTM label has been removed.

@istio-testing istio-testing removed lgtm needs-rebase Indicates a PR needs to be rebased before being merged labels May 14, 2018
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented May 16, 2018

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

Test name Commit Details Rerun command
prow/istio-pilot-e2e-envoyv2-v1alpha3.sh 896a456 link /test istio-pilot-e2e-envoyv2-v1alpha3
prow/istio-pilot-e2e.sh 896a456 link /test istio-pilot-e2e
prow/e2e-bookInfoTests.sh 896a456 link /test e2e-bookInfo
prow/e2e-bookInfoTests-v1alpha3.sh 896a456 link /test e2e-bookInfo-envoyv2-v1alpha3
prow/e2e-simpleTests.sh 896a456 link /test e2e-simple
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.

@rshriram rshriram closed this May 22, 2018
@ostromart ostromart mentioned this pull request Jun 8, 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

@costinm costinm Awaiting requested review from costinm

1 more reviewer

@ayj ayj ayj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ayj ayj

@chxchx chxchx

@geeknoid geeknoid

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@yusuoh @istio-merge-robot @chxchx @istio-testing @ayj @googlebot @rshriram @geeknoid