Skip to content

Commit

Permalink
DaemonSetSpec.Template should not be a pointer.
Browse files Browse the repository at this point in the history 
Pod template for DaemonSets isn't optional, like Deployments and Jobs,
so the DaemonSetSpec.Template field should not be a pointer.
  • Loading branch information
@madhusudancs
madhusudancs committed Jan 29, 2016
1 parent 992a859 commit 73fb6dc
Show file tree
Hide file tree
Showing 13 changed files with 52 additions and 76 deletions.
2 changes: 1 addition & 1 deletion pkg/apis/extensions/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ type DaemonSetSpec struct {
// 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"`
Template api.PodTemplateSpec `json:"template"`

// Update strategy to replace existing DaemonSet pods with new pods.
UpdateStrategy DaemonSetUpdateStrategy `json:"updateStrategy,omitempty"`
Expand Down
20 changes: 4 additions & 16 deletions pkg/apis/extensions/v1beta1/conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,14 +443,8 @@ func Convert_extensions_DaemonSetSpec_To_v1beta1_DaemonSetSpec(in *extensions.Da
} 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_api_PodTemplateSpec_To_v1_PodTemplateSpec(&in.Template, &out.Template, s); err != nil {
return err
}
if err := Convert_extensions_DaemonSetUpdateStrategy_To_v1beta1_DaemonSetUpdateStrategy(&in.UpdateStrategy, &out.UpdateStrategy, s); err != nil {
return err
Expand All @@ -473,14 +467,8 @@ func Convert_v1beta1_DaemonSetSpec_To_extensions_DaemonSetSpec(in *DaemonSetSpec
} 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_v1_PodTemplateSpec_To_api_PodTemplateSpec(&in.Template, &out.Template, s); err != nil {
return err
}
if err := Convert_v1beta1_DaemonSetUpdateStrategy_To_extensions_DaemonSetUpdateStrategy(&in.UpdateStrategy, &out.UpdateStrategy, s); err != nil {
return err
Expand Down
6 changes: 2 additions & 4 deletions pkg/apis/extensions/v1beta1/defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,8 @@ func addDefaultingFuncs(scheme *runtime.Scheme) {
}
},
func(obj *DaemonSet) {
var labels map[string]string
if obj.Spec.Template != nil {
labels = obj.Spec.Template.Labels
}
labels := obj.Spec.Template.Labels

// TODO: support templates defined elsewhere when we support them in the API
if labels != nil {
if obj.Spec.Selector == nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/extensions/v1beta1/defaults_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestSetDefaultDaemonSet(t *testing.T) {
defaultIntOrString := intstr.FromInt(1)
defaultLabels := map[string]string{"foo": "bar"}
period := int64(v1.DefaultTerminationGracePeriodSeconds)
defaultTemplate := &v1.PodTemplateSpec{
defaultTemplate := v1.PodTemplateSpec{
Spec: v1.PodSpec{
DNSPolicy: v1.DNSClusterFirst,
RestartPolicy: v1.RestartPolicyAlways,
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/extensions/v1beta1/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ type DaemonSetSpec struct {
// 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"`
Template v1.PodTemplateSpec `json:"template"`

// Update strategy to replace existing DaemonSet pods with new pods.
UpdateStrategy DaemonSetUpdateStrategy `json:"updateStrategy,omitempty"`
Expand Down
12 changes: 3 additions & 9 deletions pkg/apis/extensions/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func ValidateDaemonSetStatusUpdate(controller, oldController *extensions.DaemonS
}

// ValidateDaemonSetTemplateUpdate tests that certain fields in the daemon set's pod template are not updated.
func ValidateDaemonSetTemplateUpdate(podTemplate, oldPodTemplate *api.PodTemplateSpec, fldPath *field.Path) field.ErrorList {
func ValidateDaemonSetTemplateUpdate(podTemplate, oldPodTemplate api.PodTemplateSpec, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
podSpec := podTemplate.Spec
// podTemplate.Spec is not a pointer, so we can modify NodeSelector and NodeName directly.
Expand Down Expand Up @@ -210,20 +210,14 @@ func ValidateDaemonSetUpdateStrategy(strategy *extensions.DaemonSetUpdateStrateg
func ValidateDaemonSetSpec(spec *extensions.DaemonSetSpec, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

// 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`"))
}

allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(spec.Template, fldPath.Child("template"))...)
allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(&spec.Template, fldPath.Child("template"))...)
// Daemons typically run on more than one node, so mark Read-Write persistent disks as invalid.
allErrs = append(allErrs, apivalidation.ValidateReadOnlyPersistentDisks(spec.Template.Spec.Volumes, fldPath.Child("template", "spec", "volumes"))...)
// RestartPolicy has already been first-order validated as per ValidatePodTemplateSpec().
Expand Down
58 changes: 29 additions & 29 deletions pkg/apis/extensions/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,15 +376,15 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplateAbc.Template,
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,
Template: validPodTemplateAbc.Template,
UpdateStrategy: validUpdateStrategy,
},
},
Expand All @@ -394,15 +394,15 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplateAbc.Template,
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,
Template: validPodTemplateAbc2.Template,
UpdateStrategy: validUpdateStrategy,
},
},
Expand All @@ -412,15 +412,15 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplateAbc.Template,
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,
Template: validPodTemplateNodeSelector.Template,
UpdateStrategy: validUpdateStrategy,
},
},
Expand All @@ -439,15 +439,15 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplateAbc.Template,
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,
Template: validPodTemplateAbc.Template,
UpdateStrategy: validUpdateStrategy,
},
},
Expand All @@ -457,15 +457,15 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplateAbc.Template,
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,
Template: validPodTemplateAbc.Template,
UpdateStrategy: validUpdateStrategy,
},
},
Expand All @@ -475,15 +475,15 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplateAbc.Template,
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,
Template: invalidPodTemplate.Template,
UpdateStrategy: validUpdateStrategy,
},
},
Expand All @@ -493,15 +493,15 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplateAbc.Template,
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,
Template: validPodTemplateDef.Template,
UpdateStrategy: validUpdateStrategy,
},
},
Expand All @@ -511,15 +511,15 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplateAbc.Template,
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,
Template: readWriteVolumePodTemplate.Template,
UpdateStrategy: validUpdateStrategy,
},
},
Expand All @@ -529,15 +529,15 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplateAbc.Template,
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,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.RollingUpdateDaemonSetStrategyType,
RollingUpdate: nil,
Expand Down Expand Up @@ -590,15 +590,15 @@ func TestValidateDaemonSet(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplate.Template,
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,
Template: validPodTemplate.Template,
UpdateStrategy: validUpdateStrategy,
},
},
Expand All @@ -614,27 +614,27 @@ func TestValidateDaemonSet(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplate.Template,
Template: validPodTemplate.Template,
},
},
"missing-namespace": {
ObjectMeta: api.ObjectMeta{Name: "abc-123"},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplate.Template,
Template: validPodTemplate.Template,
},
},
"empty selector": {
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Template: &validPodTemplate.Template,
Template: validPodTemplate.Template,
},
},
"selector_doesnt_match": {
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
Template: &validPodTemplate.Template,
Template: validPodTemplate.Template,
},
},
"invalid template": {
Expand All @@ -653,7 +653,7 @@ func TestValidateDaemonSet(t *testing.T) {
},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplate.Template,
Template: validPodTemplate.Template,
},
},
"invalid_label 2": {
Expand All @@ -665,7 +665,7 @@ func TestValidateDaemonSet(t *testing.T) {
},
},
Spec: extensions.DaemonSetSpec{
Template: &invalidPodTemplate.Template,
Template: invalidPodTemplate.Template,
},
},
"invalid_annotation": {
Expand All @@ -678,7 +678,7 @@ func TestValidateDaemonSet(t *testing.T) {
},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &validPodTemplate.Template,
Template: validPodTemplate.Template,
},
},
"invalid restart policy 1": {
Expand All @@ -688,7 +688,7 @@ func TestValidateDaemonSet(t *testing.T) {
},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &api.PodTemplateSpec{
Template: api.PodTemplateSpec{
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicyOnFailure,
DNSPolicy: api.DNSClusterFirst,
Expand All @@ -707,7 +707,7 @@ func TestValidateDaemonSet(t *testing.T) {
},
Spec: extensions.DaemonSetSpec{
Selector: &extensions.LabelSelector{MatchLabels: validSelector},
Template: &api.PodTemplateSpec{
Template: api.PodTemplateSpec{
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicyNever,
DNSPolicy: api.DNSClusterFirst,
Expand Down
Loading

0 comments on commit 73fb6dc

Please sign in to comment.