From c37701113864c144380a46ebb69aa50894f3dbaf Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Tue, 23 Aug 2016 11:56:12 -0700 Subject: [PATCH 1/2] Add eviction e2e tests. Also refactor the test a bit. --- .../typed/core/v1/fake/fake_pod_expansion.go | 12 ++ .../typed/core/v1/pod_expansion.go | 6 + test/e2e/disruption.go | 160 +++++++++++++----- 3 files changed, 132 insertions(+), 46 deletions(-) diff --git a/pkg/client/clientset_generated/release_1_4/typed/core/v1/fake/fake_pod_expansion.go b/pkg/client/clientset_generated/release_1_4/typed/core/v1/fake/fake_pod_expansion.go index a166f756cea..373ebf6bd29 100644 --- a/pkg/client/clientset_generated/release_1_4/typed/core/v1/fake/fake_pod_expansion.go +++ b/pkg/client/clientset_generated/release_1_4/typed/core/v1/fake/fake_pod_expansion.go @@ -18,6 +18,7 @@ package fake import ( "k8s.io/kubernetes/pkg/api/v1" + policy "k8s.io/kubernetes/pkg/apis/policy/v1alpha1" "k8s.io/kubernetes/pkg/client/restclient" "k8s.io/kubernetes/pkg/client/testing/core" ) @@ -44,3 +45,14 @@ func (c *FakePods) GetLogs(name string, opts *v1.PodLogOptions) *restclient.Requ _, _ = c.Fake.Invokes(action, &v1.Pod{}) return &restclient.Request{} } + +func (c *FakePods) Evict(eviction *policy.Eviction) error { + action := core.CreateActionImpl{} + action.Verb = "create" + action.Resource = podsResource + action.Subresource = "evictions" + action.Object = eviction + + _, err := c.Fake.Invokes(action, eviction) + return err +} diff --git a/pkg/client/clientset_generated/release_1_4/typed/core/v1/pod_expansion.go b/pkg/client/clientset_generated/release_1_4/typed/core/v1/pod_expansion.go index fba1fd069cc..beb61d4ff02 100644 --- a/pkg/client/clientset_generated/release_1_4/typed/core/v1/pod_expansion.go +++ b/pkg/client/clientset_generated/release_1_4/typed/core/v1/pod_expansion.go @@ -19,12 +19,14 @@ package v1 import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/v1" + policy "k8s.io/kubernetes/pkg/apis/policy/v1alpha1" "k8s.io/kubernetes/pkg/client/restclient" ) // The PodExpansion interface allows manually adding extra methods to the PodInterface. type PodExpansion interface { Bind(binding *v1.Binding) error + Evict(eviction *policy.Eviction) error GetLogs(name string, opts *v1.PodLogOptions) *restclient.Request } @@ -33,6 +35,10 @@ func (c *pods) Bind(binding *v1.Binding) error { return c.client.Post().Namespace(c.ns).Resource("pods").Name(binding.Name).SubResource("binding").Body(binding).Do().Error() } +func (c *pods) Evict(eviction *policy.Eviction) error { + return c.client.Post().Namespace(c.ns).Resource("pods").Name(eviction.Name).SubResource("eviction").Body(eviction).Do().Error() +} + // Get constructs a request for getting the logs for a pod func (c *pods) GetLogs(name string, opts *v1.PodLogOptions) *restclient.Request { return c.client.Get().Namespace(c.ns).Name(name).Resource("pods").SubResource("log").VersionedParams(opts, api.ParameterCodec) diff --git a/test/e2e/disruption.go b/test/e2e/disruption.go index 2435f953b3a..21a2b5cbbb6 100644 --- a/test/e2e/disruption.go +++ b/test/e2e/disruption.go @@ -31,7 +31,7 @@ import ( "k8s.io/kubernetes/test/e2e/framework" ) -var _ = framework.KubeDescribe("DisruptionController [Feature:PodDisruptionbudget]", func() { +var _ = framework.KubeDescribe("DisruptionController", func() { f := framework.NewDefaultFramework("disruption") var ns string var cs *release_1_4.Clientset @@ -42,55 +42,17 @@ var _ = framework.KubeDescribe("DisruptionController [Feature:PodDisruptionbudge }) It("should create a PodDisruptionBudget", func() { - pdb := policy.PodDisruptionBudget{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: ns, - }, - Spec: policy.PodDisruptionBudgetSpec{ - Selector: &unversioned.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, - MinAvailable: intstr.FromString("1%"), - }, - } - _, err := cs.Policy().PodDisruptionBudgets(ns).Create(&pdb) - Expect(err).NotTo(HaveOccurred()) + createPodDisruptionBudgetOrDie(cs, ns, intstr.FromString("1%")) }) It("should update PodDisruptionBudget status", func() { - pdb := policy.PodDisruptionBudget{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: ns, - }, - Spec: policy.PodDisruptionBudgetSpec{ - Selector: &unversioned.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, - MinAvailable: intstr.FromInt(2), - }, - } - _, err := cs.Policy().PodDisruptionBudgets(ns).Create(&pdb) - Expect(err).NotTo(HaveOccurred()) - for i := 0; i < 2; i++ { - pod := &api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: fmt.Sprintf("pod-%d", i), - Namespace: ns, - Labels: map[string]string{"foo": "bar"}, - }, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "busybox", - Image: "gcr.io/google_containers/echoserver:1.4", - }, - }, - RestartPolicy: api.RestartPolicyAlways, - }, - } + createPodDisruptionBudgetOrDie(cs, ns, intstr.FromInt(2)) - _, err := cs.Pods(ns).Create(pod) - framework.ExpectNoError(err, "Creating pod %q in namespace %q", pod.Name, ns) - } - err = wait.PollImmediate(framework.Poll, 60*time.Second, func() (bool, error) { + createPodsOrDie(cs, ns, 2) + + // Since disruptionAllowed starts out false, if we see it ever become true, + // that means the controller is working. + err := wait.PollImmediate(framework.Poll, 60*time.Second, func() (bool, error) { pdb, err := cs.Policy().PodDisruptionBudgets(ns).Get("foo") if err != nil { return false, err @@ -101,4 +63,110 @@ var _ = framework.KubeDescribe("DisruptionController [Feature:PodDisruptionbudge }) + It("should allow an eviction when there is no PDB", func() { + createPodsOrDie(cs, ns, 1) + + pod, err := cs.Pods(ns).Get("pod-0") + Expect(err).NotTo(HaveOccurred()) + + e := &policy.Eviction{ + ObjectMeta: api.ObjectMeta{ + Name: pod.Name, + Namespace: ns, + }, + } + + err = cs.Pods(ns).Evict(e) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should not allow an eviction when too few pods", func() { + createPodDisruptionBudgetOrDie(cs, ns, intstr.FromInt(2)) + + createPodsOrDie(cs, ns, 1) + + pod, err := cs.Pods(ns).Get("pod-0") + Expect(err).NotTo(HaveOccurred()) + + e := &policy.Eviction{ + ObjectMeta: api.ObjectMeta{ + Name: pod.Name, + Namespace: ns, + }, + } + + // Since disruptionAllowed starts out false, wait at least 60s hoping that + // this gives the controller enough time to have truly set the status. + time.Sleep(60 * time.Second) + + err = cs.Pods(ns).Evict(e) + Expect(err).Should(MatchError("Cannot evict pod as it would violate the pod's disruption budget.")) + }) + + It("should allow an eviction when enough pods", func() { + createPodDisruptionBudgetOrDie(cs, ns, intstr.FromInt(2)) + + createPodsOrDie(cs, ns, 2) + + pod, err := cs.Pods(ns).Get("pod-0") + Expect(err).NotTo(HaveOccurred()) + + e := &policy.Eviction{ + ObjectMeta: api.ObjectMeta{ + Name: pod.Name, + Namespace: ns, + }, + } + + // Since disruptionAllowed starts out false, if an eviction is ever allowed, + // that means the controller is working. + err = wait.PollImmediate(framework.Poll, 60*time.Second, func() (bool, error) { + err = cs.Pods(ns).Evict(e) + if err != nil { + return false, nil + } else { + return true, nil + } + }) + Expect(err).NotTo(HaveOccurred()) + }) }) + +func createPodDisruptionBudgetOrDie(cs *release_1_4.Clientset, ns string, minAvailable intstr.IntOrString) { + pdb := policy.PodDisruptionBudget{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + Namespace: ns, + }, + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &unversioned.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, + MinAvailable: minAvailable, + }, + } + _, err := cs.Policy().PodDisruptionBudgets(ns).Create(&pdb) + Expect(err).NotTo(HaveOccurred()) +} + +func createPodsOrDie(cs *release_1_4.Clientset, ns string, n int) { + for i := 0; i < n; i++ { + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: fmt.Sprintf("pod-%d", i), + Namespace: ns, + Labels: map[string]string{"foo": "bar"}, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "busybox", + Image: "gcr.io/google_containers/echoserver:1.4", + }, + }, + RestartPolicy: api.RestartPolicyAlways, + }, + } + + _, err := cs.Pods(ns).Create(pod) + framework.ExpectNoError(err, "Creating pod %q in namespace %q", pod.Name, ns) + } +} From 7d89a4f6971ca011970eb930667fd27429b7744a Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Tue, 6 Sep 2016 12:09:50 -0700 Subject: [PATCH 2/2] // client-go/copy.sh --- .../src/k8s.io/client-go/1.4/Godeps/Godeps.json | 2 +- .../typed/core/v1/fake/fake_pod_expansion.go | 12 ++++++++++++ .../1.4/kubernetes/typed/core/v1/pod_expansion.go | 6 ++++++ .../client-go/1.4/pkg/api/validation/validation.go | 6 ++++-- .../k8s.io/client-go/1.4/pkg/apis/apps/register.go | 1 + .../client-go/1.4/pkg/apis/certificates/doc.go | 2 +- .../k8s.io/client-go/1.4/pkg/apis/policy/types.go | 2 +- .../1.4/pkg/apis/policy/v1alpha1/generated.proto | 2 +- .../1.4/pkg/apis/policy/v1alpha1/types.go | 2 +- .../policy/v1alpha1/types_swagger_doc_generated.go | 2 +- .../k8s.io/client-go/1.4/pkg/util/errors/errors.go | 14 +++++++++++++- staging/src/k8s.io/client-go/1.4/testing/fake.go | 12 ++++++------ 12 files changed, 48 insertions(+), 15 deletions(-) diff --git a/staging/src/k8s.io/client-go/1.4/Godeps/Godeps.json b/staging/src/k8s.io/client-go/1.4/Godeps/Godeps.json index bf3838d6f57..3177e4a949a 100644 --- a/staging/src/k8s.io/client-go/1.4/Godeps/Godeps.json +++ b/staging/src/k8s.io/client-go/1.4/Godeps/Godeps.json @@ -1,6 +1,6 @@ { "ImportPath": "k8s.io/client-go/1.4", - "GoVersion": "go1.6", + "GoVersion": "go1.7", "GodepVersion": "v74", "Packages": [ "./..." diff --git a/staging/src/k8s.io/client-go/1.4/kubernetes/typed/core/v1/fake/fake_pod_expansion.go b/staging/src/k8s.io/client-go/1.4/kubernetes/typed/core/v1/fake/fake_pod_expansion.go index 91e539f4c63..ced693a2188 100644 --- a/staging/src/k8s.io/client-go/1.4/kubernetes/typed/core/v1/fake/fake_pod_expansion.go +++ b/staging/src/k8s.io/client-go/1.4/kubernetes/typed/core/v1/fake/fake_pod_expansion.go @@ -18,6 +18,7 @@ package fake import ( "k8s.io/client-go/1.4/pkg/api/v1" + policy "k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1" "k8s.io/client-go/1.4/rest" "k8s.io/client-go/1.4/testing" ) @@ -44,3 +45,14 @@ func (c *FakePods) GetLogs(name string, opts *v1.PodLogOptions) *rest.Request { _, _ = c.Fake.Invokes(action, &v1.Pod{}) return &rest.Request{} } + +func (c *FakePods) Evict(eviction *policy.Eviction) error { + action := testing.CreateActionImpl{} + action.Verb = "create" + action.Resource = podsResource + action.Subresource = "evictions" + action.Object = eviction + + _, err := c.Fake.Invokes(action, eviction) + return err +} diff --git a/staging/src/k8s.io/client-go/1.4/kubernetes/typed/core/v1/pod_expansion.go b/staging/src/k8s.io/client-go/1.4/kubernetes/typed/core/v1/pod_expansion.go index 8ece7798e3e..064a9805a67 100644 --- a/staging/src/k8s.io/client-go/1.4/kubernetes/typed/core/v1/pod_expansion.go +++ b/staging/src/k8s.io/client-go/1.4/kubernetes/typed/core/v1/pod_expansion.go @@ -19,12 +19,14 @@ package v1 import ( "k8s.io/client-go/1.4/pkg/api" "k8s.io/client-go/1.4/pkg/api/v1" + policy "k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1" "k8s.io/client-go/1.4/rest" ) // The PodExpansion interface allows manually adding extra methods to the PodInterface. type PodExpansion interface { Bind(binding *v1.Binding) error + Evict(eviction *policy.Eviction) error GetLogs(name string, opts *v1.PodLogOptions) *rest.Request } @@ -33,6 +35,10 @@ func (c *pods) Bind(binding *v1.Binding) error { return c.client.Post().Namespace(c.ns).Resource("pods").Name(binding.Name).SubResource("binding").Body(binding).Do().Error() } +func (c *pods) Evict(eviction *policy.Eviction) error { + return c.client.Post().Namespace(c.ns).Resource("pods").Name(eviction.Name).SubResource("eviction").Body(eviction).Do().Error() +} + // Get constructs a request for getting the logs for a pod func (c *pods) GetLogs(name string, opts *v1.PodLogOptions) *rest.Request { return c.client.Get().Namespace(c.ns).Name(name).Resource("pods").SubResource("log").VersionedParams(opts, api.ParameterCodec) diff --git a/staging/src/k8s.io/client-go/1.4/pkg/api/validation/validation.go b/staging/src/k8s.io/client-go/1.4/pkg/api/validation/validation.go index 1512b54c4e3..483a88d71f0 100644 --- a/staging/src/k8s.io/client-go/1.4/pkg/api/validation/validation.go +++ b/staging/src/k8s.io/client-go/1.4/pkg/api/validation/validation.go @@ -3435,9 +3435,11 @@ func validateEndpointAddress(address *api.EndpointAddress, fldPath *field.Path, if len(address.Hostname) > 0 { allErrs = append(allErrs, ValidateDNS1123Label(address.Hostname, fldPath.Child("hostname"))...) } - // During endpoint update, validate NodeName is DNS1123 compliant and transition rules allow the update + // During endpoint update, verify that NodeName is a DNS subdomain and transition rules allow the update if address.NodeName != nil { - allErrs = append(allErrs, ValidateDNS1123Label(*address.NodeName, fldPath.Child("nodeName"))...) + for _, msg := range ValidateNodeName(*address.NodeName, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("nodeName"), *address.NodeName, msg)) + } } allErrs = append(allErrs, validateEpAddrNodeNameTransition(address, ipToNodeName, fldPath.Child("nodeName"))...) if len(allErrs) > 0 { diff --git a/staging/src/k8s.io/client-go/1.4/pkg/apis/apps/register.go b/staging/src/k8s.io/client-go/1.4/pkg/apis/apps/register.go index 28225023e57..57b6a610df9 100644 --- a/staging/src/k8s.io/client-go/1.4/pkg/apis/apps/register.go +++ b/staging/src/k8s.io/client-go/1.4/pkg/apis/apps/register.go @@ -50,6 +50,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &PetSet{}, &PetSetList{}, &api.ListOptions{}, + &api.DeleteOptions{}, ) return nil } diff --git a/staging/src/k8s.io/client-go/1.4/pkg/apis/certificates/doc.go b/staging/src/k8s.io/client-go/1.4/pkg/apis/certificates/doc.go index fd28fd007ad..cce031e2867 100644 --- a/staging/src/k8s.io/client-go/1.4/pkg/apis/certificates/doc.go +++ b/staging/src/k8s.io/client-go/1.4/pkg/apis/certificates/doc.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -// +groupName=certificates.k8s.io // +k8s:deepcopy-gen=package,register +// +groupName=certificates.k8s.io package certificates diff --git a/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/types.go b/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/types.go index 8e4b49602d4..18b1b861040 100644 --- a/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/types.go +++ b/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/types.go @@ -72,7 +72,7 @@ type PodDisruptionBudgetList struct { // Eviction evicts a pod from its node subject to certain policies and safety constraints. // This is a subresource of Pod. A request to cause such an eviction is -// created by POSTing to .../pods/foo/evictions. +// created by POSTing to .../pods//evictions. type Eviction struct { unversioned.TypeMeta `json:",inline"` diff --git a/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/generated.proto b/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/generated.proto index 2bc3b524b8b..8fb5c2be63f 100644 --- a/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/generated.proto +++ b/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/generated.proto @@ -32,7 +32,7 @@ option go_package = "v1alpha1"; // Eviction evicts a pod from its node subject to certain policies and safety constraints. // This is a subresource of Pod. A request to cause such an eviction is -// created by POSTing to .../pods/foo/evictions. +// created by POSTing to .../pods//evictions. message Eviction { // ObjectMeta describes the pod that is being evicted. optional k8s.io.kubernetes.pkg.api.v1.ObjectMeta metadata = 1; diff --git a/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/types.go b/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/types.go index 1da3ebbcd33..a7150a21aac 100644 --- a/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/types.go +++ b/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/types.go @@ -71,7 +71,7 @@ type PodDisruptionBudgetList struct { // Eviction evicts a pod from its node subject to certain policies and safety constraints. // This is a subresource of Pod. A request to cause such an eviction is -// created by POSTing to .../pods/foo/evictions. +// created by POSTing to .../pods//evictions. type Eviction struct { unversioned.TypeMeta `json:",inline"` diff --git a/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/types_swagger_doc_generated.go b/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/types_swagger_doc_generated.go index 4711780e339..7750b1007fe 100644 --- a/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/client-go/1.4/pkg/apis/policy/v1alpha1/types_swagger_doc_generated.go @@ -28,7 +28,7 @@ package v1alpha1 // AUTO-GENERATED FUNCTIONS START HERE var map_Eviction = map[string]string{ - "": "Eviction evicts a pod from its node subject to certain policies and safety constraints. This is a subresource of Pod. A request to cause such an eviction is created by POSTing to .../pods/foo/evictions.", + "": "Eviction evicts a pod from its node subject to certain policies and safety constraints. This is a subresource of Pod. A request to cause such an eviction is created by POSTing to .../pods//evictions.", "metadata": "ObjectMeta describes the pod that is being evicted.", "deleteOptions": "DeleteOptions may be provided", } diff --git a/staging/src/k8s.io/client-go/1.4/pkg/util/errors/errors.go b/staging/src/k8s.io/client-go/1.4/pkg/util/errors/errors.go index 0445c142bea..42631f21622 100644 --- a/staging/src/k8s.io/client-go/1.4/pkg/util/errors/errors.go +++ b/staging/src/k8s.io/client-go/1.4/pkg/util/errors/errors.go @@ -31,11 +31,23 @@ type Aggregate interface { // NewAggregate converts a slice of errors into an Aggregate interface, which // is itself an implementation of the error interface. If the slice is empty, // this returns nil. +// It will check if any of the element of input error list is nil, to avoid +// nil pointer panic when call Error(). func NewAggregate(errlist []error) Aggregate { if len(errlist) == 0 { return nil } - return aggregate(errlist) + // In case of input error list contains nil + var errs []error + for _, e := range errlist { + if e != nil { + errs = append(errs, e) + } + } + if len(errs) == 0 { + return nil + } + return aggregate(errs) } // This helper implements the error and Errors interfaces. Keeping it private diff --git a/staging/src/k8s.io/client-go/1.4/testing/fake.go b/staging/src/k8s.io/client-go/1.4/testing/fake.go index b220cc0bfd3..18434465412 100644 --- a/staging/src/k8s.io/client-go/1.4/testing/fake.go +++ b/staging/src/k8s.io/client-go/1.4/testing/fake.go @@ -55,7 +55,7 @@ type Reactor interface { type WatchReactor interface { // Handles indicates whether or not this Reactor deals with a given action Handles(action Action) bool - // React handles a watch action and returns results. It may choose to delegate by indicated handled=false + // React handles a watch action and returns results. It may choose to delegate by indicating handled=false React(action Action) (handled bool, ret watch.Interface, err error) } @@ -63,20 +63,20 @@ type WatchReactor interface { type ProxyReactor interface { // Handles indicates whether or not this Reactor deals with a given action Handles(action Action) bool - // React handles a watch action and returns results. It may choose to delegate by indicated handled=false + // React handles a watch action and returns results. It may choose to delegate by indicating handled=false React(action Action) (handled bool, ret rest.ResponseWrapper, err error) } // ReactionFunc is a function that returns an object or error for a given Action. If "handled" is false, -// then the test client will continue ignore the results and continue to the next ReactionFunc +// then the test client will ignore the results and continue to the next ReactionFunc type ReactionFunc func(action Action) (handled bool, ret runtime.Object, err error) // WatchReactionFunc is a function that returns a watch interface. If "handled" is false, -// then the test client will continue ignore the results and continue to the next ReactionFunc +// then the test client will ignore the results and continue to the next ReactionFunc type WatchReactionFunc func(action Action) (handled bool, ret watch.Interface, err error) // ProxyReactionFunc is a function that returns a ResponseWrapper interface for a given Action. If "handled" is false, -// then the test client will continue ignore the results and continue to the next ProxyReactionFunc +// then the test client will ignore the results and continue to the next ProxyReactionFunc type ProxyReactionFunc func(action Action) (handled bool, ret rest.ResponseWrapper, err error) // AddReactor appends a reactor to the end of the chain @@ -184,7 +184,7 @@ func (c *Fake) ClearActions() { c.actions = make([]Action, 0) } -// Actions returns a chronologically ordered slice fake actions called on the fake client +// Actions returns a chronologically ordered slice fake actions called on the fake client. func (c *Fake) Actions() []Action { c.RLock() defer c.RUnlock()