sig-storage: initial config for kubernetes-csi/csi-lib-utils #10309

pohly · 2018-12-03T15:51:07Z

kubernetes-csi/csi-lib-utils is a brand-new stand-alone repo with some
Go utility package (one at the moment, more to come). "make test"
inside it enforces coding style and that unit tests pass, which is
something that we want to ensure before merging any PR.

This is the first config for sig-storage, hence the new OWNERS file.

Fixes: kubernetes-csi/csi-lib-utils#2

k8s-ci-robot · 2018-12-03T15:51:08Z

Welcome @pohly! It looks like this is your first PR to kubernetes/test-infra 🎉🎉

pohly · 2018-12-03T16:02:44Z

/cc @msau42 @saad-ali

If there's a way to test this config fragment before merging, the please let me know - as it stands, it's basically completely untested.

make test in the repo under test of course passes.

krzyzacy · 2018-12-03T18:19:25Z

config/jobs/kubernetes-sigs/sig-storage/csi/csi-lib-utils-config.yaml

+presubmits:
+  github.com/kubernetes-csi/csi-lib-utils:
+  - name: pull-sig-storage-csi-lib-utils
+    always_run: true


you can flip always_run to false to start with, and you can trigger it on your repo with /test pull-sig-storage-csi-lib-utils

I had considered that, but as we don't have a lot going on in that repo I wasn't worried about breaking anything by making the testing mandatory right away.

@msau42 your call - we can also keep it disabled and do another PR here once we know that it works.

always_run is fine. i don't expect much activity yet

krzyzacy · 2018-12-03T18:21:38Z

also create new jobs with pod-utils now :-) it's as easy as https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes-sigs/kind/kind-presubmits.yaml#L2-L12 and you can drop most of the args

pohly · 2018-12-03T21:42:13Z

@krzyzacy Thanks for the pointer to "Create a new job" - when I asked about prow on Slack the other day, I was only pointed to prow's own README.md and existing examples, so I started from those.

Your example uses the gcr.io/k8s-testimages/kubekins-e2e:v20181129-33929460d-master image and it's ./hack/ci/build-all.sh script. But you were just using that as an example for something that uses podutils and not suggesting that I switch to that image, right?

pohly · 2018-12-03T21:46:03Z

@krzyzacy is the directory structure okay? The org is the "kubernetes-csi" working group inside the "sig-storage" SIG, the repo is "csi-lib-utils" - I wasn't quite sure where to put the config .yaml, so I created a directory for "sig-storage" with a "csi" sub-directory for the CSI specific repos.

msau42 · 2018-12-04T03:26:53Z

config/jobs/kubernetes-sigs/sig-storage/OWNERS

@@ -0,0 +1,5 @@
+approvers:


I would have the directory structure match the actual github repo structure, so "kubernetes-csi/csi-lib-utils"

So no "sig-storage", i.e. config/jobs/kubernetes-sigs/kubernetes-csi/csi-lib-utils/csi-lib-utils-config.yaml and config/jobs/kubernetes-sigs/kubernetes-csi/OWNERS?

No kubernetes-sigs either (it is an actual GitHub org).

pohly · 2018-12-04T15:50:41Z

@msau42 @krzyzacy updated, please have another look.

krzyzacy · 2018-12-04T18:04:08Z

if this is a new org, did you configure webhooks? Also you need to add an entry, something like https://github.com/kubernetes/test-infra/blob/master/prow/plugins.yaml#L300-L308 (you can choose what plugins you want to enable)

pohly · 2018-12-04T18:43:23Z

Sen Lu <[email protected]> writes:

if this is a new org, did you configure webhooks?

The kubernetes-csi org itself isn't new and has been part of Kubernetes for a while now, so I think the webhooks are in place. For example, the PR dashboard shows the PRs in the new repo.

Also you need to add an entry, something like https://github.com/kubernetes/test-infra/blob/master/prow/plugins.yaml#L300-L308 (you can choose what plugins you want to enable)

Isn't that already covered by the entry for "kubernetes-csi"? https://github.com/kubernetes/test-infra/blob/f61cf6eab16515ffc32a85489d56847aee9f408a/prow/plugins.yaml#L445-L463 Or is there perhaps some plugin missing? I have no idea what each plugin does, but looking at the other entries perhaps "trigger" is missing? In other words, add this? ``` kubernetes-csi/csi-lib-utils: - trigger ```

krzyzacy · 2018-12-04T18:47:34Z

@pohly sorry, missed that 😂 For some reason I read it as a brand new org, nvm

edit: and yes, please add

kubernetes-csi/csi-lib-utils:
- trigger

if that's not present.

kubernetes-csi/csi-lib-utils is a brand-new stand-alone repo with some Go utility package (one at the moment, more to come). "make test" inside it enforces coding style and that unit tests pass, which is something that we want to ensure before merging any PR. This is the first config for kubernetes-csi, hence the new OWNERS file. Fixes: kubernetes-csi/csi-lib-utils#2

pohly · 2018-12-04T19:45:33Z

@krzyzacy I've added the "trigger" plugin.

msau42 · 2018-12-04T20:02:35Z

/lgtm

k8s-ci-robot · 2018-12-04T20:02:38Z

LGTM label has been added.

Git tree hash: fe574c6461b3cf361887bf5ba988cae127b70178

krzyzacy · 2018-12-04T20:40:10Z

config/jobs/kubernetes-csi/csi-lib-utils/csi-lib-utils-config.yaml

+      containers:
+      # This image was chosen over the more often used kubekins-e2e because
+      # it is smaller and has everything we need (basically just a Go environment).
+      - image: gcr.io/k8s-testimages/gcloud-in-go:v20180927-6b4facbe6


if you just need go, you can also choose to use something like golang:1.11.2

I was trying to use something that might have been pulled already, i.e. something which is also used by other jobs. Not sure whether that makes sense?

But I agree, now that all of the special logic is handled by the pod-utils, a generic image should work just as well.

Either way, I'd like to get this merged and perhaps it can be tweaked further later on. We have quite a few repos under kubernetes-csi that might benefit from closer integration with Prow, so I expect that we'll come back to this.

You might want to control go versions explicitly - feel free to come back and tune this if you are hitting issues.

krzyzacy · 2018-12-04T21:18:41Z

/lgtm

k8s-ci-robot · 2018-12-04T21:18:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: krzyzacy, msau42, pohly

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:

~~config/OWNERS~~ [krzyzacy]
~~prow/OWNERS~~ [krzyzacy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2018-12-04T21:30:43Z

@pohly: Updated the following 2 configmaps:

job-config configmap using the following files:
- key csi-lib-utils-config.yaml using file config/jobs/kubernetes-csi/csi-lib-utils/csi-lib-utils-config.yaml
plugins configmap using the following files:
- key plugins.yaml using file prow/plugins.yaml

In response to this:

kubernetes-csi/csi-lib-utils is a brand-new stand-alone repo with some
Go utility package (one at the moment, more to come). "make test"
inside it enforces coding style and that unit tests pass, which is
something that we want to ensure before merging any PR.

This is the first config for sig-storage, hence the new OWNERS file.

Fixes: kubernetes-csi/csi-lib-utils#2

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.

pohly · 2018-12-04T22:03:01Z

Now that this is merged, should /test pull-sig-storage-csi-lib-utils work right away?

I tried that in kubernetes-csi/csi-lib-utils#4 (comment) but so far nothing has been triggered, at least nothing is visible in github.

krzyzacy · 2018-12-04T22:10:27Z

config/jobs/kubernetes-csi/csi-lib-utils/csi-lib-utils-config.yaml

@@ -0,0 +1,17 @@
+presubmits:
+  github.com/kubernetes-csi/csi-lib-utils:


ooooops, drop github.com here 😂 (prow key jobs off org/repo pattern)

Thanks for debugging it. Fix is in #10340.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/config Issues or PRs related to code in /config labels Dec 3, 2018

k8s-ci-robot requested review from BenTheElder and krzyzacy December 3, 2018 15:51

pohly force-pushed the sig-storage-csi-lib-utils branch from f0afd49 to 4c6eded Compare December 3, 2018 16:02

k8s-ci-robot requested review from msau42 and saad-ali December 3, 2018 16:02

krzyzacy reviewed Dec 3, 2018

View reviewed changes

msau42 reviewed Dec 4, 2018

View reviewed changes

pohly force-pushed the sig-storage-csi-lib-utils branch from 4c6eded to 7531f85 Compare December 4, 2018 15:50

pohly force-pushed the sig-storage-csi-lib-utils branch from 7531f85 to 21b20c3 Compare December 4, 2018 19:45

k8s-ci-robot added the area/prow Issues or PRs related to prow label Dec 4, 2018

k8s-ci-robot assigned msau42 Dec 4, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 4, 2018

krzyzacy reviewed Dec 4, 2018

View reviewed changes

k8s-ci-robot assigned krzyzacy Dec 4, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 4, 2018

k8s-ci-robot merged commit 3085040 into kubernetes:master Dec 4, 2018

krzyzacy reviewed Dec 4, 2018

View reviewed changes

krzyzacy mentioned this pull request Dec 4, 2018

support also CSI <= 0.3.0 kubernetes-csi/csi-lib-utils#4

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sig-storage: initial config for kubernetes-csi/csi-lib-utils #10309

sig-storage: initial config for kubernetes-csi/csi-lib-utils #10309

pohly commented Dec 3, 2018

k8s-ci-robot commented Dec 3, 2018

pohly commented Dec 3, 2018

krzyzacy Dec 3, 2018 •

edited

Loading

pohly Dec 3, 2018

msau42 Dec 4, 2018

krzyzacy commented Dec 3, 2018

pohly commented Dec 3, 2018

pohly commented Dec 3, 2018

msau42 Dec 4, 2018

pohly Dec 4, 2018

msau42 Dec 4, 2018

pohly commented Dec 4, 2018

krzyzacy commented Dec 4, 2018

pohly commented Dec 4, 2018 via email

krzyzacy commented Dec 4, 2018 •

edited

Loading

pohly commented Dec 4, 2018

msau42 commented Dec 4, 2018

k8s-ci-robot commented Dec 4, 2018

krzyzacy Dec 4, 2018

pohly Dec 4, 2018

krzyzacy Dec 4, 2018

krzyzacy commented Dec 4, 2018

k8s-ci-robot commented Dec 4, 2018

k8s-ci-robot commented Dec 4, 2018

pohly commented Dec 4, 2018

krzyzacy Dec 4, 2018 •

edited

Loading

pohly Dec 4, 2018

		@@ -0,0 +1,17 @@
		presubmits:
		github.com/kubernetes-csi/csi-lib-utils:

sig-storage: initial config for kubernetes-csi/csi-lib-utils #10309

sig-storage: initial config for kubernetes-csi/csi-lib-utils #10309

Conversation

pohly commented Dec 3, 2018

k8s-ci-robot commented Dec 3, 2018

pohly commented Dec 3, 2018

krzyzacy Dec 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzyzacy commented Dec 3, 2018

pohly commented Dec 3, 2018

pohly commented Dec 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pohly commented Dec 4, 2018

krzyzacy commented Dec 4, 2018

pohly commented Dec 4, 2018 via email

krzyzacy commented Dec 4, 2018 • edited Loading

pohly commented Dec 4, 2018

msau42 commented Dec 4, 2018

k8s-ci-robot commented Dec 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzyzacy commented Dec 4, 2018

k8s-ci-robot commented Dec 4, 2018

k8s-ci-robot commented Dec 4, 2018

pohly commented Dec 4, 2018

krzyzacy Dec 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzyzacy Dec 3, 2018 •

edited

Loading

krzyzacy commented Dec 4, 2018 •

edited

Loading

krzyzacy Dec 4, 2018 •

edited

Loading