Make VolumeSource not be a pointer

There's no reason for it to be a pointer.
duda · Jan 21, 2015 · 819803b · 819803b
1 parent a9cbc58
commit 819803b
Show file tree

Hide file tree

Showing 18 changed files with 45 additions and 50 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -68,7 +68,7 @@ func TestClusterDNS(c *client.Client) bool {
 			Volumes: []api.Volume{
 				{
 					Name: "results",
-					Source: &api.VolumeSource{
+					Source: api.VolumeSource{
 						EmptyDir: &api.EmptyDir{},
 					},
 				},