Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adjust replication controller validation to be more flexible. Fix docs. #4151

Merged
merged 1 commit into from
Feb 6, 2015
Merged

Adjust replication controller validation to be more flexible. Fix docs. #4151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions docs/volumes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ A Volume with a GCEPersistentDisk property allows access to files on a Google Co
There are some restrictions when using a GCEPersistentDisk:
- the nodes (what the kubelet runs on) need to be GCE VMs
- those VMs need to be in the same GCE project and zone as the PD
- avoid creating multiple pods that use the same Volume
- if multiple pods refer to the same Volume and both are scheduled on the same machine, regardless of whether they are read-only or read-write, then the second pod scheduled will fail.
- Replication controllers can only be created for pods that use read-only mounts.
- avoid creating multiple pods that use the same Volume if any mount it read/write.
- if a pod P already mounts a volume read/write, and a second pod Q attempts to use the volume, regardless of if it tries to use it read-only or read/write, Q will fail.
- if a pod P already mounts a volume read-only, and a second pod Q attempts to use the volume read/write, Q will fail.
- replication controllers with replicas > 1 can only be created for pods that use read-only mounts.

#### Creating a PD
Before you can use a GCE PD with a pod, you need to create it and format it.
Expand Down
11 changes: 6 additions & 5 deletions pkg/api/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,6 @@ func ValidateReplicationControllerUpdate(oldController, controller *api.Replicat
allErrs := errs.ValidationErrorList{}
allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldController.ObjectMeta, &controller.ObjectMeta).Prefix("metadata")...)
allErrs = append(allErrs, ValidateReplicationControllerSpec(&controller.Spec).Prefix("spec")...)

return allErrs
}

Expand All @@ -646,7 +645,7 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs
if !selector.Matches(labels) {
allErrs = append(allErrs, errs.NewFieldInvalid("template.labels", spec.Template.Labels, "selector does not match template"))
}
allErrs = append(allErrs, ValidatePodTemplateSpec(spec.Template).Prefix("template")...)
allErrs = append(allErrs, ValidatePodTemplateSpec(spec.Template, spec.Replicas).Prefix("template")...)
// RestartPolicy has already been first-order validated as per ValidatePodTemplateSpec().
if spec.Template.Spec.RestartPolicy.Always == nil {
// TODO: should probably be Unsupported
Expand All @@ -658,12 +657,14 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs
}

// ValidatePodTemplateSpec validates the spec of a pod template
func ValidatePodTemplateSpec(spec *api.PodTemplateSpec) errs.ValidationErrorList {
func ValidatePodTemplateSpec(spec *api.PodTemplateSpec, replicas int) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
allErrs = append(allErrs, ValidateLabels(spec.Labels, "labels")...)
allErrs = append(allErrs, ValidateAnnotations(spec.Annotations, "annotations")...)
allErrs = append(allErrs, ValidatePodSpec(&spec.Spec).Prefix("spec")...)
allErrs = append(allErrs, ValidateReadOnlyPersistentDisks(spec.Spec.Volumes).Prefix("spec.volumes")...)
if replicas > 1 {
allErrs = append(allErrs, ValidateReadOnlyPersistentDisks(spec.Spec.Volumes).Prefix("spec.volumes")...)
}
return allErrs
}

Expand All @@ -672,7 +673,7 @@ func ValidateReadOnlyPersistentDisks(volumes []api.Volume) errs.ValidationErrorL
for _, vol := range volumes {
if vol.Source.GCEPersistentDisk != nil {
if vol.Source.GCEPersistentDisk.ReadOnly == false {
allErrs = append(allErrs, errs.NewFieldInvalid("GCEPersistentDisk.ReadOnly", false, "must be true"))
allErrs = append(allErrs, errs.NewFieldInvalid("GCEPersistentDisk.ReadOnly", false, "ReadOnly must be true for replicated pods > 1, as GCE PD can only be mounted on multiple machines if it is read-only."))
}
}
}
Expand Down
178 changes: 175 additions & 3 deletions pkg/api/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1400,6 +1400,166 @@ func TestValidateService(t *testing.T) {
}
}

func TestValidateReplicationControllerUpdate(t *testing.T) {
validSelector := map[string]string{"a": "b"}
validPodTemplate := api.PodTemplate{
Spec: api.PodTemplateSpec{
ObjectMeta: api.ObjectMeta{
Labels: validSelector,
},
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
DNSPolicy: api.DNSClusterFirst,
},
},
}
readWriteVolumePodTemplate := api.PodTemplate{
Spec: api.PodTemplateSpec{
ObjectMeta: api.ObjectMeta{
Labels: validSelector,
},
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
DNSPolicy: api.DNSClusterFirst,
Volumes: []api.Volume{{Name: "gcepd", Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{"my-PD", "ext4", 1, false}}}},
},
},
}
invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"}
invalidPodTemplate := api.PodTemplate{
Spec: api.PodTemplateSpec{
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
DNSPolicy: api.DNSClusterFirst,
},
ObjectMeta: api.ObjectMeta{
Labels: invalidSelector,
},
},
}
type rcUpdateTest struct {
old api.ReplicationController
update api.ReplicationController
}
successCases := []rcUpdateTest{
{
old: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Spec,
},
},
update: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Replicas: 3,
Selector: validSelector,
Template: &validPodTemplate.Spec,
},
},
},
{
old: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Spec,
},
},
update: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Replicas: 1,
Selector: validSelector,
Template: &readWriteVolumePodTemplate.Spec,
},
},
},
}
for _, successCase := range successCases {
if errs := ValidateReplicationControllerUpdate(&successCase.old, &successCase.update); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
}
errorCases := map[string]rcUpdateTest{
"more than one read/write": {
old: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Spec,
},
},
update: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Replicas: 2,
Selector: validSelector,
Template: &readWriteVolumePodTemplate.Spec,
},
},
},
"invalid selector": {
old: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Spec,
},
},
update: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Replicas: 2,
Selector: invalidSelector,
Template: &validPodTemplate.Spec,
},
},
},
"invalid pod": {
old: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Spec,
},
},
update: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Replicas: 2,
Selector: validSelector,
Template: &invalidPodTemplate.Spec,
},
},
},
"negative replicas": {
old: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Spec,
},
},
update: api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Replicas: -1,
Selector: validSelector,
Template: &validPodTemplate.Spec,
},
},
},
}
for testName, errorCase := range errorCases {
if errs := ValidateReplicationControllerUpdate(&errorCase.old, &errorCase.update); len(errs) == 0 {
t.Errorf("expected failure: %s", testName)
}
}

}

func TestValidateReplicationController(t *testing.T) {
validSelector := map[string]string{"a": "b"}
validPodTemplate := api.PodTemplate{
Expand All @@ -1413,8 +1573,11 @@ func TestValidateReplicationController(t *testing.T) {
},
},
}
invalidVolumePodTemplate := api.PodTemplate{
readWriteVolumePodTemplate := api.PodTemplate{
Spec: api.PodTemplateSpec{
ObjectMeta: api.ObjectMeta{
Labels: validSelector,
},
Spec: api.PodSpec{
Volumes: []api.Volume{{Name: "gcepd", Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{"my-PD", "ext4", 1, false}}}},
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
Expand Down Expand Up @@ -1449,6 +1612,14 @@ func TestValidateReplicationController(t *testing.T) {
Template: &validPodTemplate.Spec,
},
},
{
ObjectMeta: api.ObjectMeta{Name: "abc-123", Namespace: api.NamespaceDefault},
Spec: api.ReplicationControllerSpec{
Replicas: 1,
Selector: validSelector,
Template: &readWriteVolumePodTemplate.Spec,
},
},
}
for _, successCase := range successCases {
if errs := ValidateReplicationController(&successCase); len(errs) != 0 {
Expand Down Expand Up @@ -1490,11 +1661,12 @@ func TestValidateReplicationController(t *testing.T) {
Selector: validSelector,
},
},
"read-write persistent disk": {
"read-write persistent disk with > 1 pod": {
ObjectMeta: api.ObjectMeta{Name: "abc"},
Spec: api.ReplicationControllerSpec{
Replicas: 2,
Selector: validSelector,
Template: &invalidVolumePodTemplate.Spec,
Template: &readWriteVolumePodTemplate.Spec,
},
},
"negative_replicas": {
Expand Down