Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds daemonset conformance tests #60456

Merged
merged 1 commit into from Mar 1, 2018

Conversation

@kow3ns
Copy link
Member

kow3ns commented Feb 26, 2018

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.
@@ -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

This comment has been minimized.

@enisoc

enisoc Feb 26, 2018

Member

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

@kow3ns kow3ns added this to In Progress in Workloads Feb 26, 2018

@@ -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

This comment has been minimized.

@bgrant0607

bgrant0607 Feb 26, 2018

Member

Typo: defer

@kow3ns kow3ns force-pushed the kow3ns:ds-conformance branch 2 times, most recently from 7278ddd to 76a9656 Feb 26, 2018

@janetkuo janetkuo self-assigned this Feb 27, 2018

/*
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.

This comment has been minimized.

@janetkuo

janetkuo Feb 27, 2018

Member

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

This comment has been minimized.

@kow3ns

kow3ns Feb 27, 2018

Author Member

making the distinction makes sense to me

This comment has been minimized.

@bgrant0607

bgrant0607 Feb 27, 2018

Member

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

This comment has been minimized.

@kow3ns

kow3ns Feb 27, 2018

Author Member

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.

@kow3ns kow3ns force-pushed the kow3ns:ds-conformance branch from 76a9656 to 4ec6853 Feb 27, 2018

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

This comment has been minimized.

@janetkuo

janetkuo Feb 27, 2018

Member

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

@@ -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

This comment has been minimized.

@janetkuo

janetkuo Feb 27, 2018

Member

Testname: DaemonSet-FailedPodCreation

This comment has been minimized.

@janetkuo

janetkuo Feb 27, 2018

Member

Or "Testname: DaemonSet-FailedPodRetryCreation"

This comment has been minimized.

@bgrant0607

bgrant0607 Feb 27, 2018

Member

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

@kow3ns kow3ns force-pushed the kow3ns:ds-conformance branch 2 times, most recently from 3ac3d8f to afedbf9 Feb 27, 2018

@bgrant0607

This comment has been minimized.

Copy link
Member

bgrant0607 commented Feb 27, 2018

/approve

@bgrant0607

This comment has been minimized.

Copy link
Member

bgrant0607 commented Feb 27, 2018

I believe all prior comments are now addressed.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Feb 27, 2018

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"

This comment has been minimized.

@janetkuo

janetkuo Feb 27, 2018

Member

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

@kow3ns

This comment has been minimized.

Copy link
Member Author

kow3ns commented Feb 27, 2018

/restest

@ixdy

This comment has been minimized.

Copy link
Member

ixdy commented Feb 27, 2018

/retest

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

@kow3ns kow3ns force-pushed the kow3ns:ds-conformance branch from afedbf9 to 1f16519 Feb 28, 2018

@k8s-github-robot k8s-github-robot removed the lgtm label Feb 28, 2018

@kow3ns kow3ns force-pushed the kow3ns:ds-conformance branch from 1f16519 to dcc3d25 Feb 28, 2018

@janetkuo

This comment has been minimized.

Copy link
Member

janetkuo commented Mar 1, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Mar 1, 2018

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 1, 2018

[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:

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

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented Mar 1, 2018

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-github-robot k8s-github-robot merged commit 5520e07 into kubernetes:master Mar 1, 2018

14 checks passed

Submit Queue Queued to run github e2e tests a second time.
Details
cla/linuxfoundation kow3ns authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-unit Context retired. Status moved to "pull-kubernetes-integration".
pull-kubernetes-verify Job succeeded.
Details

Workloads automation moved this from In Progress to Done Mar 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.