Cleanup: remove deprecated fields the proper way

This commit is contained in:
Daniel Smith 2014-08-26 17:28:36 -07:00
parent 1b24fe880a
commit 2a62d72140
6 changed files with 181 additions and 93 deletions

66
pkg/api/conversion.go Normal file
View File

@ -0,0 +1,66 @@
/*
Copyright 2014 Google Inc. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package api
import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
)
func init() {
// TODO: Consider inverting dependency chain-- imagine v1beta1 package
// registering all of these functions. Then, if you want to be able to understand
// v1beta1 objects, you just import that package for its side effects.
AddConversionFuncs(
// EnvVar's Key is deprecated in favor of Name.
func(in *EnvVar, out *v1beta1.EnvVar) error {
out.Value = in.Value
out.Key = in.Name
out.Name = in.Name
return nil
},
func(in *v1beta1.EnvVar, out *EnvVar) error {
out.Value = in.Value
if in.Name != "" {
out.Name = in.Name
} else {
out.Name = in.Key
}
return nil
},
// Path & MountType are deprecated.
func(in *VolumeMount, out *v1beta1.VolumeMount) error {
out.Name = in.Name
out.ReadOnly = in.ReadOnly
out.MountPath = in.MountPath
out.Path = in.MountPath
out.MountType = "" // MountType is ignored.
return nil
},
func(in *v1beta1.VolumeMount, out *VolumeMount) error {
out.Name = in.Name
out.ReadOnly = in.ReadOnly
if in.MountPath == "" {
out.MountPath = in.Path
} else {
out.MountPath = in.MountPath
}
return nil
},
)
}

112
pkg/api/conversion_test.go Normal file
View File

@ -0,0 +1,112 @@
/*
Copyright 2014 Google Inc. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package api
import (
"reflect"
"testing"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
)
func TestEnvConversion(t *testing.T) {
nonCanonical := []v1beta1.EnvVar{
{Key: "EV"},
{Key: "EV", Name: "EX"},
}
canonical := []EnvVar{
{Name: "EV"},
{Name: "EX"},
}
for i := range nonCanonical {
var got EnvVar
err := Convert(&nonCanonical[i], &got)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if e, a := canonical[i], got; !reflect.DeepEqual(e, a) {
t.Errorf("expected %v, got %v", e, a)
}
}
// Test conversion the other way, too.
for i := range canonical {
var got v1beta1.EnvVar
err := Convert(&canonical[i], &got)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if e, a := canonical[i].Name, got.Key; e != a {
t.Errorf("expected %v, got %v", e, a)
}
if e, a := canonical[i].Name, got.Name; e != a {
t.Errorf("expected %v, got %v", e, a)
}
}
}
func TestVolumeMountConversionToOld(t *testing.T) {
table := []struct {
in VolumeMount
out v1beta1.VolumeMount
}{
{
in: VolumeMount{Name: "foo", MountPath: "/dev/foo", ReadOnly: true},
out: v1beta1.VolumeMount{Name: "foo", MountPath: "/dev/foo", Path: "/dev/foo", ReadOnly: true},
},
}
for _, item := range table {
got := v1beta1.VolumeMount{}
err := Convert(&item.in, &got)
if err != nil {
t.Errorf("Unexpected error: %v", err)
continue
}
if e, a := item.out, got; !reflect.DeepEqual(e, a) {
t.Errorf("Expected: %#v, got %#v", e, a)
}
}
}
func TestVolumeMountConversionToNew(t *testing.T) {
table := []struct {
in v1beta1.VolumeMount
out VolumeMount
}{
{
in: v1beta1.VolumeMount{Name: "foo", MountPath: "/dev/foo", ReadOnly: true},
out: VolumeMount{Name: "foo", MountPath: "/dev/foo", ReadOnly: true},
}, {
in: v1beta1.VolumeMount{Name: "foo", MountPath: "/dev/foo", Path: "/dev/bar", ReadOnly: true},
out: VolumeMount{Name: "foo", MountPath: "/dev/foo", ReadOnly: true},
}, {
in: v1beta1.VolumeMount{Name: "foo", Path: "/dev/bar", ReadOnly: true},
out: VolumeMount{Name: "foo", MountPath: "/dev/bar", ReadOnly: true},
},
}
for _, item := range table {
got := VolumeMount{}
err := Convert(&item.in, &got)
if err != nil {
t.Errorf("Unexpected error: %v", err)
continue
}
if e, a := item.out, got; !reflect.DeepEqual(e, a) {
t.Errorf("Expected: %#v, got %#v", e, a)
}
}
}

View File

@ -43,10 +43,9 @@ type resourceVersioner interface {
var Codec codec
var ResourceVersioner resourceVersioner
var conversionScheme *conversion.Scheme
var conversionScheme = conversion.NewScheme()
func init() {
conversionScheme = conversion.NewScheme()
conversionScheme.InternalVersion = ""
conversionScheme.ExternalVersion = "v1beta1"
conversionScheme.MetaInsertionFactory = metaInsertion{}
@ -83,30 +82,6 @@ func init() {
v1beta1.Binding{},
)
// TODO: when we get more of this stuff, move to its own file. This is not a
// good home for lots of conversion functions.
// TODO: Consider inverting dependency chain-- imagine v1beta1 package
// registering all of these functions. Then, if you want to be able to understand
// v1beta1 objects, you just import that package for its side effects.
AddConversionFuncs(
// EnvVar's Key is deprecated in favor of Name.
func(in *EnvVar, out *v1beta1.EnvVar) error {
out.Value = in.Value
out.Key = in.Name
out.Name = in.Name
return nil
},
func(in *v1beta1.EnvVar, out *EnvVar) error {
out.Value = in.Value
if in.Name != "" {
out.Name = in.Name
} else {
out.Name = in.Key
}
return nil
},
)
Codec = conversionScheme
ResourceVersioner = NewJSONBaseResourceVersioner()
}

View File

@ -116,13 +116,7 @@ type VolumeMount struct {
// Optional: Defaults to false (read-write).
ReadOnly bool `yaml:"readOnly,omitempty" json:"readOnly,omitempty"`
// Required.
// Exactly one of the following must be set. If both are set, prefer MountPath.
// DEPRECATED: Path will be removed in a future version of the API.
MountPath string `yaml:"mountPath,omitempty" json:"mountPath,omitempty"`
Path string `yaml:"path,omitempty" json:"path,omitempty"`
// One of: "LOCAL" (local volume) or "HOST" (external mount from the host). Default: LOCAL.
// DEPRECATED: MountType will be removed in a future version of the API.
MountType string `yaml:"mountType,omitempty" json:"mountType,omitempty"`
}
// EnvVar represents an environment variable present in a Container

View File

@ -22,7 +22,6 @@ import (
errs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/golang/glog"
)
func validateVolumes(volumes []Volume) (util.StringSet, errs.ErrorList) {
@ -142,17 +141,7 @@ func validateVolumeMounts(mounts []VolumeMount, volumes util.StringSet) errs.Err
mErrs = append(mErrs, errs.NewNotFound("name", mnt.Name))
}
if len(mnt.MountPath) == 0 {
// Backwards compat.
if len(mnt.Path) == 0 {
mErrs = append(mErrs, errs.NewRequired("mountPath", mnt.MountPath))
} else {
glog.Warning("DEPRECATED: VolumeMount.Path has been replaced by VolumeMount.MountPath")
mnt.MountPath = mnt.Path
mnt.Path = ""
}
}
if len(mnt.MountType) != 0 {
glog.Warning("DEPRECATED: VolumeMount.MountType will be removed. The Volume struct will handle types")
mErrs = append(mErrs, errs.NewRequired("mountPath", mnt.MountPath))
}
allErrs = append(allErrs, mErrs.PrefixIndex(i)...)
}

View File

@ -17,12 +17,10 @@ limitations under the License.
package api
import (
"reflect"
"strings"
"testing"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
)
@ -154,42 +152,6 @@ func TestValidateEnv(t *testing.T) {
}
}
func TestEnvConversion(t *testing.T) {
nonCanonical := []v1beta1.EnvVar{
{Key: "EV"},
{Key: "EV", Name: "EX"},
}
canonical := []EnvVar{
{Name: "EV"},
{Name: "EX"},
}
for i := range nonCanonical {
var got EnvVar
err := Convert(&nonCanonical[i], &got)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if e, a := canonical[i], got; !reflect.DeepEqual(e, a) {
t.Errorf("expected %v, got %v", e, a)
}
}
// Test conversion the other way, too.
for i := range canonical {
var got v1beta1.EnvVar
err := Convert(&canonical[i], &got)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if e, a := canonical[i].Name, got.Key; e != a {
t.Errorf("expected %v, got %v", e, a)
}
if e, a := canonical[i].Name, got.Name; e != a {
t.Errorf("expected %v, got %v", e, a)
}
}
}
func TestValidateVolumeMounts(t *testing.T) {
volumes := util.NewStringSet("abc", "123", "abc-123")
@ -202,16 +164,6 @@ func TestValidateVolumeMounts(t *testing.T) {
t.Errorf("expected success: %v", errs)
}
nonCanonicalCase := []VolumeMount{
{Name: "abc", Path: "/foo"},
}
if errs := validateVolumeMounts(nonCanonicalCase, volumes); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
if nonCanonicalCase[0].MountPath != "/foo" {
t.Errorf("expected canonicalized values: %+v", nonCanonicalCase[0])
}
errorCases := map[string][]VolumeMount{
"empty name": {{Name: "", MountPath: "/foo"}},
"name not found": {{Name: "", MountPath: "/foo"}},
@ -293,7 +245,7 @@ func TestValidateManifest(t *testing.T) {
},
VolumeMounts: []VolumeMount{
{Name: "vol1", MountPath: "/foo"},
{Name: "vol1", Path: "/bar"},
{Name: "vol1", MountPath: "/bar"},
},
},
},