Skip to content

Commit

Permalink
add pod template spec validation for StatefulSet
Browse files Browse the repository at this point in the history
  • Loading branch information
shyung committed Apr 9, 2021
1 parent 8300553 commit 069c44f
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 16 deletions.
41 changes: 31 additions & 10 deletions pkg/apis/apps/validation/validation.go
Expand Up @@ -45,8 +45,13 @@ func ValidateStatefulSetName(name string, prefix bool) []string {
return apimachineryvalidation.NameIsDNSSubdomain(name, prefix)
}

// ValidatePodTemplateSpecForStatefulSet validates the given template and ensures that it is in accordance with the desired selector.
func ValidatePodTemplateSpecForStatefulSet(template *api.PodTemplateSpec, selector labels.Selector, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
// ValidatePodTemplateSpecForStatefulSet validates the PodTemplateSpec generated with
// template and volumeClaimTemplates of StatefulSet spec, and ensures that it is in
// accordance with the desired selector.
func ValidatePodTemplateSpecForStatefulSet(
template *api.PodTemplateSpec, volumeClaimTemplates []api.PersistentVolumeClaim,
selector labels.Selector, fldPath *field.Path, opts apivalidation.PodValidationOptions,
) field.ErrorList {
allErrs := field.ErrorList{}
if template == nil {
allErrs = append(allErrs, field.Required(fldPath, ""))
Expand All @@ -58,13 +63,28 @@ func ValidatePodTemplateSpecForStatefulSet(template *api.PodTemplateSpec, select
allErrs = append(allErrs, field.Invalid(fldPath.Child("metadata", "labels"), template.Labels, "`selector` does not match template `labels`"))
}
}
// TODO: Add validation for PodSpec, currently this will check volumes, which we know will
// fail. We should really check that the union of the given volumes and volumeClaims match
// volume mounts in the containers.
// allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(template, fldPath)...)
allErrs = append(allErrs, unversionedvalidation.ValidateLabels(template.Labels, fldPath.Child("labels"))...)
allErrs = append(allErrs, apivalidation.ValidateAnnotations(template.Annotations, fldPath.Child("annotations"))...)
allErrs = append(allErrs, apivalidation.ValidatePodSpecificAnnotations(template.Annotations, &template.Spec, fldPath.Child("annotations"), opts)...)

// Generate newTemplate with template and volumeClaimTemplates.
volumesMap := make(map[string]api.Volume)
for i := range template.Spec.Volumes {
volume := template.Spec.Volumes[i]
volumesMap[volume.Name] = volume
}
for i := range volumeClaimTemplates {
claim := volumeClaimTemplates[i]
volumesMap[claim.Name] = api.Volume{
Name: claim.Name,
VolumeSource: api.VolumeSource{PersistentVolumeClaim: &api.PersistentVolumeClaimVolumeSource{ClaimName: claim.Name}},
}
}
newVolumes := make([]api.Volume, 0, len(volumesMap))
for _, v := range volumesMap {
newVolumes = append(newVolumes, v)
}
newTemplate := template.DeepCopy()
newTemplate.Spec.Volumes = newVolumes

allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(newTemplate, fldPath, opts)...)
}
return allErrs
}
Expand Down Expand Up @@ -122,7 +142,8 @@ func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path, op
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, ""))
} else {
allErrs = append(allErrs, ValidatePodTemplateSpecForStatefulSet(&spec.Template, selector, fldPath.Child("template"), opts)...)
allErrs = append(allErrs, ValidatePodTemplateSpecForStatefulSet(
&spec.Template, spec.VolumeClaimTemplates, selector, fldPath.Child("template"), opts)...)
}

if spec.Template.Spec.RestartPolicy != api.RestartPolicyAlways {
Expand Down
9 changes: 8 additions & 1 deletion pkg/apis/apps/validation/validation_test.go
Expand Up @@ -44,7 +44,14 @@ func TestValidateStatefulSet(t *testing.T) {
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicyAlways,
DNSPolicy: api.DNSClusterFirst,
Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}},
Containers: []api.Container{
{
Name: "abc",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: api.TerminationMessageReadFile,
},
},
},
},
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/registry/apps/statefulset/storage/storage_test.go
Expand Up @@ -69,9 +69,10 @@ func validNewStatefulSet() *apps.StatefulSet {
Spec: api.PodSpec{
Containers: []api.Container{
{
Name: "test",
Image: "test_image",
ImagePullPolicy: api.PullIfNotPresent,
Name: "test",
Image: "test_image",
ImagePullPolicy: api.PullIfNotPresent,
TerminationMessagePolicy: api.TerminationMessageReadFile,
},
},
RestartPolicy: api.RestartPolicyAlways,
Expand Down
9 changes: 8 additions & 1 deletion pkg/registry/apps/statefulset/strategy_test.go
Expand Up @@ -44,7 +44,14 @@ func TestStatefulSetStrategy(t *testing.T) {
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicyAlways,
DNSPolicy: api.DNSClusterFirst,
Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}},
Containers: []api.Container{
{
Name: "abc",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: api.TerminationMessageReadFile,
},
},
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion test/integration/etcd/data.go
Expand Up @@ -111,7 +111,7 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes
ExpectedEtcdPath: "/registry/deployments/" + namespace + "/deployment4",
},
gvr("apps", "v1", "statefulsets"): {
Stub: `{"metadata": {"name": "ss3"}, "spec": {"selector": {"matchLabels": {"a": "b"}}, "template": {"metadata": {"labels": {"a": "b"}}}}}`,
Stub: `{"metadata": {"name": "ss3"}, "spec": {"selector": {"matchLabels": {"a": "b"}}, "template": {"metadata": {"labels": {"a": "b"}}, "spec": {"containers": [{"image": "` + image + `", "name": "container4"}]}}}}`,
ExpectedEtcdPath: "/registry/statefulsets/" + namespace + "/ss3",
},
gvr("apps", "v1", "replicasets"): {
Expand Down

0 comments on commit 069c44f

Please sign in to comment.