diff --git a/docs/volumes.md b/docs/volumes.md index 59c04636829..7bbb6b23f31 100644 --- a/docs/volumes.md +++ b/docs/volumes.md @@ -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. diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 1a9c4e85532..af47e3e0ad9 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -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 } @@ -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 @@ -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 } @@ -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.")) } } } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index df0ab23adf2..3e680f0f905 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -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{ @@ -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{}}, @@ -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 { @@ -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": {