From bd3b476bbf41771a0e8065e0e784a74590b461e0 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Tue, 5 Jan 2016 13:21:38 -0800 Subject: [PATCH] Update types, add defaulting code, conversions and validation. --- pkg/apis/extensions/types.go | 29 ++- pkg/apis/extensions/v1beta1/conversion.go | 124 ++++++++++ pkg/apis/extensions/v1beta1/defaults.go | 19 ++ pkg/apis/extensions/v1beta1/defaults_test.go | 158 +++++++++--- pkg/apis/extensions/v1beta1/types.go | 29 ++- pkg/apis/extensions/validation/validation.go | 35 ++- .../extensions/validation/validation_test.go | 225 +++++++++++++++--- pkg/registry/daemonset/etcd/etcd_test.go | 21 ++ 8 files changed, 558 insertions(+), 82 deletions(-) diff --git a/pkg/apis/extensions/types.go b/pkg/apis/extensions/types.go index 1315653f911..e0ae4986cf4 100644 --- a/pkg/apis/extensions/types.go +++ b/pkg/apis/extensions/types.go @@ -312,11 +312,11 @@ type DaemonSetUpdateStrategy struct { // Type of daemon set update. Only "RollingUpdate" is supported at this time. Default is RollingUpdate. Type DaemonSetUpdateStrategyType `json:"type,omitempty"` - // Rolling update config params. Present only if DeploymentStrategyType = + // Rolling update config params. Present only if DaemonSetUpdateStrategy = // RollingUpdate. //--- // TODO: Update this to follow our convention for oneOf, whatever we decide it - // to be. Same as DeploymentStrategy.RollingUpdate + // to be. Same as DeploymentStrategy.RollingUpdate. RollingUpdate *RollingUpdateDaemonSet `json:"rollingUpdate,omitempty"` } @@ -324,7 +324,7 @@ type DaemonSetUpdateStrategyType string const ( // Replace the old daemons by new ones using rolling update i.e replace them on each node one after the other. - RollingUpdateDaemonSetUpdateStrategyType DaemonSetUpdateStrategyType = "RollingUpdate" + RollingUpdateDaemonSetStrategyType DaemonSetUpdateStrategyType = "RollingUpdate" ) // Spec to control the desired behavior of daemon set rolling update. @@ -333,6 +333,7 @@ type RollingUpdateDaemonSet struct { // update. Value can be an absolute number (ex: 5) or a percentage of total // number of DaemonSet pods at the start of the update (ex: 10%). Absolute // number is calculated from percentage by rounding up. + // This cannot be 0. // Default value is 1. // Example: when this is set to 30%, 30% of the currently running DaemonSet // pods can be stopped for an update at any given time. The update starts @@ -358,17 +359,33 @@ type DaemonSetSpec struct { // More info: http://releases.k8s.io/HEAD/docs/user-guide/labels.md#label-selectors Selector *LabelSelector `json:"selector,omitempty"` - // The update strategy to use to replace existing DaemonSet with a new one. - Strategy DaemonSetUpdateStrategy `json:"strategy,omitempty"` - // Template is the object that describes the pod that will be created. // The DaemonSet will create exactly one copy of this pod on every node // that matches the template's node selector (or on every node if no node // selector is specified). // More info: http://releases.k8s.io/HEAD/docs/user-guide/replication-controller.md#pod-template Template *api.PodTemplateSpec `json:"template,omitempty"` + + // Update strategy to replace existing DaemonSet pods with new pods. + UpdateStrategy DaemonSetUpdateStrategy `json:"updateStrategy,omitempty"` + + // Label key that is added to DaemonSet pods to distinguish between old and + // new pod templates during DaemonSet update. + // Users can set this to an empty string to indicate that the system should + // not add any label. If unspecified, system uses + // DefaultDaemonSetUniqueLabelKey("daemonset.kubernetes.io/podTemplateHash"). + // Value of this key is hash of DaemonSetSpec.PodTemplateSpec. + // No label is added if this is set to empty string. + UniqueLabelKey string `json:"uniqueLabelKey,omitempty"` } +const ( + // DefaultDaemonSetUniqueLabelKey is the default key of the labels that is added + // to daemon set pods to distinguish between old and new pod templates during + // DaemonSet update. See DaemonSetSpec's UniqueLabelKey field for more information. + DefaultDaemonSetUniqueLabelKey string = "daemonset.kubernetes.io/podTemplateHash" +) + // DaemonSetStatus represents the current status of a daemon set. type DaemonSetStatus struct { // CurrentNumberScheduled is the number of nodes that are running at least 1 diff --git a/pkg/apis/extensions/v1beta1/conversion.go b/pkg/apis/extensions/v1beta1/conversion.go index 7228aa986c4..2f74270e23f 100644 --- a/pkg/apis/extensions/v1beta1/conversion.go +++ b/pkg/apis/extensions/v1beta1/conversion.go @@ -38,6 +38,12 @@ func addConversionFuncs(scheme *runtime.Scheme) { Convert_v1beta1_DeploymentStrategy_To_extensions_DeploymentStrategy, Convert_extensions_RollingUpdateDeployment_To_v1beta1_RollingUpdateDeployment, Convert_v1beta1_RollingUpdateDeployment_To_extensions_RollingUpdateDeployment, + Convert_extensions_DaemonSetSpec_To_v1beta1_DaemonSetSpec, + Convert_v1beta1_DaemonSetSpec_To_extensions_DaemonSetSpec, + Convert_extensions_DaemonSetUpdateStrategy_To_v1beta1_DaemonSetUpdateStrategy, + Convert_v1beta1_DaemonSetUpdateStrategy_To_extensions_DaemonSetUpdateStrategy, + Convert_extensions_RollingUpdateDaemonSet_To_v1beta1_RollingUpdateDaemonSet, + Convert_v1beta1_RollingUpdateDaemonSet_To_extensions_RollingUpdateDaemonSet, ) if err != nil { // If one of the conversion functions is malformed, detect it immediately. @@ -387,3 +393,121 @@ func Convert_v1_PodSecurityContext_To_api_PodSecurityContext(in *v1.PodSecurityC } return nil } + +func Convert_extensions_DaemonSetSpec_To_v1beta1_DaemonSetSpec(in *extensions.DaemonSetSpec, out *DaemonSetSpec, s conversion.Scope) error { + if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found { + defaulting.(func(*extensions.DaemonSetSpec))(in) + } + // unable to generate simple pointer conversion for extensions.LabelSelector -> v1beta1.LabelSelector + if in.Selector != nil { + out.Selector = new(LabelSelector) + if err := Convert_extensions_LabelSelector_To_v1beta1_LabelSelector(in.Selector, out.Selector, s); err != nil { + return err + } + } else { + out.Selector = nil + } + // unable to generate simple pointer conversion for api.PodTemplateSpec -> v1.PodTemplateSpec + if in.Template != nil { + out.Template = new(v1.PodTemplateSpec) + if err := Convert_api_PodTemplateSpec_To_v1_PodTemplateSpec(in.Template, out.Template, s); err != nil { + return err + } + } else { + out.Template = nil + } + if err := Convert_extensions_DaemonSetUpdateStrategy_To_v1beta1_DaemonSetUpdateStrategy(&in.UpdateStrategy, &out.UpdateStrategy, s); err != nil { + return err + } + out.UniqueLabelKey = new(string) + *out.UniqueLabelKey = in.UniqueLabelKey + return nil +} + +func Convert_v1beta1_DaemonSetSpec_To_extensions_DaemonSetSpec(in *DaemonSetSpec, out *extensions.DaemonSetSpec, s conversion.Scope) error { + if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found { + defaulting.(func(*DaemonSetSpec))(in) + } + // unable to generate simple pointer conversion for v1beta1.LabelSelector -> extensions.LabelSelector + if in.Selector != nil { + out.Selector = new(extensions.LabelSelector) + if err := Convert_v1beta1_LabelSelector_To_extensions_LabelSelector(in.Selector, out.Selector, s); err != nil { + return err + } + } else { + out.Selector = nil + } + // unable to generate simple pointer conversion for v1.PodTemplateSpec -> api.PodTemplateSpec + if in.Template != nil { + out.Template = new(api.PodTemplateSpec) + if err := Convert_v1_PodTemplateSpec_To_api_PodTemplateSpec(in.Template, out.Template, s); err != nil { + return err + } + } else { + out.Template = nil + } + if err := Convert_v1beta1_DaemonSetUpdateStrategy_To_extensions_DaemonSetUpdateStrategy(&in.UpdateStrategy, &out.UpdateStrategy, s); err != nil { + return err + } + if in.UniqueLabelKey != nil { + out.UniqueLabelKey = *in.UniqueLabelKey + } + return nil +} + +func Convert_extensions_DaemonSetUpdateStrategy_To_v1beta1_DaemonSetUpdateStrategy(in *extensions.DaemonSetUpdateStrategy, out *DaemonSetUpdateStrategy, s conversion.Scope) error { + if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found { + defaulting.(func(*extensions.DaemonSetUpdateStrategy))(in) + } + out.Type = DaemonSetUpdateStrategyType(in.Type) + if in.RollingUpdate != nil { + out.RollingUpdate = new(RollingUpdateDaemonSet) + if err := Convert_extensions_RollingUpdateDaemonSet_To_v1beta1_RollingUpdateDaemonSet(in.RollingUpdate, out.RollingUpdate, s); err != nil { + return err + } + } else { + out.RollingUpdate = nil + } + return nil +} + +func Convert_v1beta1_DaemonSetUpdateStrategy_To_extensions_DaemonSetUpdateStrategy(in *DaemonSetUpdateStrategy, out *extensions.DaemonSetUpdateStrategy, s conversion.Scope) error { + if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found { + defaulting.(func(*DaemonSetUpdateStrategy))(in) + } + out.Type = extensions.DaemonSetUpdateStrategyType(in.Type) + if in.RollingUpdate != nil { + out.RollingUpdate = new(extensions.RollingUpdateDaemonSet) + if err := Convert_v1beta1_RollingUpdateDaemonSet_To_extensions_RollingUpdateDaemonSet(in.RollingUpdate, out.RollingUpdate, s); err != nil { + return err + } + } else { + out.RollingUpdate = nil + } + return nil +} + +func Convert_extensions_RollingUpdateDaemonSet_To_v1beta1_RollingUpdateDaemonSet(in *extensions.RollingUpdateDaemonSet, out *RollingUpdateDaemonSet, s conversion.Scope) error { + if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found { + defaulting.(func(*extensions.RollingUpdateDaemonSet))(in) + } + if out.MaxUnavailable == nil { + out.MaxUnavailable = &intstr.IntOrString{} + } + if err := s.Convert(&in.MaxUnavailable, out.MaxUnavailable, 0); err != nil { + return err + } + out.MinReadySeconds = int32(in.MinReadySeconds) + return nil +} + +func Convert_v1beta1_RollingUpdateDaemonSet_To_extensions_RollingUpdateDaemonSet(in *RollingUpdateDaemonSet, out *extensions.RollingUpdateDaemonSet, s conversion.Scope) error { + if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found { + defaulting.(func(*RollingUpdateDaemonSet))(in) + } + if err := s.Convert(in.MaxUnavailable, &out.MaxUnavailable, 0); err != nil { + return err + } + out.MinReadySeconds = int(in.MinReadySeconds) + return nil +} diff --git a/pkg/apis/extensions/v1beta1/defaults.go b/pkg/apis/extensions/v1beta1/defaults.go index adb72f6a35b..98a4abbfb2f 100644 --- a/pkg/apis/extensions/v1beta1/defaults.go +++ b/pkg/apis/extensions/v1beta1/defaults.go @@ -44,6 +44,25 @@ func addDefaultingFuncs(scheme *runtime.Scheme) { obj.Labels = labels } } + updateStrategy := &obj.Spec.UpdateStrategy + if updateStrategy.Type == "" { + updateStrategy.Type = RollingUpdateDaemonSetStrategyType + } + if updateStrategy.Type == RollingUpdateDaemonSetStrategyType { + if updateStrategy.RollingUpdate == nil { + rollingUpdate := RollingUpdateDaemonSet{} + updateStrategy.RollingUpdate = &rollingUpdate + } + if updateStrategy.RollingUpdate.MaxUnavailable == nil { + // Set default MaxUnavailable as 1 by default. + maxUnavailable := intstr.FromInt(1) + updateStrategy.RollingUpdate.MaxUnavailable = &maxUnavailable + } + } + if obj.Spec.UniqueLabelKey == nil { + obj.Spec.UniqueLabelKey = new(string) + *obj.Spec.UniqueLabelKey = DefaultDaemonSetUniqueLabelKey + } }, func(obj *Deployment) { // Default labels and selector to labels from pod template spec. diff --git a/pkg/apis/extensions/v1beta1/defaults_test.go b/pkg/apis/extensions/v1beta1/defaults_test.go index 89e07cf8f1e..bf8d56af3d6 100644 --- a/pkg/apis/extensions/v1beta1/defaults_test.go +++ b/pkg/apis/extensions/v1beta1/defaults_test.go @@ -30,59 +30,155 @@ import ( ) func TestSetDefaultDaemonSet(t *testing.T) { + defaultIntOrString := intstr.FromInt(1) + defaultLabels := map[string]string{"foo": "bar"} + period := int64(v1.DefaultTerminationGracePeriodSeconds) + defaultTemplate := &v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + DNSPolicy: v1.DNSClusterFirst, + RestartPolicy: v1.RestartPolicyAlways, + SecurityContext: &v1.PodSecurityContext{}, + TerminationGracePeriodSeconds: &period, + }, + ObjectMeta: v1.ObjectMeta{ + Labels: defaultLabels, + }, + } tests := []struct { - ds *DaemonSet - expectLabelsChange bool + original *DaemonSet + expected *DaemonSet }{ - { - ds: &DaemonSet{ + { // Labels change/defaulting test. + original: &DaemonSet{ Spec: DaemonSetSpec{ - Template: &v1.PodTemplateSpec{ - ObjectMeta: v1.ObjectMeta{ - Labels: map[string]string{ - "foo": "bar", - }, - }, - }, + Template: defaultTemplate, + }, + }, + expected: &DaemonSet{ + ObjectMeta: v1.ObjectMeta{ + Labels: defaultLabels, + }, + Spec: DaemonSetSpec{ + Selector: &LabelSelector{ + MatchLabels: defaultLabels, + }, + Template: defaultTemplate, + UpdateStrategy: DaemonSetUpdateStrategy{ + Type: RollingUpdateDaemonSetStrategyType, + RollingUpdate: &RollingUpdateDaemonSet{ + MaxUnavailable: &defaultIntOrString, + }, + }, + UniqueLabelKey: newString(DefaultDaemonSetUniqueLabelKey), }, }, - expectLabelsChange: true, }, - { - ds: &DaemonSet{ + { // Labels change/defaulting test. + original: &DaemonSet{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{ "bar": "foo", }, }, Spec: DaemonSetSpec{ - Template: &v1.PodTemplateSpec{ - ObjectMeta: v1.ObjectMeta{ - Labels: map[string]string{ - "foo": "bar", - }, + Template: defaultTemplate, + UpdateStrategy: DaemonSetUpdateStrategy{ + Type: RollingUpdateDaemonSetStrategyType, + RollingUpdate: &RollingUpdateDaemonSet{ + MaxUnavailable: &defaultIntOrString, }, }, }, }, - expectLabelsChange: false, + expected: &DaemonSet{ + ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{ + "bar": "foo", + }, + }, + Spec: DaemonSetSpec{ + Selector: &LabelSelector{ + MatchLabels: defaultLabels, + }, + Template: defaultTemplate, + UpdateStrategy: DaemonSetUpdateStrategy{ + Type: RollingUpdateDaemonSetStrategyType, + RollingUpdate: &RollingUpdateDaemonSet{ + MaxUnavailable: &defaultIntOrString, + }, + }, + UniqueLabelKey: newString(DefaultDaemonSetUniqueLabelKey), + }, + }, + }, + { // Update strategy. + original: &DaemonSet{}, + expected: &DaemonSet{ + Spec: DaemonSetSpec{ + UpdateStrategy: DaemonSetUpdateStrategy{ + Type: RollingUpdateDaemonSetStrategyType, + RollingUpdate: &RollingUpdateDaemonSet{ + MaxUnavailable: &defaultIntOrString, + }, + }, + UniqueLabelKey: newString(DefaultDaemonSetUniqueLabelKey), + }, + }, + }, + { // Update strategy. + original: &DaemonSet{ + Spec: DaemonSetSpec{ + UpdateStrategy: DaemonSetUpdateStrategy{ + RollingUpdate: &RollingUpdateDaemonSet{}, + }, + }, + }, + expected: &DaemonSet{ + Spec: DaemonSetSpec{ + UpdateStrategy: DaemonSetUpdateStrategy{ + Type: RollingUpdateDaemonSetStrategyType, + RollingUpdate: &RollingUpdateDaemonSet{ + MaxUnavailable: &defaultIntOrString, + }, + }, + UniqueLabelKey: newString(DefaultDaemonSetUniqueLabelKey), + }, + }, + }, + { // Custom unique label key. + original: &DaemonSet{ + Spec: DaemonSetSpec{ + UpdateStrategy: DaemonSetUpdateStrategy{ + RollingUpdate: &RollingUpdateDaemonSet{}, + }, + UniqueLabelKey: newString("customDaemonSetKey"), + }, + }, + expected: &DaemonSet{ + Spec: DaemonSetSpec{ + UpdateStrategy: DaemonSetUpdateStrategy{ + Type: RollingUpdateDaemonSetStrategyType, + RollingUpdate: &RollingUpdateDaemonSet{ + MaxUnavailable: &defaultIntOrString, + }, + }, + UniqueLabelKey: newString("customDaemonSetKey"), + }, + }, }, } - for _, test := range tests { - ds := test.ds - obj2 := roundTrip(t, runtime.Object(ds)) - ds2, ok := obj2.(*DaemonSet) + for i, test := range tests { + original := test.original + expected := test.expected + obj2 := roundTrip(t, runtime.Object(original)) + got, ok := obj2.(*DaemonSet) if !ok { - t.Errorf("unexpected object: %v", ds2) + t.Errorf("(%d) unexpected object: %v", i, got) t.FailNow() } - if test.expectLabelsChange != reflect.DeepEqual(ds2.Labels, ds2.Spec.Template.Labels) { - if test.expectLabelsChange { - t.Errorf("expected: %v, got: %v", ds2.Spec.Template.Labels, ds2.Labels) - } else { - t.Errorf("unexpected equality: %v", ds.Labels) - } + if !reflect.DeepEqual(got.Spec, expected.Spec) { + t.Errorf("(%d) got different than expected\ngot:\n\t%+v\nexpected:\n\t%+v", i, got.Spec, expected.Spec) } } } diff --git a/pkg/apis/extensions/v1beta1/types.go b/pkg/apis/extensions/v1beta1/types.go index fd04457aeb2..ba7339fc154 100644 --- a/pkg/apis/extensions/v1beta1/types.go +++ b/pkg/apis/extensions/v1beta1/types.go @@ -302,11 +302,11 @@ type DaemonSetUpdateStrategy struct { // Type of daemon set update. Only "RollingUpdate" is supported at this time. Default is RollingUpdate. Type DaemonSetUpdateStrategyType `json:"type,omitempty"` - // Rolling update config params. Present only if DeploymentStrategyType = + // Rolling update config params. Present only if DaemonSetUpdateStrategy = // RollingUpdate. //--- // TODO: Update this to follow our convention for oneOf, whatever we decide it - // to be. Same as DeploymentStrategy.RollingUpdate + // to be. Same as DeploymentStrategy.RollingUpdate. RollingUpdate *RollingUpdateDaemonSet `json:"rollingUpdate,omitempty"` } @@ -314,7 +314,7 @@ type DaemonSetUpdateStrategyType string const ( // Replace the old daemons by new ones using rolling update i.e replace them on each node one after the other. - RollingUpdateDaemonSetUpdateStrategyType DaemonSetUpdateStrategyType = "RollingUpdate" + RollingUpdateDaemonSetStrategyType DaemonSetUpdateStrategyType = "RollingUpdate" ) // Spec to control the desired behavior of daemon set rolling update. @@ -323,6 +323,7 @@ type RollingUpdateDaemonSet struct { // update. Value can be an absolute number (ex: 5) or a percentage of total // number of DaemonSet pods at the start of the update (ex: 10%). Absolute // number is calculated from percentage by rounding up. + // This cannot be 0. // Default value is 1. // Example: when this is set to 30%, 30% of the currently running DaemonSet // pods can be stopped for an update at any given time. The update starts @@ -348,17 +349,33 @@ type DaemonSetSpec struct { // More info: http://releases.k8s.io/HEAD/docs/user-guide/labels.md#label-selectors Selector *LabelSelector `json:"selector,omitempty"` - // The update strategy to use to replace existing DaemonSet with a new one. - Strategy DaemonSetUpdateStrategy `json:"strategy,omitempty"` - // Template is the object that describes the pod that will be created. // The DaemonSet will create exactly one copy of this pod on every node // that matches the template's node selector (or on every node if no node // selector is specified). // More info: http://releases.k8s.io/HEAD/docs/user-guide/replication-controller.md#pod-template Template *v1.PodTemplateSpec `json:"template,omitempty"` + + // Update strategy to replace existing DaemonSet pods with new pods. + UpdateStrategy DaemonSetUpdateStrategy `json:"updateStrategy,omitempty"` + + // Label key that is added to DaemonSet pods to distinguish between old and + // new pod templates during DaemonSet update. + // Users can set this to an empty string to indicate that the system should + // not add any label. If unspecified, system uses + // DefaultDaemonSetUniqueLabelKey("daemonset.kubernetes.io/podTemplateHash"). + // Value of this key is hash of DaemonSetSpec.PodTemplateSpec. + // No label is added if this is set to empty string. + UniqueLabelKey *string `json:"uniqueLabelKey,omitempty"` } +const ( + // DefaultDaemonSetUniqueLabelKey is the default key of the labels that is added + // to daemon set pods to distinguish between old and new pod templates during + // DaemonSet update. See DaemonSetSpec's UniqueLabelKey field for more information. + DefaultDaemonSetUniqueLabelKey string = "daemonset.kubernetes.io/podTemplateHash" +) + // DaemonSetStatus represents the current status of a daemon set. type DaemonSetStatus struct { // CurrentNumberScheduled is the number of nodes that are running at least 1 diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index 86b82371bd1..79ff6624d22 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -179,17 +179,46 @@ func ValidateDaemonSetTemplateUpdate(podTemplate, oldPodTemplate *api.PodTemplat return allErrs } +func ValidateRollingUpdateDaemonSet(rollingUpdate *extensions.RollingUpdateDaemonSet, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + allErrs = append(allErrs, ValidatePositiveIntOrPercent(rollingUpdate.MaxUnavailable, fldPath.Child("maxUnavailable"))...) + if getIntOrPercentValue(rollingUpdate.MaxUnavailable) == 0 { + // MaxUnavailable cannot be 0. + allErrs = append(allErrs, field.Invalid(fldPath.Child("maxUnavailable"), rollingUpdate.MaxUnavailable, "cannot be 0")) + } + // Validate that MaxUnavailable is not more than 100%. + allErrs = append(allErrs, IsNotMoreThan100Percent(rollingUpdate.MaxUnavailable, fldPath.Child("maxUnavailable"))...) + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(rollingUpdate.MinReadySeconds), fldPath.Child("minReadySeconds"))...) + return allErrs +} + +func ValidateDaemonSetUpdateStrategy(strategy *extensions.DaemonSetUpdateStrategy, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + // Only rolling update is supported at this time. + if strategy.Type != extensions.RollingUpdateDaemonSetStrategyType { + allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), strategy.Type, "RollingUpdate is the only supported type")) + } + // Make sure RollingUpdate field isn't nil. + if strategy.RollingUpdate == nil { + allErrs = append(allErrs, field.Required(fldPath.Child("rollingUpdate"), "")) + return allErrs + } + allErrs = append(allErrs, ValidateRollingUpdateDaemonSet(strategy.RollingUpdate, fldPath.Child("rollingUpdate"))...) + return allErrs +} + // ValidateDaemonSetSpec tests if required fields in the DaemonSetSpec are set. func ValidateDaemonSetSpec(spec *extensions.DaemonSetSpec, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...) - + // The order of these checks is important because spec.Template is tested for nil value here + // before accessing its labels in the following check. if spec.Template == nil { allErrs = append(allErrs, field.Required(fldPath.Child("template"), "")) return allErrs } + allErrs = append(allErrs, ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...) selector, err := extensions.LabelSelectorAsSelector(spec.Selector) if err == nil && !selector.Matches(labels.Set(spec.Template.Labels)) { allErrs = append(allErrs, field.Invalid(fldPath.Child("template", "metadata", "labels"), spec.Template.Labels, "`selector` does not match template `labels`")) @@ -203,6 +232,8 @@ func ValidateDaemonSetSpec(spec *extensions.DaemonSetSpec, fldPath *field.Path) allErrs = append(allErrs, field.NotSupported(fldPath.Child("template", "spec", "restartPolicy"), spec.Template.Spec.RestartPolicy, []string{string(api.RestartPolicyAlways)})) } + allErrs = append(allErrs, ValidateDaemonSetUpdateStrategy(&spec.UpdateStrategy, fldPath.Child("updateStrategy"))...) + return allErrs } diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go index 62f61cde021..d071fcf9665 100644 --- a/pkg/apis/extensions/validation/validation_test.go +++ b/pkg/apis/extensions/validation/validation_test.go @@ -283,6 +283,13 @@ func TestValidateDaemonSetUpdate(t *testing.T) { validSelector2 := map[string]string{"c": "d"} invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"} + validUpdateStrategy := extensions.DaemonSetUpdateStrategy{ + Type: extensions.RollingUpdateDaemonSetStrategyType, + RollingUpdate: &extensions.RollingUpdateDaemonSet{ + MaxUnavailable: intstr.FromInt(1), + }, + } + validPodSpecAbc := api.PodSpec{ RestartPolicy: api.RestartPolicyAlways, DNSPolicy: api.DNSClusterFirst, @@ -368,15 +375,17 @@ func TestValidateDaemonSetUpdate(t *testing.T) { old: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, update: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, }, @@ -384,15 +393,17 @@ func TestValidateDaemonSetUpdate(t *testing.T) { old: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, update: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector2}, - Template: &validPodTemplateAbc2.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector2}, + Template: &validPodTemplateAbc2.Template, + UpdateStrategy: validUpdateStrategy, }, }, }, @@ -400,15 +411,17 @@ func TestValidateDaemonSetUpdate(t *testing.T) { old: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, update: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateNodeSelector.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateNodeSelector.Template, + UpdateStrategy: validUpdateStrategy, }, }, }, @@ -425,15 +438,17 @@ func TestValidateDaemonSetUpdate(t *testing.T) { old: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, update: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, }, @@ -441,15 +456,17 @@ func TestValidateDaemonSetUpdate(t *testing.T) { old: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, update: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: invalidSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: invalidSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, }, @@ -457,15 +474,17 @@ func TestValidateDaemonSetUpdate(t *testing.T) { old: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, update: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &invalidPodTemplate.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &invalidPodTemplate.Template, + UpdateStrategy: validUpdateStrategy, }, }, }, @@ -473,15 +492,17 @@ func TestValidateDaemonSetUpdate(t *testing.T) { old: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, update: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateDef.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateDef.Template, + UpdateStrategy: validUpdateStrategy, }, }, }, @@ -489,15 +510,38 @@ func TestValidateDaemonSetUpdate(t *testing.T) { old: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplateAbc.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, }, }, update: extensions.DaemonSet{ ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &readWriteVolumePodTemplate.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &readWriteVolumePodTemplate.Template, + UpdateStrategy: validUpdateStrategy, + }, + }, + }, + "invalid update strategy": { + old: extensions.DaemonSet{ + ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault}, + Spec: extensions.DaemonSetSpec{ + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: validUpdateStrategy, + }, + }, + update: extensions.DaemonSet{ + ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, + Spec: extensions.DaemonSetSpec{ + Selector: &extensions.LabelSelector{MatchLabels: invalidSelector}, + Template: &validPodTemplateAbc.Template, + UpdateStrategy: extensions.DaemonSetUpdateStrategy{ + Type: extensions.RollingUpdateDaemonSetStrategyType, + RollingUpdate: nil, + }, }, }, }, @@ -511,6 +555,12 @@ func TestValidateDaemonSetUpdate(t *testing.T) { func TestValidateDaemonSet(t *testing.T) { validSelector := map[string]string{"a": "b"} + validUpdateStrategy := extensions.DaemonSetUpdateStrategy{ + Type: extensions.RollingUpdateDaemonSetStrategyType, + RollingUpdate: &extensions.RollingUpdateDaemonSet{ + MaxUnavailable: intstr.FromInt(1), + }, + } validPodTemplate := api.PodTemplate{ Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ @@ -539,15 +589,17 @@ func TestValidateDaemonSet(t *testing.T) { { ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplate.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplate.Template, + UpdateStrategy: validUpdateStrategy, }, }, { ObjectMeta: api.ObjectMeta{Name: "abc-123", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ - Selector: &extensions.LabelSelector{MatchLabels: validSelector}, - Template: &validPodTemplate.Template, + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplate.Template, + UpdateStrategy: validUpdateStrategy, }, }, } @@ -585,7 +637,7 @@ func TestValidateDaemonSet(t *testing.T) { Template: &validPodTemplate.Template, }, }, - "invalid manifest": { + "invalid template": { ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault}, Spec: extensions.DaemonSetSpec{ Selector: &extensions.LabelSelector{MatchLabels: validSelector}, @@ -667,6 +719,104 @@ func TestValidateDaemonSet(t *testing.T) { }, }, }, + "invalid update strategy - Type is not RollingUpdate": { + ObjectMeta: api.ObjectMeta{ + Name: "abc-123", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.DaemonSetSpec{ + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplate.Template, + UpdateStrategy: extensions.DaemonSetUpdateStrategy{ + Type: "", + RollingUpdate: &extensions.RollingUpdateDaemonSet{ + MaxUnavailable: intstr.FromInt(1), + }, + }, + }, + }, + "invalid update strategy - RollingUpdate field is nil": { + ObjectMeta: api.ObjectMeta{ + Name: "abc-123", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.DaemonSetSpec{ + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplate.Template, + UpdateStrategy: extensions.DaemonSetUpdateStrategy{ + Type: extensions.RollingUpdateDaemonSetStrategyType, + RollingUpdate: nil, + }, + }, + }, + "invalid update strategy - MaxUnavailable is 0": { + ObjectMeta: api.ObjectMeta{ + Name: "abc-123", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.DaemonSetSpec{ + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplate.Template, + UpdateStrategy: extensions.DaemonSetUpdateStrategy{ + Type: extensions.RollingUpdateDaemonSetStrategyType, + RollingUpdate: &extensions.RollingUpdateDaemonSet{ + MaxUnavailable: intstr.FromInt(0), + MinReadySeconds: 1, + }, + }, + }, + }, + "invalid update strategy - MaxUnavailable is greater than 100%": { + ObjectMeta: api.ObjectMeta{ + Name: "abc-123", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.DaemonSetSpec{ + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplate.Template, + UpdateStrategy: extensions.DaemonSetUpdateStrategy{ + Type: extensions.RollingUpdateDaemonSetStrategyType, + RollingUpdate: &extensions.RollingUpdateDaemonSet{ + MaxUnavailable: intstr.FromString("150%"), + MinReadySeconds: 1, + }, + }, + }, + }, + "invalid update strategy - MaxUnavailable is negative": { + ObjectMeta: api.ObjectMeta{ + Name: "abc-123", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.DaemonSetSpec{ + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplate.Template, + UpdateStrategy: extensions.DaemonSetUpdateStrategy{ + Type: extensions.RollingUpdateDaemonSetStrategyType, + RollingUpdate: &extensions.RollingUpdateDaemonSet{ + MaxUnavailable: intstr.FromInt(-1), + MinReadySeconds: 0, + }, + }, + }, + }, + "invalid update strategy - MinReadySeconds is negative": { + ObjectMeta: api.ObjectMeta{ + Name: "abc-123", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.DaemonSetSpec{ + Selector: &extensions.LabelSelector{MatchLabels: validSelector}, + Template: &validPodTemplate.Template, + UpdateStrategy: extensions.DaemonSetUpdateStrategy{ + Type: extensions.RollingUpdateDaemonSetStrategyType, + RollingUpdate: &extensions.RollingUpdateDaemonSet{ + MaxUnavailable: intstr.FromInt(-1), + MinReadySeconds: -1, + }, + }, + }, + }, } for k, v := range errorCases { errs := ValidateDaemonSet(&v) @@ -676,6 +826,7 @@ func TestValidateDaemonSet(t *testing.T) { for i := range errs { field := errs[i].Field if !strings.HasPrefix(field, "spec.template.") && + !strings.HasPrefix(field, "spec.updateStrategy") && field != "metadata.name" && field != "metadata.namespace" && field != "spec.selector" && diff --git a/pkg/registry/daemonset/etcd/etcd_test.go b/pkg/registry/daemonset/etcd/etcd_test.go index f23cf75daf1..de37afd71c3 100755 --- a/pkg/registry/daemonset/etcd/etcd_test.go +++ b/pkg/registry/daemonset/etcd/etcd_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/kubernetes/pkg/registry/registrytest" "k8s.io/kubernetes/pkg/runtime" etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing" + "k8s.io/kubernetes/pkg/util/intstr" ) func newStorage(t *testing.T) (*REST, *StatusREST, *etcdtesting.EtcdTestServer) { @@ -59,6 +60,13 @@ func newValidDaemonSet() *extensions.DaemonSet { DNSPolicy: api.DNSClusterFirst, }, }, + UpdateStrategy: extensions.DaemonSetUpdateStrategy{ + Type: extensions.RollingUpdateDaemonSetStrategyType, + RollingUpdate: &extensions.RollingUpdateDaemonSet{ + MaxUnavailable: intstr.FromInt(1), + }, + }, + UniqueLabelKey: "foo-label", }, } } @@ -81,6 +89,14 @@ func TestCreate(t *testing.T) { Template: validDaemonSet.Spec.Template, }, }, + // invalid update strategy + &extensions.DaemonSet{ + Spec: extensions.DaemonSetSpec{ + Selector: validDaemonSet.Spec.Selector, + Template: validDaemonSet.Spec.Template, + UniqueLabelKey: validDaemonSet.Spec.UniqueLabelKey, + }, + }, ) } @@ -118,6 +134,11 @@ func TestUpdate(t *testing.T) { object.Spec.Selector = &extensions.LabelSelector{MatchLabels: map[string]string{}} return object }, + func(obj runtime.Object) runtime.Object { + object := obj.(*extensions.DaemonSet) + object.Spec.UpdateStrategy = extensions.DaemonSetUpdateStrategy{} + return object + }, ) }