Adds daemonset conformance tests #60456

kow3ns · 2018-02-26T21:17:40Z

What this PR does / why we need it: Adds conformance tests for deamonset

Conformance tests are added for the DaemonSet kinds in the apps/v1 group version. Deprecated versions of DaemonSet will not be tested for conformance, and conformance is only applicable to release 1.10 and later.

enisoc · 2018-02-26T21:26:19Z

test/e2e/apps/daemon_set.go

@@ -108,7 +108,12 @@ var _ = SIGDescribe("Daemon set [Serial]", func() {
 		Expect(err).NotTo(HaveOccurred())
 	})

-	It("should run and stop simple daemon", func() {
+	/*
+	  Testname: Simple DaemonSet Creation


I don't know if it matters, but the example and all existing Testnames in the repo have no spaces.

https://github.com/kubernetes/kubernetes/blob/master/test/conformance/cf_header.md

bgrant0607 · 2018-02-26T22:19:28Z

test/e2e/apps/daemon_set.go

@@ -175,6 +185,8 @@ var _ = SIGDescribe("Daemon set [Serial]", func() {
 		Expect(err).NotTo(HaveOccurred())
 	})

+	// We deffer adding this test to conformance pending the disposition of moving DaemonSet scheduling logic to the


Typo: defer

janetkuo · 2018-02-27T00:14:16Z

test/e2e/apps/daemon_set.go

+	/*
+	  Testname: DaemonSet-Creation
+	  Description: A conformant Kubernetes distribution MUST support the creation of DaemonSets. When a DaemonSet
+	  Pod is deleted, the DaemonSet controller MUST recreate the Pod.


nit: we never "recreate" a Pod but simply create a new one.

making the distinction makes sense to me

I think this terminology is ok in this case because the pod is associated with the node. The terminology I generally discourage is "reschedule".

I'll go with Janet's suggestion because its conformance and someone might infer that we are updating the Pod in place. In general I wouldn't read "recreate" as in place update, but it can't hurt.

janetkuo · 2018-02-27T00:15:50Z

test/e2e/apps/daemon_set.go

-	It("should retry creating failed daemon pods", func() {
+	/*
+	  Testname: DaemonSet-FailedPodRecreation
+	  Description: A conformant Kubernetes distribution MUST recreate DaemonSet Pods when they fail.


Same comment here for "recreate" (L238 - L239)

janetkuo · 2018-02-27T00:17:33Z

test/e2e/apps/daemon_set.go

@@ -223,7 +235,11 @@ var _ = SIGDescribe("Daemon set [Serial]", func() {
 			NotTo(HaveOccurred(), "error waiting for daemon pod to not be running on nodes")
 	})

-	It("should retry creating failed daemon pods", func() {
+	/*
+	  Testname: DaemonSet-FailedPodRecreation


Testname: DaemonSet-FailedPodCreation

Or "Testname: DaemonSet-FailedPodRetryCreation"

Yes, this one reads oddly. It sounds like the Pod is not having fun. :-)

bgrant0607 · 2018-02-27T01:21:49Z

/approve

bgrant0607 · 2018-02-27T01:23:06Z

I believe all prior comments are now addressed.

/lgtm

janetkuo · 2018-02-27T01:37:18Z

test/conformance/testdata/conformance.txt

+test/e2e/apps/daemon_set.go: "should run and stop complex daemon"
+test/e2e/apps/daemon_set.go: "should retry creating failed daemon pods"
+test/e2e/apps/daemon_set.go: "Should update pod when spec was updated and update strategy is RollingUpdate"
+test/e2e/apps/daemon_set.go: "Should rollback without unnecessary restarts"


Not capitalize "Should" to make it consistent with other tests?

kow3ns · 2018-02-27T01:53:47Z

/restest

ixdy · 2018-02-27T01:57:19Z

/retest

janetkuo · 2018-03-01T22:03:27Z

/lgtm

k8s-ci-robot · 2018-03-01T22:03:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bgrant0607, janetkuo, kow3ns

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:

~~test/conformance/testdata/OWNERS~~ [bgrant0607]
~~test/e2e/apps/OWNERS~~ [bgrant0607,janetkuo,kow3ns]

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

k8s-github-robot · 2018-03-01T23:23:53Z

Automatic merge from submit-queue (batch tested with PRs 52077, 60456, 60591). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 26, 2018

k8s-ci-robot requested review from mfojtik and smarterclayton February 26, 2018 21:17

kow3ns assigned bgrant0607 Feb 26, 2018

kow3ns requested review from janetkuo and enisoc February 26, 2018 21:18

enisoc reviewed Feb 26, 2018

View reviewed changes

bgrant0607 reviewed Feb 26, 2018

View reviewed changes

kow3ns force-pushed the ds-conformance branch 2 times, most recently from 7278ddd to 76a9656 Compare February 26, 2018 23:12

janetkuo self-assigned this Feb 27, 2018

janetkuo reviewed Feb 27, 2018

View reviewed changes

kow3ns force-pushed the ds-conformance branch from 76a9656 to 4ec6853 Compare February 27, 2018 00:16

janetkuo reviewed Feb 27, 2018

View reviewed changes

kow3ns force-pushed the ds-conformance branch 2 times, most recently from 3ac3d8f to afedbf9 Compare February 27, 2018 01:12

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

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

janetkuo reviewed Feb 27, 2018

View reviewed changes

kow3ns added this to the v1.10 milestone Feb 27, 2018

kow3ns added the status/approved-for-milestone label Feb 27, 2018

kow3ns force-pushed the ds-conformance branch from afedbf9 to 1f16519 Compare February 28, 2018 20:37

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 28, 2018

Adds daemonset conformance tests

dcc3d25

kow3ns force-pushed the ds-conformance branch from 1f16519 to dcc3d25 Compare February 28, 2018 20:44

janetkuo approved these changes Mar 1, 2018

View reviewed changes

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

k8s-github-robot merged commit 5520e07 into kubernetes:master Mar 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds daemonset conformance tests #60456

Adds daemonset conformance tests #60456

kow3ns commented Feb 26, 2018

enisoc Feb 26, 2018

bgrant0607 Feb 26, 2018

janetkuo Feb 27, 2018

kow3ns Feb 27, 2018

bgrant0607 Feb 27, 2018

kow3ns Feb 27, 2018

janetkuo Feb 27, 2018

janetkuo Feb 27, 2018

janetkuo Feb 27, 2018 •

edited

Loading

bgrant0607 Feb 27, 2018

bgrant0607 commented Feb 27, 2018

bgrant0607 commented Feb 27, 2018

janetkuo Feb 27, 2018

kow3ns commented Feb 27, 2018

ixdy commented Feb 27, 2018

janetkuo commented Mar 1, 2018

k8s-ci-robot commented Mar 1, 2018

k8s-github-robot commented Mar 1, 2018

Adds daemonset conformance tests #60456

Adds daemonset conformance tests #60456

Conversation

kow3ns commented Feb 26, 2018

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

janetkuo Feb 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgrant0607 commented Feb 27, 2018

bgrant0607 commented Feb 27, 2018

Choose a reason for hiding this comment

kow3ns commented Feb 27, 2018

ixdy commented Feb 27, 2018

janetkuo commented Mar 1, 2018

k8s-ci-robot commented Mar 1, 2018

k8s-github-robot commented Mar 1, 2018

janetkuo Feb 27, 2018 •

edited

Loading