Prow presubmit job to build mlkube.io container. #4951

jlewi · 2017-10-10T13:19:06Z

This job is supposed to build a Docker container with the CRD. Once
this works we will add another job to actually run the tests.
We create a new image to be used by this job.
- The image needs docker so that we can do a Docker build.

k8s-ci-robot · 2017-10-10T13:19:08Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: helpdesk@rt.linuxfoundation.org

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.

jlewi · 2017-10-10T13:19:34Z

@krzyzacy @foxish PTAL.

* This job is supposed to build a Docker container with the CRD. Once this works we will add another job to actually run the tests. * We create a new image to be used by this job. * The image needs docker so that we can do a Docker build.

krzyzacy · 2017-10-10T17:42:35Z

prow/config.yaml

+          ports:
+          # TODO(jlewi): Do we need this port?
+          - containerPort: 9999
+            hostPort: 9999


the port is for node affinity since they shares a git cache, you probably don't need it here.

krzyzacy · 2017-10-10T17:45:29Z

prow/config.yaml

+          env:
+            - name: GOOGLE_APPLICATION_CREDENTIALS
+              value: /etc/service-account/service-account.json
+          # TODO(jlewi): Do we need privileged mode in order to run Docker?


you probably need to figure out kubernetes/kubernetes#1806, have you tried locally?

I've tried running my container locally using docker run but not running on K8s. Going to try it on my test prow cluster soon.

krzyzacy · 2017-10-10T17:45:46Z

/assign

BenTheElder · 2017-10-10T21:19:09Z

prow/config.yaml

+            secretName: service-account
+        - name: docker
+          hostPath:
+            path: /var/run/docker.sock


/cc @ixdy, I think we don't intend to allow jobs access to the host docker socket.

@BenTheElder Can you suggest an alternative approach?

@jlewi what I've been doing is using bazel to build/push the image instead, not sure if that's feasible to you

* Need to specify namespace, resync_period and some other values or the config won't be parseable.

* Add pre and postsubmit jobs for mlkube. * Remove the periodic job; I don't think we want that.

* Fix conflict with mlkube-e2e periodic job.

* We probably want this and the testgrid requires it.

jlewi · 2017-10-12T19:04:37Z

@krzyzacy This is ready for another look but there are are some test failures I could use help with.

verify_test is complaining that there is no test-grid associated with my presubmit job.

//jobs:config_test is failing complaining about duplicate projects. I added an entry here because otherwise the //prow/config:go_default_test failed complaining

jobs_test.go:152: Cannot find test-infra/jobs/mlkube-build-presubmit.sh for mlkube-build-presubmit

but it looks like my fix might have caused other problems.

krzyzacy · 2017-10-12T19:13:16Z

jobs/config.json

@@ -9907,6 +9907,27 @@
      "UNKNOWN"
    ]
  },
+  "mlkube-build-presubmit": {


you'll need an equivalent one for mlkube-build-postsubmit

krzyzacy · 2017-10-12T19:14:14Z

jobs/config.json

+      "--extract=gke",
+      "--gcp-cloud-sdk=gs://cloud-sdk-testing/ci/staging",
+      "--gcp-node-image=gci",
+      "--gcp-project=mlkube-testing",


for the presubmit error you need to add something to https://github.com/kubernetes/test-infra/blob/master/jobs/config_test.py#L681-L791

krzyzacy · 2017-10-12T19:15:05Z

I think we don't want to allow sock mapping on prow pods. We should either use bazel to build docker images, or we need to implement some docker-in-docker service in prow.

This reverts commit 7507edb. If we load all presubmits we end up with failures due to other presubmits with no testgroup.

krzyzacy · 2017-10-13T16:56:15Z

hit by #4989

…ubmit tests. * All tests should be Prow jobs; delete jobs/config.json entry for the periodic job. * All jobs should use the same container image for the tests. * Create dashboard tabs on the bigdata dashboard. Remove tabs from the apps dashboard.

jlewi · 2017-10-13T18:18:12Z

@krzyzacy This is ready for another review.

Only test failure appears to be due to pull-test-infra-bazel flakes upon installing gubernator requirements.txt #4989
I made the existing periodic test that @foxish had added consistent with the new pre/post submit tests
- I created dashboards for each test on the bigdata dashboard group
- Before it was using both big-data and apps
I removed the use of docker build and used GCR instead to build the image
- As discussed offline building the images with bazel might be better but I didn't want to introduce
  bazel in the mlkube toolchain just for this.

krzyzacy · 2017-10-13T18:20:25Z

jobs/config_test.py

+            # using this project there will just be one entry.
+            'mlkube-build-presubmit': 'mlkube-*',
+            'mlkube-build-postsubmit': 'mlkube-*',
+            'ci-kubernetes-e2e-mlkube-gke': 'mlkube-*',


now you probably don't need this block. The test basically scans duplicated projects from config.json.

krzyzacy · 2017-10-13T18:25:00Z

testgrid/config/config.yaml

+    test_group_name: mlkube-build-postsubmit
+  - name: mlkube-build-presubmit
+    description: Presubmit tests of TfJob CRD running on stable GKE.
+    test_group_name: mlkube-build-presubmit
  notifications:
  - summary: Please configure this skeleton dashboard and remove this notification
    context_link: https://github.com/kubernetes/test-infra/tree/master/testgrid/config


(probably can delete this notification now)

jlewi · 2017-10-13T18:55:27Z

Thanks.

krzyzacy · 2017-10-13T18:56:10Z

prow/config.yaml

+          secretName: service-account
+      - name: docker
+        hostPath:
+          path: /var/run/docker.sock


remove the sock map here as well?

krzyzacy · 2017-10-13T18:58:24Z

prow/config.yaml

@@ -2489,6 +2489,31 @@ presubmits:
        hostPath:
          path: /mnt/disks/ssd0

+  jlewi/mlkube.io:


hummm, I feel like our bot might not have access to your repo, you might want to configure somewhere around https://github.com/kubernetes/test-infra/blob/master/prow/plugins.yaml#L5-L11

also https://github.com/kubernetes/test-infra/blob/master/prow/plugins.yaml#L37-L38

Thanks. I also think I need to configure webhooks on my repo but I'll do that after this PR is done.

* Configure trigger plugin for mlkube.io repo.

krzyzacy · 2017-10-13T19:12:09Z

it's a good start and probably we have more stuff need to configure. we'll see how this goes :-)

/lgtm

k8s-github-robot · 2017-10-13T19:13:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi, krzyzacy

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

Needs approval from an approver in each of these OWNERS Files:

~~OWNERS~~ [krzyzacy]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-ci-robot · 2017-10-13T19:56:06Z

@jlewi: I updated Prow config for you!

I updated Prow plugins config for you!

In response to this:

This job is supposed to build a Docker container with the CRD. Once
this works we will add another job to actually run the tests.

We create a new image to be used by this job.

The image needs docker so that we can do a Docker build.

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.

jlewi · 2017-10-13T21:21:52Z

Thanks

krzyzacy · 2017-10-13T21:48:39Z

(whoops, forgot to ask you to squash 😂 )

jlewi requested a review from krzyzacy as a code owner October 10, 2017 13:19

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 10, 2017

jlewi force-pushed the mlkube_prow branch from d0328d2 to cf8ce96 Compare October 10, 2017 13:23

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Oct 10, 2017

krzyzacy reviewed Oct 10, 2017

View reviewed changes

k8s-ci-robot assigned krzyzacy Oct 10, 2017

BenTheElder reviewed Oct 10, 2017

View reviewed changes

Fix starter.yaml

01d41a2

* Need to specify namespace, resync_period and some other values or the config won't be parseable.

jlewi requested review from cjwagner and 0xmichalis as code owners October 10, 2017 23:22

* mlkube image moved to mlkube.io repo.

49da9b2

* Add pre and postsubmit jobs for mlkube. * Remove the periodic job; I don't think we want that.

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 12, 2017

jlewi added 6 commits October 12, 2017 10:56

Merge branch 'master' into mlkube_prow

6c44b46

* Fix conflict with mlkube-e2e periodic job.

Restore the mlkube periodic job.

39f7008

* We probably want this and the testgrid requires it.

Sort the configs.

d212e7b

Add a context to the presubmit job.

d8ba6c9

Fix the //prow/config:go_default_test

b0d10e5

Sort the configs.

6a4bcec

krzyzacy reviewed Oct 12, 2017

View reviewed changes

jlewi added 2 commits October 13, 2017 07:24

Revert "* Try to fix failure"

988f657

This reverts commit 7507edb. If we load all presubmits we end up with failures due to other presubmits with no testgroup.

Include presubmits from jlewi/mlkube.io

5d6a304

jlewi added 2 commits October 13, 2017 10:10

gofmt.

0090983

Merge branch 'master' into mlkube_prow

80da1c6

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 13, 2017

gofmt.

23987bf

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 13, 2017

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 13, 2017

krzyzacy reviewed Oct 13, 2017

View reviewed changes

Address code review comments.

9e76d5f

krzyzacy reviewed Oct 13, 2017

View reviewed changes

* Remove reference to socket.

6603279

* Configure trigger plugin for mlkube.io repo.

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 13, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 13, 2017

k8s-ci-robot merged commit ad548b4 into kubernetes:master Oct 13, 2017

jlewi mentioned this pull request Oct 13, 2017

Integrate with Prow for Continuous Testing kubeflow/training-operator#46

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prow presubmit job to build mlkube.io container. #4951

Prow presubmit job to build mlkube.io container. #4951

jlewi commented Oct 10, 2017

k8s-ci-robot commented Oct 10, 2017

jlewi commented Oct 10, 2017

krzyzacy Oct 10, 2017

krzyzacy Oct 10, 2017

jlewi Oct 10, 2017

krzyzacy commented Oct 10, 2017

BenTheElder Oct 10, 2017

jlewi Oct 12, 2017

krzyzacy Oct 12, 2017

jlewi commented Oct 12, 2017

krzyzacy Oct 12, 2017

krzyzacy Oct 12, 2017

krzyzacy commented Oct 12, 2017

krzyzacy commented Oct 13, 2017

jlewi commented Oct 13, 2017

krzyzacy Oct 13, 2017

jlewi Oct 13, 2017

krzyzacy Oct 13, 2017

jlewi Oct 13, 2017

jlewi commented Oct 13, 2017

krzyzacy Oct 13, 2017

jlewi Oct 13, 2017

krzyzacy Oct 13, 2017

krzyzacy Oct 13, 2017

jlewi Oct 13, 2017

krzyzacy commented Oct 13, 2017

k8s-github-robot commented Oct 13, 2017

k8s-ci-robot commented Oct 13, 2017

jlewi commented Oct 13, 2017

krzyzacy commented Oct 13, 2017

Prow presubmit job to build mlkube.io container. #4951

Prow presubmit job to build mlkube.io container. #4951

Conversation

jlewi commented Oct 10, 2017

k8s-ci-robot commented Oct 10, 2017

jlewi commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzyzacy commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlewi commented Oct 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzyzacy commented Oct 12, 2017

krzyzacy commented Oct 13, 2017

jlewi commented Oct 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlewi commented Oct 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzyzacy commented Oct 13, 2017

k8s-github-robot commented Oct 13, 2017

k8s-ci-robot commented Oct 13, 2017

@jlewi: I updated Prow config for you!

jlewi commented Oct 13, 2017

krzyzacy commented Oct 13, 2017