github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-30 06:54:01 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #5738 from smarterclayton/cleanup_clients

Browse Source 
Clients should not check conditions, UpdateStatus() is inconsistent
This commit is contained in:
Yu-Ju Hong 2015-03-25 13:59:47 -07:00
commit 6145b3b9c4
19 changed files with 32 additions and 163 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/client/client_test.go
View File

@ -35,7 +35,7 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/version"
)
const nameRequiredError = "name is required parameter to Get"
const nameRequiredError = "resource name may not be empty"
type testRequest struct {
Method string

5
pkg/client/endpoints.go
View File

@ -17,7 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@ -72,10 +71,6 @@ func (c *endpoints) List(selector labels.Selector) (result *api.EndpointsList, e
// Get returns information about the endpoints for a particular service.
func (c *endpoints) Get(name string) (result *api.Endpoints, err error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result = &api.Endpoints{}
err = c.r.Get().Namespace(c.ns).Resource("endpoints").Name(name).Do().Into(result)
return

5
pkg/client/events.go
View File

@ -17,7 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@ -114,10 +113,6 @@ func (e *events) List(label labels.Selector, field fields.Selector) (*api.EventL
// Get returns the given event, or an error.
func (e *events) Get(name string) (*api.Event, error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result := &api.Event{}
err := e.client.Get().
NamespaceIfScoped(e.namespace, len(e.namespace) > 0).

4
pkg/client/fake_pods.go
View File

@ -65,7 +65,7 @@ func (c *FakePods) Bind(bind *api.Binding) error {
return nil
}
func (c *FakePods) UpdateStatus(name string, status *api.PodStatus) (*api.Pod, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "update-status-pod", Value: name})
func (c *FakePods) UpdateStatus(pod *api.Pod) (*api.Pod, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "update-status-pod", Value: pod.Name})
return &api.Pod{}, nil
}

2
pkg/client/fake_resource_quotas.go
View File

@ -55,7 +55,7 @@ func (c *FakeResourceQuotas) Update(resourceQuota *api.ResourceQuota) (*api.Reso
return &api.ResourceQuota{}, nil
}
func (c *FakeResourceQuotas) Status(resourceQuota *api.ResourceQuota) (*api.ResourceQuota, error) {
func (c *FakeResourceQuotas) UpdateStatus(resourceQuota *api.ResourceQuota) (*api.ResourceQuota, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "update-status-resourceQuota", Value: resourceQuota.Name})
c.Fake.ResourceQuotaStatus = *resourceQuota
return &api.ResourceQuota{}, nil

5
pkg/client/limit_ranges.go
View File

@ -17,7 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@ -64,10 +63,6 @@ func (c *limitRanges) List(selector labels.Selector) (result *api.LimitRangeList
// Get takes the name of the limitRange, and returns the corresponding Pod object, and an error if it occurs
func (c *limitRanges) Get(name string) (result *api.LimitRange, err error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result = &api.LimitRange{}
err = c.r.Get().Namespace(c.ns).Resource("limitRanges").Name(name).Do().Into(result)
return

5
pkg/client/minions.go
View File

@ -17,7 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@ -74,10 +73,6 @@ func (c *nodes) List() (*api.NodeList, error) {
// Get gets an existing node.
func (c *nodes) Get(name string) (*api.Node, error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result := &api.Node{}
err := c.r.Get().Resource(c.resourceName()).Name(name).Do().Into(result)
return result, err

5
pkg/client/namespaces.go
View File

@ -17,7 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@ -104,10 +103,6 @@ func (c *namespaces) Status(namespace *api.Namespace) (result *api.Namespace, er
// Get gets an existing namespace
func (c *namespaces) Get(name string) (*api.Namespace, error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result := &api.Namespace{}
err := c.r.Get().Resource("namespaces").Name(name).Do().Into(result)
return result, err

20
pkg/client/pods.go
View File

@ -17,9 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
@ -40,7 +37,7 @@ type PodInterface interface {
Update(pod *api.Pod) (*api.Pod, error)
Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error)
Bind(binding *api.Binding) error
UpdateStatus(name string, status *api.PodStatus) (*api.Pod, error)
UpdateStatus(pod *api.Pod) (*api.Pod, error)
}
// pods implements PodsNamespacer interface
@ -66,10 +63,6 @@ func (c *pods) List(selector labels.Selector) (result *api.PodList, err error) {
// Get takes the name of the pod, and returns the corresponding Pod object, and an error if it occurs
func (c *pods) Get(name string) (result *api.Pod, err error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result = &api.Pod{}
err = c.r.Get().Namespace(c.ns).Resource("pods").Name(name).Do().Into(result)
return
@ -90,10 +83,6 @@ func (c *pods) Create(pod *api.Pod) (result *api.Pod, err error) {
// Update takes the representation of a pod to update. Returns the server's representation of the pod, and an error, if it occurs.
func (c *pods) Update(pod *api.Pod) (result *api.Pod, err error) {
result = &api.Pod{}
if len(pod.ResourceVersion) == 0 {
err = fmt.Errorf("invalid update object, missing resource version: %v", pod)
return
}
err = c.r.Put().Namespace(c.ns).Resource("pods").Name(pod.Name).Body(pod).Do().Into(result)
return
}
@ -116,13 +105,8 @@ func (c *pods) Bind(binding *api.Binding) error {
}
// UpdateStatus takes the name of the pod and the new status. Returns the server's representation of the pod, and an error, if it occurs.
func (c *pods) UpdateStatus(name string, newStatus *api.PodStatus) (result *api.Pod, err error) {
func (c *pods) UpdateStatus(pod *api.Pod) (result *api.Pod, err error) {
result = &api.Pod{}
pod, err := c.Get(name)
if err != nil {
return
}
pod.Status = *newStatus
err = c.r.Put().Namespace(c.ns).Resource("pods").Name(pod.Name).SubResource("status").Body(pod).Do().Into(result)
return
}

11
pkg/client/replication_controllers.go
View File

@ -17,9 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
@ -61,10 +58,6 @@ func (c *replicationControllers) List(selector labels.Selector) (result *api.Rep
// Get returns information about a particular replication controller.
func (c *replicationControllers) Get(name string) (result *api.ReplicationController, err error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result = &api.ReplicationController{}
err = c.r.Get().Namespace(c.ns).Resource("replicationControllers").Name(name).Do().Into(result)
return
@ -80,10 +73,6 @@ func (c *replicationControllers) Create(controller *api.ReplicationController) (
// Update updates an existing replication controller.
func (c *replicationControllers) Update(controller *api.ReplicationController) (result *api.ReplicationController, err error) {
result = &api.ReplicationController{}
if len(controller.ResourceVersion) == 0 {
err = fmt.Errorf("invalid update object, missing resource version: %v", controller)
return
}
err = c.r.Put().Namespace(c.ns).Resource("replicationControllers").Name(controller.Name).Body(controller).Do().Into(result)
return
}

4
pkg/client/request.go
View File

@ -190,6 +190,10 @@ func (r *Request) Name(resourceName string) *Request {
if r.err != nil {
return r
}
if len(resourceName) == 0 {
r.err = fmt.Errorf("resource name may not be empty")
return r
}
if len(r.resourceName) != 0 {
r.err = fmt.Errorf("resource name already set to %q, cannot change to %q", r.resourceName, resourceName)
return r

19
pkg/client/resource_quotas.go
View File

@ -17,9 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
@ -38,7 +35,7 @@ type ResourceQuotaInterface interface {
Delete(name string) error
Create(resourceQuota *api.ResourceQuota) (*api.ResourceQuota, error)
Update(resourceQuota *api.ResourceQuota) (*api.ResourceQuota, error)
Status(resourceQuota *api.ResourceQuota) (*api.ResourceQuota, error)
UpdateStatus(resourceQuota *api.ResourceQuota) (*api.ResourceQuota, error)
Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error)
}
@ -65,10 +62,6 @@ func (c *resourceQuotas) List(selector labels.Selector) (result *api.ResourceQuo
// Get takes the name of the resourceQuota, and returns the corresponding ResourceQuota object, and an error if it occurs
func (c *resourceQuotas) Get(name string) (result *api.ResourceQuota, err error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result = &api.ResourceQuota{}
err = c.r.Get().Namespace(c.ns).Resource("resourceQuotas").Name(name).Do().Into(result)
return
@ -89,21 +82,13 @@ func (c *resourceQuotas) Create(resourceQuota *api.ResourceQuota) (result *api.R
// Update takes the representation of a resourceQuota to update spec. Returns the server's representation of the resourceQuota, and an error, if it occurs.
func (c *resourceQuotas) Update(resourceQuota *api.ResourceQuota) (result *api.ResourceQuota, err error) {
result = &api.ResourceQuota{}
if len(resourceQuota.ResourceVersion) == 0 {
err = fmt.Errorf("invalid update object, missing resource version: %v", resourceQuota)
return
}
err = c.r.Put().Namespace(c.ns).Resource("resourceQuotas").Name(resourceQuota.Name).Body(resourceQuota).Do().Into(result)
return
}
// Status takes the representation of a resourceQuota to update status. Returns the server's representation of the resourceQuota, and an error, if it occurs.
func (c *resourceQuotas) Status(resourceQuota *api.ResourceQuota) (result *api.ResourceQuota, err error) {
func (c *resourceQuotas) UpdateStatus(resourceQuota *api.ResourceQuota) (result *api.ResourceQuota, err error) {
result = &api.ResourceQuota{}
if len(resourceQuota.ResourceVersion) == 0 {
err = fmt.Errorf("invalid update object, missing resource version: %v", resourceQuota)
return
}
err = c.r.Put().Namespace(c.ns).Resource("resourceQuotas").Name(resourceQuota.Name).SubResource("status").Body(resourceQuota).Do().Into(result)
return
}

30
pkg/client/resource_quotas_test.go
View File

@ -174,38 +174,10 @@ func TestResourceQuotaStatusUpdate(t *testing.T) {
Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200, Body: resourceQuota},
}
response, err := c.Setup().ResourceQuotas(ns).Status(resourceQuota)
response, err := c.Setup().ResourceQuotas(ns).UpdateStatus(resourceQuota)
c.Validate(t, response, err)
}
func TestInvalidResourceQuotaUpdate(t *testing.T) {
ns := api.NamespaceDefault
resourceQuota := &api.ResourceQuota{
ObjectMeta: api.ObjectMeta{
Name: "abc",
Namespace: "foo",
},
Spec: api.ResourceQuotaSpec{
Hard: api.ResourceList{
api.ResourceCPU: resource.MustParse("100"),
api.ResourceMemory: resource.MustParse("10000"),
api.ResourcePods: resource.MustParse("10"),
api.ResourceServices: resource.MustParse("10"),
api.ResourceReplicationControllers: resource.MustParse("10"),
api.ResourceQuotas: resource.MustParse("10"),
},
},
}
c := &testClient{
Request: testRequest{Method: "PUT", Path: testapi.ResourcePath(getResourceQuotasResoureName(), ns, "abc"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200, Body: resourceQuota},
}
_, err := c.Setup().ResourceQuotas(ns).Update(resourceQuota)
if err == nil {
t.Errorf("Expected an error due to missing ResourceVersion")
}
}
func TestResourceQuotaDelete(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{

16
pkg/client/secrets.go
View File

@ -17,9 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
@ -54,10 +51,6 @@ func newSecrets(c *Client, ns string) *secrets {
}
func (s *secrets) Create(secret *api.Secret) (*api.Secret, error) {
if s.namespace != "" && secret.Namespace != s.namespace {
return nil, fmt.Errorf("can't create a secret with namespace '%v' in namespace '%v'", secret.Namespace, s.namespace)
}
result := &api.Secret{}
err := s.client.Post().
Namespace(secret.Namespace).
@ -86,10 +79,6 @@ func (s *secrets) List(label labels.Selector, field fields.Selector) (*api.Secre
// Get returns the given secret, or an error.
func (s *secrets) Get(name string) (*api.Secret, error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result := &api.Secret{}
err := s.client.Get().
Namespace(s.namespace).
@ -124,11 +113,6 @@ func (s *secrets) Delete(name string) error {
func (s *secrets) Update(secret *api.Secret) (result *api.Secret, err error) {
result = &api.Secret{}
if len(secret.ResourceVersion) == 0 {
err = fmt.Errorf("invalid update object, missing resource version: %v", secret)
return
}
err = s.client.Put().
Namespace(s.namespace).
Resource("secrets").

40
pkg/client/services.go
View File

@ -17,8 +17,6 @@ limitations under the License.
package client
import (
"errors"
"fmt"
"time"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@ -71,10 +69,6 @@ func (c *services) List(selector labels.Selector) (result *api.ServiceList, err
// Get returns information about a particular service.
func (c *services) Get(name string) (result *api.Service, err error) {
if len(name) == 0 {
return nil, errors.New("name is required parameter to Get")
}
result = &api.Service{}
err = c.r.Get().Namespace(c.ns).Resource("services").Name(name).Do().Into(result)
return
@ -83,48 +77,20 @@ func (c *services) Get(name string) (result *api.Service, err error) {
// Create creates a new service.
func (c *services) Create(svc *api.Service) (result *api.Service, err error) {
result = &api.Service{}
// v1beta3 does not allow POST without a namespace.
needNamespace := !api.PreV1Beta3(c.r.APIVersion())
namespace := c.ns
if needNamespace && len(namespace) == 0 {
namespace = api.NamespaceDefault
}
request := c.r.Post()
request.Timeout(extendedTimeout)
err = request.Namespace(namespace).Resource("services").Body(svc).Do().Into(result)
err = c.r.Post().Timeout(extendedTimeout).Namespace(c.ns).Resource("services").Body(svc).Do().Into(result)
return
}
// Update updates an existing service.
func (c *services) Update(svc *api.Service) (result *api.Service, err error) {
result = &api.Service{}
if len(svc.ResourceVersion) == 0 {
err = fmt.Errorf("invalid update object, missing resource version: %v", svc)
return
}
// v1beta3 does not allow PUT without a namespace.
needNamespace := !api.PreV1Beta3(c.r.APIVersion())
namespace := c.ns
if needNamespace && len(namespace) == 0 {
namespace = api.NamespaceDefault
}
request := c.r.Put()
request.Timeout(extendedTimeout)
err = request.Namespace(namespace).Resource("services").Name(svc.Name).Body(svc).Do().Into(result)
err = c.r.Put().Timeout(extendedTimeout).Timeout(extendedTimeout).Namespace(c.ns).Resource("services").Name(svc.Name).Body(svc).Do().Into(result)
return
}
// Delete deletes an existing service.
func (c *services) Delete(name string) error {
// v1beta3 does not allow DELETE without a namespace.
needNamespace := !api.PreV1Beta3(c.r.APIVersion())
namespace := c.ns
if needNamespace && len(namespace) == 0 {
namespace = api.NamespaceDefault
}
request := c.r.Delete()
request.Timeout(extendedTimeout)
return request.Namespace(c.ns).Resource("services").Name(name).Do().Error()
return c.r.Delete().Timeout(extendedTimeout).Namespace(c.ns).Resource("services").Name(name).Do().Error()
}
// Watch returns a watch.Interface that watches the requested services.

16
pkg/kubelet/status_manager.go
View File

@ -106,16 +106,26 @@ func (s *statusManager) syncBatch() error {
podFullName := kubecontainer.GetPodFullName(pod)
status := syncRequest.status
_, err := s.kubeClient.Pods(pod.Namespace).UpdateStatus(pod.Name, &status)
var err error
statusPod := &api.Pod{
ObjectMeta: pod.ObjectMeta,
}
// TODO: make me easier to express from client code
if statusPod, err = s.kubeClient.Pods(statusPod.Namespace).Get(statusPod.Name); err == nil {
statusPod.Status = status
}
if err == nil {
statusPod, err = s.kubeClient.Pods(pod.Namespace).UpdateStatus(statusPod)
// TODO: handle conflict as a retry, make that easier too.
}
if err != nil {
// We failed to update status. In order to make sure we retry next time
// we delete cached value. This may result in an additional update, but
// this is ok.
s.DeletePodStatus(podFullName)
return fmt.Errorf("error updating status for pod %q: %v", pod.Name, err)
} else {
glog.V(3).Infof("Status for pod %q updated successfully", pod.Name)
}
glog.V(3).Infof("Status for pod %q updated successfully", pod.Name)
return nil
}

2
pkg/kubelet/status_manager_test.go
View File

@ -110,5 +110,5 @@ func TestSyncBatch(t *testing.T) {
if err != nil {
t.Errorf("unexpected syncing error: %v", err)
}
verifyActions(t, syncer.kubeClient, []string{"update-status-pod"})
verifyActions(t, syncer.kubeClient, []string{"get-pod", "update-status-pod"})
}

2
pkg/resourcequota/resource_quota_controller.go
View File

@ -208,7 +208,7 @@ func (rm *ResourceQuotaManager) syncResourceQuota(quota api.ResourceQuota) (err
// update the usage only if it changed
if dirty {
_, err = rm.kubeClient.ResourceQuotas(usage.Namespace).Status(&usage)
_, err = rm.kubeClient.ResourceQuotas(usage.Namespace).UpdateStatus(&usage)
return err
}
return nil

2
plugin/pkg/admission/resourcequota/admission.go
View File

@ -123,7 +123,7 @@ func (q *quota) Admit(a admission.Attributes) (err error) {
Annotations: quota.Annotations},
}
usage.Status = *status
_, err = q.client.ResourceQuotas(usage.Namespace).Status(&usage)
_, err = q.client.ResourceQuotas(usage.Namespace).UpdateStatus(&usage)
if err != nil {
return apierrors.NewForbidden(a.GetResource(), name, fmt.Errorf("Unable to %s %s at this time because there was an error enforcing quota", a.GetOperation(), a.GetResource()))
}