HealthyRegistry should only decorate minions, not omit them

Fixes #3098
This commit is contained in:
Clayton Coleman 2015-01-05 22:23:27 -05:00
parent cae572290b
commit a0d711816d
4 changed files with 54 additions and 45 deletions

View File

@ -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
}

View File

@ -86,19 +86,23 @@ func TestFiltering(t *testing.T) {
delegate: mockMinionRegistry,
client: &notMinion{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'")
}
}

View File

@ -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)
}
}

View File

@ -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 {