2nd iteration of Gobindata + RepoRoot removals. #25584

jayunit100 · 2016-05-13T19:16:53Z

Part of my overall life mission to kill reporoot.

Fixes part of #24348

cc @kubernetes/sig-testing .

jayunit100 · 2016-05-13T19:17:42Z

this is untested locally, but this should pass e2e's.

jayunit100 · 2016-05-16T14:30:42Z

@k8s-bot test this please issue #IGNORE

jayunit100 · 2016-05-16T20:04:48Z

@timothysc @spiffxp @ixdy PTAL

timothysc · 2016-05-16T20:47:50Z

It's still a command line option..? What else needs it?

spiffxp · 2016-05-16T21:03:09Z

test/e2e/scheduler_predicates.go

-		nodeSelectionRoot := filepath.Join(framework.TestContext.RepoRoot, "test/e2e/testing-manifests/node-selection")
-		testPodPath := filepath.Join(nodeSelectionRoot, "pod-with-node-affinity.yaml")
-		framework.RunKubectlOrDie("create", "-f", testPodPath, fmt.Sprintf("--namespace=%v", ns))
+		affinityPod := string(framework.ReadOrDie("test/e2e/node-selection/pod-with-node-affinity.yaml")[:])


I'm guessing since this merged you'll want to change this back to testing-manifests? #25564

Agh I guess i should automate this whole damn thing to check parity

googlebot · 2016-05-17T15:35:21Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

k8s-bot · 2016-07-29T22:07:27Z

GCE e2e build/test passed for commit 6166083.

ixdy · 2016-07-29T22:07:38Z

test/e2e_node/jenkins/e2e-node-jenkins.sh

@@ -28,7 +28,16 @@ set -x

 . $1

+if [ "$INSTALL_GODEP" = true ] ; then


minor style nit: prefer [[ ]] format for conditionals and ${var} format

I'm also not sure where INSTALL_GODEP is set.

i think i took the need for this out a while ago when e2e-node was updated, i left the if in here just in case it was needed again. should i delete entirely ?

i just went ahead and removed this whole step. the new e2e-node-jenkins doesnt need this on any machine, it used to require it for some reason iirc on my mac (or some other machine i ran this on?)...

Please do all this in a rule in a makefile, so humans can call as much or as little of the build process as they want.

k8s-github-robot · 2016-07-29T22:07:44Z

Automatic merge from submit-queue

ixdy · 2016-07-29T22:09:46Z

test/e2e_node/jenkins/e2e-node-jenkins.sh

 make generated_files
+
+# TODO converge build steps with hack/build-go some day if possible.
+go generate test/e2e/framework/gobindata_util.go


do the node e2e tests even use any of the examples/ files? @pwittrock @timstclair

Framework compilation is required by node e2e

ixdy · 2016-07-29T22:10:36Z

basically LG, and thanks for doing this, but please take a look at the few comments I left.

jayunit100 · 2016-07-29T22:43:38Z

@ixdy submit queue pushed it , I'll do a follow on pr tonite

@spxtr

Automatic merge from submit-queue [minor] couple of quick cleanups for kill reporoot quick fixes from last one, cc @spxtr @ixdy follow on to #25584

lavalamp · 2016-08-02T17:36:09Z

@ixdy, somehow this passed all of our test jobs even though it has a bunch of unresolved merge conflicts in test-cmd.sh. Why did no presubmit catch this?

ixdy · 2016-08-02T17:48:40Z

well, as far as git was concerned, they were resolved, I think.

spxtr · 2016-08-02T17:55:02Z

It was a bad rebase. I'm not sure why test-cmd wasn't run before merging.

jayunit100 · 2016-08-02T19:48:14Z

interesting. i guess the entire test-cmd file was overwritten during that rebase.

is this conversation related to #17579 ? "ci hides test-cmd failures".

ixdy · 2016-08-02T22:05:55Z

The reason that hack/test-cmd.sh was broken but CI didn't notice is that CI is now using hack/make-rules/test-cmd.sh (via make test-cmd) instead of hack/test-cmd.sh (as of #25978 merging).

This is obviously super confusing. @thockin were you planning to delete the hack/* versions of scripts, or at least put HUGE WARNINGS that folks should not be using them anymore?

ixdy · 2016-08-02T22:06:52Z

... oh, that's what it does. ha. OK, nothing to see here.

ixdy · 2016-08-02T22:11:34Z

@jayunit100 do you remember if you had any real changes to hack/test-cmd.sh? I'm guessing not, since everything is still passing...

jayunit100 · 2016-08-03T00:10:15Z

Iirc no, but the pr is old and I've been working on other things. I'll be back in kubernetes land on the 15th

kevin-wangzefeng · 2016-08-03T14:34:41Z

test/e2e/scheduler_predicates.go

@@ -1213,8 +1211,7 @@ var _ = framework.KubeDescribe("SchedulerPredicates [Serial]", func() {

 		By("Trying to launch a pod that with PodAffinity & PodAntiAffinity setting as embedded JSON string in the annotation value.")
 		labelPodName := "with-newlabels"
-		nodeSelectionRoot := filepath.Join(framework.TestContext.RepoRoot, "test/e2e/testing-manifests/node-selection")
-		testPodPath := filepath.Join(nodeSelectionRoot, "pod-with-pod-affinity.yaml")
+		testPodPath := string(framework.ReadOrDie("test/e2e/testing-manifests/node-selection/pos-with-node-affinity.yaml"))


The change here is not correct, should not read the file content.

testPodPath := "test/e2e/testing-manifests/node-selection/pod-with-pod-affinity.yaml"

Actually this also doesn't work, as such file doesn't exist (or something) and kubectl can't read gobindata (I think). The point is - it doesn't work as well. @jayunit100 - I'm going to spend half a day fixing this, is using gobindata really, really, really worth it? @thockin

I just sent a PR #30061 to change it back, feel free to close if you have better solution :)

I was exaggerating, as I was annoyed by this problem and the fact that somehow gobindeps PR went in even though it broke the test completely. We really, really, really should make serial suite blocking, however painful it may be. @lavalamp @fejta

What was missing in the PR automation?

we don't run [Serial] tests per-PR.

…tes-affinity-tests Automatic merge from submit-queue fix creating pod from file failure in scheduler-predicates fix #29816 ref #25584 (comment) and #25584 (comment)

@jayunit100

Automatic merge from submit-queue Install go-bindata in cross-build image Another follow-up to #25584. We need `go-bindata` to create `test/e2e/generated`, and downloading it with `go get` at build time is painful for a variety of reasons. We can just include it in the cross-build image and not worry about it, especially as it updates very infrequently. This fixes `hack/update-generated-protobuf.sh` as well. cc @jayunit100 @soltysh

lavalamp · 2016-08-04T17:07:24Z

hack/update-bindata.sh

@@ -0,0 +1,59 @@
+#!/bin/bash


@thockin has done a bunch of work to get our Makefile doing stuff like this automatically. The Makefile's topological sort ensures that it is always done when necessary and (depending how fancy) not done when unnecessary.

I would strongly recommend that you produce either your own makefile for building the test job, or (probably better) add a rule in the main makefile.

lavalamp · 2016-08-04T17:09:32Z

@jayunit100 sorry I didn't realize this was merged already, but please take a look at the comments I just left, maybe you can fix in a followup.

googlebot added the cla: yes label May 13, 2016

k8s-github-robot assigned spxtr May 13, 2016

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels May 13, 2016

jayunit100 force-pushed the kill-repo-root-slb branch 2 times, most recently from 7435036 to f2d0eda Compare May 13, 2016 19:56

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2016

jayunit100 changed the title ~~kill repo-root : test/e2e/serviceloadbalancers.go~~ [wip] kill repo-root : test/e2e/serviceloadbalancers.go May 16, 2016

jayunit100 force-pushed the kill-repo-root-slb branch from f2d0eda to 8d1cc0b Compare May 16, 2016 14:55

k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 16, 2016

jayunit100 force-pushed the kill-repo-root-slb branch 2 times, most recently from 9719918 to 96e16de Compare May 16, 2016 18:42

jayunit100 changed the title ~~[wip] kill repo-root : test/e2e/serviceloadbalancers.go~~ 2nd iteration of Gobindata + RepoRoot removals. May 16, 2016

timothysc added this to the v1.3 milestone May 16, 2016

timothysc added area/test priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. labels May 16, 2016

spiffxp reviewed May 16, 2016
View reviewed changes

jayunit100 force-pushed the kill-repo-root-slb branch from 96e16de to 4e54e08 Compare May 17, 2016 15:35

googlebot added cla: no and removed cla: yes labels May 17, 2016

k8s-github-robot removed the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 17, 2016

ixdy reviewed Jul 29, 2016
View reviewed changes

k8s-github-robot merged commit df0d582 into kubernetes:master Jul 29, 2016

ixdy reviewed Jul 29, 2016
View reviewed changes

jayunit100 mentioned this pull request Jul 30, 2016

[minor] couple of quick cleanups for kill reporoot #29813

Merged

deads2k mentioned this pull request Aug 2, 2016

fix hack/test-cmd #29918

Merged

ixdy mentioned this pull request Aug 2, 2016

Install go-bindata in cross-build image #29947

Merged

ixdy mentioned this pull request Aug 2, 2016

CI and test-cmd #29938

Closed

kevin-wangzefeng reviewed Aug 3, 2016
View reviewed changes

This was referenced Aug 3, 2016

[k8s.io] SchedulerPredicates [Serial] validates that embedding the JSON NodeAffinity setting as a string in the annotation value work {Kubernetes e2e suite} #29816

Closed

fix creating pod from file failure in scheduler-predicates #29996

Merged

This was referenced Aug 4, 2016

[k8s.io] SchedulerPredicates [Serial] validates that embedding the JSON NodeAffinity setting as a string in the annotation value work {Kubernetes e2e suite} #30018

Closed

fix nodeAffinity e2e file not exist failure #30061

Closed

lavalamp reviewed Aug 4, 2016
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2nd iteration of Gobindata + RepoRoot removals. #25584

2nd iteration of Gobindata + RepoRoot removals. #25584

jayunit100 commented May 13, 2016

jayunit100 commented May 13, 2016

jayunit100 commented May 16, 2016

jayunit100 commented May 16, 2016 •

edited

Loading

timothysc commented May 16, 2016 •

edited

Loading

spiffxp May 16, 2016 •

edited

Loading

jayunit100 May 16, 2016

googlebot commented May 17, 2016

k8s-bot commented Jul 29, 2016

ixdy Jul 29, 2016

ixdy Jul 29, 2016

jayunit100 Jul 30, 2016

jayunit100 Jul 30, 2016 •

edited

Loading

lavalamp Aug 4, 2016

k8s-github-robot commented Jul 29, 2016

ixdy Jul 29, 2016

jayunit100 Jul 29, 2016

ixdy commented Jul 29, 2016

jayunit100 commented Jul 29, 2016

lavalamp commented Aug 2, 2016

ixdy commented Aug 2, 2016

spxtr commented Aug 2, 2016

jayunit100 commented Aug 2, 2016

ixdy commented Aug 2, 2016 •

edited

Loading

ixdy commented Aug 2, 2016

ixdy commented Aug 2, 2016

jayunit100 commented Aug 3, 2016

kevin-wangzefeng Aug 3, 2016 •

edited

Loading

gmarek Aug 4, 2016

kevin-wangzefeng Aug 4, 2016

gmarek Aug 4, 2016

timothysc Aug 4, 2016

ixdy Aug 4, 2016

lavalamp Aug 4, 2016

lavalamp commented Aug 4, 2016

		@@ -28,7 +28,16 @@ set -x

		. $1

		if [ "$INSTALL_GODEP" = true ] ; then

2nd iteration of Gobindata + RepoRoot removals. #25584

2nd iteration of Gobindata + RepoRoot removals. #25584

Conversation

jayunit100 commented May 13, 2016

jayunit100 commented May 13, 2016

jayunit100 commented May 16, 2016

jayunit100 commented May 16, 2016 • edited Loading

timothysc commented May 16, 2016 • edited Loading

spiffxp May 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

googlebot commented May 17, 2016

k8s-bot commented Jul 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayunit100 Jul 30, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-github-robot commented Jul 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ixdy commented Jul 29, 2016

jayunit100 commented Jul 29, 2016

lavalamp commented Aug 2, 2016

ixdy commented Aug 2, 2016

spxtr commented Aug 2, 2016

jayunit100 commented Aug 2, 2016

ixdy commented Aug 2, 2016 • edited Loading

ixdy commented Aug 2, 2016

ixdy commented Aug 2, 2016

jayunit100 commented Aug 3, 2016

kevin-wangzefeng Aug 3, 2016 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lavalamp commented Aug 4, 2016

jayunit100 commented May 16, 2016 •

edited

Loading

timothysc commented May 16, 2016 •

edited

Loading

spiffxp May 16, 2016 •

edited

Loading

jayunit100 Jul 30, 2016 •

edited

Loading

ixdy commented Aug 2, 2016 •

edited

Loading

kevin-wangzefeng Aug 3, 2016 •

edited

Loading