Skip to content

Commit

Permalink
Merge pull request kubernetes#5001 from brendandburns/api3
Browse files Browse the repository at this point in the history 
Embed VolumeSource in v1beta3 and internal.
  • Loading branch information
@thockin
thockin committed Mar 4, 2015
2 parents 3891ad1 + fb90b56 commit fdea725
Show file tree
Hide file tree
Showing 29 changed files with 122 additions and 96 deletions.
3 changes: 1 addition & 2 deletions examples/cassandra/v1beta3/cassandra-controller.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,5 @@ spec:
name: data
volumes:
- name: data
source:
emptyDir: {}
emptyDir: {}

10 changes: 4 additions & 6 deletions examples/mysql-wordpress-pd/v1beta3/mysql.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ spec:
mountPath: /var/lib/mysql
volumes:
- name: mysql-persistent-storage
source:
# emptyDir: {}
persistentDisk:
# This GCE PD must already exist.
pdName: mysql-disk
fsType: ext4
persistentDisk:
# This GCE PD must already exist.
pdName: mysql-disk
fsType: ext4

10 changes: 4 additions & 6 deletions examples/mysql-wordpress-pd/v1beta3/wordpress.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ spec:
mountPath: /var/www/html
volumes:
- name: wordpress-persistent-storage
source:
# emptyDir: {}
persistentDisk:
# This GCE PD must already exist.
pdName: wordpress-disk
fsType: ext4
persistentDisk:
# This GCE PD must already exist.
pdName: wordpress-disk
fsType: ext4

4 changes: 2 additions & 2 deletions pkg/api/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ type Volume struct {
// Required: This must be a DNS_LABEL. Each volume in a pod must have
// a unique name.
Name string `json:"name"`
// Source represents the location and type of a volume to mount.
// The VolumeSource represents the location and type of a volume to mount.
// This is optional for now. If not specified, the Volume is implied to be an EmptyDir.
// This implied behavior is deprecated and will be removed in a future version.
Source VolumeSource `json:"source,omitempty"`
VolumeSource `json:"inline,omitempty"`
}

// VolumeSource represents the source location of a volume to mount.
Expand Down
15 changes: 15 additions & 0 deletions pkg/api/v1beta1/conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,21 @@ func init() {
return nil
},

func(in *newer.Volume, out *Volume, s conversion.Scope) error {
if err := s.Convert(&in.VolumeSource, &out.Source, 0); err != nil {
return err
}
out.Name = in.Name
return nil
},
func(in *Volume, out *newer.Volume, s conversion.Scope) error {
if err := s.Convert(&in.Source, &out.VolumeSource, 0); err != nil {
return err
}
out.Name = in.Name
return nil
},

// VolumeSource's HostDir is deprecated in favor of HostPath.
// TODO: It would be great if I could just map field names to
// convert or else maybe say "convert all members of this
Expand Down
16 changes: 16 additions & 0 deletions pkg/api/v1beta2/conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,22 @@ func init() {
}
return nil
},

func(in *newer.Volume, out *Volume, s conversion.Scope) error {
if err := s.Convert(&in.VolumeSource, &out.Source, 0); err != nil {
return err
}
out.Name = in.Name
return nil
},
func(in *Volume, out *newer.Volume, s conversion.Scope) error {
if err := s.Convert(&in.Source, &out.VolumeSource, 0); err != nil {
return err
}
out.Name = in.Name
return nil
},

func(in *newer.VolumeSource, out *VolumeSource, s conversion.Scope) error {
if err := s.Convert(&in.EmptyDir, &out.EmptyDir, 0); err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/v1beta3/defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import (
func init() {
api.Scheme.AddDefaultingFuncs(
func(obj *Volume) {
if util.AllPtrFieldsNil(&obj.Source) {
obj.Source = VolumeSource{
if util.AllPtrFieldsNil(&obj.VolumeSource) {
obj.VolumeSource = VolumeSource{
EmptyDir: &EmptyDirVolumeSource{},
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/v1beta3/defaults_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestSetDefaulPodSpec(t *testing.T) {
if policy.Never != nil || policy.OnFailure != nil || policy.Always == nil {
t.Errorf("Expected only policy.Always is set, got: %s", policy)
}
vsource := bp2.Spec.Volumes[0].Source
vsource := bp2.Spec.Volumes[0].VolumeSource
if vsource.EmptyDir == nil {
t.Errorf("Expected non-empty volume is set, got: %s", vsource.EmptyDir)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/v1beta3/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ type Volume struct {
// Source represents the location and type of a volume to mount.
// This is optional for now. If not specified, the Volume is implied to be an EmptyDir.
// This implied behavior is deprecated and will be removed in a future version.
Source VolumeSource `json:"source,omitempty"`
VolumeSource `json:"inline,omitempty"`
}

// VolumeSource represents the source location of a valume to mount.
Expand Down
6 changes: 3 additions & 3 deletions pkg/api/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func validateVolumes(volumes []api.Volume) (util.StringSet, errs.ValidationError

allNames := util.StringSet{}
for i, vol := range volumes {
el := validateSource(&vol.Source).Prefix("source")
el := validateSource(&vol.VolumeSource).Prefix("source")
if len(vol.Name) == 0 {
el = append(el, errs.NewFieldRequired("name", vol.Name))
} else if !util.IsDNSLabel(vol.Name) {
Expand Down Expand Up @@ -793,8 +793,8 @@ func ValidatePodTemplateSpec(spec *api.PodTemplateSpec, replicas int) errs.Valid
func ValidateReadOnlyPersistentDisks(volumes []api.Volume) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
for _, vol := range volumes {
if vol.Source.GCEPersistentDisk != nil {
if vol.Source.GCEPersistentDisk.ReadOnly == false {
if vol.GCEPersistentDisk != nil {
if vol.GCEPersistentDisk.ReadOnly == false {
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
44 changes: 22 additions & 22 deletions pkg/api/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,13 @@ func TestValidateAnnotations(t *testing.T) {

func TestValidateVolumes(t *testing.T) {
successCase := []api.Volume{
{Name: "abc", Source: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/path1"}}},
{Name: "123", Source: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/path2"}}},
{Name: "abc-123", Source: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/path3"}}},
{Name: "empty", Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
{Name: "gcepd", Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{"my-PD", "ext4", 1, false}}},
{Name: "gitrepo", Source: api.VolumeSource{GitRepo: &api.GitRepoVolumeSource{"my-repo", "hashstring"}}},
{Name: "secret", Source: api.VolumeSource{Secret: &api.SecretVolumeSource{api.ObjectReference{Namespace: api.NamespaceDefault, Name: "my-secret", Kind: "Secret"}}}},
{Name: "abc", VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/path1"}}},
{Name: "123", VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/path2"}}},
{Name: "abc-123", VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/path3"}}},
{Name: "empty", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
{Name: "gcepd", VolumeSource: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{"my-PD", "ext4", 1, false}}},
{Name: "gitrepo", VolumeSource: api.VolumeSource{GitRepo: &api.GitRepoVolumeSource{"my-repo", "hashstring"}}},
{Name: "secret", VolumeSource: api.VolumeSource{Secret: &api.SecretVolumeSource{api.ObjectReference{Namespace: api.NamespaceDefault, Name: "my-secret", Kind: "Secret"}}}},
}
names, errs := validateVolumes(successCase)
if len(errs) != 0 {
Expand All @@ -168,10 +168,10 @@ func TestValidateVolumes(t *testing.T) {
T errors.ValidationErrorType
F string
}{
"zero-length name": {[]api.Volume{{Name: "", Source: emptyVS}}, errors.ValidationErrorTypeRequired, "[0].name"},
"name > 63 characters": {[]api.Volume{{Name: strings.Repeat("a", 64), Source: emptyVS}}, errors.ValidationErrorTypeInvalid, "[0].name"},
"name not a DNS label": {[]api.Volume{{Name: "a.b.c", Source: emptyVS}}, errors.ValidationErrorTypeInvalid, "[0].name"},
"name not unique": {[]api.Volume{{Name: "abc", Source: emptyVS}, {Name: "abc", Source: emptyVS}}, errors.ValidationErrorTypeDuplicate, "[1].name"},
"zero-length name": {[]api.Volume{{Name: "", VolumeSource: emptyVS}}, errors.ValidationErrorTypeRequired, "[0].name"},
"name > 63 characters": {[]api.Volume{{Name: strings.Repeat("a", 64), VolumeSource: emptyVS}}, errors.ValidationErrorTypeInvalid, "[0].name"},
"name not a DNS label": {[]api.Volume{{Name: "a.b.c", VolumeSource: emptyVS}}, errors.ValidationErrorTypeInvalid, "[0].name"},
"name not unique": {[]api.Volume{{Name: "abc", VolumeSource: emptyVS}, {Name: "abc", VolumeSource: emptyVS}}, errors.ValidationErrorTypeDuplicate, "[1].name"},
}
for k, v := range errorCases {
_, errs := validateVolumes(v.V)
Expand Down Expand Up @@ -648,8 +648,8 @@ func TestValidateManifest(t *testing.T) {
{
Version: "v1beta1",
ID: "abc",
Volumes: []api.Volume{{Name: "vol1", Source: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/vol1"}}},
{Name: "vol2", Source: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/vol2"}}}},
Volumes: []api.Volume{{Name: "vol1", VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/vol1"}}},
{Name: "vol2", VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{"/mnt/vol2"}}}},
Containers: []api.Container{
{
Name: "abc",
Expand Down Expand Up @@ -699,7 +699,7 @@ func TestValidateManifest(t *testing.T) {
"invalid volume name": {
Version: "v1beta1",
ID: "abc",
Volumes: []api.Volume{{Name: "vol.1", Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}},
Volumes: []api.Volume{{Name: "vol.1", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}},
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
DNSPolicy: api.DNSClusterFirst,
},
Expand All @@ -722,14 +722,14 @@ func TestValidateManifest(t *testing.T) {
func TestValidatePodSpec(t *testing.T) {
successCases := []api.PodSpec{
{ // Populate basic fields, leave defaults for most.
Volumes: []api.Volume{{Name: "vol", Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}},
Volumes: []api.Volume{{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}},
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
DNSPolicy: api.DNSClusterFirst,
},
{ // Populate all fields.
Volumes: []api.Volume{
{Name: "vol", Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
},
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
Expand Down Expand Up @@ -778,7 +778,7 @@ func TestValidatePod(t *testing.T) {
{ // Basic fields.
ObjectMeta: api.ObjectMeta{Name: "123", Namespace: "ns"},
Spec: api.PodSpec{
Volumes: []api.Volume{{Name: "vol", Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}},
Volumes: []api.Volume{{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}},
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
DNSPolicy: api.DNSClusterFirst,
Expand All @@ -788,7 +788,7 @@ func TestValidatePod(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "abc.123.do-re-mi", Namespace: "ns"},
Spec: api.PodSpec{
Volumes: []api.Volume{
{Name: "vol", Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
},
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
Expand Down Expand Up @@ -1080,7 +1080,7 @@ func TestValidateBoundPods(t *testing.T) {
{ // Basic fields.
ObjectMeta: api.ObjectMeta{Name: "123", Namespace: "ns"},
Spec: api.PodSpec{
Volumes: []api.Volume{{Name: "vol", Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}},
Volumes: []api.Volume{{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}},
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
DNSPolicy: api.DNSClusterFirst,
Expand All @@ -1090,7 +1090,7 @@ func TestValidateBoundPods(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "abc.123.do-re-mi", Namespace: "ns"},
Spec: api.PodSpec{
Volumes: []api.Volume{
{Name: "vol", Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
},
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
Expand Down Expand Up @@ -1552,7 +1552,7 @@ func TestValidateReplicationControllerUpdate(t *testing.T) {
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
DNSPolicy: api.DNSClusterFirst,
Volumes: []api.Volume{{Name: "gcepd", Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{"my-PD", "ext4", 1, false}}}},
Volumes: []api.Volume{{Name: "gcepd", VolumeSource: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{"my-PD", "ext4", 1, false}}}},
},
},
}
Expand Down Expand Up @@ -1710,7 +1710,7 @@ func TestValidateReplicationController(t *testing.T) {
Labels: validSelector,
},
Spec: api.PodSpec{
Volumes: []api.Volume{{Name: "gcepd", Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{"my-PD", "ext4", 1, false}}}},
Volumes: []api.Volume{{Name: "gcepd", VolumeSource: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{"my-PD", "ext4", 1, false}}}},
RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
DNSPolicy: api.DNSClusterFirst,
},
Expand Down
8 changes: 4 additions & 4 deletions pkg/kubectl/cmd/util/helpers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ func TestMerge(t *testing.T) {
Spec: api.PodSpec{
Volumes: []api.Volume{
{
Name: "v1",
Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}},
Name: "v1",
VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}},
},
{
Name: "v2",
Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}},
Name: "v2",
VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}},
},
},
RestartPolicy: api.RestartPolicy{
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/config/file_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func ExampleManifestAndPod(id string) (v1beta1.ContainerManifest, api.BoundPod)
Volumes: []api.Volume{
{
Name: "host-dir",
Source: api.VolumeSource{
VolumeSource: api.VolumeSource{
HostPath: &api.HostPathVolumeSource{"/dir/path"},
},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/kubelet_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1018,8 +1018,8 @@ func TestMountExternalVolumes(t *testing.T) {
Spec: api.PodSpec{
Volumes: []api.Volume{
{
Name: "vol1",
Source: api.VolumeSource{},
Name: "vol1",
VolumeSource: api.VolumeSource{},
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/volume/empty_dir/empty_dir.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ func (plugin *emptyDirPlugin) CanSupport(spec *api.Volume) bool {
return false
}

if util.AllPtrFieldsNil(&spec.Source) {
if util.AllPtrFieldsNil(&spec.VolumeSource) {
return true
}
if spec.Source.EmptyDir != nil {
if spec.EmptyDir != nil {
return true
}
return false
Expand Down
12 changes: 6 additions & 6 deletions pkg/kubelet/volume/empty_dir/empty_dir_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ func TestCanSupport(t *testing.T) {
if plug.Name() != "kubernetes.io/empty-dir" {
t.Errorf("Wrong name: %s", plug.Name())
}
if !plug.CanSupport(&api.Volume{Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}) {
if !plug.CanSupport(&api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}) {
t.Errorf("Expected true")
}
if !plug.CanSupport(&api.Volume{Source: api.VolumeSource{}}) {
if !plug.CanSupport(&api.Volume{VolumeSource: api.VolumeSource{}}) {
t.Errorf("Expected true")
}
}
Expand All @@ -53,8 +53,8 @@ func TestPlugin(t *testing.T) {
t.Errorf("Can't find the plugin by name")
}
spec := &api.Volume{
Name: "vol1",
Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}},
Name: "vol1",
VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}},
}
builder, err := plug.NewBuilder(spec, types.UID("poduid"))
if err != nil {
Expand Down Expand Up @@ -134,11 +134,11 @@ func TestPluginLegacy(t *testing.T) {
if plug.Name() != "empty" {
t.Errorf("Wrong name: %s", plug.Name())
}
if plug.CanSupport(&api.Volume{Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}) {
if plug.CanSupport(&api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}) {
t.Errorf("Expected false")
}

if _, err := plug.NewBuilder(&api.Volume{Source: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}, types.UID("poduid")); err == nil {
if _, err := plug.NewBuilder(&api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}, types.UID("poduid")); err == nil {
t.Errorf("Expected failiure")
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/kubelet/volume/gce_pd/gce_pd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (plugin *gcePersistentDiskPlugin) CanSupport(spec *api.Volume) bool {
return false
}

if spec.Source.GCEPersistentDisk != nil {
if spec.GCEPersistentDisk != nil {
return true
}
return false
Expand All @@ -81,13 +81,13 @@ func (plugin *gcePersistentDiskPlugin) newBuilderInternal(spec *api.Volume, podU
return nil, fmt.Errorf("legacy mode: can not create new instances")
}

pdName := spec.Source.GCEPersistentDisk.PDName
fsType := spec.Source.GCEPersistentDisk.FSType
pdName := spec.GCEPersistentDisk.PDName
fsType := spec.GCEPersistentDisk.FSType
partition := ""
if spec.Source.GCEPersistentDisk.Partition != 0 {
partition = strconv.Itoa(spec.Source.GCEPersistentDisk.Partition)
if spec.GCEPersistentDisk.Partition != 0 {
partition = strconv.Itoa(spec.GCEPersistentDisk.Partition)
}
readOnly := spec.Source.GCEPersistentDisk.ReadOnly
readOnly := spec.GCEPersistentDisk.ReadOnly

return &gcePersistentDisk{
podUID: podUID,
Expand Down
Loading

0 comments on commit fdea725

Please sign in to comment.