diff --git a/pkg/registry/minion/healthy_registry.go b/pkg/registry/minion/healthy_registry.go index 1e8878cf1a9..dfdaa9875fc 100644 --- a/pkg/registry/minion/healthy_registry.go +++ b/pkg/registry/minion/healthy_registry.go @@ -22,8 +22,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/health" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" - - "github.com/golang/glog" ) type HealthyRegistry struct { @@ -40,23 +38,10 @@ func NewHealthyRegistry(delegate Registry, client client.KubeletHealthChecker) R func (r *HealthyRegistry) GetMinion(ctx api.Context, minionID string) (*api.Node, error) { minion, err := r.delegate.GetMinion(ctx, minionID) - if err != nil { - return minion, err - } - if minion == nil { - return nil, ErrDoesNotExist - } if err != nil { return nil, err } - status, err := r.client.HealthCheck(minionID) - if err != nil { - return nil, err - } - if status == health.Unhealthy { - return nil, ErrNotHealty - } - return minion, nil + return r.checkMinion(minion), nil } func (r *HealthyRegistry) DeleteMinion(ctx api.Context, minionID string) error { @@ -72,26 +57,42 @@ func (r *HealthyRegistry) UpdateMinion(ctx api.Context, minion *api.Node) error } func (r *HealthyRegistry) ListMinions(ctx api.Context) (currentMinions *api.NodeList, err error) { - result := &api.NodeList{} list, err := r.delegate.ListMinions(ctx) if err != nil { - return result, err + return nil, err } - for _, minion := range list.Items { - status, err := r.client.HealthCheck(minion.Name) - if err != nil { - glog.V(1).Infof("%#v failed health check with error: %v", minion, err) - continue - } - if status == health.Healthy { - result.Items = append(result.Items, minion) - } else { - glog.Errorf("%#v failed a health check, ignoring.", minion) - } + for i := range list.Items { + list.Items[i] = *r.checkMinion(&list.Items[i]) } - return result, nil + return list, nil } func (r *HealthyRegistry) WatchMinions(ctx api.Context, label, field labels.Selector, resourceVersion string) (watch.Interface, error) { - return r.delegate.WatchMinions(ctx, label, field, resourceVersion) + w, err := r.delegate.WatchMinions(ctx, label, field, resourceVersion) + if err != nil { + return nil, err + } + return watch.Filter(w, watch.FilterFunc(func(in watch.Event) (watch.Event, bool) { + if node, ok := in.Object.(*api.Node); ok && node != nil { + in.Object = r.checkMinion(node) + } + return in, true + })), nil +} + +func (r *HealthyRegistry) checkMinion(node *api.Node) *api.Node { + condition := api.ConditionFull + switch status, err := r.client.HealthCheck(node.Name); { + case err != nil: + condition = api.ConditionUnknown + case status == health.Unhealthy: + condition = api.ConditionNone + } + // TODO: distinguish other conditions like Reachable/Live, and begin storing this + // data on nodes directly via sync loops. + node.Status.Conditions = append(node.Status.Conditions, api.NodeCondition{ + Kind: api.NodeReady, + Status: condition, + }) + return node } diff --git a/pkg/registry/minion/healthy_registry_test.go b/pkg/registry/minion/healthy_registry_test.go index cae721a99c0..f7a30ca6d21 100644 --- a/pkg/registry/minion/healthy_registry_test.go +++ b/pkg/registry/minion/healthy_registry_test.go @@ -86,19 +86,23 @@ func TestFiltering(t *testing.T) { delegate: mockMinionRegistry, client: ¬Minion{minion: "m1"}, } - expected := []string{"m2", "m3"} + expected := []string{"m1", "m2", "m3"} list, err := healthy.ListMinions(ctx) if err != nil { t.Errorf("unexpected error: %v", err) } - if !reflect.DeepEqual(list, registrytest.MakeMinionList(expected, api.NodeResources{})) { + expectedMinions := registrytest.MakeMinionList(expected, api.NodeResources{}) + expectedMinions.Items[0].Status.Conditions = []api.NodeCondition{{Kind: api.NodeReady, Status: api.ConditionNone}} + expectedMinions.Items[1].Status.Conditions = []api.NodeCondition{{Kind: api.NodeReady, Status: api.ConditionFull}} + expectedMinions.Items[2].Status.Conditions = []api.NodeCondition{{Kind: api.NodeReady, Status: api.ConditionFull}} + if !reflect.DeepEqual(list, expectedMinions) { t.Errorf("Expected %v, Got %v", expected, list) } minion, err := healthy.GetMinion(ctx, "m1") - if err == nil { - t.Errorf("unexpected non-error") + if err != nil { + t.Errorf("unexpected error: %v", err) } - if minion != nil { - t.Errorf("Unexpected presence of 'm1'") + if minion == nil { + t.Errorf("Unexpected empty 'm1'") } } diff --git a/pkg/registry/minion/rest_test.go b/pkg/registry/minion/rest_test.go index 16946e801bb..4b4666d06bb 100644 --- a/pkg/registry/minion/rest_test.go +++ b/pkg/registry/minion/rest_test.go @@ -34,8 +34,8 @@ func TestMinionRegistryREST(t *testing.T) { if obj, err := ms.Get(ctx, "bar"); err != nil || obj.(*api.Node).Name != "bar" { t.Errorf("missing expected object") } - if _, err := ms.Get(ctx, "baz"); err != ErrDoesNotExist { - t.Errorf("has unexpected object") + if _, err := ms.Get(ctx, "baz"); !errors.IsNotFound(err) { + t.Errorf("has unexpected error: %v", err) } c, err := ms.Create(ctx, &api.Node{ObjectMeta: api.ObjectMeta{Name: "baz"}}) @@ -61,8 +61,8 @@ func TestMinionRegistryREST(t *testing.T) { if s, ok := obj.Object.(*api.Status); !ok || s.Status != api.StatusSuccess { t.Errorf("delete return value was weird: %#v", obj) } - if _, err := ms.Get(ctx, "bar"); err != ErrDoesNotExist { - t.Errorf("delete didn't actually delete") + if _, err := ms.Get(ctx, "bar"); !errors.IsNotFound(err) { + t.Errorf("delete didn't actually delete: %v", err) } _, err = ms.Delete(ctx, "bar") @@ -105,8 +105,8 @@ func TestMinionRegistryHealthCheck(t *testing.T) { if m, ok := result.Object.(*api.Node); !ok || m.Name != "m1" { t.Errorf("insert return value was weird: %#v", result) } - if _, err := ms.Get(ctx, "m1"); err == nil { - t.Errorf("node is unhealthy, expect no result from apiserver") + if _, err := ms.Get(ctx, "m1"); err != nil { + t.Errorf("node is unhealthy, expect no error: %v", err) } } diff --git a/pkg/registry/registrytest/minion.go b/pkg/registry/registrytest/minion.go index c0d1b91c344..13db8ff2592 100644 --- a/pkg/registry/registrytest/minion.go +++ b/pkg/registry/registrytest/minion.go @@ -20,6 +20,7 @@ import ( "sync" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" ) @@ -80,12 +81,15 @@ func (r *MinionRegistry) UpdateMinion(ctx api.Context, minion *api.Node) error { func (r *MinionRegistry) GetMinion(ctx api.Context, minionID string) (*api.Node, error) { r.Lock() defer r.Unlock() + if r.Err != nil { + return nil, r.Err + } for _, node := range r.Minions.Items { if node.Name == minionID { - return &node, r.Err + return &node, nil } } - return nil, r.Err + return nil, errors.NewNotFound("node", minionID) } func (r *MinionRegistry) DeleteMinion(ctx api.Context, minionID string) error {