From ec22c2dd825044319f81d97edb4e4642dbff61bc Mon Sep 17 00:00:00 2001 From: Vishnu Kannan Date: Tue, 18 Aug 2015 11:14:52 -0700 Subject: [PATCH] Address comments. --- pkg/client/unversioned/cache/listers.go | 19 +++++++++---------- pkg/client/unversioned/cache/listers_test.go | 10 +++++----- pkg/client/{ => unversioned}/daemon.go | 5 ++++- pkg/client/{ => unversioned}/daemon_test.go | 2 +- .../testclient/fake_daemons.go | 4 ++++ pkg/registry/daemon/doc.go | 2 +- pkg/registry/daemon/etcd/etcd_test.go | 17 ----------------- pkg/registry/daemon/rest.go | 9 +++------ pkg/registry/generic/etcd/etcd.go | 2 +- 9 files changed, 28 insertions(+), 42 deletions(-) rename pkg/client/{ => unversioned}/daemon.go (96%) rename pkg/client/{ => unversioned}/daemon_test.go (99%) rename pkg/client/{ => unversioned}/testclient/fake_daemons.go (93%) diff --git a/pkg/client/unversioned/cache/listers.go b/pkg/client/unversioned/cache/listers.go index b4accf4c95..9cdf07862a 100644 --- a/pkg/client/unversioned/cache/listers.go +++ b/pkg/client/unversioned/cache/listers.go @@ -248,10 +248,10 @@ func (s *StoreToDaemonLister) List() (daemons []api.Daemon, err error) { return daemons, nil } -// GetPodDaemon returns a list of daemon daemons managing a pod. Returns an error iff no matching daemons are found. -func (s *StoreToDaemonLister) GetPodDaemon(pod *api.Pod) (daemons []api.Daemon, err error) { +// GetPodDaemons returns a list of daemons managing a pod. Returns an error iff no matching daemons are found. +func (s *StoreToDaemonLister) GetPodDaemons(pod *api.Pod) (daemons []api.Daemon, err error) { var selector labels.Selector - var dc api.Daemon + var daemonController api.Daemon if len(pod.Labels) == 0 { err = fmt.Errorf("No daemons found for pod %v because it has no labels", pod.Name) @@ -259,18 +259,17 @@ func (s *StoreToDaemonLister) GetPodDaemon(pod *api.Pod) (daemons []api.Daemon, } for _, m := range s.Store.List() { - dc = *m.(*api.Daemon) - if dc.Namespace != pod.Namespace { + daemonController = *m.(*api.Daemon) + if daemonController.Namespace != pod.Namespace { continue } - labelSet := labels.Set(dc.Spec.Selector) - selector = labels.Set(dc.Spec.Selector).AsSelector() + selector = labels.Set(daemonController.Spec.Selector).AsSelector() - // If an dc with a nil or empty selector creeps in, it should match nothing, not everything. - if labelSet.AsSelector().Empty() || !selector.Matches(labels.Set(pod.Labels)) { + // If a daemonController with a nil or empty selector creeps in, it should match nothing, not everything. + if selector.Empty() || !selector.Matches(labels.Set(pod.Labels)) { continue } - daemons = append(daemons, dc) + daemons = append(daemons, daemonController) } if len(daemons) == 0 { err = fmt.Errorf("Could not find daemons for pod %s in namespace %s with labels: %v", pod.Name, pod.Namespace, pod.Labels) diff --git a/pkg/client/unversioned/cache/listers_test.go b/pkg/client/unversioned/cache/listers_test.go index 3f7eecfb0b..ecc99de665 100644 --- a/pkg/client/unversioned/cache/listers_test.go +++ b/pkg/client/unversioned/cache/listers_test.go @@ -64,7 +64,7 @@ func TestStoreToReplicationControllerLister(t *testing.T) { }, outRCNames: util.NewStringSet("basic"), }, - // No pod lables + // No pod labels { inRCs: []*api.ReplicationController{ { @@ -186,7 +186,7 @@ func TestStoreToDaemonLister(t *testing.T) { }, outDCNames: util.NewStringSet("basic", "complex", "complex2"), }, - // No pod lables + // No pod labels { inDCs: []*api.Daemon{ { @@ -200,7 +200,7 @@ func TestStoreToDaemonLister(t *testing.T) { pod := &api.Pod{ ObjectMeta: api.ObjectMeta{Name: "pod1", Namespace: "ns"}, } - return lister.GetPodDaemon(pod) + return lister.GetPodDaemons(pod) }, outDCNames: util.NewStringSet(), expectErr: true, @@ -220,7 +220,7 @@ func TestStoreToDaemonLister(t *testing.T) { Labels: map[string]string{"foo": "bar"}, }, } - return lister.GetPodDaemon(pod) + return lister.GetPodDaemons(pod) }, outDCNames: util.NewStringSet(), expectErr: true, @@ -249,7 +249,7 @@ func TestStoreToDaemonLister(t *testing.T) { Namespace: "ns", }, } - return lister.GetPodDaemon(pod) + return lister.GetPodDaemons(pod) }, outDCNames: util.NewStringSet("bar"), }, diff --git a/pkg/client/daemon.go b/pkg/client/unversioned/daemon.go similarity index 96% rename from pkg/client/daemon.go rename to pkg/client/unversioned/daemon.go index 75a5db397f..052ba95a54 100644 --- a/pkg/client/daemon.go +++ b/pkg/client/unversioned/daemon.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package client +package unversioned import ( "k8s.io/kubernetes/pkg/api" @@ -47,6 +47,9 @@ func newDaemons(c *Client, namespace string) *daemons { return &daemons{c, namespace} } +// Ensure statically that daemons implements DaemonInterface. +var _ DaemonInterface = &daemons{} + func (c *daemons) List(selector labels.Selector) (result *api.DaemonList, err error) { result = &api.DaemonList{} err = c.r.Get().Namespace(c.ns).Resource("daemons").LabelsSelectorParam(selector).Do().Into(result) diff --git a/pkg/client/daemon_test.go b/pkg/client/unversioned/daemon_test.go similarity index 99% rename from pkg/client/daemon_test.go rename to pkg/client/unversioned/daemon_test.go index d3dd5b74e8..6e5ecbf38f 100644 --- a/pkg/client/daemon_test.go +++ b/pkg/client/unversioned/daemon_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package client +package unversioned import ( "testing" diff --git a/pkg/client/testclient/fake_daemons.go b/pkg/client/unversioned/testclient/fake_daemons.go similarity index 93% rename from pkg/client/testclient/fake_daemons.go rename to pkg/client/unversioned/testclient/fake_daemons.go index 6a172f8564..f484b51e5c 100644 --- a/pkg/client/testclient/fake_daemons.go +++ b/pkg/client/unversioned/testclient/fake_daemons.go @@ -18,6 +18,7 @@ package testclient import ( "k8s.io/kubernetes/pkg/api" + kClientLib "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/fields" "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/watch" @@ -39,6 +40,9 @@ const ( CreateDaemonAction = "create-daemon" ) +// Ensure statically that FakeDaemons implements DaemonInterface. +var _ kClientLib.DaemonInterface = &FakeDaemons{} + func (c *FakeDaemons) Get(name string) (*api.Daemon, error) { obj, err := c.Fake.Invokes(NewGetAction("daemons", c.Namespace, name), &api.Daemon{}) return obj.(*api.Daemon), err diff --git a/pkg/registry/daemon/doc.go b/pkg/registry/daemon/doc.go index abfc57ec01..a245200438 100644 --- a/pkg/registry/daemon/doc.go +++ b/pkg/registry/daemon/doc.go @@ -14,6 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package daemon provides Registry interface and it's RESTStorage +// Package daemon provides Registry interface and its RESTStorage // implementation for storing Daemon api objects. package daemon diff --git a/pkg/registry/daemon/etcd/etcd_test.go b/pkg/registry/daemon/etcd/etcd_test.go index d117d772aa..fe4f09f71a 100755 --- a/pkg/registry/daemon/etcd/etcd_test.go +++ b/pkg/registry/daemon/etcd/etcd_test.go @@ -543,28 +543,11 @@ func TestEtcdWatchControllersFields(t *testing.T) { testFieldMap := map[int][]fields.Set{ PASS: { - {"status.currentNumberScheduled": "2"}, - {"status.numberMisscheduled": "1"}, - {"status.desiredNumberScheduled": "4"}, {"metadata.name": "foo"}, - {"status.currentNumberScheduled": "2", "status.numberMisscheduled": "1"}, - {"status.currentNumberScheduled": "2", "status.desiredNumberScheduled": "4"}, - {"status.currentNumberScheduled": "2", "metadata.name": "foo"}, - {"status.desiredNumberScheduled": "4", "metadata.name": "foo"}, - {"status.currentNumberScheduled": "2", "status.desiredNumberScheduled": "4", "metadata.name": "foo"}, - {"status.currentNumberScheduled": "2", "status.numberMisscheduled": "1", "status.desiredNumberScheduled": "4"}, - {"status.currentNumberScheduled": "2", "status.numberMisscheduled": "1", "status.desiredNumberScheduled": "4", "metadata.name": "foo"}, }, FAIL: { - {"status.currentNumberScheduled": "1"}, - {"status.numberMisscheduled": "0"}, - {"status.desiredNumberScheduled": "5"}, {"metadata.name": "bar"}, {"name": "foo"}, - {"status.replicas": "0"}, - {"status.currentNumberScheduled": "2", "status.desiredNumberScheduled": "3"}, - {"status.numberMisscheduled": "3", "status.desiredNumberScheduled": "5"}, - {"status.currentNumberScheduled": "2", "status.desiredNumberScheduled": "4", "metadata.name": "foox"}, }, } testEtcdActions := []string{ diff --git a/pkg/registry/daemon/rest.go b/pkg/registry/daemon/rest.go index 31b3409c36..7adea048c8 100644 --- a/pkg/registry/daemon/rest.go +++ b/pkg/registry/daemon/rest.go @@ -19,7 +19,6 @@ package daemon import ( "fmt" "reflect" - "strconv" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/validation" @@ -92,17 +91,15 @@ func (daemonStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) f return append(validationErrorList, updateErrorList...) } +// AllowUnconditionalUpdate is the default update policy for daemon objects. func (daemonStrategy) AllowUnconditionalUpdate() bool { return true } -// DaemonToSelectableFields returns a label set that represents the object. +// DaemonToSelectableFields returns a field set that represents the object. func DaemonToSelectableFields(daemon *api.Daemon) fields.Set { return fields.Set{ - "metadata.name": daemon.Name, - "status.currentNumberScheduled": strconv.Itoa(daemon.Status.CurrentNumberScheduled), - "status.numberMisscheduled": strconv.Itoa(daemon.Status.NumberMisscheduled), - "status.desiredNumberScheduled": strconv.Itoa(daemon.Status.DesiredNumberScheduled), + "metadata.name": daemon.Name, } } diff --git a/pkg/registry/generic/etcd/etcd.go b/pkg/registry/generic/etcd/etcd.go index b1b39e5705..3242d20bc7 100644 --- a/pkg/registry/generic/etcd/etcd.go +++ b/pkg/registry/generic/etcd/etcd.go @@ -273,7 +273,7 @@ func (e *Etcd) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool return nil, nil, err } if newVersion != version { - return nil, nil, kubeerr.NewConflict(e.EndpointName, name, fmt.Errorf("the object has been modified; please apply your changes to the latest version and try again %+v, %+v", version, newVersion)) + return nil, nil, kubeerr.NewConflict(e.EndpointName, name, fmt.Errorf("the object has been modified; please apply your changes to the latest version and try again")) } } if err := rest.BeforeUpdate(e.UpdateStrategy, ctx, obj, existing); err != nil {