From aa9b9b9fa8ad3425a03fa7406048aabfe1f2b3ed Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 29 Aug 2014 12:15:30 -0700 Subject: [PATCH] Invert api and api/v1beta1 dependencies 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. --- examples/examples_test.go | 1 + pkg/api/helper.go | 17 ------- pkg/api/helper_test.go | 63 ++++++++++++------------ pkg/api/{ => v1beta1}/conversion.go | 27 +++++----- pkg/api/{ => v1beta1}/conversion_test.go | 47 +++++++++--------- pkg/api/v1beta1/register.go | 40 +++++++++++++++ pkg/apiserver/operation_test.go | 2 + pkg/client/client.go | 1 + pkg/kubelet/config/etcd.go | 1 + pkg/master/master.go | 1 + pkg/registry/binding/storage_test.go | 1 + pkg/registry/etcd/etcd_test.go | 1 + pkg/tools/decoder_test.go | 1 + 13 files changed, 121 insertions(+), 82 deletions(-) rename pkg/api/{ => v1beta1}/conversion.go (67%) rename pkg/api/{ => v1beta1}/conversion_test.go (78%) create mode 100644 pkg/api/v1beta1/register.go diff --git a/examples/examples_test.go b/examples/examples_test.go index de7fc721e6c..b7d8cfda0f7 100644 --- a/examples/examples_test.go +++ b/examples/examples_test.go @@ -25,6 +25,7 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/golang/glog" ) diff --git a/pkg/api/helper.go b/pkg/api/helper.go index db85cd32a9f..3484690487b 100644 --- a/pkg/api/helper.go +++ b/pkg/api/helper.go @@ -20,7 +20,6 @@ import ( "fmt" "reflect" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" "gopkg.in/v1/yaml" ) @@ -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() diff --git a/pkg/api/helper_test.go b/pkg/api/helper_test.go index ac0f7680ee7..87883670b54 100644 --- a/pkg/api/helper_test.go +++ b/pkg/api/helper_test.go @@ -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" @@ -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" @@ -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 = "" @@ -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 @@ -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 @@ -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. @@ -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) { @@ -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) { @@ -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"}`) diff --git a/pkg/api/conversion.go b/pkg/api/v1beta1/conversion.go similarity index 67% rename from pkg/api/conversion.go rename to pkg/api/v1beta1/conversion.go index 39dfb653d88..1f5d699362c 100644 --- a/pkg/api/conversion.go +++ b/pkg/api/v1beta1/conversion.go @@ -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 @@ -43,7 +46,7 @@ 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 @@ -51,7 +54,7 @@ func init() { 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 == "" { @@ -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) diff --git a/pkg/api/conversion_test.go b/pkg/api/v1beta1/conversion_test.go similarity index 78% rename from pkg/api/conversion_test.go rename to pkg/api/v1beta1/conversion_test.go index 11e2e71868e..a0f1b80b94e 100644 --- a/pkg/api/conversion_test.go +++ b/pkg/api/v1beta1/conversion_test.go @@ -14,26 +14,29 @@ See the License for the specific language governing permissions and limitations under the License. */ -package api +package v1beta1_test import ( "reflect" "testing" + newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" ) +var Convert = newer.Convert + func TestEnvConversion(t *testing.T) { nonCanonical := []v1beta1.EnvVar{ {Key: "EV"}, {Key: "EV", Name: "EX"}, } - canonical := []EnvVar{ + canonical := []newer.EnvVar{ {Name: "EV"}, {Name: "EX"}, } for i := range nonCanonical { - var got EnvVar + var got newer.EnvVar err := Convert(&nonCanonical[i], &got) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -61,11 +64,11 @@ func TestEnvConversion(t *testing.T) { func TestVolumeMountConversionToOld(t *testing.T) { table := []struct { - in VolumeMount + in newer.VolumeMount out v1beta1.VolumeMount }{ { - in: VolumeMount{Name: "foo", MountPath: "/dev/foo", ReadOnly: true}, + in: newer.VolumeMount{Name: "foo", MountPath: "/dev/foo", ReadOnly: true}, out: v1beta1.VolumeMount{Name: "foo", MountPath: "/dev/foo", Path: "/dev/foo", ReadOnly: true}, }, } @@ -85,21 +88,21 @@ func TestVolumeMountConversionToOld(t *testing.T) { func TestVolumeMountConversionToNew(t *testing.T) { table := []struct { in v1beta1.VolumeMount - out VolumeMount + out newer.VolumeMount }{ { in: v1beta1.VolumeMount{Name: "foo", MountPath: "/dev/foo", ReadOnly: true}, - out: VolumeMount{Name: "foo", MountPath: "/dev/foo", ReadOnly: true}, + out: newer.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}, + out: newer.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}, + out: newer.VolumeMount{Name: "foo", MountPath: "/dev/bar", ReadOnly: true}, }, } for _, item := range table { - got := VolumeMount{} + got := newer.VolumeMount{} err := Convert(&item.in, &got) if err != nil { t.Errorf("Unexpected error: %v", err) @@ -115,39 +118,39 @@ func TestMinionListConversionToNew(t *testing.T) { oldMinion := func(id string) v1beta1.Minion { return v1beta1.Minion{JSONBase: v1beta1.JSONBase{ID: id}} } - newMinion := func(id string) Minion { - return Minion{JSONBase: JSONBase{ID: id}} + newMinion := func(id string) newer.Minion { + return newer.Minion{JSONBase: newer.JSONBase{ID: id}} } oldMinions := []v1beta1.Minion{ oldMinion("foo"), oldMinion("bar"), } - newMinions := []Minion{ + newMinions := []newer.Minion{ newMinion("foo"), newMinion("bar"), } table := []struct { oldML *v1beta1.MinionList - newML *MinionList + newML *newer.MinionList }{ { oldML: &v1beta1.MinionList{Items: oldMinions}, - newML: &MinionList{Items: newMinions}, + newML: &newer.MinionList{Items: newMinions}, }, { oldML: &v1beta1.MinionList{Minions: oldMinions}, - newML: &MinionList{Items: newMinions}, + newML: &newer.MinionList{Items: newMinions}, }, { oldML: &v1beta1.MinionList{ Items: oldMinions, Minions: []v1beta1.Minion{oldMinion("baz")}, }, - newML: &MinionList{Items: newMinions}, + newML: &newer.MinionList{Items: newMinions}, }, } for _, item := range table { - got := &MinionList{} + got := &newer.MinionList{} err := Convert(item.oldML, got) if err != nil { t.Errorf("Unexpected error: %v", err) @@ -162,19 +165,19 @@ func TestMinionListConversionToOld(t *testing.T) { oldMinion := func(id string) v1beta1.Minion { return v1beta1.Minion{JSONBase: v1beta1.JSONBase{ID: id}} } - newMinion := func(id string) Minion { - return Minion{JSONBase: JSONBase{ID: id}} + newMinion := func(id string) newer.Minion { + return newer.Minion{JSONBase: newer.JSONBase{ID: id}} } oldMinions := []v1beta1.Minion{ oldMinion("foo"), oldMinion("bar"), } - newMinions := []Minion{ + newMinions := []newer.Minion{ newMinion("foo"), newMinion("bar"), } - newML := &MinionList{Items: newMinions} + newML := &newer.MinionList{Items: newMinions} oldML := &v1beta1.MinionList{ Items: oldMinions, Minions: oldMinions, diff --git a/pkg/api/v1beta1/register.go b/pkg/api/v1beta1/register.go new file mode 100644 index 00000000000..cd4cb5a2d94 --- /dev/null +++ b/pkg/api/v1beta1/register.go @@ -0,0 +1,40 @@ +/* +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 v1beta1 + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" +) + +func init() { + api.AddKnownTypes("v1beta1", + PodList{}, + Pod{}, + ReplicationControllerList{}, + ReplicationController{}, + ServiceList{}, + Service{}, + MinionList{}, + Minion{}, + Status{}, + ServerOpList{}, + ServerOp{}, + ContainerManifestList{}, + Endpoints{}, + Binding{}, + ) +} diff --git a/pkg/apiserver/operation_test.go b/pkg/apiserver/operation_test.go index a322f958782..e74f3592a2e 100644 --- a/pkg/apiserver/operation_test.go +++ b/pkg/apiserver/operation_test.go @@ -25,7 +25,9 @@ import ( "testing" "time" + // TODO: remove dependency on api, apiserver should be generic "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" ) func TestOperation(t *testing.T) { diff --git a/pkg/client/client.go b/pkg/client/client.go index 04e2529c13b..85b632e40aa 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -26,6 +26,7 @@ import ( "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/version" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" diff --git a/pkg/kubelet/config/etcd.go b/pkg/kubelet/config/etcd.go index 1538f517751..bf220b48f2b 100644 --- a/pkg/kubelet/config/etcd.go +++ b/pkg/kubelet/config/etcd.go @@ -23,6 +23,7 @@ import ( "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet" "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" diff --git a/pkg/master/master.go b/pkg/master/master.go index 8ef62aec879..0f3d7f5cda4 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -21,6 +21,7 @@ import ( "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" diff --git a/pkg/registry/binding/storage_test.go b/pkg/registry/binding/storage_test.go index a9711e5d8b2..be62dcdd9a9 100644 --- a/pkg/registry/binding/storage_test.go +++ b/pkg/registry/binding/storage_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) diff --git a/pkg/registry/etcd/etcd_test.go b/pkg/registry/etcd/etcd_test.go index c2780a5a67f..cd45031d026 100644 --- a/pkg/registry/etcd/etcd_test.go +++ b/pkg/registry/etcd/etcd_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" diff --git a/pkg/tools/decoder_test.go b/pkg/tools/decoder_test.go index 6fabc5646b7..dc7fd636249 100644 --- a/pkg/tools/decoder_test.go +++ b/pkg/tools/decoder_test.go @@ -24,6 +24,7 @@ import ( "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" )