Modifying trusty jobs to use GCI. #480

maisem · 2016-08-31T17:27:20Z

The KUBE_NODE_OS_DISTRIBUTION and KUBE_OS_DISTRIBUTION variables are meaningless for GKE, removing them as well.

This change is

wonderfly · 2016-08-31T18:42:56Z

Thanks for making the change! Quick question though, without these vars, how does GKE know what distro the nodes are running?

spxtr · 2016-08-31T18:46:09Z

lgtm once @wonderfly's question is resolved.

vishh · 2016-08-31T18:50:14Z

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

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 66 [r1] (raw file):

                export GINKGO_TEST_ARGS="--ginkgo.skip=\[Slow\]|\[Serial\]|\[Disruptive\]|\[Flaky\]|\[Feature:.+\]"
                export PROJECT="k8s-jkns-e2e-gke-ci"
                export KUBE_NODE_OS_DISTRIBUTION="debian"

Would it better to set the image type explicitly here as well? Here and elsewhere

Comments from Reviewable

maisem · 2016-08-31T19:28:05Z

@wonderfly - if nothing is set (i.e. no argument is passed through the gcloud CLI) we use the server side default (which is currently container_vm).

You can check the server default by running

$ gcloud container get-server-config
Fetching server config for us-central1-b
defaultClusterVersion: 1.3.6
defaultImageType: CONTAINER_VM
validImageTypes:
- CONTAINER_VM
- GCI
validNodeVersions:
- 1.3.6
- 1.3.5
- 1.2.6
- 1.1.8

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

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 66 [r1] (raw file):

Previously, vishh (Vish Kannan) wrote…

Would it better to set the image type explicitly here as well? Here and elsewhere

Not setting it would use the server side default (which is container vm).

You can check the server default by running

$ gcloud container get-server-config
Fetching server config for us-central1-b
defaultClusterVersion: 1.3.6
defaultImageType: CONTAINER_VM
validImageTypes:
- CONTAINER_VM
- GCI
validNodeVersions:
- 1.3.6
- 1.3.5
- 1.2.6
- 1.1.8

Comments from Reviewable

wonderfly · 2016-08-31T19:49:57Z

@maisem So the trick is to set node_image_type when calling the gcloud cli?

vishh · 2016-08-31T21:23:07Z

I'd prefer our testing to not depend on server side defaulting because if
there is a bug there, we will be oblivious to the fact that GKE tests are
using GCI instead of container-vm.
The defaulting should be tested separately. WDYT?

On Wed, Aug 31, 2016 at 12:49 PM, Daniel Wang [email protected]
wrote:

@maisem https://github.com/maisem So the trick is to set node_image_type
when calling the gcloud cli?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#480 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGvIKO3mFl54okom3-uBZOn1ceb6zwxXks5qldrlgaJpZM4Jx3K6
.

wonderfly · 2016-08-31T22:58:36Z

I'd prefer our testing to not depend on server side defaulting

Makes sense to me

wonderfly · 2016-08-31T23:01:30Z

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 447 [r1] (raw file):

The variable "KUBE_OS_DISTRIBUTION" is set to "trusty" so that

the OS dependendent test cases will use Trusty in their assertions.

These comments are out of date too

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 450 [r1] (raw file):

# the OS dependendent test cases will use Trusty in their assertions.
- project:
    name: kubernetes-e2e-gke-trusty

Since you are updating the jobs to use GCI instead of trusty, can you update the job names too?

Comments from Reviewable

maisem · 2016-09-01T04:03:07Z

I'd prefer our testing to not depend on server side defaulting

The tests should be targeting the default configuration (because that's majority of the clusters) and in addition we should have tests that target a specific configuration (e.g. GCI).
This is also what we do for version selection in a number of tests on the release branches.

wonderfly · 2016-09-01T17:54:08Z

SG. Let's make that discussion/change in a separate thread and proceed with this PR? The Trusty jobs are really old and need to be updated sooner than later.

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

Comments from Reviewable

vishh · 2016-09-01T18:03:50Z

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 66 [r1] (raw file):

Previously, maisem (Maisem Ali) wrote…

Not setting it would use the server side default (which is container vm).

You can check the server default by running
$ gcloud container get-server-config
Fetching server config for us-central1-b
defaultClusterVersion: 1.3.6
defaultImageType: CONTAINER_VM
validImageTypes:
- CONTAINER_VM
- GCI
validNodeVersions:
- 1.3.6
- 1.3.5
- 1.2.6
- 1.1.8

We chatted offline. This change is necessary since `KUBE_NODE_OS_DISTRIBUTION` is not respected in the case of GKE. The question of defaulting will be dealt with in a separate PR

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 450 [r1] (raw file):

Previously, wonderfly (Daniel Wang) wrote…

Since you are updating the jobs to use GCI instead of trusty, can you update the job names too?

+1. Trusty does not make sense anymore

Comments from Reviewable

maisem · 2016-09-01T18:16:17Z

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 447 [r1] (raw file):

Previously, wonderfly (Daniel Wang) wrote…

The variable "KUBE_OS_DISTRIBUTION" is set to "trusty" so that

the OS dependendent test cases will use Trusty in their assertions.

These comments are out of date too

Done.

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 450 [r1] (raw file):

Previously, vishh (Vish Kannan) wrote…

+1. Trusty does not make sense anymore

Done.

Comments from Reviewable

wonderfly · 2016-09-01T18:31:08Z

Review status: 0 of 1 files reviewed at latest revision, 4 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 447 [r3] (raw file):

kubernetes-e2e-gke-gci-release-1.2
Ah, I just realized that all the GKE version pinned jobs are pretty old too - kubernetes-e2e-gke-version-pinned all still pin to release-1.2 and there's a TODO that never got finished: https://github.com/kubernetes/test-infra/blob/master/jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml#L364.

I am okay with deal with that in a separate PR, but imo it needs to be resolved soon.

Comments from Reviewable

maisem · 2016-09-01T20:09:38Z

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 447 [r3] (raw file):

Previously, wonderfly (Daniel Wang) wrote…

kubernetes-e2e-gke-gci-release-1.2
Ah, I just realized that all the GKE version pinned jobs are pretty old too - kubernetes-e2e-gke-version-pinned all still pin to release-1.2 and there's a TODO that never got finished: https://github.com/kubernetes/test-infra/blob/master/jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml#L364.

I am okay with deal with that in a separate PR, but imo it needs to be resolved soon.

I was going to update these to 1.3 when we cut 1.4 and add jobs for the 1.4 branch.

Comments from Reviewable

wonderfly · 2016-09-01T21:18:20Z

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 447 [r3] (raw file):

Previously, maisem (Maisem Ali) wrote…

I was going to update these to 1.3 when we cut 1.4 and add jobs for the 1.4 branch.

SG

Comments from Reviewable

wonderfly · 2016-09-01T21:18:32Z

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

Comments from Reviewable

vishh · 2016-09-01T21:45:23Z

Review status: 0 of 1 files reviewed at latest revision, 4 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 452 [r3] (raw file):

    emails: '[email protected]'
    gke-suffix:
        - 'gke-gci-test':  # kubernetes-e2e-gke-gci-test

The test name is determined by this suffix. Shouldn't the release-1.2 suffix be added here?

Comments from Reviewable

spxtr · 2016-09-06T17:27:32Z

LGTM

updating comment Renaming trusty jobs to GCI. Adding release-1.2 to the test names.

maisem · 2016-09-08T20:38:47Z

Review status: 0 of 1 files reviewed at latest revision, 4 unresolved discussions.

jenkins/job-configs/kubernetes-jenkins/kubernetes-e2e-gke.yaml, line 452 [r3] (raw file):

Previously, vishh (Vish Kannan) wrote…

The test name is determined by this suffix. Shouldn't the release-1.2 suffix be added here?

Done.

Comments from Reviewable

vishh · 2016-09-08T20:40:45Z

Review status: 0 of 1 files reviewed at latest revision, 4 unresolved discussions.

Comments from Reviewable

Automatic merge from submit-queue Delete branch if auto PR has been merged **Release note**: ```release-note none ```

APICoverage-Recorder in the commit knative/test-infra#463 was writting an instance of v1beta1.AdmissionResponse to the http.ResponseWriter when it should be writting v1beta1.AdmissionReview this changeset fixes the issue.

googlebot added the cla: yes label Aug 31, 2016

maisem assigned spxtr Aug 31, 2016

spxtr assigned wonderfly Aug 31, 2016

maisem force-pushed the trusty branch from 5aefaeb to 16af3bf Compare September 1, 2016 18:11

maisem force-pushed the trusty branch from 16af3bf to 7375731 Compare September 2, 2016 20:01

maisem force-pushed the trusty branch from 7375731 to a96c3af Compare September 8, 2016 20:32

Modifying trusty jobs to use GCI.

6ca3df4

updating comment Renaming trusty jobs to GCI. Adding release-1.2 to the test names.

maisem force-pushed the trusty branch from a96c3af to 6ca3df4 Compare September 8, 2016 20:34

vishh merged commit 17ac268 into kubernetes:master Sep 8, 2016

maisem deleted the trusty branch September 8, 2016 20:41

ostromart pushed a commit to ostromart/test-infra that referenced this pull request Jul 26, 2019

Delete branch once PR merged (kubernetes#480)

8267d16

Automatic merge from submit-queue Delete branch if auto PR has been merged **Release note**: ```release-note none ```

k8s-ci-robot mentioned this pull request Nov 21, 2022

Prow test force reload #28076

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modifying trusty jobs to use GCI. #480

Modifying trusty jobs to use GCI. #480

maisem commented Aug 31, 2016 •

edited by pwittrock

Loading

wonderfly commented Aug 31, 2016

spxtr commented Aug 31, 2016 •

edited

Loading

vishh commented Aug 31, 2016

maisem commented Aug 31, 2016

wonderfly commented Aug 31, 2016

vishh commented Aug 31, 2016

wonderfly commented Aug 31, 2016

wonderfly commented Aug 31, 2016

the OS dependendent test cases will use Trusty in their assertions.

maisem commented Sep 1, 2016

wonderfly commented Sep 1, 2016

vishh commented Sep 1, 2016

maisem commented Sep 1, 2016

the OS dependendent test cases will use Trusty in their assertions.

wonderfly commented Sep 1, 2016

maisem commented Sep 1, 2016

wonderfly commented Sep 1, 2016

wonderfly commented Sep 1, 2016

vishh commented Sep 1, 2016

spxtr commented Sep 6, 2016

maisem commented Sep 8, 2016

vishh commented Sep 8, 2016

Modifying trusty jobs to use GCI. #480

Modifying trusty jobs to use GCI. #480

Conversation

maisem commented Aug 31, 2016 • edited by pwittrock Loading

wonderfly commented Aug 31, 2016

spxtr commented Aug 31, 2016 • edited Loading

vishh commented Aug 31, 2016

maisem commented Aug 31, 2016

wonderfly commented Aug 31, 2016

vishh commented Aug 31, 2016

wonderfly commented Aug 31, 2016

wonderfly commented Aug 31, 2016

the OS dependendent test cases will use Trusty in their assertions.

maisem commented Sep 1, 2016

wonderfly commented Sep 1, 2016

vishh commented Sep 1, 2016

maisem commented Sep 1, 2016

the OS dependendent test cases will use Trusty in their assertions.

wonderfly commented Sep 1, 2016

maisem commented Sep 1, 2016

wonderfly commented Sep 1, 2016

wonderfly commented Sep 1, 2016

vishh commented Sep 1, 2016

spxtr commented Sep 6, 2016

maisem commented Sep 8, 2016

vishh commented Sep 8, 2016

maisem commented Aug 31, 2016 •

edited by pwittrock

Loading

spxtr commented Aug 31, 2016 •

edited

Loading