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.

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

Make VolumeSource not be a pointer #3661

Merged
merged 1 commit into from
Jan 21, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions pkg/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/v1beta2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/v1beta3/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 5 additions & 9 deletions pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions pkg/api/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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}}}},
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/config/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
},
Expand All @@ -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"},
},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/kubelet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ func TestMountExternalVolumes(t *testing.T) {
Volumes: []api.Volume{
{
Name: "vol1",
Source: &api.VolumeSource{},
Source: api.VolumeSource{},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/volume/empty_dir/empty_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 6 additions & 7 deletions pkg/kubelet/volume/empty_dir/empty_dir_test.go
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.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")
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/volume/gce_pd/gce_pd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/kubelet/volume/gce_pd/gce_pd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/volume/git_repo/git_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/kubelet/volume/git_repo/git_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/volume/host_path/host_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pkg/kubelet/volume/host_path/host_path_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/scheduler/predicates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand All @@ -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",
},
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/cluster_dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestClusterDNS(c *client.Client) bool {
Volumes: []api.Volume{
{
Name: "results",
Source: &api.VolumeSource{
Source: api.VolumeSource{
EmptyDir: &api.EmptyDir{},
},
},
Expand Down