Skip to content

Make existing e2e tests to use container-vm explicitly#417

Merged
vishh merged 1 commit intokubernetes:masterfrom
vishh:force-debian
Aug 22, 2016
Merged

Make existing e2e tests to use container-vm explicitly#417
vishh merged 1 commit intokubernetes:masterfrom
vishh:force-debian

Conversation

@vishh
Copy link
Copy Markdown
Contributor

@vishh vishh commented Aug 19, 2016 •
edited
Loading

This is required to prepare for the switch to gci as default.
For kubernetes/kubernetes#25276

This change is Reviewable

@vishh
Copy link
Copy Markdown
Contributor Author

vishh commented Aug 19, 2016

cc @pwittrock

@vishh vishh mentioned this pull request Aug 19, 2016
Merged
@wonderfly
Copy link
Copy Markdown
Contributor

wonderfly commented Aug 19, 2016

nit: The tests are using containervm. So consider changing the commit message to something like Make tests use container-vm explicitly.

@dchen1107 dchen1107 self-assigned this Aug 19, 2016
@dchen1107
Copy link
Copy Markdown
Member

dchen1107 commented Aug 19, 2016

+1 on #417 (comment)

@dchen1107
Copy link
Copy Markdown
Member

dchen1107 commented Aug 19, 2016

Besides above nit, LGTM

@vishh vishh changed the title Force existing tests to use container-vm Make existing e2e tests to use container-vm explicitly Aug 19, 2016
@vishh
Copy link
Copy Markdown
Contributor Author

vishh commented Aug 19, 2016

PR title updated

@dchen1107
Copy link
Copy Markdown
Member

dchen1107 commented Aug 19, 2016

LGTM

@dchen1107 dchen1107 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 19, 2016
@vishh
Copy link
Copy Markdown
Contributor Author

vishh commented Aug 19, 2016

ping @kubernetes/test-infra-maintainers

@spxtr
Copy link
Copy Markdown
Contributor

spxtr commented Aug 19, 2016

It seems like excess copy-paste. Which jobs don't need this set?

Probably fine, though.

Reviewed 6 of 6 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

jenkins/job-configs/kubernetes-jenkins-pull/kubernetes-pull.yaml, line 330 [r1] (raw file):

            export KUBE_FASTBUILD=true
            export KUBE_GCS_UPDATE_LATEST=n

nit: whitespace here and in a few other places.

Comments from Reviewable

@vishh
Copy link
Copy Markdown
Contributor Author

vishh commented Aug 19, 2016

I plan on adding tests that will use gci instead of container-vm next.
After that, k8s HEAD will switch to using gci by default. This PR is
necessary to prevent existing tests from accidentally testing gci instead
of container-vm.

On Fri, Aug 19, 2016 at 2:16 PM, Joe Finney notifications@github.com
wrote:

It seems like excess copy-paste. Which jobs don't need this set?

Probably fine, though.

Reviewed 6 of 6 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved

discussion.

jenkins/job-configs/kubernetes-jenkins-pull/kubernetes-pull.yaml, line
330 [r1]
https://reviewable.io:443/reviews/kubernetes/test-infra/417#-KPZe7wF3k1M5CNHKqIP:-KPZe7wF3k1M5CNHKqIQ:bmhmjmo
(raw file

test-infra/jenkins/job-configs/kubernetes-jenkins-pull/kubernetes-pull.yaml

Line 330 in f3832a4

):

        export KUBE_FASTBUILD=true
        export KUBE_GCS_UPDATE_LATEST=n

nit: whitespace here and in a few other places.

Comments from Reviewable
https://reviewable.io:443/reviews/kubernetes/test-infra/417#-:-KPZeEymEmJj5tH5GcNE:bo03kcu

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#417 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGvIKPwZuEb6k7LKE74wh-R4l21mIGTqks5qhh0qgaJpZM4Jo0YO
.

@vishh vishh force-pushed the force-debian branch 2 times, most recently from e71c9bb to b854978 Compare August 19, 2016 22:18
@vishh
Copy link
Copy Markdown
Contributor Author

vishh commented Aug 19, 2016

@spxtr PTAL.

Review status: 5 of 6 files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

@spxtr
Copy link
Copy Markdown
Contributor

spxtr commented Aug 19, 2016

Review status: 5 of 6 files reviewed at latest revision, 2 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins-pull/kubernetes-pull.yaml, line 329 [r2] (raw file):

            export KUBE_RUN_FROM_OUTPUT=y
            export KUBE_FASTBUILD=true
            export KUBE_GCS_UPDATE_LATEST=n

Sorry, I meant you should remove all trailing whitespace. I don't think you need any whitespace on blank lines.

Comments from Reviewable

@vishh
Make existing tests to use container-vm explicitly to prepare for the…
c759109 
… switch to gci as default

Signed-off-by: Vishnu kannan <vishnuk@google.com>
@vishh
Copy link
Copy Markdown
Contributor Author

vishh commented Aug 22, 2016

@spxtr PTAL

Review status: 5 of 6 files reviewed at latest revision, 2 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins-pull/kubernetes-pull.yaml, line 330 [r1] (raw file):

Previously, spxtr (Joe Finney) wrote…

nit: whitespace here and in a few other places.

Oops. I did not notice the trailing spaces.

Comments from Reviewable

@spxtr
Copy link
Copy Markdown
Contributor

spxtr commented Aug 22, 2016

:lgtm:

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

@vishh
Copy link
Copy Markdown
Contributor Author

vishh commented Aug 22, 2016

Review status: all files reviewed at latest revision, 1 unresolved discussion.

jenkins/job-configs/kubernetes-jenkins-pull/kubernetes-pull.yaml, line 329 [r2] (raw file):

Previously, spxtr (Joe Finney) wrote…

Sorry, I meant you should remove all trailing whitespace. I don't think you need any whitespace on blank lines.

Done.

Comments from Reviewable

@vishh vishh merged commit 00171d0 into kubernetes:master Aug 22, 2016
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request Aug 26, 2016
Merge pull request #31023 from vishh/gci-default
1b38ba6 
Automatic merge from submit-queue

Use upgraded container-vm by default on worker nodes for GCE k8s clusters

For #25276
Depends on kubernetes/test-infra#417
foxish pushed a commit to foxish/test-infra that referenced this pull request Jan 21, 2017
@eparis
Merge pull request kubernetes#417 from eparis/test-cleanups
3f45053 
[mungegithub] Moar tests!
ostromart pushed a commit to ostromart/test-infra that referenced this pull request Jul 26, 2019
@sebastienvas @istio-merge-robot
Explicitly set node selector (kubernetes#417)
2d8363a 
Automatic merge from submit-queue

Explicitly set node selector

Prow has 2 node pool. It is possible that some jobs are scheduled on the wrong pool explaining why we experience kubernetes#396.

```release-note
NONE
```
@clarketm clarketm mentioned this pull request Oct 24, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@spxtr spxtr

@dchen1107 dchen1107

Labels

lgtm "Looks good to me", indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vishh @wonderfly @dchen1107 @spxtr @googlebot