Skip to content

Commit

Permalink
Invert api and api/v1beta1 dependencies
Browse files Browse the repository at this point in the history 
This is some cleanup that has been needed for a while.
There's still one more step that could usefully be done, which is to
split up our api package into the part that provides the helper
functions and the part that provides the internal types. That can come
later.

The v1beta1 package is now a good example of what an api plugin should
do to version its types.
  • Loading branch information
@lavalamp
lavalamp committed Aug 29, 2014
1 parent d13e59c commit aa9b9b9
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 82 deletions.
1 change: 1 addition & 0 deletions examples/examples_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"testing"

"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
_ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
"github.com/golang/glog"
)

Expand Down
17 changes: 0 additions & 17 deletions pkg/api/helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"fmt"
"reflect"

"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
"gopkg.in/v1/yaml"
)
Expand Down Expand Up @@ -65,22 +64,6 @@ func init() {
Endpoints{},
Binding{},
)
AddKnownTypes("v1beta1",
v1beta1.PodList{},
v1beta1.Pod{},
v1beta1.ReplicationControllerList{},
v1beta1.ReplicationController{},
v1beta1.ServiceList{},
v1beta1.Service{},
v1beta1.MinionList{},
v1beta1.Minion{},
v1beta1.Status{},
v1beta1.ServerOpList{},
v1beta1.ServerOp{},
v1beta1.ContainerManifestList{},
v1beta1.Endpoints{},
v1beta1.Binding{},
)

Codec = conversionScheme
ResourceVersioner = NewJSONBaseResourceVersioner()
Expand Down
63 changes: 32 additions & 31 deletions pkg/api/helper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package api
package api_test

import (
"encoding/json"
Expand All @@ -23,6 +23,8 @@ import (
"reflect"
"testing"

"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
_ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/fsouza/go-dockerclient"
"github.com/google/gofuzz"
Expand All @@ -32,7 +34,7 @@ var fuzzIters = flag.Int("fuzz_iters", 50, "How many fuzzing iterations to do.")

// apiObjectFuzzer can randomly populate api objects.
var apiObjectFuzzer = fuzz.New().NilChance(.5).NumElements(1, 1).Funcs(
func(j *JSONBase, c fuzz.Continue) {
func(j *api.JSONBase, c fuzz.Continue) {
// We have to customize the randomization of JSONBases because their
// APIVersion and Kind must remain blank in memory.
j.APIVersion = ""
Expand Down Expand Up @@ -105,20 +107,20 @@ func objDiff(a, b interface{}) string {
func runTest(t *testing.T, source interface{}) {
name := reflect.TypeOf(source).Elem().Name()
apiObjectFuzzer.Fuzz(source)
j, err := FindJSONBase(source)
j, err := api.FindJSONBase(source)
if err != nil {
t.Fatalf("Unexpected error %v for %#v", err, source)
}
j.SetKind("")
j.SetAPIVersion("")

data, err := Encode(source)
data, err := api.Encode(source)
if err != nil {
t.Errorf("%v: %v (%#v)", name, err, source)
return
}

obj2, err := Decode(data)
obj2, err := api.Decode(data)
if err != nil {
t.Errorf("%v: %v", name, err)
return
Expand All @@ -129,7 +131,7 @@ func runTest(t *testing.T, source interface{}) {
}
}
obj3 := reflect.New(reflect.TypeOf(source).Elem()).Interface()
err = DecodeInto(data, obj3)
err = api.DecodeInto(data, obj3)
if err != nil {
t.Errorf("2: %v: %v", name, err)
return
Expand All @@ -142,22 +144,21 @@ func runTest(t *testing.T, source interface{}) {
}

func TestTypes(t *testing.T) {
// TODO: auto-fill all fields.
table := []interface{}{
&PodList{},
&Pod{},
&ServiceList{},
&Service{},
&ReplicationControllerList{},
&ReplicationController{},
&MinionList{},
&Minion{},
&Status{},
&ServerOpList{},
&ServerOp{},
&ContainerManifestList{},
&Endpoints{},
&Binding{},
&api.PodList{},
&api.Pod{},
&api.ServiceList{},
&api.Service{},
&api.ReplicationControllerList{},
&api.ReplicationController{},
&api.MinionList{},
&api.Minion{},
&api.Status{},
&api.ServerOpList{},
&api.ServerOp{},
&api.ContainerManifestList{},
&api.Endpoints{},
&api.Binding{},
}
for _, item := range table {
// Try a few times, since runTest uses random values.
Expand All @@ -168,16 +169,16 @@ func TestTypes(t *testing.T) {
}

func TestEncode_NonPtr(t *testing.T) {
pod := Pod{
pod := api.Pod{
Labels: map[string]string{"name": "foo"},
}
obj := interface{}(pod)
data, err := Encode(obj)
obj2, err2 := Decode(data)
data, err := api.Encode(obj)
obj2, err2 := api.Decode(data)
if err != nil || err2 != nil {
t.Fatalf("Failure: '%v' '%v'", err, err2)
}
if _, ok := obj2.(*Pod); !ok {
if _, ok := obj2.(*api.Pod); !ok {
t.Fatalf("Got wrong type")
}
if !reflect.DeepEqual(obj2, &pod) {
Expand All @@ -186,16 +187,16 @@ func TestEncode_NonPtr(t *testing.T) {
}

func TestEncode_Ptr(t *testing.T) {
pod := &Pod{
pod := &api.Pod{
Labels: map[string]string{"name": "foo"},
}
obj := interface{}(pod)
data, err := Encode(obj)
obj2, err2 := Decode(data)
data, err := api.Encode(obj)
obj2, err2 := api.Decode(data)
if err != nil || err2 != nil {
t.Fatalf("Failure: '%v' '%v'", err, err2)
}
if _, ok := obj2.(*Pod); !ok {
if _, ok := obj2.(*api.Pod); !ok {
t.Fatalf("Got wrong type")
}
if !reflect.DeepEqual(obj2, pod) {
Expand All @@ -205,11 +206,11 @@ func TestEncode_Ptr(t *testing.T) {

func TestBadJSONRejection(t *testing.T) {
badJSONMissingKind := []byte(`{ }`)
if _, err := Decode(badJSONMissingKind); err == nil {
if _, err := api.Decode(badJSONMissingKind); err == nil {
t.Errorf("Did not reject despite lack of kind field: %s", badJSONMissingKind)
}
badJSONUnknownType := []byte(`{"kind": "bar"}`)
if _, err1 := Decode(badJSONUnknownType); err1 == nil {
if _, err1 := api.Decode(badJSONUnknownType); err1 == nil {
t.Errorf("Did not reject despite use of unknown type: %s", badJSONUnknownType)
}
/*badJSONKindMismatch := []byte(`{"kind": "Pod"}`)
Expand Down
27 changes: 15 additions & 12 deletions pkg/api/conversion.go → pkg/api/v1beta1/conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,28 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package api
package v1beta1

import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
// Alias this so it can be easily changed when we cut the next version.
newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
// Also import under original name for Convert and AddConversionFuncs.
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
)

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(
// Shortcut for sub-conversions. TODO: This should possibly be refactored
// such that this convert function is passed to each conversion func.
Convert := api.Convert
api.AddConversionFuncs(
// EnvVar's Key is deprecated in favor of Name.
func(in *EnvVar, out *v1beta1.EnvVar) error {
func(in *newer.EnvVar, out *EnvVar) error {
out.Value = in.Value
out.Key = in.Name
out.Name = in.Name
return nil
},
func(in *v1beta1.EnvVar, out *EnvVar) error {
func(in *EnvVar, out *newer.EnvVar) error {
out.Value = in.Value
if in.Name != "" {
out.Name = in.Name
Expand All @@ -43,15 +46,15 @@ func init() {
},

// Path & MountType are deprecated.
func(in *VolumeMount, out *v1beta1.VolumeMount) error {
func(in *newer.VolumeMount, out *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 {
func(in *VolumeMount, out *newer.VolumeMount) error {
out.Name = in.Name
out.ReadOnly = in.ReadOnly
if in.MountPath == "" {
Expand All @@ -63,13 +66,13 @@ func init() {
},

// MinionList.Items had a wrong name in v1beta1
func(in *MinionList, out *v1beta1.MinionList) error {
func(in *newer.MinionList, out *MinionList) error {
Convert(&in.JSONBase, &out.JSONBase)
Convert(&in.Items, &out.Items)
out.Minions = out.Items
return nil
},
func(in *v1beta1.MinionList, out *MinionList) error {
func(in *MinionList, out *newer.MinionList) error {
Convert(&in.JSONBase, &out.JSONBase)
if len(in.Items) == 0 {
Convert(&in.Minions, &out.Items)
Expand Down
Loading

0 comments on commit aa9b9b9

Please sign in to comment.