Fix update validation for pods w/init containers.

This commit is contained in:
Ivan Shvedunov 2016-07-04 23:30:54 +03:00
parent 63bb2810d2
commit 02baa44948
4 changed files with 121 additions and 20 deletions

View File

@ -1960,6 +1960,23 @@ func ValidatePodSecurityContext(securityContext *api.PodSecurityContext, spec *a
return allErrs
}
func validateContainerUpdates(newContainers, oldContainers []api.Container, fldPath *field.Path) (allErrs field.ErrorList, stop bool) {
allErrs = field.ErrorList{}
if len(newContainers) != len(oldContainers) {
//TODO: Pinpoint the specific container that causes the invalid error after we have strategic merge diff
allErrs = append(allErrs, field.Forbidden(fldPath, "pod updates may not add or remove containers"))
return allErrs, true
}
// validate updated container images
for i, ctr := range newContainers {
if len(ctr.Image) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Index(i).Child("image"), ""))
}
}
return allErrs, false
}
// ValidatePodUpdate tests to see if the update is legal for an end user to make. newPod is updated with fields
// that cannot be changed.
func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList {
@ -1967,21 +1984,21 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList {
allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath)
allErrs = append(allErrs, ValidatePodSpecificAnnotations(newPod.ObjectMeta.Annotations, fldPath.Child("annotations"))...)
specPath := field.NewPath("spec")
if len(newPod.Spec.Containers) != len(oldPod.Spec.Containers) {
//TODO: Pinpoint the specific container that causes the invalid error after we have strategic merge diff
allErrs = append(allErrs, field.Forbidden(specPath.Child("containers"), "pod updates may not add or remove containers"))
return allErrs
}
// validate updateable fields:
// 1. containers[*].image
// 2. spec.activeDeadlineSeconds
// 2. initContainers[*].image
// 3. 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"), ""))
}
containerErrs, stop := validateContainerUpdates(newPod.Spec.Containers, oldPod.Spec.Containers, specPath.Child("containers"))
allErrs = append(allErrs, containerErrs...)
if stop {
return allErrs
}
containerErrs, stop = validateContainerUpdates(newPod.Spec.InitContainers, oldPod.Spec.InitContainers, specPath.Child("initContainers"))
allErrs = append(allErrs, containerErrs...)
if stop {
return allErrs
}
// validate updated spec.activeDeadlineSeconds. two types of updates are allowed:
@ -2013,6 +2030,13 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList {
newContainers = append(newContainers, container)
}
mungedPod.Spec.Containers = newContainers
// munge initContainers[*].image
var newInitContainers []api.Container
for ix, container := range mungedPod.Spec.InitContainers {
container.Image = oldPod.Spec.InitContainers[ix].Image
newInitContainers = append(newInitContainers, container)
}
mungedPod.Spec.InitContainers = newInitContainers
// munge spec.activeDeadlineSeconds
mungedPod.Spec.ActiveDeadlineSeconds = nil
if oldPod.Spec.ActiveDeadlineSeconds != nil {

View File

@ -1907,8 +1907,9 @@ func TestValidatePodSpec(t *testing.T) {
Volumes: []api.Volume{
{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
},
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
RestartPolicy: api.RestartPolicyAlways,
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
InitContainers: []api.Container{{Name: "ictr", Image: "iimage", ImagePullPolicy: "IfNotPresent"}},
RestartPolicy: api.RestartPolicyAlways,
NodeSelector: map[string]string{
"key": "value",
},
@ -1999,6 +2000,12 @@ func TestValidatePodSpec(t *testing.T) {
RestartPolicy: api.RestartPolicyAlways,
DNSPolicy: api.DNSClusterFirst,
},
"bad init container": {
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
InitContainers: []api.Container{{}},
RestartPolicy: api.RestartPolicyAlways,
DNSPolicy: api.DNSClusterFirst,
},
"bad DNS policy": {
DNSPolicy: api.DNSPolicy("invalid"),
RestartPolicy: api.RestartPolicyAlways,
@ -3022,6 +3029,35 @@ func TestValidatePodUpdate(t *testing.T) {
false,
"more containers",
},
{
api.Pod{
ObjectMeta: api.ObjectMeta{
Name: "foo",
},
Spec: api.PodSpec{
InitContainers: []api.Container{
{
Image: "foo:V1",
},
},
},
},
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
Spec: api.PodSpec{
InitContainers: []api.Container{
{
Image: "foo:V2",
},
{
Image: "bar:V2",
},
},
},
},
false,
"more init containers",
},
{
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
@ -3070,6 +3106,30 @@ func TestValidatePodUpdate(t *testing.T) {
true,
"image change",
},
{
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
Spec: api.PodSpec{
InitContainers: []api.Container{
{
Image: "foo:V1",
},
},
},
},
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
Spec: api.PodSpec{
InitContainers: []api.Container{
{
Image: "foo:V2",
},
},
},
},
true,
"init container image change",
},
{
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
@ -3092,6 +3152,28 @@ func TestValidatePodUpdate(t *testing.T) {
false,
"image change to empty",
},
{
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
Spec: api.PodSpec{
InitContainers: []api.Container{
{},
},
},
},
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
Spec: api.PodSpec{
InitContainers: []api.Container{
{
Image: "foo:V2",
},
},
},
},
false,
"init container image change to empty",
},
{
api.Pod{
Spec: api.PodSpec{},

View File

@ -131,7 +131,6 @@ func addGrouplessTypes() {
api.Scheme.AddKnownTypes(grouplessGroupVersion,
&apiservertesting.Simple{}, &apiservertesting.SimpleList{}, &ListOptions{},
&api.DeleteOptions{}, &apiservertesting.SimpleGetOptions{}, &apiservertesting.SimpleRoot{})
api.Scheme.AddKnownTypes(grouplessGroupVersion, &api.Pod{})
api.Scheme.AddKnownTypes(grouplessInternalGroupVersion,
&apiservertesting.Simple{}, &apiservertesting.SimpleList{}, &api.ListOptions{},
&apiservertesting.SimpleGetOptions{}, &apiservertesting.SimpleRoot{})

View File

@ -31,6 +31,7 @@ import (
"k8s.io/kubernetes/pkg/api/rest"
"k8s.io/kubernetes/pkg/api/testapi"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/api/v1"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/pkg/types"
"k8s.io/kubernetes/pkg/util/diff"
@ -186,12 +187,7 @@ func (tc *patchTestCase) Run(t *testing.T) {
namer := &testNamer{namespace, name}
copier := runtime.ObjectCopier(api.Scheme)
resource := unversioned.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}
versionedObj, err := api.Scheme.ConvertToVersion(&api.Pod{}, unversioned.GroupVersion{Version: "v1"})
if err != nil {
t.Errorf("%s: unexpected error: %v", tc.name, err)
return
}
versionedObj := &v1.Pod{}
for _, patchType := range []api.PatchType{api.JSONPatchType, api.MergePatchType, api.StrategicMergePatchType} {
// TODO SUPPORT THIS!