Integration test for reading Pods via http & file #5785

wojtek-t · 2015-03-23T13:16:26Z

Extend hack/integration_test.sh to test reading Pods (in addition to ContainerManifest) via Kubelet http & file channel.

Fixes #3372
It's also part of #5630

cc @vmarmol @yujuhong @bgrant0607

wojtek-t · 2015-03-23T13:18:57Z

cmd/integration/integration.go

@@ -855,6 +879,29 @@ func ServeCachedManifestFile() (servingAddress string) {
 }

 const (
+	testManifestFile = `kind: Pod


This change isn't exactly what we would like to do, because it only tests whether reading Pods works.
Do you have any suggestions how to make it test both scenarios (without significant changes to this file)?

Is anything using this const?

Yes - it is used by ServeCachedManifestFile.
What I would ideally like to do is to have "two modes" - one testing reading pods from http given by manifest, and the other one reading pods from http given PodSpec. But currently all tests are using the same "http endpoint".

FYI: I found an easy way to test both modes (i.e. reading both Pods and ContainerManifest from http channel). Basically, in the integration test we have two Kubelets, so I did small changes that the first one is reading Pods from http, and the second is reading ContainerManifest from http.
I hope that you're fine with that.

roberthbailey · 2015-03-23T17:24:35Z

cmd/integration/integration.go

@@ -306,37 +306,61 @@ func podRunning(c *client.Client, podNamespace string, podID string) wait.Condit
 }

 func runStaticPodTest(c *client.Client, configFilePath string) {
-	manifest := `version: v1beta2
-id: static-pod
+	hostname, _ := os.Hostname()


@smarterclayton just removed os.Hostname from the integration tests because it was causing issues running them on a mac.

Also we should be using the name from the kubelet (in the integration test, it is always "localhost" for files)

----- Original Message -----

@@ -306,37 +306,61 @@ func podRunning(c *client.Client, podNamespace
string, podID string) wait.Condit
}

func runStaticPodTest(c *client.Client, configFilePath string) {

manifest := `version: v1beta2
-id: static-pod

hostname, _ := os.Hostname()

@smarterclayton just removed os.Hostname from the integration tests because
it was causing issues running them on a mac.

Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/5785/files#r26958255

Yes - I've seen that, but thanks for notifying. I will fix that tomorrow.

vmarmol · 2015-03-23T17:51:19Z

@wojtek-t this is already strictly better than what we have today. We can probably add a Get() to the pods and verify the fields in the pod if you want to also test the values.

wojtek-t · 2015-03-24T11:08:00Z

I have no idea how this worked yesterday. Basically, I rebased the PR today morning and it stopped working. But the real problem is that it couldn't parsed the pods in the format that I used yesterday. I spent an hour fighting with it but it didn't lead me anywhere and I gave up.

I changed the data format that I was using and now it seems to work.

PTAL

smarterclayton · 2015-03-24T13:30:29Z

When the YAML change landed trailing commas became invalid JSON (the strict Go JSON parser does not allow them, the looser YAML parser did)

----- Original Message -----

I have no idea how this worked yesterday. Basically, I rebased the PR today
morning and it stopped working. But the real problem is that it couldn't
parsed the pods in the format that I used yesterday. I spent an hour
fighting with it but it didn't lead me anywhere and I gave up.

I changed the data format that I was using and now it seems to work.

PTAL

Reply to this email directly or view it on GitHub:
#5785 (comment)

wojtek-t · 2015-03-24T13:41:47Z

My problems weren't related to commas. I had the following yaml:
kind: Pod
apiversion: v1beta3
metadata:
name: static-pod-from-spec
spec:
containers:
- name: static-container
image: kubernetes/pause
and the error I was getting was: "couldn't get version/kind: invalid character 'k' looking for beginning of value"

Anyway, I think that the current version also tests what we wanted to test.

smarterclayton · 2015-03-24T14:39:56Z

We don't support YAML anymore from the API :)

----- Original Message -----

My problems weren't related to commas. I had the following yaml:
kind: Pod
apiversion: v1beta3
metadata:
name: static-pod-from-spec
spec:
containers:
- name: static-container
image: kubernetes/pause
and the error I was getting was: "couldn't get version/kind: invalid
character 'k' looking for beginning of value"

Anyway, I think that the current version also tests what we wanted to test.

Reply to this email directly or view it on GitHub:
#5785 (comment)

wojtek-t · 2015-03-24T14:41:41Z

Thanks @smarterclayton - I know there are plans for it, but didn't know that it already happened.
Can you point me to the PR that disabled it?

vmarmol · 2015-03-24T15:11:13Z

@wojtek-t #5533

wojtek-t · 2015-03-24T15:21:27Z

Thanks - that was merged yesterday and that explains why it worked for me yesterday.

Anyway, it seems that the current version is exactly what we would like to test.

wojtek-t · 2015-03-24T15:22:03Z

BTW - I looked into the test failure and it seems that it's a flake that is unrelated to my changes.

vmarmol · 2015-03-24T15:22:41Z

Thanks @wojtek-t I kicked off Travis and will merge when that is green :) it's half-way through

Integration test for reading Pods via http & file

wojtek-t · 2015-03-24T15:38:06Z

Thanks!

smarterclayton · 2015-03-24T15:45:30Z

#5533

On Mar 24, 2015, at 10:41 AM, Wojciech Tyczynski notifications@github.com wrote:

Thanks @smarterclayton - I know there are plans for it, but didn't know that it already happened.
Can you point me to the PR that disabled it?

—
Reply to this email directly or view it on GitHub.

googlebot added the cla: yes label Mar 23, 2015

wojtek-t reviewed Mar 23, 2015
View reviewed changes

wojtek-t mentioned this pull request Mar 23, 2015

Kubelet config sources should use the provided hostname, not lookup os.Hostname() #5775

Merged

vmarmol self-assigned this Mar 23, 2015

roberthbailey reviewed Mar 23, 2015
View reviewed changes

Integration test for reading Pods via http & file

f80cac1

wojtek-t force-pushed the check_reading_pods_in_integration branch from b29973a to f80cac1 Compare March 24, 2015 11:08

vmarmol added a commit that referenced this pull request Mar 24, 2015

Merge pull request #5785 from wojtek-t/check_reading_pods_in_integration

940f4b5

Integration test for reading Pods via http & file

vmarmol merged commit 940f4b5 into kubernetes:master Mar 24, 2015

wojtek-t deleted the check_reading_pods_in_integration branch March 27, 2015 10:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integration test for reading Pods via http & file #5785

Integration test for reading Pods via http & file #5785

wojtek-t commented Mar 23, 2015

wojtek-t Mar 23, 2015

vmarmol Mar 23, 2015

wojtek-t Mar 23, 2015

wojtek-t Mar 24, 2015

roberthbailey Mar 23, 2015

smarterclayton Mar 23, 2015

wojtek-t Mar 23, 2015

wojtek-t Mar 24, 2015

vmarmol commented Mar 23, 2015

wojtek-t commented Mar 24, 2015

smarterclayton commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

smarterclayton commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

vmarmol commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

vmarmol commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

smarterclayton commented Mar 24, 2015

Integration test for reading Pods via http & file #5785

Integration test for reading Pods via http & file #5785

Conversation

wojtek-t commented Mar 23, 2015

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

Choose a reason for hiding this comment

vmarmol commented Mar 23, 2015

wojtek-t commented Mar 24, 2015

smarterclayton commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

smarterclayton commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

vmarmol commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

vmarmol commented Mar 24, 2015

wojtek-t commented Mar 24, 2015

smarterclayton commented Mar 24, 2015