Allow pod.Spec.ActiveDeadlineSeconds to be updateable

pull/6/head
Paul Morie 2016-01-26 15:52:14 -05:00
parent 87fbfdc953
commit 0b82d0b491
6 changed files with 466 additions and 11 deletions

View File

@ -1485,17 +1485,56 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList {
allErrs = append(allErrs, field.Forbidden(specPath.Child("containers"), "pod updates may not add or remove containers")) allErrs = append(allErrs, field.Forbidden(specPath.Child("containers"), "pod updates may not add or remove containers"))
return allErrs return allErrs
} }
pod := *newPod
// Tricky, we need to copy the container list so that we don't overwrite the update // validate updateable fields:
// 1. containers[*].image
// 2. spec.activeDeadlineSeconds
// validate updated container images
for i, ctr := range newPod.Spec.Containers {
if len(ctr.Image) == 0 {
allErrs = append(allErrs, field.Required(specPath.Child("containers").Index(i).Child("image"), ""))
}
}
// validate updated spec.activeDeadlineSeconds. two types of updates are allowed:
// 1. from nil to a positive value
// 2. from a positive value to a lesser, non-negative value
if newPod.Spec.ActiveDeadlineSeconds != nil {
newActiveDeadlineSeconds := *newPod.Spec.ActiveDeadlineSeconds
if newActiveDeadlineSeconds < 0 {
allErrs = append(allErrs, field.Invalid(specPath.Child("activeDeadlineSeconds"), newActiveDeadlineSeconds, isNegativeErrorMsg))
return allErrs
}
if oldPod.Spec.ActiveDeadlineSeconds != nil {
oldActiveDeadlineSeconds := *oldPod.Spec.ActiveDeadlineSeconds
if oldActiveDeadlineSeconds < newActiveDeadlineSeconds {
allErrs = append(allErrs, field.Invalid(specPath.Child("activeDeadlineSeconds"), newActiveDeadlineSeconds, "must be less than or equal to previous value"))
return allErrs
}
}
} else if oldPod.Spec.ActiveDeadlineSeconds != nil {
allErrs = append(allErrs, field.Invalid(specPath.Child("activeDeadlineSeconds"), newPod.Spec.ActiveDeadlineSeconds, "must not update from a positive integer to nil value"))
}
// handle updateable fields by munging those fields prior to deep equal comparison.
mungedPod := *newPod
// munge containers[*].image
var newContainers []api.Container var newContainers []api.Container
for ix, container := range pod.Spec.Containers { for ix, container := range mungedPod.Spec.Containers {
container.Image = oldPod.Spec.Containers[ix].Image container.Image = oldPod.Spec.Containers[ix].Image
newContainers = append(newContainers, container) newContainers = append(newContainers, container)
} }
pod.Spec.Containers = newContainers mungedPod.Spec.Containers = newContainers
if !api.Semantic.DeepEqual(pod.Spec, oldPod.Spec) { // munge spec.activeDeadlineSeconds
mungedPod.Spec.ActiveDeadlineSeconds = nil
if oldPod.Spec.ActiveDeadlineSeconds != nil {
activeDeadlineSeconds := *oldPod.Spec.ActiveDeadlineSeconds
mungedPod.Spec.ActiveDeadlineSeconds = &activeDeadlineSeconds
}
if !api.Semantic.DeepEqual(mungedPod.Spec, oldPod.Spec) {
//TODO: Pinpoint the specific field that causes the invalid error after we have strategic merge diff //TODO: Pinpoint the specific field that causes the invalid error after we have strategic merge diff
allErrs = append(allErrs, field.Forbidden(specPath, "pod updates may not change fields other than `containers[*].image`")) allErrs = append(allErrs, field.Forbidden(specPath, "pod updates may not change fields other than `containers[*].image` or `spec.activeDeadlineSeconds`"))
} }
return allErrs return allErrs

View File

@ -1924,9 +1924,17 @@ func TestValidatePod(t *testing.T) {
} }
func TestValidatePodUpdate(t *testing.T) { func TestValidatePodUpdate(t *testing.T) {
now := unversioned.Now() var (
grace := int64(30) activeDeadlineSecondsZero = int64(0)
grace2 := int64(31) activeDeadlineSecondsNegative = int64(-30)
activeDeadlineSecondsPositive = int64(30)
activeDeadlineSecondsLarger = int64(31)
now = unversioned.Now()
grace = int64(30)
grace2 = int64(31)
)
tests := []struct { tests := []struct {
a api.Pod a api.Pod
b api.Pod b api.Pod
@ -2061,6 +2069,150 @@ func TestValidatePodUpdate(t *testing.T) {
true, true,
"image change", "image change",
}, },
{
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
Spec: api.PodSpec{
Containers: []api.Container{
{},
},
},
},
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
Spec: api.PodSpec{
Containers: []api.Container{
{
Image: "foo:V2",
},
},
},
},
false,
"image change to empty",
},
{
api.Pod{
Spec: api.PodSpec{},
},
api.Pod{
Spec: api.PodSpec{},
},
true,
"activeDeadlineSeconds no change, nil",
},
{
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsPositive,
},
},
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsPositive,
},
},
true,
"activeDeadlineSeconds no change, set",
},
{
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsPositive,
},
},
api.Pod{},
true,
"activeDeadlineSeconds change to positive from nil",
},
{
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsPositive,
},
},
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsLarger,
},
},
true,
"activeDeadlineSeconds change to smaller positive",
},
{
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsLarger,
},
},
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsPositive,
},
},
false,
"activeDeadlineSeconds change to larger positive",
},
{
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsNegative,
},
},
api.Pod{},
false,
"activeDeadlineSeconds change to negative from nil",
},
{
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsNegative,
},
},
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsPositive,
},
},
false,
"activeDeadlineSeconds change to negative from positive",
},
{
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsZero,
},
},
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsPositive,
},
},
true,
"activeDeadlineSeconds change to zero from positive",
},
{
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsZero,
},
},
api.Pod{},
true,
"activeDeadlineSeconds change to zero from nil",
},
{
api.Pod{},
api.Pod{
Spec: api.PodSpec{
ActiveDeadlineSeconds: &activeDeadlineSecondsPositive,
},
},
false,
"activeDeadlineSeconds change to nil from positive",
},
{ {
api.Pod{ api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"}, ObjectMeta: api.ObjectMeta{Name: "foo"},
@ -2149,11 +2301,11 @@ func TestValidatePodUpdate(t *testing.T) {
errs := ValidatePodUpdate(&test.a, &test.b) errs := ValidatePodUpdate(&test.a, &test.b)
if test.isValid { if test.isValid {
if len(errs) != 0 { if len(errs) != 0 {
t.Errorf("unexpected invalid: %s %v, %v", test.test, test.a, test.b) t.Errorf("unexpected invalid: %s (%+v)\nA: %+v\nB: %+v", test.test, errs, test.a, test.b)
} }
} else { } else {
if len(errs) == 0 { if len(errs) == 0 {
t.Errorf("unexpected valid: %s %v, %v", test.test, test.a, test.b) t.Errorf("unexpected valid: %s\nA: %+v\nB: %+v", test.test, test.a, test.b)
} }
} }
} }

View File

@ -200,6 +200,11 @@ func (f *Framework) afterEach() {
f.Client = nil f.Client = nil
} }
// WaitForPodTerminated waits for the pod to be terminated with the given reason.
func (f *Framework) WaitForPodTerminated(podName, reason string) error {
return waitForPodTerminatedInNamespace(f.Client, podName, reason, f.Namespace.Name)
}
// WaitForPodRunning waits for the pod to run in the namespace. // WaitForPodRunning waits for the pod to run in the namespace.
func (f *Framework) WaitForPodRunning(podName string) error { func (f *Framework) WaitForPodRunning(podName string) error {
return waitForPodRunningInNamespace(f.Client, podName, f.Namespace.Name) return waitForPodRunningInNamespace(f.Client, podName, f.Namespace.Name)

View File

@ -470,6 +470,86 @@ var _ = Describe("Pods", func() {
Logf("Pod update OK") Logf("Pod update OK")
}) })
It("should allow activeDeadlineSeconds to be updated [Conformance]", func() {
podClient := framework.Client.Pods(framework.Namespace.Name)
By("creating the pod")
name := "pod-update-activedeadlineseconds-" + string(util.NewUUID())
value := strconv.Itoa(time.Now().Nanosecond())
pod := &api.Pod{
ObjectMeta: api.ObjectMeta{
Name: name,
Labels: map[string]string{
"name": "foo",
"time": value,
},
},
Spec: api.PodSpec{
Containers: []api.Container{
{
Name: "nginx",
Image: "gcr.io/google_containers/nginx:1.7.9",
Ports: []api.ContainerPort{{ContainerPort: 80}},
LivenessProbe: &api.Probe{
Handler: api.Handler{
HTTPGet: &api.HTTPGetAction{
Path: "/index.html",
Port: intstr.FromInt(8080),
},
},
InitialDelaySeconds: 30,
},
},
},
},
}
By("submitting the pod to kubernetes")
defer func() {
By("deleting the pod")
podClient.Delete(pod.Name, api.NewDeleteOptions(0))
}()
pod, err := podClient.Create(pod)
if err != nil {
Failf("Failed to create pod: %v", err)
}
expectNoError(framework.WaitForPodRunning(pod.Name))
By("verifying the pod is in kubernetes")
selector := labels.SelectorFromSet(labels.Set(map[string]string{"time": value}))
options := api.ListOptions{LabelSelector: selector}
pods, err := podClient.List(options)
Expect(len(pods.Items)).To(Equal(1))
// Standard get, update retry loop
expectNoError(wait.Poll(time.Millisecond*500, time.Second*30, func() (bool, error) {
By("updating the pod")
value = strconv.Itoa(time.Now().Nanosecond())
if pod == nil { // on retries we need to re-get
pod, err = podClient.Get(name)
if err != nil {
return false, fmt.Errorf("failed to get pod: %v", err)
}
}
newDeadline := int64(5)
pod.Spec.ActiveDeadlineSeconds = &newDeadline
pod, err = podClient.Update(pod)
if err == nil {
Logf("Successfully updated pod")
return true, nil
}
if errors.IsConflict(err) {
Logf("Conflicting update to pod, re-get and re-update: %v", err)
pod = nil // re-get it when we retry
return false, nil
}
return false, fmt.Errorf("failed to update pod: %v", err)
}))
expectNoError(framework.WaitForPodTerminated(pod.Name, "DeadlineExceeded"))
})
It("should contain environment variables for services [Conformance]", func() { It("should contain environment variables for services [Conformance]", func() {
// Make a pod that will be a service. // Make a pod that will be a service.
// This pod serves its hostname via HTTP. // This pod serves its hostname via HTTP.

View File

@ -788,6 +788,22 @@ func waitForPodNotPending(c *client.Client, ns, podName string) error {
}) })
} }
// waitForPodTerminatedInNamespace returns an error if it took too long for the pod
// to terminate or if the pod terminated with an unexpected reason.
func waitForPodTerminatedInNamespace(c *client.Client, podName, reason, namespace string) error {
return waitForPodCondition(c, namespace, podName, "terminated due to deadline exceeded", podStartTimeout, func(pod *api.Pod) (bool, error) {
if pod.Status.Phase == api.PodFailed {
if pod.Status.Reason == reason {
return true, nil
} else {
return true, fmt.Errorf("Expected pod %n/%n to be terminated with reason %v, got reason: ", namespace, podName, reason, pod.Status.Reason)
}
}
return false, nil
})
}
// waitForPodSuccessInNamespace returns nil if the pod reached state success, or an error if it reached failure or ran too long. // waitForPodSuccessInNamespace returns nil if the pod reached state success, or an error if it reached failure or ran too long.
func waitForPodSuccessInNamespace(c *client.Client, podName string, contName string, namespace string) error { func waitForPodSuccessInNamespace(c *client.Client, podName string, contName string, namespace string) error {
return waitForPodCondition(c, namespace, podName, "success or failure", podStartTimeout, func(pod *api.Pod) (bool, error) { return waitForPodCondition(c, namespace, podName, "success or failure", podStartTimeout, func(pod *api.Pod) (bool, error) {

163
test/integration/pods.go Normal file
View File

@ -0,0 +1,163 @@
// +build integration,!no-etcd
/*
Copyright 2015 The Kubernetes Authors All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package integration
import (
"fmt"
"net/http"
"net/http/httptest"
"testing"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/testapi"
client "k8s.io/kubernetes/pkg/client/unversioned"
"k8s.io/kubernetes/pkg/master"
"k8s.io/kubernetes/test/integration/framework"
)
func TestPodUpdateActiveDeadlineSeconds(t *testing.T) {
var m *master.Master
s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
m.Handler.ServeHTTP(w, req)
}))
// TODO: Uncomment when fix #19254
// defer s.Close()
ns := "pod-activedeadline-update"
masterConfig := framework.NewIntegrationTestMasterConfig()
m, err := master.New(masterConfig)
if err != nil {
t.Fatalf("Error in bringing up the master: %v", err)
}
framework.DeleteAllEtcdKeys()
client := client.NewOrDie(&client.Config{Host: s.URL, ContentConfig: client.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}})
var (
iZero = int64(0)
i30 = int64(30)
i60 = int64(60)
iNeg = int64(-1)
)
prototypePod := func() *api.Pod {
return &api.Pod{
ObjectMeta: api.ObjectMeta{
Name: "XXX",
},
Spec: api.PodSpec{
Containers: []api.Container{
{
Name: "fake-name",
Image: "fakeimage",
},
},
},
}
}
cases := []struct {
name string
original *int64
update *int64
valid bool
}{
{
name: "no change, nil",
original: nil,
update: nil,
valid: true,
},
{
name: "no change, set",
original: &i30,
update: &i60,
valid: true,
},
{
name: "change to positive from nil",
original: nil,
update: &i60,
valid: true,
},
{
name: "change to smaller positive",
original: &i60,
update: &i30,
valid: true,
},
{
name: "change to larger positive",
original: &i30,
update: &i60,
valid: false,
},
{
name: "change to negative from positive",
original: &i30,
update: &iNeg,
valid: false,
},
{
name: "change to negative from nil",
original: nil,
update: &iNeg,
valid: false,
},
{
name: "change to zero from positive",
original: &i30,
update: &iZero,
valid: true,
},
{
name: "change to zero from nil",
original: nil,
update: &iZero,
valid: true,
},
{
name: "change to nil from positive",
original: &i30,
update: nil,
valid: false,
},
}
for i, tc := range cases {
pod := prototypePod()
pod.Spec.ActiveDeadlineSeconds = tc.original
pod.ObjectMeta.Name = fmt.Sprintf("activedeadlineseconds-test-%v", i)
if _, err := client.Pods(ns).Create(pod); err != nil {
t.Errorf("Failed to create pod: %v", err)
}
pod.Spec.ActiveDeadlineSeconds = tc.update
_, err := client.Pods(ns).Update(pod)
if tc.valid && err != nil {
t.Errorf("%v: failed to update pod: %v", tc.name, err)
} else if !tc.valid && err == nil {
t.Errorf("%v: unexpected allowed update to pod", tc.name)
}
deletePodOrErrorf(t, client, ns, pod.Name)
}
}