Integrate circle CI with Testgrid by chxchx · Pull Request #4649 · istio/istio

chxchx · 2018-03-30T00:30:10Z

The integration will start with mixer and simple e2e tests. Will expand to other jobs in subsequent PRs.

Results are now available on
https://k8s-testgrid.appspot.com/istio#circleci-e2e-mixer
https://k8s-testgrid.appspot.com/istio#circleci-e2e-simple

Those are test results from this PR exclusively. After this is merged, the results will be more interesting.

The ci_to_gubernator binary is built from istio/test-infra#769. Feel free to also take a look.

codecov · 2018-03-30T01:31:36Z

Codecov Report

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

@@           Coverage Diff           @@
##           master   #4649    +/-   ##
=======================================
+ Coverage      73%     73%    +1%     
=======================================
  Files         311     311            
  Lines       25949   26144   +195     
=======================================
+ Hits        18826   19061   +235     
+ Misses       6364    6318    -46     
- Partials      759     765     +6

Impacted Files	Coverage Δ
pilot/pkg/config/memory/monitor.go	`82% <0%> (-9%)`	⬇️
mixer/adapter/stackdriver/log/log.go	`67% <0%> (-5%)`	⬇️
pilot/pkg/serviceregistry/kube/queue.go	`86% <0%> (-3%)`	⬇️
...olarwinds/internal/papertrail/papertrail_logger.go	`59% <0%> (-3%)`	⬇️
mixer/adapter/circonus/circonus.go	`71% <0%> (-2%)`	⬇️
mixer/adapter/opa/opa.go	`79% <0%> (-1%)`	⬇️
mixer/adapter/list/list.go	`100% <0%> (ø)`	⬇️
mixer/adapter/solarwinds/metrics_handler.go	`84% <0%> (ø)`	⬇️
mixer/adapter/stdio/stdio.go	`100% <0%> (ø)`	⬆️
pilot/pkg/serviceregistry/kube/controller.go	`67% <0%> (ø)`	⬆️
... and 14 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 6e443e5...e6edcd8. Read the comment docs.

costinm · 2018-03-30T06:36:19Z

.circleci/Dockerfile

How big is it ? Is it a script ? Can we check it in ?

Yeah I removed this from docker image and create a separate scripts in bin/. See istio/test-infra#769 for the source of ci_to_gubernator

chxchx · 2018-04-12T22:07:41Z

PTAL @costinm @sebastienvas

The makefile is left intact and logics moved to circle ci config yaml, which uses anchors to reuse the same routines. More details are in my previous comment.

costinm

Better - but still small comments.

costinm · 2018-04-12T22:13:54Z

.circleci/config.yml

The make targets should internally tee, but into $OUT/... not tmp

This likely has the same problem we had in the past with pipefail.

Every run step in circle ci is executed in the #!/bin/bash -eo pipefail environment, so pipe into tee directly is fine and this command will exit with the same exit code as the make e2e_* target. I had the same concern as you do but then I noticed quite a few usage of this fashion in the circle ci config yaml so I think it is fine.

I would prefer instrumenting the tee command outside of the makefile as different CI handles the log differently and developers may not care storing the logs separately besides seeing them on console already (just like we don't generate the junit xml by default and actually do so in a second steps)

costinm · 2018-04-12T22:15:23Z

.circleci/config.yml

junit.xml is used by circleci for its internal reporting - and should be generated under out/tetss (see store_test_results)

chxchx · 2018-04-17T19:54:27Z

PTAL @costinm

sebastienvas · 2018-04-19T20:45:10Z

bin/ci2gubernator.sh

Please remove the wget part.

sebastienvas · 2018-04-19T20:46:44Z

/lgtm

chxchx · 2018-04-19T21:00:05Z

PTAL

costinm · 2018-04-20T04:07:44Z

/test

costinm

/lgtm
/approve

sebastienvas · 2018-04-20T21:51:32Z

/lgtm

istio-testing · 2018-04-20T21:51:37Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~.circleci/OWNERS~~ [costinm]
~~bin/OWNERS~~ [costinm]
~~tests/OWNERS~~ [costinm,sebastienvas]

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 · 2018-04-20T22:01:22Z

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

istio-testing · 2018-04-20T23:15:53Z

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

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

istio-merge-robot · 2018-04-20T23:17:28Z

Automatic merge from submit-queue.

ldemailly · 2018-04-23T22:45:23Z

looks like this broke circleci - any idea how come it went through undetected ?

bin/ci2gubernator.sh: line 25: CIRCLE_PR_NUMBER: unbound variable

eg https://circleci.com/gh/istio/istio/79135

chxchx · 2018-04-23T23:45:12Z

As the discussion from slack, the issue seems to happen on PRs from a branch in istio/istio (same repo) and not for PRs from forked repos. Pending further investigation, the immediate fix is here #5137.

ldemailly · 2018-04-24T00:42:30Z

thx for the quick fix, please file a follow up issue

ldemailly · 2018-04-27T01:17:18Z

.circleci/config.yml

            make docker.all generate_yaml
      - run: bin/testEnvRootMinikube.sh wait
      - run: docker images
-      - run: make test/minikube/auth/e2e_simple


hey I had fixed this test to be running with auth (it needs to) - this PR reverted my fix

ldemailly · 2018-04-27T01:20:57Z

fixing the e2e simple change from auth -> noauth; back to auth in
https://github.com/istio/istio/pull/5241/files#diff-1d37e48f9ceff6d8030570cd36286a61

chxchx · 2018-04-27T19:42:48Z

Sorry that was probably messed up when I was fixing merge conflicts. Good catch!

googlebot added the cla: yes label Mar 30, 2018

chxchx force-pushed the circleTestgrid branch from df56a4d to 813c370 Compare March 30, 2018 01:31

chxchx force-pushed the circleTestgrid branch from 813c370 to 5262966 Compare March 30, 2018 01:31

costinm reviewed Mar 30, 2018

View reviewed changes

chxchx force-pushed the circleTestgrid branch 9 times, most recently from af2b305 to 84fd1b3 Compare March 30, 2018 21:19

istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 3, 2018

chxchx force-pushed the circleTestgrid branch from a27dfa2 to ec4db74 Compare April 3, 2018 00:08

istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 3, 2018

chxchx force-pushed the circleTestgrid branch from ec4db74 to 94640dc Compare April 3, 2018 00:21

istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 3, 2018

chxchx force-pushed the circleTestgrid branch from 94640dc to 04426c0 Compare April 3, 2018 00:26

istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 3, 2018

chxchx force-pushed the circleTestgrid branch 9 times, most recently from 4e91016 to c7ea090 Compare April 3, 2018 23:04

chxchx force-pushed the circleTestgrid branch 10 times, most recently from 70c6fcb to 842c361 Compare April 11, 2018 22:25

costinm reviewed Apr 12, 2018

View reviewed changes

sebastienvas reviewed Apr 19, 2018

View reviewed changes

bin/ci2gubernator.sh Outdated

Copy link
Copy Markdown

Contributor

sebastienvas Apr 19, 2018

Choose a reason for hiding this comment

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

Please remove the wget part.

costinm approved these changes Apr 20, 2018

View reviewed changes

integrate circle ci with testgrid

e6edcd8

ldemailly reviewed Apr 27, 2018

View reviewed changes

Conversation

chxchx commented Mar 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chxchx commented Apr 12, 2018

Uh oh!

costinm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chxchx Apr 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chxchx commented Apr 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebastienvas commented Apr 19, 2018

Uh oh!

chxchx commented Apr 19, 2018

Uh oh!

costinm commented Apr 20, 2018

Uh oh!

costinm left a comment

Choose a reason for hiding this comment

Uh oh!

sebastienvas commented Apr 20, 2018

Uh oh!

istio-testing commented Apr 20, 2018

Uh oh!

istio-merge-robot commented Apr 20, 2018

Uh oh!

istio-testing commented Apr 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

istio-merge-robot commented Apr 20, 2018

Uh oh!

ldemailly commented Apr 23, 2018

Uh oh!

chxchx commented Apr 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldemailly commented Apr 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldemailly commented Apr 27, 2018

Uh oh!

chxchx commented Apr 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

chxchx commented Mar 30, 2018 •

edited

Loading

codecov bot commented Mar 30, 2018 •

edited

Loading

chxchx Apr 13, 2018 •

edited

Loading

istio-testing commented Apr 20, 2018 •

edited

Loading

chxchx commented Apr 23, 2018 •

edited

Loading