From ccc56d3c3c741e9b57715dd2e7407a90cb8526fc Mon Sep 17 00:00:00 2001 From: gmarek Date: Wed, 8 Apr 2015 11:32:47 +0200 Subject: [PATCH] Update NodeStatus use subresources. --- pkg/client/nodes.go | 11 +++++++ pkg/client/testclient/fake_nodes.go | 5 ++++ .../controller/nodecontroller.go | 2 +- .../controller/nodecontroller_test.go | 18 ++++++++---- pkg/kubelet/kubelet.go | 2 +- pkg/kubelet/kubelet_test.go | 2 +- pkg/master/master.go | 4 ++- pkg/registry/minion/etcd/etcd.go | 21 ++++++++++++-- pkg/registry/minion/etcd/etcd_test.go | 2 +- pkg/registry/minion/rest.go | 29 ++++++++++++++++--- pkg/registry/pod/rest.go | 1 - 11 files changed, 80 insertions(+), 17 deletions(-) diff --git a/pkg/client/nodes.go b/pkg/client/nodes.go index 8e36a91e350..1397a5e6944 100644 --- a/pkg/client/nodes.go +++ b/pkg/client/nodes.go @@ -35,6 +35,7 @@ type NodeInterface interface { List(selector labels.Selector) (*api.NodeList, error) Delete(name string) error Update(*api.Node) (*api.Node, error) + UpdateStatus(*api.Node) (*api.Node, error) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) } @@ -94,6 +95,16 @@ func (c *nodes) Update(node *api.Node) (*api.Node, error) { return result, err } +func (c *nodes) UpdateStatus(node *api.Node) (*api.Node, error) { + result := &api.Node{} + if len(node.ResourceVersion) == 0 { + err := fmt.Errorf("invalid update object, missing resource version: %v", node) + return nil, err + } + err := c.r.Put().Resource(c.resourceName()).Name(node.Name).SubResource("status").Body(node).Do().Into(result) + return result, err +} + // Watch returns a watch.Interface that watches the requested nodes. func (c *nodes) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) { return c.r.Get(). diff --git a/pkg/client/testclient/fake_nodes.go b/pkg/client/testclient/fake_nodes.go index b58378fbb28..b3e0cc7dc7f 100644 --- a/pkg/client/testclient/fake_nodes.go +++ b/pkg/client/testclient/fake_nodes.go @@ -54,6 +54,11 @@ func (c *FakeNodes) Update(minion *api.Node) (*api.Node, error) { return obj.(*api.Node), err } +func (c *FakeNodes) UpdateStatus(minion *api.Node) (*api.Node, error) { + obj, err := c.Fake.Invokes(FakeAction{Action: "update-status-node", Value: minion}, &api.Node{}) + return obj.(*api.Node), err +} + func (c *FakeNodes) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) { c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-nodes", Value: resourceVersion}) return c.Fake.Watch, c.Fake.Err diff --git a/pkg/cloudprovider/controller/nodecontroller.go b/pkg/cloudprovider/controller/nodecontroller.go index 1be036c275e..d3e1eadaff0 100644 --- a/pkg/cloudprovider/controller/nodecontroller.go +++ b/pkg/cloudprovider/controller/nodecontroller.go @@ -413,7 +413,7 @@ func (nc *NodeController) tryUpdateNodeStatus(node *api.Node) (time.Duration, ap } } if !api.Semantic.DeepEqual(nc.getCondition(&node.Status, api.NodeReady), lastReadyCondition) { - if _, err = nc.kubeClient.Nodes().Update(node); err != nil { + if _, err = nc.kubeClient.Nodes().UpdateStatus(node); err != nil { glog.Errorf("Error updating node %s: %v", node.Name, err) return gracePeriod, lastReadyCondition, readyCondition, err } else { diff --git a/pkg/cloudprovider/controller/nodecontroller_test.go b/pkg/cloudprovider/controller/nodecontroller_test.go index 42c73d65196..48479cdd9fb 100644 --- a/pkg/cloudprovider/controller/nodecontroller_test.go +++ b/pkg/cloudprovider/controller/nodecontroller_test.go @@ -56,10 +56,11 @@ type FakeNodeHandler struct { Existing []*api.Node // Output - CreatedNodes []*api.Node - DeletedNodes []*api.Node - UpdatedNodes []*api.Node - RequestCount int + CreatedNodes []*api.Node + DeletedNodes []*api.Node + UpdatedNodes []*api.Node + UpdatedNodeStatuses []*api.Node + RequestCount int } func (c *FakeNodeHandler) Nodes() client.NodeInterface { @@ -124,6 +125,13 @@ func (m *FakeNodeHandler) Update(node *api.Node) (*api.Node, error) { return node, nil } +func (m *FakeNodeHandler) UpdateStatus(node *api.Node) (*api.Node, error) { + nodeCopy := *node + m.UpdatedNodeStatuses = append(m.UpdatedNodeStatuses, &nodeCopy) + m.RequestCount++ + return node, nil +} + func (m *FakeNodeHandler) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) { return nil, nil } @@ -1053,7 +1061,7 @@ func TestMonitorNodeStatusUpdateStatus(t *testing.T) { if item.expectedRequestCount != item.fakeNodeHandler.RequestCount { t.Errorf("expected %v call, but got %v.", item.expectedRequestCount, item.fakeNodeHandler.RequestCount) } - if !api.Semantic.DeepEqual(item.expectedNodes, item.fakeNodeHandler.UpdatedNodes) { + if len(item.fakeNodeHandler.UpdatedNodes) > 0 && !api.Semantic.DeepEqual(item.expectedNodes, item.fakeNodeHandler.UpdatedNodes) { t.Errorf("expected nodes %+v, got %+v", item.expectedNodes[0], item.fakeNodeHandler.UpdatedNodes[0]) } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 5f5de897551..3a8d3f83814 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1843,7 +1843,7 @@ func (kl *Kubelet) tryUpdateNodeStatus() error { kl.recordNodeOnlineEvent() } - _, err = kl.kubeClient.Nodes().Update(node) + _, err = kl.kubeClient.Nodes().UpdateStatus(node) return err } diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 6bd5036e6ab..695254687aa 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -3113,7 +3113,7 @@ func TestUpdateNewNodeStatus(t *testing.T) { if err := kubelet.updateNodeStatus(); err != nil { t.Errorf("unexpected error: %v", err) } - if len(kubeClient.Actions) != 2 || kubeClient.Actions[1].Action != "update-node" { + if len(kubeClient.Actions) != 2 || kubeClient.Actions[1].Action != "update-status-node" { t.Fatalf("unexpected actions: %v", kubeClient.Actions) } updatedNode, ok := kubeClient.Actions[1].Value.(*api.Node) diff --git a/pkg/master/master.go b/pkg/master/master.go index 8d88769b850..5ce5c2a7cb4 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -376,7 +376,7 @@ func (m *Master) init(c *Config) { endpointsStorage := endpointsetcd.NewStorage(c.EtcdHelper) m.endpointRegistry = endpoint.NewRegistry(endpointsStorage) - nodeStorage := nodeetcd.NewStorage(c.EtcdHelper, c.KubeletClient) + nodeStorage, nodeStatusStorage := nodeetcd.NewStorage(c.EtcdHelper, c.KubeletClient) m.nodeRegistry = minion.NewRegistry(nodeStorage) // TODO: split me up into distinct storage registries @@ -397,7 +397,9 @@ func (m *Master) init(c *Config) { "services": service.NewStorage(m.serviceRegistry, c.Cloud, m.nodeRegistry, m.endpointRegistry, m.portalNet, c.ClusterName), "endpoints": endpointsStorage, "minions": nodeStorage, + "minions/status": nodeStatusStorage, "nodes": nodeStorage, + "nodes/status": nodeStatusStorage, "events": event.NewStorage(eventRegistry), "limitRanges": limitrange.NewStorage(limitRangeRegistry), diff --git a/pkg/registry/minion/etcd/etcd.go b/pkg/registry/minion/etcd/etcd.go index 0c22589d933..298bcdb9efc 100644 --- a/pkg/registry/minion/etcd/etcd.go +++ b/pkg/registry/minion/etcd/etcd.go @@ -34,8 +34,22 @@ type REST struct { connection client.ConnectionInfoGetter } +// StatusREST implements the REST endpoint for changing the status of a pod. +type StatusREST struct { + store *etcdgeneric.Etcd +} + +func (r *StatusREST) New() runtime.Object { + return &api.Node{} +} + +// Update alters the status subset of an object. +func (r *StatusREST) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool, error) { + return r.store.Update(ctx, obj) +} + // NewStorage returns a RESTStorage object that will work against nodes. -func NewStorage(h tools.EtcdHelper, connection client.ConnectionInfoGetter) *REST { +func NewStorage(h tools.EtcdHelper, connection client.ConnectionInfoGetter) (*REST, *StatusREST) { prefix := "/registry/minions" store := &etcdgeneric.Etcd{ NewFunc: func() runtime.Object { return &api.Node{} }, @@ -58,7 +72,10 @@ func NewStorage(h tools.EtcdHelper, connection client.ConnectionInfoGetter) *RES Helper: h, } - return &REST{store, connection} + statusStore := *store + statusStore.UpdateStrategy = minion.StatusStrategy + + return &REST{store, connection}, &StatusREST{store: &statusStore} } // Implement Redirector. diff --git a/pkg/registry/minion/etcd/etcd_test.go b/pkg/registry/minion/etcd/etcd_test.go index 78248a28367..e8d88f18abc 100644 --- a/pkg/registry/minion/etcd/etcd_test.go +++ b/pkg/registry/minion/etcd/etcd_test.go @@ -55,7 +55,7 @@ func newHelper(t *testing.T) (*tools.FakeEtcdClient, tools.EtcdHelper) { func newStorage(t *testing.T) (*REST, *tools.FakeEtcdClient) { fakeEtcdClient, h := newHelper(t) - storage := NewStorage(h, fakeConnectionInfoGetter{}) + storage, _ := NewStorage(h, fakeConnectionInfoGetter{}) return storage, fakeEtcdClient } diff --git a/pkg/registry/minion/rest.go b/pkg/registry/minion/rest.go index 38df7bfba2b..f25ecd2bb2d 100644 --- a/pkg/registry/minion/rest.go +++ b/pkg/registry/minion/rest.go @@ -56,14 +56,14 @@ func (nodeStrategy) AllowCreateOnUpdate() bool { // PrepareForCreate clears fields that are not allowed to be set by end users on creation. func (nodeStrategy) PrepareForCreate(obj runtime.Object) { _ = obj.(*api.Node) - // Nodes allow *all* fields, including status, to be set. + // Nodes allow *all* fields, including status, to be set on create. } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. func (nodeStrategy) PrepareForUpdate(obj, old runtime.Object) { - _ = obj.(*api.Node) - _ = old.(*api.Node) - // Nodes allow *all* fields, including status, to be set. + newNode := obj.(*api.Node) + oldNode := old.(*api.Node) + newNode.Status = oldNode.Status } // Validate validates a new node. @@ -77,6 +77,27 @@ func (nodeStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fie return validation.ValidateMinionUpdate(old.(*api.Node), obj.(*api.Node)) } +type nodeStatusStrategy struct { + nodeStrategy +} + +var StatusStrategy = nodeStatusStrategy{Strategy} + +func (nodeStatusStrategy) PrepareForCreate(obj runtime.Object) { + _ = obj.(*api.Node) + // Nodes allow *all* fields, including status, to be set on create. +} + +func (nodeStatusStrategy) PrepareForUpdate(obj, old runtime.Object) { + newNode := obj.(*api.Node) + oldNode := old.(*api.Node) + newNode.Spec = oldNode.Spec +} + +func (nodeStatusStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fielderrors.ValidationErrorList { + return validation.ValidateMinionUpdate(old.(*api.Node), obj.(*api.Node)) +} + // ResourceGetter is an interface for retrieving resources by ResourceLocation. type ResourceGetter interface { Get(api.Context, string) (runtime.Object, error) diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go index 04aab008267..874bf69a123 100644 --- a/pkg/registry/pod/rest.go +++ b/pkg/registry/pod/rest.go @@ -43,7 +43,6 @@ type podStrategy struct { // Strategy is the default logic that applies when creating and updating Pod // objects via the REST API. -// TODO: Create other strategies for updating status, bindings, etc var Strategy = podStrategy{api.Scheme, api.SimpleNameGenerator} // NamespaceScoped is true for pods.