diff --git a/pkg/api/types.go b/pkg/api/types.go index af4feac3f39..745c7d986ff 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -145,10 +145,10 @@ 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"` + Source VolumeSource `json:"source,omitempty"` } -// VolumeSource represents the source location of a valume to mount. +// VolumeSource represents the source location of a volume to mount. // Only one of its members may be specified. type VolumeSource struct { // HostPath represents file or directory on the host machine that is diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 3a589ebc0ea..1f11c12bbd7 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -79,7 +79,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" description:"location and type of volume to mount; at most one of HostDir, EmptyDir, GCEPersistentDisk, or GitRepo; default is EmptyDir"` + Source VolumeSource `json:"source,omitempty" description:"location and type of volume to mount; at most one of HostDir, EmptyDir, GCEPersistentDisk, or GitRepo; default is EmptyDir"` } // VolumeSource represents the source location of a valume to mount. diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 55cc457b73f..3f1ef0bbacb 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -53,7 +53,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" description:"location and type of volume to mount; at most one of HostDir, EmptyDir, GCEPersistentDisk, or GitRepo; default is EmptyDir"` + Source VolumeSource `json:"source,omitempty" description:"location and type of volume to mount; at most one of HostDir, EmptyDir, GCEPersistentDisk, or GitRepo; default is EmptyDir"` } // VolumeSource represents the source location of a valume to mount. diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index e2322f2b8ac..67972e64d1a 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -164,7 +164,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"` + Source VolumeSource `json:"source,omitempty"` } // VolumeSource represents the source location of a valume to mount. diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 1ac27118f64..4386f7edeaa 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -40,14 +40,7 @@ func validateVolumes(volumes []api.Volume) (util.StringSet, errs.ValidationError allNames := util.StringSet{} for i := range volumes { vol := &volumes[i] // so we can set default values - el := errs.ValidationErrorList{} - if vol.Source == nil { - // TODO: Enforce that a source is set once we deprecate the implied form. - vol.Source = &api.VolumeSource{ - EmptyDir: &api.EmptyDir{}, - } - } - el = validateSource(vol.Source).Prefix("source") + el := validateSource(&vol.Source).Prefix("source") if len(vol.Name) == 0 { el = append(el, errs.NewFieldRequired("name", vol.Name)) } else if !util.IsDNSLabel(vol.Name) { @@ -83,7 +76,10 @@ func validateSource(source *api.VolumeSource) errs.ValidationErrorList { numVolumes++ allErrs = append(allErrs, validateGCEPersistentDisk(source.GCEPersistentDisk).Prefix("persistentDisk")...) } - if numVolumes != 1 { + if numVolumes == 0 { + // TODO: Enforce that a source is set once we deprecate the implied form. + source.EmptyDir = &api.EmptyDir{} + } else if numVolumes != 1 { allErrs = append(allErrs, errs.NewFieldInvalid("", source, "exactly 1 volume type is required")) } return allErrs diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 537a5e9aa41..8a53f5a6723 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -77,11 +77,11 @@ func TestValidateLabels(t *testing.T) { func TestValidateVolumes(t *testing.T) { successCase := []api.Volume{ {Name: "abc"}, - {Name: "123", Source: &api.VolumeSource{HostPath: &api.HostPath{"/mnt/path2"}}}, - {Name: "abc-123", Source: &api.VolumeSource{HostPath: &api.HostPath{"/mnt/path3"}}}, - {Name: "empty", Source: &api.VolumeSource{EmptyDir: &api.EmptyDir{}}}, - {Name: "gcepd", Source: &api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{"my-PD", "ext4", 1, false}}}, - {Name: "gitrepo", Source: &api.VolumeSource{GitRepo: &api.GitRepo{"my-repo", "hashstring"}}}, + {Name: "123", Source: api.VolumeSource{HostPath: &api.HostPath{"/mnt/path2"}}}, + {Name: "abc-123", Source: api.VolumeSource{HostPath: &api.HostPath{"/mnt/path3"}}}, + {Name: "empty", Source: api.VolumeSource{EmptyDir: &api.EmptyDir{}}}, + {Name: "gcepd", Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{"my-PD", "ext4", 1, false}}}, + {Name: "gitrepo", Source: api.VolumeSource{GitRepo: &api.GitRepo{"my-repo", "hashstring"}}}, } names, errs := validateVolumes(successCase) if len(errs) != 0 { @@ -414,8 +414,8 @@ func TestValidateManifest(t *testing.T) { { Version: "v1beta1", ID: "abc", - Volumes: []api.Volume{{Name: "vol1", Source: &api.VolumeSource{HostPath: &api.HostPath{"/mnt/vol1"}}}, - {Name: "vol2", Source: &api.VolumeSource{HostPath: &api.HostPath{"/mnt/vol2"}}}}, + Volumes: []api.Volume{{Name: "vol1", Source: api.VolumeSource{HostPath: &api.HostPath{"/mnt/vol1"}}}, + {Name: "vol2", Source: api.VolumeSource{HostPath: &api.HostPath{"/mnt/vol2"}}}}, Containers: []api.Container{ { Name: "abc", @@ -1061,7 +1061,7 @@ func TestValidateReplicationController(t *testing.T) { invalidVolumePodTemplate := api.PodTemplate{ Spec: api.PodTemplateSpec{ Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "gcepd", Source: &api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{"my-PD", "ext4", 1, false}}}}, + Volumes: []api.Volume{{Name: "gcepd", Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{"my-PD", "ext4", 1, false}}}}, }, }, } diff --git a/pkg/kubelet/config/file_test.go b/pkg/kubelet/config/file_test.go index a20f3edaa8d..4716f6d2606 100644 --- a/pkg/kubelet/config/file_test.go +++ b/pkg/kubelet/config/file_test.go @@ -45,7 +45,7 @@ func ExampleManifestAndPod(id string) (api.ContainerManifest, api.BoundPod) { Volumes: []api.Volume{ { Name: "host-dir", - Source: &api.VolumeSource{ + Source: api.VolumeSource{ HostPath: &api.HostPath{"/dir/path"}, }, }, @@ -67,7 +67,7 @@ func ExampleManifestAndPod(id string) (api.ContainerManifest, api.BoundPod) { Volumes: []api.Volume{ { Name: "host-dir", - Source: &api.VolumeSource{ + Source: api.VolumeSource{ HostPath: &api.HostPath{"/dir/path"}, }, }, diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 6e4548cb873..9b32f1304ff 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -967,7 +967,7 @@ func TestMountExternalVolumes(t *testing.T) { Volumes: []api.Volume{ { Name: "vol1", - Source: &api.VolumeSource{}, + Source: api.VolumeSource{}, }, }, }, diff --git a/pkg/kubelet/volume/empty_dir/empty_dir.go b/pkg/kubelet/volume/empty_dir/empty_dir.go index 6d50c18f905..2df9540f054 100644 --- a/pkg/kubelet/volume/empty_dir/empty_dir.go +++ b/pkg/kubelet/volume/empty_dir/empty_dir.go @@ -60,7 +60,7 @@ func (plugin *emptyDirPlugin) CanSupport(spec *api.Volume) bool { return false } - if spec.Source == nil || util.AllPtrFieldsNil(spec.Source) { + if util.AllPtrFieldsNil(&spec.Source) { return true } if spec.Source.EmptyDir != nil { diff --git a/pkg/kubelet/volume/empty_dir/empty_dir_test.go b/pkg/kubelet/volume/empty_dir/empty_dir_test.go index 40f32464e8a..addeb013a35 100644 --- a/pkg/kubelet/volume/empty_dir/empty_dir_test.go +++ b/pkg/kubelet/volume/empty_dir/empty_dir_test.go @@ -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.EmptyDir{}}}) { + if !plug.CanSupport(&api.Volume{Source: api.VolumeSource{EmptyDir: &api.EmptyDir{}}}) { t.Errorf("Expected true") } - if !plug.CanSupport(&api.Volume{Source: nil}) { + if !plug.CanSupport(&api.Volume{Source: api.VolumeSource{}}) { t.Errorf("Expected true") } } @@ -54,7 +54,7 @@ func TestPlugin(t *testing.T) { } spec := &api.Volume{ Name: "vol1", - Source: &api.VolumeSource{EmptyDir: &api.EmptyDir{}}, + Source: api.VolumeSource{EmptyDir: &api.EmptyDir{}}, } builder, err := plug.NewBuilder(spec, types.UID("poduid")) if err != nil { @@ -107,8 +107,7 @@ func TestPluginBackCompat(t *testing.T) { t.Errorf("Can't find the plugin by name") } spec := &api.Volume{ - Name: "vol1", - Source: nil, + Name: "vol1", } builder, err := plug.NewBuilder(spec, types.UID("poduid")) if err != nil { @@ -135,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.EmptyDir{}}}) { + if plug.CanSupport(&api.Volume{Source: api.VolumeSource{EmptyDir: &api.EmptyDir{}}}) { t.Errorf("Expected false") } - if _, err := plug.NewBuilder(&api.Volume{Source: &api.VolumeSource{EmptyDir: &api.EmptyDir{}}}, types.UID("poduid")); err == nil { + if _, err := plug.NewBuilder(&api.Volume{Source: api.VolumeSource{EmptyDir: &api.EmptyDir{}}}, types.UID("poduid")); err == nil { t.Errorf("Expected failiure") } diff --git a/pkg/kubelet/volume/gce_pd/gce_pd.go b/pkg/kubelet/volume/gce_pd/gce_pd.go index 1f47060255a..d93b9ff7291 100644 --- a/pkg/kubelet/volume/gce_pd/gce_pd.go +++ b/pkg/kubelet/volume/gce_pd/gce_pd.go @@ -63,7 +63,7 @@ func (plugin *gcePersistentDiskPlugin) CanSupport(spec *api.Volume) bool { return false } - if spec.Source != nil && spec.Source.GCEPersistentDisk != nil { + if spec.Source.GCEPersistentDisk != nil { return true } return false diff --git a/pkg/kubelet/volume/gce_pd/gce_pd_test.go b/pkg/kubelet/volume/gce_pd/gce_pd_test.go index b7387e333a3..01129bd5fab 100644 --- a/pkg/kubelet/volume/gce_pd/gce_pd_test.go +++ b/pkg/kubelet/volume/gce_pd/gce_pd_test.go @@ -37,7 +37,7 @@ func TestCanSupport(t *testing.T) { if plug.Name() != "kubernetes.io/gce-pd" { t.Errorf("Wrong name: %s", plug.Name()) } - if !plug.CanSupport(&api.Volume{Source: &api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{}}}) { + if !plug.CanSupport(&api.Volume{Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{}}}) { t.Errorf("Expected true") } } @@ -88,7 +88,7 @@ func TestPlugin(t *testing.T) { } spec := &api.Volume{ Name: "vol1", - Source: &api.VolumeSource{ + Source: api.VolumeSource{ GCEPersistentDisk: &api.GCEPersistentDisk{ PDName: "pd", FSType: "ext4", @@ -155,11 +155,11 @@ func TestPluginLegacy(t *testing.T) { if plug.Name() != "gce-pd" { t.Errorf("Wrong name: %s", plug.Name()) } - if plug.CanSupport(&api.Volume{Source: &api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{}}}) { + if plug.CanSupport(&api.Volume{Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{}}}) { t.Errorf("Expected false") } - if _, err := plug.NewBuilder(&api.Volume{Source: &api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{}}}, types.UID("poduid")); err == nil { + if _, err := plug.NewBuilder(&api.Volume{Source: api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{}}}, types.UID("poduid")); err == nil { t.Errorf("Expected failiure") } diff --git a/pkg/kubelet/volume/git_repo/git_repo.go b/pkg/kubelet/volume/git_repo/git_repo.go index 0739c0dce22..1409444ba22 100644 --- a/pkg/kubelet/volume/git_repo/git_repo.go +++ b/pkg/kubelet/volume/git_repo/git_repo.go @@ -63,7 +63,7 @@ func (plugin *gitRepoPlugin) CanSupport(spec *api.Volume) bool { return false } - if spec.Source != nil && spec.Source.GitRepo != nil { + if spec.Source.GitRepo != nil { return true } return false diff --git a/pkg/kubelet/volume/git_repo/git_repo_test.go b/pkg/kubelet/volume/git_repo/git_repo_test.go index 105d937d5c3..f4443bcf9ce 100644 --- a/pkg/kubelet/volume/git_repo/git_repo_test.go +++ b/pkg/kubelet/volume/git_repo/git_repo_test.go @@ -49,7 +49,7 @@ func TestCanSupport(t *testing.T) { if plug.Name() != "kubernetes.io/git-repo" { t.Errorf("Wrong name: %s", plug.Name()) } - if !plug.CanSupport(&api.Volume{Source: &api.VolumeSource{GitRepo: &api.GitRepo{}}}) { + if !plug.CanSupport(&api.Volume{Source: api.VolumeSource{GitRepo: &api.GitRepo{}}}) { t.Errorf("Expected true") } } @@ -110,7 +110,7 @@ func TestPlugin(t *testing.T) { } spec := &api.Volume{ Name: "vol1", - Source: &api.VolumeSource{ + Source: api.VolumeSource{ GitRepo: &api.GitRepo{ Repository: "https://github.com/GoogleCloudPlatform/kubernetes.git", Revision: "2a30ce65c5ab586b98916d83385c5983edd353a1", @@ -168,11 +168,11 @@ func TestPluginLegacy(t *testing.T) { if plug.Name() != "git" { t.Errorf("Wrong name: %s", plug.Name()) } - if plug.CanSupport(&api.Volume{Source: &api.VolumeSource{GitRepo: &api.GitRepo{}}}) { + if plug.CanSupport(&api.Volume{Source: api.VolumeSource{GitRepo: &api.GitRepo{}}}) { t.Errorf("Expected false") } - if _, err := plug.NewBuilder(&api.Volume{Source: &api.VolumeSource{GitRepo: &api.GitRepo{}}}, types.UID("poduid")); err == nil { + if _, err := plug.NewBuilder(&api.Volume{Source: api.VolumeSource{GitRepo: &api.GitRepo{}}}, types.UID("poduid")); err == nil { t.Errorf("Expected failiure") } diff --git a/pkg/kubelet/volume/host_path/host_path.go b/pkg/kubelet/volume/host_path/host_path.go index 176a7b0b157..9e34e3e75b3 100644 --- a/pkg/kubelet/volume/host_path/host_path.go +++ b/pkg/kubelet/volume/host_path/host_path.go @@ -46,7 +46,7 @@ func (plugin *hostPathPlugin) Name() string { } func (plugin *hostPathPlugin) CanSupport(spec *api.Volume) bool { - if spec.Source != nil && spec.Source.HostPath != nil { + if spec.Source.HostPath != nil { return true } return false diff --git a/pkg/kubelet/volume/host_path/host_path_test.go b/pkg/kubelet/volume/host_path/host_path_test.go index 7b7a3903b9c..2914fd46cf5 100644 --- a/pkg/kubelet/volume/host_path/host_path_test.go +++ b/pkg/kubelet/volume/host_path/host_path_test.go @@ -35,10 +35,10 @@ func TestCanSupport(t *testing.T) { if plug.Name() != "kubernetes.io/host-path" { t.Errorf("Wrong name: %s", plug.Name()) } - if !plug.CanSupport(&api.Volume{Source: &api.VolumeSource{HostPath: &api.HostPath{}}}) { + if !plug.CanSupport(&api.Volume{Source: api.VolumeSource{HostPath: &api.HostPath{}}}) { t.Errorf("Expected true") } - if plug.CanSupport(&api.Volume{Source: nil}) { + if plug.CanSupport(&api.Volume{Source: api.VolumeSource{}}) { t.Errorf("Expected false") } } @@ -53,7 +53,7 @@ func TestPlugin(t *testing.T) { } spec := &api.Volume{ Name: "vol1", - Source: &api.VolumeSource{HostPath: &api.HostPath{"/vol1"}}, + Source: api.VolumeSource{HostPath: &api.HostPath{"/vol1"}}, } builder, err := plug.NewBuilder(spec, types.UID("poduid")) if err != nil { diff --git a/pkg/scheduler/predicates_test.go b/pkg/scheduler/predicates_test.go index 5f91ba248f8..9ef6c11b5b8 100644 --- a/pkg/scheduler/predicates_test.go +++ b/pkg/scheduler/predicates_test.go @@ -272,7 +272,7 @@ func TestDiskConflicts(t *testing.T) { volState := api.PodSpec{ Volumes: []api.Volume{ { - Source: &api.VolumeSource{ + Source: api.VolumeSource{ GCEPersistentDisk: &api.GCEPersistentDisk{ PDName: "foo", }, @@ -283,7 +283,7 @@ func TestDiskConflicts(t *testing.T) { volState2 := api.PodSpec{ Volumes: []api.Volume{ { - Source: &api.VolumeSource{ + Source: api.VolumeSource{ GCEPersistentDisk: &api.GCEPersistentDisk{ PDName: "bar", }, diff --git a/test/e2e/cluster_dns.go b/test/e2e/cluster_dns.go index 55ded3a5df8..90e8b5f58b2 100644 --- a/test/e2e/cluster_dns.go +++ b/test/e2e/cluster_dns.go @@ -73,7 +73,7 @@ func TestClusterDNS(c *client.Client) bool { Volumes: []api.Volume{ { Name: "results", - Source: &api.VolumeSource{ + Source: api.VolumeSource{ EmptyDir: &api.EmptyDir{}, }, },