diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 7fa4eb001ff..4bde732fbb9 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -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 diff --git a/pkg/client/endpoints.go b/pkg/client/endpoints.go index 7a2406bf188..7679d162b7d 100644 --- a/pkg/client/endpoints.go +++ b/pkg/client/endpoints.go @@ -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 diff --git a/pkg/client/events.go b/pkg/client/events.go index a40a87991a2..3ffd2bfb7b1 100644 --- a/pkg/client/events.go +++ b/pkg/client/events.go @@ -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). diff --git a/pkg/client/fake_pods.go b/pkg/client/fake_pods.go index 970904cca27..56f35019213 100644 --- a/pkg/client/fake_pods.go +++ b/pkg/client/fake_pods.go @@ -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 } diff --git a/pkg/client/fake_resource_quotas.go b/pkg/client/fake_resource_quotas.go index 5024ec47560..f9546bea895 100644 --- a/pkg/client/fake_resource_quotas.go +++ b/pkg/client/fake_resource_quotas.go @@ -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 diff --git a/pkg/client/limit_ranges.go b/pkg/client/limit_ranges.go index d234643a1b9..5792d272adb 100644 --- a/pkg/client/limit_ranges.go +++ b/pkg/client/limit_ranges.go @@ -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 diff --git a/pkg/client/minions.go b/pkg/client/minions.go index eb257444090..6d42fa3150f 100644 --- a/pkg/client/minions.go +++ b/pkg/client/minions.go @@ -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 diff --git a/pkg/client/namespaces.go b/pkg/client/namespaces.go index 04ad150a3d3..7e5c377545f 100644 --- a/pkg/client/namespaces.go +++ b/pkg/client/namespaces.go @@ -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 diff --git a/pkg/client/pods.go b/pkg/client/pods.go index fc50cbc65f7..42e5f920c83 100644 --- a/pkg/client/pods.go +++ b/pkg/client/pods.go @@ -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 } diff --git a/pkg/client/replication_controllers.go b/pkg/client/replication_controllers.go index cefbe61eaf4..dbcf28d4153 100644 --- a/pkg/client/replication_controllers.go +++ b/pkg/client/replication_controllers.go @@ -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 } diff --git a/pkg/client/request.go b/pkg/client/request.go index 5b94e979e2f..58a1a8477e9 100644 --- a/pkg/client/request.go +++ b/pkg/client/request.go @@ -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 diff --git a/pkg/client/resource_quotas.go b/pkg/client/resource_quotas.go index 891989f2d82..740a8282e66 100644 --- a/pkg/client/resource_quotas.go +++ b/pkg/client/resource_quotas.go @@ -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 } diff --git a/pkg/client/resource_quotas_test.go b/pkg/client/resource_quotas_test.go index 7d77a809c25..ff0d1978cfb 100644 --- a/pkg/client/resource_quotas_test.go +++ b/pkg/client/resource_quotas_test.go @@ -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{ diff --git a/pkg/client/secrets.go b/pkg/client/secrets.go index 65819064838..710704ef2f2 100644 --- a/pkg/client/secrets.go +++ b/pkg/client/secrets.go @@ -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"). diff --git a/pkg/client/services.go b/pkg/client/services.go index b75172dc812..416dff781a9 100644 --- a/pkg/client/services.go +++ b/pkg/client/services.go @@ -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. diff --git a/pkg/kubelet/status_manager.go b/pkg/kubelet/status_manager.go index 3065a79ec14..2d8f4449c6c 100644 --- a/pkg/kubelet/status_manager.go +++ b/pkg/kubelet/status_manager.go @@ -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 } diff --git a/pkg/kubelet/status_manager_test.go b/pkg/kubelet/status_manager_test.go index ced98525bb4..41ffe00cf07 100644 --- a/pkg/kubelet/status_manager_test.go +++ b/pkg/kubelet/status_manager_test.go @@ -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"}) } diff --git a/pkg/resourcequota/resource_quota_controller.go b/pkg/resourcequota/resource_quota_controller.go index 2bc96fa5c9b..93006679b88 100644 --- a/pkg/resourcequota/resource_quota_controller.go +++ b/pkg/resourcequota/resource_quota_controller.go @@ -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 diff --git a/plugin/pkg/admission/resourcequota/admission.go b/plugin/pkg/admission/resourcequota/admission.go index b2e5d611c31..c4dacafe288 100644 --- a/plugin/pkg/admission/resourcequota/admission.go +++ b/plugin/pkg/admission/resourcequota/admission.go @@ -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())) }