Skip to content

Commit

Permalink
Merge pull request #78914 from liggitt/pod-spec-defaults
Browse files Browse the repository at this point in the history 
Add tests to detect default changes to podspec and podspectemplate defaults
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Jun 27, 2019
2 parents c5dc18b + 617309c commit 9c8827f
Showing 1 changed file with 277 additions and 1 deletion.
278 changes: 277 additions & 1 deletion pkg/apis/core/v1/defaults_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@ limitations under the License.
package v1_test

import (
"encoding/json"
"fmt"
"reflect"
"strings"
"testing"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/kubernetes/pkg/api/legacyscheme"
corev1 "k8s.io/kubernetes/pkg/apis/core/v1"
Expand All @@ -34,6 +37,279 @@ import (
_ "k8s.io/kubernetes/pkg/api/testapi"
)

// TestWorkloadDefaults detects changes to defaults within PodTemplateSpec.
// Defaulting changes within this type can cause spurious rollouts of workloads on API server update.
func TestWorkloadDefaults(t *testing.T) {
rc := &v1.ReplicationController{Spec: v1.ReplicationControllerSpec{Template: &v1.PodTemplateSpec{}}}
template := rc.Spec.Template
// New defaults under PodTemplateSpec are only acceptable if they would not be applied when reading data from a previous release.
// Forbidden: adding a new field `MyField *bool` and defaulting it to a non-nil value
// Forbidden: defaulting an existing field `MyField *bool` when it was previously not defaulted
// Forbidden: changing an existing default value
// Allowed: adding a new field `MyContainer *MyType` and defaulting a child of that type (e.g. `MyContainer.MyChildField`) if and only if MyContainer is non-nil
expectedDefaults := map[string]string{
".Spec.Containers[0].Env[0].ValueFrom.FieldRef.APIVersion": `"v1"`,
".Spec.Containers[0].ImagePullPolicy": `"IfNotPresent"`,
".Spec.Containers[0].Lifecycle.PostStart.HTTPGet.Path": `"/"`,
".Spec.Containers[0].Lifecycle.PostStart.HTTPGet.Scheme": `"HTTP"`,
".Spec.Containers[0].Lifecycle.PreStop.HTTPGet.Path": `"/"`,
".Spec.Containers[0].Lifecycle.PreStop.HTTPGet.Scheme": `"HTTP"`,
".Spec.Containers[0].LivenessProbe.FailureThreshold": `3`,
".Spec.Containers[0].LivenessProbe.Handler.HTTPGet.Path": `"/"`,
".Spec.Containers[0].LivenessProbe.Handler.HTTPGet.Scheme": `"HTTP"`,
".Spec.Containers[0].LivenessProbe.PeriodSeconds": `10`,
".Spec.Containers[0].LivenessProbe.SuccessThreshold": `1`,
".Spec.Containers[0].LivenessProbe.TimeoutSeconds": `1`,
".Spec.Containers[0].Ports[0].Protocol": `"TCP"`,
".Spec.Containers[0].ReadinessProbe.FailureThreshold": `3`,
".Spec.Containers[0].ReadinessProbe.Handler.HTTPGet.Path": `"/"`,
".Spec.Containers[0].ReadinessProbe.Handler.HTTPGet.Scheme": `"HTTP"`,
".Spec.Containers[0].ReadinessProbe.PeriodSeconds": `10`,
".Spec.Containers[0].ReadinessProbe.SuccessThreshold": `1`,
".Spec.Containers[0].ReadinessProbe.TimeoutSeconds": `1`,
".Spec.Containers[0].TerminationMessagePath": `"/dev/termination-log"`,
".Spec.Containers[0].TerminationMessagePolicy": `"File"`,
".Spec.DNSPolicy": `"ClusterFirst"`,
".Spec.InitContainers[0].Env[0].ValueFrom.FieldRef.APIVersion": `"v1"`,
".Spec.InitContainers[0].ImagePullPolicy": `"IfNotPresent"`,
".Spec.InitContainers[0].Lifecycle.PostStart.HTTPGet.Path": `"/"`,
".Spec.InitContainers[0].Lifecycle.PostStart.HTTPGet.Scheme": `"HTTP"`,
".Spec.InitContainers[0].Lifecycle.PreStop.HTTPGet.Path": `"/"`,
".Spec.InitContainers[0].Lifecycle.PreStop.HTTPGet.Scheme": `"HTTP"`,
".Spec.InitContainers[0].LivenessProbe.FailureThreshold": `3`,
".Spec.InitContainers[0].LivenessProbe.Handler.HTTPGet.Path": `"/"`,
".Spec.InitContainers[0].LivenessProbe.Handler.HTTPGet.Scheme": `"HTTP"`,
".Spec.InitContainers[0].LivenessProbe.PeriodSeconds": `10`,
".Spec.InitContainers[0].LivenessProbe.SuccessThreshold": `1`,
".Spec.InitContainers[0].LivenessProbe.TimeoutSeconds": `1`,
".Spec.InitContainers[0].Ports[0].Protocol": `"TCP"`,
".Spec.InitContainers[0].ReadinessProbe.FailureThreshold": `3`,
".Spec.InitContainers[0].ReadinessProbe.Handler.HTTPGet.Path": `"/"`,
".Spec.InitContainers[0].ReadinessProbe.Handler.HTTPGet.Scheme": `"HTTP"`,
".Spec.InitContainers[0].ReadinessProbe.PeriodSeconds": `10`,
".Spec.InitContainers[0].ReadinessProbe.SuccessThreshold": `1`,
".Spec.InitContainers[0].ReadinessProbe.TimeoutSeconds": `1`,
".Spec.InitContainers[0].TerminationMessagePath": `"/dev/termination-log"`,
".Spec.InitContainers[0].TerminationMessagePolicy": `"File"`,
".Spec.RestartPolicy": `"Always"`,
".Spec.SchedulerName": `"default-scheduler"`,
".Spec.SecurityContext": `{}`,
".Spec.TerminationGracePeriodSeconds": `30`,
".Spec.Volumes[0].VolumeSource.AzureDisk.CachingMode": `"ReadWrite"`,
".Spec.Volumes[0].VolumeSource.AzureDisk.FSType": `"ext4"`,
".Spec.Volumes[0].VolumeSource.AzureDisk.Kind": `"Shared"`,
".Spec.Volumes[0].VolumeSource.AzureDisk.ReadOnly": `false`,
".Spec.Volumes[0].VolumeSource.ConfigMap.DefaultMode": `420`,
".Spec.Volumes[0].VolumeSource.DownwardAPI.DefaultMode": `420`,
".Spec.Volumes[0].VolumeSource.DownwardAPI.Items[0].FieldRef.APIVersion": `"v1"`,
".Spec.Volumes[0].VolumeSource.EmptyDir": `{}`,
".Spec.Volumes[0].VolumeSource.HostPath.Type": `""`,
".Spec.Volumes[0].VolumeSource.ISCSI.ISCSIInterface": `"default"`,
".Spec.Volumes[0].VolumeSource.Projected.DefaultMode": `420`,
".Spec.Volumes[0].VolumeSource.Projected.Sources[0].DownwardAPI.Items[0].FieldRef.APIVersion": `"v1"`,
".Spec.Volumes[0].VolumeSource.Projected.Sources[0].ServiceAccountToken.ExpirationSeconds": `3600`,
".Spec.Volumes[0].VolumeSource.RBD.Keyring": `"/etc/ceph/keyring"`,
".Spec.Volumes[0].VolumeSource.RBD.RBDPool": `"rbd"`,
".Spec.Volumes[0].VolumeSource.RBD.RadosUser": `"admin"`,
".Spec.Volumes[0].VolumeSource.ScaleIO.FSType": `"xfs"`,
".Spec.Volumes[0].VolumeSource.ScaleIO.StorageMode": `"ThinProvisioned"`,
".Spec.Volumes[0].VolumeSource.Secret.DefaultMode": `420`,
}
defaults := detectDefaults(t, rc, reflect.ValueOf(template))
if !reflect.DeepEqual(expectedDefaults, defaults) {
t.Errorf("Defaults for PodTemplateSpec changed. This can cause spurious rollouts of workloads on API server upgrade.")
t.Logf(diff.ObjectReflectDiff(expectedDefaults, defaults))
}
}

// TestPodDefaults detects changes to defaults within PodSpec.
// Defaulting changes within this type (*especially* within containers) can cause kubelets to restart containers on API server update.
func TestPodDefaults(t *testing.T) {
pod := &v1.Pod{}
// New defaults under PodSpec are only acceptable if they would not be applied when reading data from a previous release.
// Forbidden: adding a new field `MyField *bool` and defaulting it to a non-nil value
// Forbidden: defaulting an existing field `MyField *bool` when it was previously not defaulted
// Forbidden: changing an existing default value
// Allowed: adding a new field `MyContainer *MyType` and defaulting a child of that type (e.g. `MyContainer.MyChildField`) if and only if MyContainer is non-nil
expectedDefaults := map[string]string{
".Spec.Containers[0].Env[0].ValueFrom.FieldRef.APIVersion": `"v1"`,
".Spec.Containers[0].ImagePullPolicy": `"IfNotPresent"`,
".Spec.Containers[0].Lifecycle.PostStart.HTTPGet.Path": `"/"`,
".Spec.Containers[0].Lifecycle.PostStart.HTTPGet.Scheme": `"HTTP"`,
".Spec.Containers[0].Lifecycle.PreStop.HTTPGet.Path": `"/"`,
".Spec.Containers[0].Lifecycle.PreStop.HTTPGet.Scheme": `"HTTP"`,
".Spec.Containers[0].LivenessProbe.FailureThreshold": `3`,
".Spec.Containers[0].LivenessProbe.Handler.HTTPGet.Path": `"/"`,
".Spec.Containers[0].LivenessProbe.Handler.HTTPGet.Scheme": `"HTTP"`,
".Spec.Containers[0].LivenessProbe.PeriodSeconds": `10`,
".Spec.Containers[0].LivenessProbe.SuccessThreshold": `1`,
".Spec.Containers[0].LivenessProbe.TimeoutSeconds": `1`,
".Spec.Containers[0].Ports[0].Protocol": `"TCP"`,
".Spec.Containers[0].ReadinessProbe.FailureThreshold": `3`,
".Spec.Containers[0].ReadinessProbe.Handler.HTTPGet.Path": `"/"`,
".Spec.Containers[0].ReadinessProbe.Handler.HTTPGet.Scheme": `"HTTP"`,
".Spec.Containers[0].ReadinessProbe.PeriodSeconds": `10`,
".Spec.Containers[0].ReadinessProbe.SuccessThreshold": `1`,
".Spec.Containers[0].ReadinessProbe.TimeoutSeconds": `1`,
".Spec.Containers[0].Resources.Requests": `{"":"0"}`, // this gets defaulted from the limits field
".Spec.Containers[0].TerminationMessagePath": `"/dev/termination-log"`,
".Spec.Containers[0].TerminationMessagePolicy": `"File"`,
".Spec.DNSPolicy": `"ClusterFirst"`,
".Spec.EnableServiceLinks": `true`,
".Spec.InitContainers[0].Env[0].ValueFrom.FieldRef.APIVersion": `"v1"`,
".Spec.InitContainers[0].ImagePullPolicy": `"IfNotPresent"`,
".Spec.InitContainers[0].Lifecycle.PostStart.HTTPGet.Path": `"/"`,
".Spec.InitContainers[0].Lifecycle.PostStart.HTTPGet.Scheme": `"HTTP"`,
".Spec.InitContainers[0].Lifecycle.PreStop.HTTPGet.Path": `"/"`,
".Spec.InitContainers[0].Lifecycle.PreStop.HTTPGet.Scheme": `"HTTP"`,
".Spec.InitContainers[0].LivenessProbe.FailureThreshold": `3`,
".Spec.InitContainers[0].LivenessProbe.Handler.HTTPGet.Path": `"/"`,
".Spec.InitContainers[0].LivenessProbe.Handler.HTTPGet.Scheme": `"HTTP"`,
".Spec.InitContainers[0].LivenessProbe.PeriodSeconds": `10`,
".Spec.InitContainers[0].LivenessProbe.SuccessThreshold": `1`,
".Spec.InitContainers[0].LivenessProbe.TimeoutSeconds": `1`,
".Spec.InitContainers[0].Ports[0].Protocol": `"TCP"`,
".Spec.InitContainers[0].ReadinessProbe.FailureThreshold": `3`,
".Spec.InitContainers[0].ReadinessProbe.Handler.HTTPGet.Path": `"/"`,
".Spec.InitContainers[0].ReadinessProbe.Handler.HTTPGet.Scheme": `"HTTP"`,
".Spec.InitContainers[0].ReadinessProbe.PeriodSeconds": `10`,
".Spec.InitContainers[0].ReadinessProbe.SuccessThreshold": `1`,
".Spec.InitContainers[0].ReadinessProbe.TimeoutSeconds": `1`,
".Spec.InitContainers[0].Resources.Requests": `{"":"0"}`, // this gets defaulted from the limits field
".Spec.InitContainers[0].TerminationMessagePath": `"/dev/termination-log"`,
".Spec.InitContainers[0].TerminationMessagePolicy": `"File"`,
".Spec.RestartPolicy": `"Always"`,
".Spec.SchedulerName": `"default-scheduler"`,
".Spec.SecurityContext": `{}`,
".Spec.TerminationGracePeriodSeconds": `30`,
".Spec.Volumes[0].VolumeSource.AzureDisk.CachingMode": `"ReadWrite"`,
".Spec.Volumes[0].VolumeSource.AzureDisk.FSType": `"ext4"`,
".Spec.Volumes[0].VolumeSource.AzureDisk.Kind": `"Shared"`,
".Spec.Volumes[0].VolumeSource.AzureDisk.ReadOnly": `false`,
".Spec.Volumes[0].VolumeSource.ConfigMap.DefaultMode": `420`,
".Spec.Volumes[0].VolumeSource.DownwardAPI.DefaultMode": `420`,
".Spec.Volumes[0].VolumeSource.DownwardAPI.Items[0].FieldRef.APIVersion": `"v1"`,
".Spec.Volumes[0].VolumeSource.EmptyDir": `{}`,
".Spec.Volumes[0].VolumeSource.HostPath.Type": `""`,
".Spec.Volumes[0].VolumeSource.ISCSI.ISCSIInterface": `"default"`,
".Spec.Volumes[0].VolumeSource.Projected.DefaultMode": `420`,
".Spec.Volumes[0].VolumeSource.Projected.Sources[0].DownwardAPI.Items[0].FieldRef.APIVersion": `"v1"`,
".Spec.Volumes[0].VolumeSource.Projected.Sources[0].ServiceAccountToken.ExpirationSeconds": `3600`,
".Spec.Volumes[0].VolumeSource.RBD.Keyring": `"/etc/ceph/keyring"`,
".Spec.Volumes[0].VolumeSource.RBD.RBDPool": `"rbd"`,
".Spec.Volumes[0].VolumeSource.RBD.RadosUser": `"admin"`,
".Spec.Volumes[0].VolumeSource.ScaleIO.FSType": `"xfs"`,
".Spec.Volumes[0].VolumeSource.ScaleIO.StorageMode": `"ThinProvisioned"`,
".Spec.Volumes[0].VolumeSource.Secret.DefaultMode": `420`,
}
defaults := detectDefaults(t, pod, reflect.ValueOf(pod))
if !reflect.DeepEqual(expectedDefaults, defaults) {
t.Errorf("Defaults for PodSpec changed. This can cause spurious restarts of containers on API server upgrade.")
t.Logf(diff.ObjectReflectDiff(expectedDefaults, defaults))
}
}

type testPath struct {
path string
value reflect.Value
}

func detectDefaults(t *testing.T, obj runtime.Object, v reflect.Value) map[string]string {
defaults := map[string]string{}
toVisit := []testPath{{path: "", value: v}}

for len(toVisit) > 0 {
visit := toVisit[0]
toVisit = toVisit[1:]

legacyscheme.Scheme.Default(obj)
defaultedV := visit.value
zeroV := reflect.Zero(visit.value.Type())

switch {
case visit.value.Kind() == reflect.Struct:
for fi := 0; fi < visit.value.NumField(); fi++ {
structField := visit.value.Type().Field(fi)
valueField := visit.value.Field(fi)
if valueField.CanSet() {
toVisit = append(toVisit, testPath{path: visit.path + "." + structField.Name, value: valueField})
}
}

case visit.value.Kind() == reflect.Slice:
if !visit.value.IsNil() {
// if we already have a value, we got defaulted
marshaled, _ := json.Marshal(defaultedV.Interface())
defaults[visit.path] = string(marshaled)
} else if visit.value.Type().Elem().Kind() == reflect.Struct {
if strings.HasPrefix(visit.path, ".ObjectMeta.ManagedFields[") {
break
}
// if we don't already have a value, and contain structs, add an empty item so we can recurse
item := reflect.New(visit.value.Type().Elem()).Elem()
visit.value.Set(reflect.Append(visit.value, item))
toVisit = append(toVisit, testPath{path: visit.path + "[0]", value: visit.value.Index(0)})
} else if !isPrimitive(visit.value.Type().Elem().Kind()) {
t.Logf("unhandled non-primitive slice type %s: %s", visit.path, visit.value.Type().Elem())
}

case visit.value.Kind() == reflect.Map:
if !visit.value.IsNil() {
// if we already have a value, we got defaulted
marshaled, _ := json.Marshal(defaultedV.Interface())
defaults[visit.path] = string(marshaled)
} else if visit.value.Type().Key().Kind() == reflect.String && visit.value.Type().Elem().Kind() == reflect.Struct {
if strings.HasPrefix(visit.path, ".ObjectMeta.ManagedFields[") {
break
}
// if we don't already have a value, and contain structs, add an empty item so we can recurse
item := reflect.New(visit.value.Type().Elem()).Elem()
visit.value.Set(reflect.MakeMap(visit.value.Type()))
visit.value.SetMapIndex(reflect.New(visit.value.Type().Key()).Elem(), item)
toVisit = append(toVisit, testPath{path: visit.path + "[*]", value: item})
} else if !isPrimitive(visit.value.Type().Elem().Kind()) {
t.Logf("unhandled non-primitive map type %s: %s", visit.path, visit.value.Type().Elem())
}

case visit.value.Kind() == reflect.Ptr:
if visit.value.IsNil() {
if visit.value.Type().Elem().Kind() == reflect.Struct {
visit.value.Set(reflect.New(visit.value.Type().Elem()))
toVisit = append(toVisit, testPath{path: visit.path, value: visit.value.Elem()})
} else if !isPrimitive(visit.value.Type().Elem().Kind()) {
t.Errorf("unhandled non-primitive nil ptr: %s: %s", visit.path, visit.value.Type())
}
} else {
if visit.path != "" {
marshaled, _ := json.Marshal(defaultedV.Interface())
defaults[visit.path] = string(marshaled)
}
toVisit = append(toVisit, testPath{path: visit.path, value: visit.value.Elem()})
}

case isPrimitive(visit.value.Kind()):
if !reflect.DeepEqual(defaultedV.Interface(), zeroV.Interface()) {
marshaled, _ := json.Marshal(defaultedV.Interface())
defaults[visit.path] = string(marshaled)
}

default:
t.Errorf("unhandled kind: %s: %s", visit.path, visit.value.Type())
}

}
return defaults
}

func isPrimitive(k reflect.Kind) bool {
switch k {
case reflect.String, reflect.Bool, reflect.Int32, reflect.Int64, reflect.Int:
return true
default:
return false
}
}

func roundTrip(t *testing.T, obj runtime.Object) runtime.Object {
codec := legacyscheme.Codecs.LegacyCodec(corev1.SchemeGroupVersion)
data, err := runtime.Encode(codec, obj)
Expand Down

0 comments on commit 9c8827f

Please sign in to comment.