Merge pull request #3661 from thockin/volume-source-as-non-pointer

Make VolumeSource not be a pointer
This commit is contained in:
Dawn Chen 2015-01-21 12:39:49 -08:00
commit d2ebbcaf90
18 changed files with 45 additions and 50 deletions

View File

@ -145,10 +145,10 @@ type Volume struct {
// Source represents the location and type of a volume to mount. // 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 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. // 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. // Only one of its members may be specified.
type VolumeSource struct { type VolumeSource struct {
// HostPath represents file or directory on the host machine that is // HostPath represents file or directory on the host machine that is

View File

@ -79,7 +79,7 @@ type Volume struct {
// Source represents the location and type of a volume to mount. // 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 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. // 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. // VolumeSource represents the source location of a valume to mount.

View File

@ -53,7 +53,7 @@ type Volume struct {
// Source represents the location and type of a volume to mount. // 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 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. // 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. // VolumeSource represents the source location of a valume to mount.

View File

@ -164,7 +164,7 @@ type Volume struct {
// Source represents the location and type of a volume to mount. // 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 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. // 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 valume to mount.

View File

@ -40,14 +40,7 @@ func validateVolumes(volumes []api.Volume) (util.StringSet, errs.ValidationError
allNames := util.StringSet{} allNames := util.StringSet{}
for i := range volumes { for i := range volumes {
vol := &volumes[i] // so we can set default values vol := &volumes[i] // so we can set default values
el := errs.ValidationErrorList{} el := validateSource(&vol.Source).Prefix("source")
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")
if len(vol.Name) == 0 { if len(vol.Name) == 0 {
el = append(el, errs.NewFieldRequired("name", vol.Name)) el = append(el, errs.NewFieldRequired("name", vol.Name))
} else if !util.IsDNSLabel(vol.Name) { } else if !util.IsDNSLabel(vol.Name) {
@ -83,7 +76,10 @@ func validateSource(source *api.VolumeSource) errs.ValidationErrorList {
numVolumes++ numVolumes++
allErrs = append(allErrs, validateGCEPersistentDisk(source.GCEPersistentDisk).Prefix("persistentDisk")...) 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")) allErrs = append(allErrs, errs.NewFieldInvalid("", source, "exactly 1 volume type is required"))
} }
return allErrs return allErrs

View File

@ -77,11 +77,11 @@ func TestValidateLabels(t *testing.T) {
func TestValidateVolumes(t *testing.T) { func TestValidateVolumes(t *testing.T) {
successCase := []api.Volume{ successCase := []api.Volume{
{Name: "abc"}, {Name: "abc"},
{Name: "123", Source: &api.VolumeSource{HostPath: &api.HostPath{"/mnt/path2"}}}, {Name: "123", Source: api.VolumeSource{HostPath: &api.HostPath{"/mnt/path2"}}},
{Name: "abc-123", Source: &api.VolumeSource{HostPath: &api.HostPath{"/mnt/path3"}}}, {Name: "abc-123", Source: api.VolumeSource{HostPath: &api.HostPath{"/mnt/path3"}}},
{Name: "empty", Source: &api.VolumeSource{EmptyDir: &api.EmptyDir{}}}, {Name: "empty", Source: api.VolumeSource{EmptyDir: &api.EmptyDir{}}},
{Name: "gcepd", Source: &api.VolumeSource{GCEPersistentDisk: &api.GCEPersistentDisk{"my-PD", "ext4", 1, false}}}, {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: "gitrepo", Source: api.VolumeSource{GitRepo: &api.GitRepo{"my-repo", "hashstring"}}},
} }
names, errs := validateVolumes(successCase) names, errs := validateVolumes(successCase)
if len(errs) != 0 { if len(errs) != 0 {
@ -414,8 +414,8 @@ func TestValidateManifest(t *testing.T) {
{ {
Version: "v1beta1", Version: "v1beta1",
ID: "abc", ID: "abc",
Volumes: []api.Volume{{Name: "vol1", Source: &api.VolumeSource{HostPath: &api.HostPath{"/mnt/vol1"}}}, Volumes: []api.Volume{{Name: "vol1", Source: api.VolumeSource{HostPath: &api.HostPath{"/mnt/vol1"}}},
{Name: "vol2", Source: &api.VolumeSource{HostPath: &api.HostPath{"/mnt/vol2"}}}}, {Name: "vol2", Source: api.VolumeSource{HostPath: &api.HostPath{"/mnt/vol2"}}}},
Containers: []api.Container{ Containers: []api.Container{
{ {
Name: "abc", Name: "abc",
@ -1061,7 +1061,7 @@ func TestValidateReplicationController(t *testing.T) {
invalidVolumePodTemplate := api.PodTemplate{ invalidVolumePodTemplate := api.PodTemplate{
Spec: api.PodTemplateSpec{ Spec: api.PodTemplateSpec{
Spec: api.PodSpec{ 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}}}},
}, },
}, },
} }

View File

@ -45,7 +45,7 @@ func ExampleManifestAndPod(id string) (api.ContainerManifest, api.BoundPod) {
Volumes: []api.Volume{ Volumes: []api.Volume{
{ {
Name: "host-dir", Name: "host-dir",
Source: &api.VolumeSource{ Source: api.VolumeSource{
HostPath: &api.HostPath{"/dir/path"}, HostPath: &api.HostPath{"/dir/path"},
}, },
}, },
@ -67,7 +67,7 @@ func ExampleManifestAndPod(id string) (api.ContainerManifest, api.BoundPod) {
Volumes: []api.Volume{ Volumes: []api.Volume{
{ {
Name: "host-dir", Name: "host-dir",
Source: &api.VolumeSource{ Source: api.VolumeSource{
HostPath: &api.HostPath{"/dir/path"}, HostPath: &api.HostPath{"/dir/path"},
}, },
}, },

View File

@ -967,7 +967,7 @@ func TestMountExternalVolumes(t *testing.T) {
Volumes: []api.Volume{ Volumes: []api.Volume{
{ {
Name: "vol1", Name: "vol1",
Source: &api.VolumeSource{}, Source: api.VolumeSource{},
}, },
}, },
}, },

View File

@ -60,7 +60,7 @@ func (plugin *emptyDirPlugin) CanSupport(spec *api.Volume) bool {
return false return false
} }
if spec.Source == nil || util.AllPtrFieldsNil(spec.Source) { if util.AllPtrFieldsNil(&spec.Source) {
return true return true
} }
if spec.Source.EmptyDir != nil { if spec.Source.EmptyDir != nil {

View File

@ -36,10 +36,10 @@ func TestCanSupport(t *testing.T) {
if plug.Name() != "kubernetes.io/empty-dir" { if plug.Name() != "kubernetes.io/empty-dir" {
t.Errorf("Wrong name: %s", plug.Name()) 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") t.Errorf("Expected true")
} }
if !plug.CanSupport(&api.Volume{Source: nil}) { if !plug.CanSupport(&api.Volume{Source: api.VolumeSource{}}) {
t.Errorf("Expected true") t.Errorf("Expected true")
} }
} }
@ -54,7 +54,7 @@ func TestPlugin(t *testing.T) {
} }
spec := &api.Volume{ spec := &api.Volume{
Name: "vol1", Name: "vol1",
Source: &api.VolumeSource{EmptyDir: &api.EmptyDir{}}, Source: api.VolumeSource{EmptyDir: &api.EmptyDir{}},
} }
builder, err := plug.NewBuilder(spec, types.UID("poduid")) builder, err := plug.NewBuilder(spec, types.UID("poduid"))
if err != nil { if err != nil {
@ -107,8 +107,7 @@ func TestPluginBackCompat(t *testing.T) {
t.Errorf("Can't find the plugin by name") t.Errorf("Can't find the plugin by name")
} }
spec := &api.Volume{ spec := &api.Volume{
Name: "vol1", Name: "vol1",
Source: nil,
} }
builder, err := plug.NewBuilder(spec, types.UID("poduid")) builder, err := plug.NewBuilder(spec, types.UID("poduid"))
if err != nil { if err != nil {
@ -135,11 +134,11 @@ func TestPluginLegacy(t *testing.T) {
if plug.Name() != "empty" { if plug.Name() != "empty" {
t.Errorf("Wrong name: %s", plug.Name()) 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") 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") t.Errorf("Expected failiure")
} }

View File

@ -63,7 +63,7 @@ func (plugin *gcePersistentDiskPlugin) CanSupport(spec *api.Volume) bool {
return false return false
} }
if spec.Source != nil && spec.Source.GCEPersistentDisk != nil { if spec.Source.GCEPersistentDisk != nil {
return true return true
} }
return false return false

View File

@ -37,7 +37,7 @@ func TestCanSupport(t *testing.T) {
if plug.Name() != "kubernetes.io/gce-pd" { if plug.Name() != "kubernetes.io/gce-pd" {
t.Errorf("Wrong name: %s", plug.Name()) 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") t.Errorf("Expected true")
} }
} }
@ -88,7 +88,7 @@ func TestPlugin(t *testing.T) {
} }
spec := &api.Volume{ spec := &api.Volume{
Name: "vol1", Name: "vol1",
Source: &api.VolumeSource{ Source: api.VolumeSource{
GCEPersistentDisk: &api.GCEPersistentDisk{ GCEPersistentDisk: &api.GCEPersistentDisk{
PDName: "pd", PDName: "pd",
FSType: "ext4", FSType: "ext4",
@ -155,11 +155,11 @@ func TestPluginLegacy(t *testing.T) {
if plug.Name() != "gce-pd" { if plug.Name() != "gce-pd" {
t.Errorf("Wrong name: %s", plug.Name()) 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") 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") t.Errorf("Expected failiure")
} }

View File

@ -63,7 +63,7 @@ func (plugin *gitRepoPlugin) CanSupport(spec *api.Volume) bool {
return false return false
} }
if spec.Source != nil && spec.Source.GitRepo != nil { if spec.Source.GitRepo != nil {
return true return true
} }
return false return false

View File

@ -49,7 +49,7 @@ func TestCanSupport(t *testing.T) {
if plug.Name() != "kubernetes.io/git-repo" { if plug.Name() != "kubernetes.io/git-repo" {
t.Errorf("Wrong name: %s", plug.Name()) 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") t.Errorf("Expected true")
} }
} }
@ -110,7 +110,7 @@ func TestPlugin(t *testing.T) {
} }
spec := &api.Volume{ spec := &api.Volume{
Name: "vol1", Name: "vol1",
Source: &api.VolumeSource{ Source: api.VolumeSource{
GitRepo: &api.GitRepo{ GitRepo: &api.GitRepo{
Repository: "https://github.com/GoogleCloudPlatform/kubernetes.git", Repository: "https://github.com/GoogleCloudPlatform/kubernetes.git",
Revision: "2a30ce65c5ab586b98916d83385c5983edd353a1", Revision: "2a30ce65c5ab586b98916d83385c5983edd353a1",
@ -168,11 +168,11 @@ func TestPluginLegacy(t *testing.T) {
if plug.Name() != "git" { if plug.Name() != "git" {
t.Errorf("Wrong name: %s", plug.Name()) 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") 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") t.Errorf("Expected failiure")
} }

View File

@ -46,7 +46,7 @@ func (plugin *hostPathPlugin) Name() string {
} }
func (plugin *hostPathPlugin) CanSupport(spec *api.Volume) bool { func (plugin *hostPathPlugin) CanSupport(spec *api.Volume) bool {
if spec.Source != nil && spec.Source.HostPath != nil { if spec.Source.HostPath != nil {
return true return true
} }
return false return false

View File

@ -35,10 +35,10 @@ func TestCanSupport(t *testing.T) {
if plug.Name() != "kubernetes.io/host-path" { if plug.Name() != "kubernetes.io/host-path" {
t.Errorf("Wrong name: %s", plug.Name()) 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") t.Errorf("Expected true")
} }
if plug.CanSupport(&api.Volume{Source: nil}) { if plug.CanSupport(&api.Volume{Source: api.VolumeSource{}}) {
t.Errorf("Expected false") t.Errorf("Expected false")
} }
} }
@ -53,7 +53,7 @@ func TestPlugin(t *testing.T) {
} }
spec := &api.Volume{ spec := &api.Volume{
Name: "vol1", Name: "vol1",
Source: &api.VolumeSource{HostPath: &api.HostPath{"/vol1"}}, Source: api.VolumeSource{HostPath: &api.HostPath{"/vol1"}},
} }
builder, err := plug.NewBuilder(spec, types.UID("poduid")) builder, err := plug.NewBuilder(spec, types.UID("poduid"))
if err != nil { if err != nil {

View File

@ -272,7 +272,7 @@ func TestDiskConflicts(t *testing.T) {
volState := api.PodSpec{ volState := api.PodSpec{
Volumes: []api.Volume{ Volumes: []api.Volume{
{ {
Source: &api.VolumeSource{ Source: api.VolumeSource{
GCEPersistentDisk: &api.GCEPersistentDisk{ GCEPersistentDisk: &api.GCEPersistentDisk{
PDName: "foo", PDName: "foo",
}, },
@ -283,7 +283,7 @@ func TestDiskConflicts(t *testing.T) {
volState2 := api.PodSpec{ volState2 := api.PodSpec{
Volumes: []api.Volume{ Volumes: []api.Volume{
{ {
Source: &api.VolumeSource{ Source: api.VolumeSource{
GCEPersistentDisk: &api.GCEPersistentDisk{ GCEPersistentDisk: &api.GCEPersistentDisk{
PDName: "bar", PDName: "bar",
}, },

View File

@ -73,7 +73,7 @@ func TestClusterDNS(c *client.Client) bool {
Volumes: []api.Volume{ Volumes: []api.Volume{
{ {
Name: "results", Name: "results",
Source: &api.VolumeSource{ Source: api.VolumeSource{
EmptyDir: &api.EmptyDir{}, EmptyDir: &api.EmptyDir{},
}, },
}, },