github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-24 12:15:52 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #51733 from caesarxuchao/only-relax-uninitialized-pod-validation

Browse Source 
Automatic merge from submit-queue (batch tested with PRs 51733, 51838)

Relax update validation of uninitialized pod

Split from https://github.com/kubernetes/kubernetes/pull/50344

Fix https://github.com/kubernetes/kubernetes/issues/47837

* Let the podStrategy to only call `validation.ValidatePod()` if the old pod is not initialized, so fields are mutable.
* Let the podStatusStrategy refuse updates if the old pod is not initialized.

cc @smarterclayton 

```release-note
Pod spec is mutable when the pod is uninitialized. The apiserver requires the pod spec to be valid even if it's uninitialized. Updating the status field of uninitialized pods is invalid.
```
This commit is contained in:
Kubernetes Submit Queue 2017-09-06 00:02:17 -07:00 committed by GitHub
commit 795154919d
7 changed files with 177 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/registry/core/pod/BUILD
View File

@ -19,7 +19,9 @@ go_library(
"//pkg/api/validation:go_default_library",
"//pkg/kubelet/client:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/fields:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
@ -27,9 +29,11 @@ go_library(
"//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
"//vendor/k8s.io/apiserver/pkg/features:go_default_library",
"//vendor/k8s.io/apiserver/pkg/registry/generic:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/names:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
],
)

3
pkg/registry/core/pod/storage/BUILD
View File

@ -25,11 +25,14 @@ go_test(
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
"//vendor/k8s.io/apiserver/pkg/features:go_default_library",
"//vendor/k8s.io/apiserver/pkg/registry/generic:go_default_library",
"//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/errors:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/etcd/testing:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature/testing:go_default_library",
],
)

73
pkg/registry/core/pod/storage/storage_test.go
View File

@ -32,11 +32,14 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/diff"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/features"
"k8s.io/apiserver/pkg/registry/generic"
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/apiserver/pkg/storage"
storeerr "k8s.io/apiserver/pkg/storage/errors"
etcdtesting "k8s.io/apiserver/pkg/storage/etcd/testing"
utilfeature "k8s.io/apiserver/pkg/util/feature"
utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/registry/registrytest"
"k8s.io/kubernetes/pkg/securitycontext"
@ -699,6 +702,76 @@ func TestEtcdCreateBinding(t *testing.T) {
}
}
func TestEtcdUpdateUninitialized(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.Initializers, true)()
storage, _, _, server := newStorage(t)
defer server.Terminate(t)
defer storage.Store.DestroyFunc()
ctx := genericapirequest.NewDefaultContext()
pod := validNewPod()
// add pending initializers to the pod
pod.ObjectMeta.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "init.k8s.io"}}}
if _, err := storage.Create(ctx, pod, true); err != nil {
t.Fatalf("unexpected error: %v", err)
}
podIn := *pod
// only uninitialized pod is allowed to add containers via update
podIn.Spec.Containers = append(podIn.Spec.Containers, api.Container{
Name: "foo2",
Image: "test",
ImagePullPolicy: api.PullAlways,
TerminationMessagePath: api.TerminationMessagePathDefault,
TerminationMessagePolicy: api.TerminationMessageReadFile,
SecurityContext: securitycontext.ValidInternalSecurityContextWithContainerDefaults(),
})
podIn.ObjectMeta.Initializers = nil
_, _, err := storage.Update(ctx, podIn.Name, rest.DefaultUpdatedObjectInfo(&podIn, api.Scheme))
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
obj, err := storage.Get(ctx, podIn.ObjectMeta.Name, &metav1.GetOptions{})
if err != nil {
t.Errorf("unexpected error: %v", err)
}
podOut := obj.(*api.Pod)
if podOut.GetInitializers() != nil {
t.Errorf("expect nil initializers, got %v", podOut.ObjectMeta.Initializers)
}
if !apiequality.Semantic.DeepEqual(podIn.Spec.Containers, podOut.Spec.Containers) {
t.Errorf("objects differ: %v", diff.ObjectDiff(podOut, &podIn))
}
}
func TestEtcdStatusUpdateUninitialized(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.Initializers, true)()
storage, _, statusStorage, server := newStorage(t)
defer server.Terminate(t)
defer storage.Store.DestroyFunc()
ctx := genericapirequest.NewDefaultContext()
pod := validNewPod()
// add pending initializers to the pod
pod.ObjectMeta.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "init.k8s.io"}}}
if _, err := storage.Create(ctx, pod, true); err != nil {
t.Fatalf("unexpected error: %v", err)
}
podIn := *pod
// only uninitialized pod is allowed to add containers via update
podIn.Status.Phase = api.PodRunning
podIn.ObjectMeta.Initializers = nil
_, _, err := statusStorage.Update(ctx, podIn.Name, rest.DefaultUpdatedObjectInfo(&podIn, api.Scheme))
expected := "Forbidden: must not update status when the object is uninitialized"
if err == nil {
t.Fatalf("Unexpected no err, expected %q", expected)
}
if !strings.Contains(err.Error(), expected) {
t.Errorf("unexpected error: %v, expected %q", err, expected)
}
}
func TestEtcdUpdateNotScheduled(t *testing.T) {
storage, _, _, server := newStorage(t)
defer server.Terminate(t)

34
pkg/registry/core/pod/strategy.go
View File

@ -26,7 +26,9 @@ import (
"time"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
apimachineryvalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
@ -34,9 +36,11 @@ import (
utilnet "k8s.io/apimachinery/pkg/util/net"
"k8s.io/apimachinery/pkg/util/validation/field"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/features"
"k8s.io/apiserver/pkg/registry/generic"
"k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/helper/qos"
podutil "k8s.io/kubernetes/pkg/api/pod"
@ -95,9 +99,31 @@ func (podStrategy) AllowCreateOnUpdate() bool {
return false
}
func isUpdatingUninitializedPod(old runtime.Object) (bool, error) {
if !utilfeature.DefaultFeatureGate.Enabled(features.Initializers) {
return false, nil
}
oldMeta, err := meta.Accessor(old)
if err != nil {
return false, err
}
oldInitializers := oldMeta.GetInitializers()
if oldInitializers != nil && len(oldInitializers.Pending) != 0 {
return true, nil
}
return false, nil
}
// ValidateUpdate is the default update validation for an end user.
func (podStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList {
errorList := validation.ValidatePod(obj.(*api.Pod))
uninitializedUpdate, err := isUpdatingUninitializedPod(old)
if err != nil {
return append(errorList, field.InternalError(field.NewPath("metadata"), err))
}
if uninitializedUpdate {
return errorList
}
return append(errorList, validation.ValidatePodUpdate(obj.(*api.Pod), old.(*api.Pod))...)
}
@ -166,6 +192,14 @@ func (podStatusStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, ol
}
func (podStatusStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList {
var errorList field.ErrorList
uninitializedUpdate, err := isUpdatingUninitializedPod(old)
if err != nil {
return append(errorList, field.InternalError(field.NewPath("metadata"), err))
}
if uninitializedUpdate {
return append(errorList, field.Forbidden(field.NewPath("status"), apimachineryvalidation.UninitializedStatusUpdateErrorMsg))
}
// TODO: merge valid fields after update
return validation.ValidatePodStatusUpdate(obj.(*api.Pod), old.(*api.Pod))
}

2
staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go
View File

@ -88,3 +88,5 @@ func ValidateDeleteOptions(options *metav1.DeleteOptions) field.ErrorList {
}
return allErrs
}
const UninitializedStatusUpdateErrorMsg string = `must not update status when the object is uninitialized`

1
test/e2e/BUILD
View File

@ -75,6 +75,7 @@ go_library(
"//vendor/github.com/onsi/gomega:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/api/rbac/v1beta1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/fields:go_default_library",

119
test/e2e/resource_quota.go
View File

@ -21,6 +21,7 @@ import (
"time"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
@ -147,72 +148,72 @@ var _ = framework.KubeDescribe("ResourceQuota", func() {
Expect(err).NotTo(HaveOccurred())
})
It("should create a ResourceQuota and capture the life of an uninitialized pod.", func() {
// TODO: uncomment the test when #50344 is merged.
// By("Creating a ResourceQuota")
// quotaName := "test-quota"
// resourceQuota := newTestResourceQuota(quotaName)
// resourceQuota, err := createResourceQuota(f.ClientSet, f.Namespace.Name, resourceQuota)
// Expect(err).NotTo(HaveOccurred())
It("[Feature:Initializers] should create a ResourceQuota and capture the life of an uninitialized pod.", func() {
By("Creating a ResourceQuota")
quotaName := "test-quota"
resourceQuota := newTestResourceQuota(quotaName)
resourceQuota, err := createResourceQuota(f.ClientSet, f.Namespace.Name, resourceQuota)
Expect(err).NotTo(HaveOccurred())
// By("Ensuring resource quota status is calculated")
// usedResources := v1.ResourceList{}
// usedResources[v1.ResourceQuotas] = resource.MustParse("1")
// err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
// Expect(err).NotTo(HaveOccurred())
By("Ensuring resource quota status is calculated")
usedResources := v1.ResourceList{}
usedResources[v1.ResourceQuotas] = resource.MustParse("1")
err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
Expect(err).NotTo(HaveOccurred())
// By("Creating an uninitialized Pod that fits quota")
// podName := "test-pod"
// requests := v1.ResourceList{}
// requests[v1.ResourceCPU] = resource.MustParse("500m")
// requests[v1.ResourceMemory] = resource.MustParse("252Mi")
// pod := newTestPodForQuota(f, podName, requests, v1.ResourceList{})
// pod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "unhandled"}}}
// _, err = f.ClientSet.Core().Pods(f.Namespace.Name).Create(pod)
// // because no one is handling the initializer, server will return a 504 timeout
// if err != nil && !errors.IsTimeout(err) {
// framework.Failf("expect err to be timeout error, got %v", err)
// }
// podToUpdate, err := f.ClientSet.Core().Pods(f.Namespace.Name).Get(podName, metav1.GetOptions{})
// Expect(err).NotTo(HaveOccurred())
By("Creating an uninitialized Pod that fits quota")
podName := "test-pod"
requests := v1.ResourceList{}
requests[v1.ResourceCPU] = resource.MustParse("500m")
requests[v1.ResourceMemory] = resource.MustParse("252Mi")
pod := newTestPodForQuota(f, podName, requests, v1.ResourceList{})
pod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "unhandled"}}}
_, err = f.ClientSet.Core().Pods(f.Namespace.Name).Create(pod)
// because no one is handling the initializer, server will return a 504 timeout
if err != nil && !errors.IsTimeout(err) {
framework.Failf("expect err to be timeout error, got %v", err)
}
podToUpdate, err := f.ClientSet.Core().Pods(f.Namespace.Name).Get(podName, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
// By("Ensuring ResourceQuota status captures the pod usage")
// usedResources[v1.ResourceQuotas] = resource.MustParse("1")
// usedResources[v1.ResourcePods] = resource.MustParse("1")
// usedResources[v1.ResourceCPU] = requests[v1.ResourceCPU]
// usedResources[v1.ResourceMemory] = requests[v1.ResourceMemory]
// err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
// Expect(err).NotTo(HaveOccurred())
By("Ensuring ResourceQuota status captures the pod usage")
usedResources[v1.ResourceQuotas] = resource.MustParse("1")
usedResources[v1.ResourcePods] = resource.MustParse("1")
usedResources[v1.ResourceCPU] = requests[v1.ResourceCPU]
usedResources[v1.ResourceMemory] = requests[v1.ResourceMemory]
err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
Expect(err).NotTo(HaveOccurred())
// By("Not allowing an uninitialized pod to be created that exceeds remaining quota")
// requests = v1.ResourceList{}
// requests[v1.ResourceCPU] = resource.MustParse("600m")
// requests[v1.ResourceMemory] = resource.MustParse("100Mi")
// pod = newTestPodForQuota(f, "fail-pod", requests, v1.ResourceList{})
// pod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "unhandled"}}}
// pod, err = f.ClientSet.Core().Pods(f.Namespace.Name).Create(pod)
// Expect(err).To(HaveOccurred())
// fmt.Println("CHAO: err=", err)
By("Not allowing an uninitialized pod to be created that exceeds remaining quota")
requests = v1.ResourceList{}
requests[v1.ResourceCPU] = resource.MustParse("600m")
requests[v1.ResourceMemory] = resource.MustParse("100Mi")
pod = newTestPodForQuota(f, "fail-pod", requests, v1.ResourceList{})
pod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "unhandled"}}}
pod, err = f.ClientSet.Core().Pods(f.Namespace.Name).Create(pod)
Expect(err).To(HaveOccurred())
fmt.Println("CHAO: err=", err)
// By("Ensuring an uninitialized pod can update its resource requirements")
// // a pod cannot dynamically update its resource requirements.
// requests = v1.ResourceList{}
// requests[v1.ResourceCPU] = resource.MustParse("100m")
// requests[v1.ResourceMemory] = resource.MustParse("100Mi")
// podToUpdate.Spec.Containers[0].Resources.Requests = requests
// _, err = f.ClientSet.Core().Pods(f.Namespace.Name).Update(podToUpdate)
// Expect(err).NotTo(HaveOccurred())
By("Ensuring an uninitialized pod can update its resource requirements")
// a pod cannot dynamically update its resource requirements.
requests = v1.ResourceList{}
requests[v1.ResourceCPU] = resource.MustParse("100m")
requests[v1.ResourceMemory] = resource.MustParse("100Mi")
podToUpdate.Spec.Containers[0].Resources.Requests = requests
_, err = f.ClientSet.Core().Pods(f.Namespace.Name).Update(podToUpdate)
Expect(err).NotTo(HaveOccurred())
// By("Ensuring attempts to update pod resource requirements did change quota usage")
// usedResources[v1.ResourceQuotas] = resource.MustParse("1")
// usedResources[v1.ResourcePods] = resource.MustParse("1")
// usedResources[v1.ResourceCPU] = requests[v1.ResourceCPU]
// usedResources[v1.ResourceMemory] = requests[v1.ResourceMemory]
// err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
// Expect(err).NotTo(HaveOccurred())
By("Ensuring attempts to update pod resource requirements did change quota usage")
usedResources[v1.ResourceQuotas] = resource.MustParse("1")
usedResources[v1.ResourcePods] = resource.MustParse("1")
usedResources[v1.ResourceCPU] = requests[v1.ResourceCPU]
usedResources[v1.ResourceMemory] = requests[v1.ResourceMemory]
err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
Expect(err).NotTo(HaveOccurred())
// TODO: uncomment the test when the replenishment_controller uses the
// sharedInformer that list/watches uninitialized objects.
// TODO: uncomment the test after 51247 is merged, in which the
// replenishment_controller uses the sharedInformer that list/watches
// uninitialized objects.
// By("Deleting the pod")
// err = f.ClientSet.Core().Pods(f.Namespace.Name).Delete(podName, metav1.NewDeleteOptions(0))
// Expect(err).NotTo(HaveOccurred())