Skip to content

prow: introduce prow #253

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 2 commits into from
Jul 10, 2017
Merged

prow: introduce prow #253

merged 2 commits into from
Jul 10, 2017

Conversation

nlandolfi
Copy link
Contributor

@nlandolfi nlandolfi commented Jun 30, 2017

Introduce prow related resources and configurations.

  1. Introduce the cluster deployment and config files for Istio
  2. Introduce docker/prowbazel image; a modified version of the image used on Jenkins, suitable for prow jobs
  3. Introduce test-infra-presubmit job to this repository.

Update 7/6/17: prow is running, view the simple deck frontend here. It is running as the prow cluster on istio-testing, so with access to the project one can also get the kubectl context and get the pods:

$ kubectl get pods
 NAME                     READY     STATUS    RESTARTS   AGE
deck-3409711596-8qs1f    1/1       Running   0          8m
deck-3409711596-jq191    1/1       Running   0          8m
deck-3409711596-xjdw3    1/1       Running   0          8m
hook-4091801138-bg7cr    1/1       Running   0          8m
plank-1139864133-2d0g3   1/1       Running   0          8m
sinker-79557021-1d9xm    1/1       Running   0          8m
tot-3401709015-lkwgm     1/1       Running   0          8m

This PR adds the test-infra-presubmit job file. This results of test runs from this file are here. Notice that we are piggy-backing on kubernetes instance of gubernator, a UI for viewing job results.

I have turned off public reporting for all prow jobs configured in prow/cluster/configmaps/config.yaml for the moment, because it was needlessly failing on other PRs (c.f. 254.

My intention is to create a pull request against pilot adding its job files in a similar manner. I will wait to see that those are passing (while keeping reporting off), then once that merges master we can turn on the public reporting. As we can turn on public reporting once this PR goes to master (because the jobs/test-infra-presubmit.sh file will be in master).

TODO:

  • Changes to bootstrap.py our k8s-test-infra fork
  • Look into modifying the checkout directory in bootstrap.py as per Seb's comment

@googlebot
Copy link

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.

Copy link
Contributor

@kyessenov kyessenov left a comment

Choose a reason for hiding this comment

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

I hope you don't expect us to review 600k lines of code :)

@nlandolfi
Copy link
Contributor Author

nlandolfi commented Jun 30, 2017 via email

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@nlandolfi
Copy link
Contributor Author

for now axing k8s-test-infra subtree because I don't modify here, and we use the k8s-prow images now (rather than build our own from source).

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

1 similar comment
@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@nlandolfi
Copy link
Contributor Author

@istio-testing bazel test this

@nlandolfi nlandolfi changed the title WIP: prow: introduce prow prow: introduce prow Jul 6, 2017
@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

docker/README.md Outdated
@@ -0,0 +1,4 @@
This directory contains standard images maintained by Istio for test infrastructure purposes.

* prowbazel - gives an environment with git, gcloud, python, bazel, and go. It is suitable for bazel-based projects.
Copy link
Member

Choose a reason for hiding this comment

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

is this deeply tied to gcloud? Can this be run on other platforms?

Copy link
Member

Choose a reason for hiding this comment

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

The reason I ask is because eventually folks would want to run tests against istio/istio on their own environments. Having a jenkins and a prow seems ripe for all sorts of issues such as test environment divergence.

Copy link
Contributor

Choose a reason for hiding this comment

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

The prowbazel docker image will be used in every job. We are actually uploading artifacts and logs to google cloud storage. Those would be publicly available. If folks want to upload to other cloud, they can modify this image to add new tools or create their own image. In a near future we ll probably use specific image for specific jobs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point @rshriram -- the prowbazel docker file in particular, not tied to GCP, just a docker file (we do host it on GCR).

to reiterate as @sebastienvas mentioned, the entrypoint of the docker file uploads logs to a gcs bucket, which is google specific. the bucket is read public.

other folks running infrastructure would likely use an image that uploads logs to their preferred storage.

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.

it's exciting to see this taking shape, nice work!

quick random question below:

@@ -0,0 +1,965 @@
#!/usr/bin/env python

# Copyright 2016 The Kubernetes Authors.
Copy link
Member

Choose a reason for hiding this comment

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

any way to reference rather than copy this large file ? or did you make changes to it (which change compared to the original ? where is the original?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have forked the kubernetest/test-infra repo here. I have a PR against our fork here. I cleaned up my changes so that they are flags which, when unspecified, do not modify current behavior, but also allow us to modify/turn off the things we need to.

@@ -150,6 +150,7 @@ presubmits:

istio/test-infra:
- name: test-infra-presubmit
skip_report: true
Copy link
Member

Choose a reason for hiding this comment

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

why ? I'm curious to see how prow reports work and I hope it doesn't have the bug that our jenkins report has of posting a comment that triggers a lot of useless github notifications ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This diff introduces the prow/test-infra-presubmit.sh, but other PRs on these repos don't yet have the file, and the job would fail, therefore I turned off reporting.

re: notifications, Prow does comment on the PR for test failures.

Copy link
Contributor

@sebastienvas sebastienvas left a comment

Choose a reason for hiding this comment

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

PTAL

docker/README.md Outdated
@@ -0,0 +1,4 @@
This directory contains standard images maintained by Istio for test infrastructure purposes.

* prowbazel - gives an environment with git, gcloud, python, bazel, and go. It is suitable for bazel-based projects.
Copy link
Contributor

Choose a reason for hiding this comment

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

The prowbazel docker image will be used in every job. We are actually uploading artifacts and logs to google cloud storage. Those would be publicly available. If folks want to upload to other cloud, they can modify this image to add new tools or create their own image. In a near future we ll probably use specific image for specific jobs.

ENV TEST_TMPDIR /home/bootstrap/.cache/bazel

# Add bootstrap, the test harness (checks out code, captures log and status)
ADD ./docker/prowbazel/bootstrap.py ${HOME}
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on our discussion with @ldemailly, this is where you would put an URL. https://docs.docker.com/engine/reference/builder/#add. The url might something like https://raw.githubusercontent.com/kubernetes/test-infra/master/jenkins/bootstrap.py


# Open shell to running prowbazel image for experimentation.
run:
docker run -it --entrypoint bash --privileged gcr.io/istio-testing/prowbazel:0.1
Copy link
Contributor

Choose a reason for hiding this comment

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

replace istio-testing with $(PROJECT) and 0.1 with $(VERSION)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, done.


if [ "$CI" == "bootstrap" ]; then
# ensure correct path
mkdir -p $GOPATH/src/istio.io
Copy link
Contributor

Choose a reason for hiding this comment

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

It kind of sucks that we need to do that on every job. Could we move that feature into bootstrap once we have our own fork ?

prow/README.md Outdated
.
├── anotherdir
│   └── ...
├── jobs
Copy link
Contributor

Choose a reason for hiding this comment

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

let s rename this to prow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,102 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

let s move this to prow/config.yaml as discussed.

@@ -0,0 +1,9 @@
# Plugin repository whitelist.
Copy link
Contributor

Choose a reason for hiding this comment

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

let move this to prow/plugins.yaml as discussed.

prow/Makefile Outdated
kubectl apply -f cluster/deck_deployment.yaml

deck-service: get-cluster-credentials
kubectl create -f cluster/deck_service.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

why not make everything apply?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call. done

@@ -0,0 +1,6 @@
apiVersion: extensions/v1beta1
kind: ThirdPartyResource
Copy link
Contributor

Choose a reason for hiding this comment

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

should probably switch to CustomResourceDefinition since TPRs are deprecated. I'd expect the upstream project to do the work fairly soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch. I created an issue upstream, I assume the plan is to make the switch.

@nlandolfi
Copy link
Contributor Author

@istio-testing bazel test this

1 similar comment
@nlandolfi
Copy link
Contributor Author

@istio-testing bazel test this

@nlandolfi
Copy link
Contributor Author

@istio-testing bazel test this

@nlandolfi
Copy link
Contributor Author

nlandolfi commented Jul 10, 2017

the prow results for the presubmit are here.

@sebastienvas

@googlebot
Copy link

CLAs look good, thanks!

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@googlebot
Copy link

CLAs look good, thanks!

@istio-testing
Copy link
Collaborator

This is a test.

@istio-testing
Copy link
Collaborator

Jenkins job test-infra/presubmit passed

@nlandolfi nlandolfi merged commit 9a2d644 into istio:master Jul 10, 2017
@istio-testing
Copy link
Collaborator

@nlandolfi: I updated Prow config for you!

I updated Prow plugins config for you!.

In response to this:

Introduce prow related resources and configurations.

  1. Introduce the cluster deployment and config files for Istio
  2. Introduce docker/prowbazel image; a modified version of the image used on Jenkins, suitable for prow jobs
  3. Introduce test-infra-presubmit job to this repository.

Update 7/6/17: prow is running, view the simple deck frontend here. It is running as the prow cluster on istio-testing, so with access to the project one can also get the kubectl context and get the pods:

$ kubectl get pods
NAME                     READY     STATUS    RESTARTS   AGE
deck-3409711596-8qs1f    1/1       Running   0          8m
deck-3409711596-jq191    1/1       Running   0          8m
deck-3409711596-xjdw3    1/1       Running   0          8m
hook-4091801138-bg7cr    1/1       Running   0          8m
plank-1139864133-2d0g3   1/1       Running   0          8m
sinker-79557021-1d9xm    1/1       Running   0          8m
tot-3401709015-lkwgm     1/1       Running   0          8m

This PR adds the test-infra-presubmit job file. This results of test runs from this file are here. Notice that we are piggy-backing on kubernetes instance of gubernator, a UI for viewing job results.

I have turned off public reporting for all prow jobs configured in prow/cluster/configmaps/config.yaml for the moment, because it was needlessly failing on other PRs (c.f. 254.

My intention is to create a pull request against pilot adding its job files in a similar manner. I will wait to see that those are passing (while keeping reporting off), then once that merges master we can turn on the public reporting. As we can turn on public reporting once this PR goes to master (because the jobs/test-infra-presubmit.sh file will be in master).

TODO:

  • Changes to bootstrap.py our k8s-test-infra fork
  • Look into modifying the checkout directory in bootstrap.py as per Seb's comment

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.

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