Improve error message when name is omitted but generateName is available

This commit is contained in:
derekwaynecarr
2015-04-14 18:03:26 -04:00
parent 4d3b66c09f
commit 81dcd8c836
17 changed files with 184 additions and 81 deletions

View File

@@ -21,7 +21,6 @@ import (
"io"
"github.com/GoogleCloudPlatform/kubernetes/pkg/admission"
apierrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
)
@@ -36,7 +35,7 @@ func init() {
type alwaysDeny struct{}
func (alwaysDeny) Admit(a admission.Attributes) (err error) {
return apierrors.NewForbidden(a.GetResource(), "", errors.New("Admission control is denying all modifications"))
return admission.NewForbidden(a, errors.New("Admission control is denying all modifications"))
}
func NewAlwaysDeny() admission.Interface {

View File

@@ -24,7 +24,7 @@ import (
func TestAdmission(t *testing.T) {
handler := NewAlwaysDeny()
err := handler.Admit(admission.NewAttributesRecord(nil, "foo", "Pod", "ignored"))
err := handler.Admit(admission.NewAttributesRecord(nil, "Pod", "foo", "Pod", "ignored"))
if err == nil {
t.Errorf("Expected error returned from admission handler")
}

View File

@@ -22,7 +22,6 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/admission"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
apierrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource"
"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
@@ -58,6 +57,9 @@ func (l *limitRanger) Admit(a admission.Attributes) (err error) {
name := "Unknown"
if obj != nil {
name, _ = meta.NewAccessor().Name(obj)
if len(name) == 0 {
name, _ = meta.NewAccessor().GenerateName(obj)
}
}
key := &api.LimitRange{
@@ -68,7 +70,7 @@ func (l *limitRanger) Admit(a admission.Attributes) (err error) {
}
items, err := l.indexer.Index("namespace", key)
if err != nil {
return apierrors.NewForbidden(a.GetResource(), name, fmt.Errorf("Unable to %s %s at this time because there was an error enforcing limit ranges", a.GetOperation(), resource))
return admission.NewForbidden(a, fmt.Errorf("Unable to %s %s at this time because there was an error enforcing limit ranges", a.GetOperation(), resource))
}
if len(items) == 0 {
return nil
@@ -79,7 +81,7 @@ func (l *limitRanger) Admit(a admission.Attributes) (err error) {
limitRange := items[i].(*api.LimitRange)
err = l.limitFunc(limitRange, a.GetResource(), a.GetObject())
if err != nil {
return err
return admission.NewForbidden(a, err)
}
}
return nil
@@ -250,11 +252,11 @@ func PodLimitFunc(limitRange *api.LimitRange, pod *api.Pod) error {
switch minOrMax {
case "Min":
if observed < enforced {
return apierrors.NewForbidden("pods", pod.Name, err)
return err
}
case "Max":
if observed > enforced {
return apierrors.NewForbidden("pods", pod.Name, err)
return err
}
}
}

View File

@@ -53,11 +53,11 @@ func (p *provision) Admit(a admission.Attributes) (err error) {
}
defaultVersion, kind, err := latest.RESTMapper.VersionAndKindForResource(a.GetResource())
if err != nil {
return err
return admission.NewForbidden(a, err)
}
mapping, err := latest.RESTMapper.RESTMapping(kind, defaultVersion)
if err != nil {
return err
return admission.NewForbidden(a, err)
}
if mapping.Scope.Name() != meta.RESTScopeNameNamespace {
return nil
@@ -71,14 +71,14 @@ func (p *provision) Admit(a admission.Attributes) (err error) {
}
_, exists, err := p.store.Get(namespace)
if err != nil {
return err
return admission.NewForbidden(a, err)
}
if exists {
return nil
}
_, err = p.client.Namespaces().Create(namespace)
if err != nil && !errors.IsAlreadyExists(err) {
return err
return admission.NewForbidden(a, err)
}
return nil
}

View File

@@ -41,7 +41,7 @@ func TestAdmission(t *testing.T) {
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, namespace, "pods", "CREATE"))
err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", "CREATE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
@@ -72,7 +72,7 @@ func TestAdmissionNamespaceExists(t *testing.T) {
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, namespace, "pods", "CREATE"))
err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", "CREATE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
@@ -96,7 +96,7 @@ func TestIgnoreAdmission(t *testing.T) {
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, namespace, "pods", "UPDATE"))
err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", "UPDATE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
@@ -123,7 +123,7 @@ func TestAdmissionNamespaceExistsUnknownToHandler(t *testing.T) {
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, namespace, "pods", "CREATE"))
err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", "CREATE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}

View File

@@ -22,7 +22,6 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/admission"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
apierrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
@@ -50,11 +49,11 @@ type exists struct {
func (e *exists) Admit(a admission.Attributes) (err error) {
defaultVersion, kind, err := latest.RESTMapper.VersionAndKindForResource(a.GetResource())
if err != nil {
return err
return admission.NewForbidden(a, err)
}
mapping, err := latest.RESTMapper.RESTMapping(kind, defaultVersion)
if err != nil {
return err
return admission.NewForbidden(a, err)
}
if mapping.Scope.Name() != meta.RESTScopeNameNamespace {
return nil
@@ -68,17 +67,12 @@ func (e *exists) Admit(a admission.Attributes) (err error) {
}
_, exists, err := e.store.Get(namespace)
if err != nil {
return err
return admission.NewForbidden(a, err)
}
if exists {
return nil
}
obj := a.GetObject()
name := "Unknown"
if obj != nil {
name, _ = meta.NewAccessor().Name(obj)
}
return apierrors.NewForbidden(kind, name, fmt.Errorf("Namespace %s does not exist", a.GetNamespace()))
return admission.NewForbidden(a, fmt.Errorf("Namespace %s does not exist", a.GetNamespace()))
}
func NewExists(c client.Interface) admission.Interface {

View File

@@ -22,7 +22,6 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/admission"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
apierrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
@@ -52,11 +51,11 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) {
}
defaultVersion, kind, err := latest.RESTMapper.VersionAndKindForResource(a.GetResource())
if err != nil {
return err
return admission.NewForbidden(a, err)
}
mapping, err := latest.RESTMapper.RESTMapping(kind, defaultVersion)
if err != nil {
return err
return admission.NewForbidden(a, err)
}
if mapping.Scope.Name() != meta.RESTScopeNameNamespace {
return nil
@@ -68,7 +67,7 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) {
},
})
if err != nil {
return err
return admission.NewForbidden(a, err)
}
if !exists {
return nil
@@ -78,12 +77,7 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) {
return nil
}
name := "Unknown"
obj := a.GetObject()
if obj != nil {
name, _ = meta.NewAccessor().Name(obj)
}
return apierrors.NewForbidden(kind, name, fmt.Errorf("Namespace %s is terminating", a.GetNamespace()))
return admission.NewForbidden(a, fmt.Errorf("Namespace %s is terminating", a.GetNamespace()))
}
func NewLifecycle(c client.Interface) admission.Interface {

View File

@@ -50,7 +50,7 @@ func TestAdmission(t *testing.T) {
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, namespaceObj.Namespace, "pods", "CREATE"))
err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespaceObj.Namespace, "pods", "CREATE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler: %v", err)
}
@@ -60,19 +60,19 @@ func TestAdmission(t *testing.T) {
store.Add(namespaceObj)
// verify create operations in the namespace cause an error
err = handler.Admit(admission.NewAttributesRecord(&pod, namespaceObj.Namespace, "pods", "CREATE"))
err = handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespaceObj.Namespace, "pods", "CREATE"))
if err == nil {
t.Errorf("Expected error rejecting creates in a namespace when it is terminating")
}
// verify update operations in the namespace can proceed
err = handler.Admit(admission.NewAttributesRecord(&pod, namespaceObj.Namespace, "pods", "UPDATE"))
err = handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespaceObj.Namespace, "pods", "UPDATE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler: %v", err)
}
// verify delete operations in the namespace can proceed
err = handler.Admit(admission.NewAttributesRecord(nil, namespaceObj.Namespace, "pods", "DELETE"))
err = handler.Admit(admission.NewAttributesRecord(nil, "Pod", namespaceObj.Namespace, "pods", "DELETE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler: %v", err)
}

View File

@@ -22,8 +22,6 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/admission"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
apierrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource"
"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
"github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache"
@@ -71,13 +69,6 @@ func (q *quota) Admit(a admission.Attributes) (err error) {
return nil
}
obj := a.GetObject()
resource := a.GetResource()
name := "Unknown"
if obj != nil {
name, _ = meta.NewAccessor().Name(obj)
}
key := &api.ResourceQuota{
ObjectMeta: api.ObjectMeta{
Namespace: a.GetNamespace(),
@@ -86,7 +77,7 @@ func (q *quota) Admit(a admission.Attributes) (err error) {
}
items, err := q.indexer.Index("namespace", key)
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(), resource))
return admission.NewForbidden(a, fmt.Errorf("Unable to %s %s at this time because there was an error enforcing quota", a.GetOperation(), a.GetResource()))
}
if len(items) == 0 {
return nil
@@ -109,7 +100,7 @@ func (q *quota) Admit(a admission.Attributes) (err error) {
dirty, err := IncrementUsage(a, status, q.client)
if err != nil {
return err
return admission.NewForbidden(a, err)
}
if dirty {
@@ -125,7 +116,7 @@ func (q *quota) Admit(a admission.Attributes) (err error) {
usage.Status = *status
_, 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()))
return admission.NewForbidden(a, fmt.Errorf("Unable to %s %s at this time because there was an error enforcing quota", a.GetOperation(), a.GetResource()))
}
}
}
@@ -136,17 +127,12 @@ func (q *quota) Admit(a admission.Attributes) (err error) {
// Return true if the usage must be recorded prior to admitting the new resource
// Return an error if the operation should not pass admission control
func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, client client.Interface) (bool, error) {
obj := a.GetObject()
resourceName := a.GetResource()
name := "Unknown"
if obj != nil {
name, _ = meta.NewAccessor().Name(obj)
}
dirty := false
set := map[api.ResourceName]bool{}
for k := range status.Hard {
set[k] = true
}
obj := a.GetObject()
// handle max counts for each kind of resource (pods, services, replicationControllers, etc.)
if a.GetOperation() == "CREATE" {
resourceName := resourceToResourceName[a.GetResource()]
@@ -154,10 +140,10 @@ func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, cli
if hardFound {
used, usedFound := status.Used[resourceName]
if !usedFound {
return false, apierrors.NewForbidden(a.GetResource(), name, fmt.Errorf("Quota usage stats are not yet known, unable to admit resource until an accurate count is completed."))
return false, fmt.Errorf("Quota usage stats are not yet known, unable to admit resource until an accurate count is completed.")
}
if used.Value() >= hard.Value() {
return false, apierrors.NewForbidden(a.GetResource(), name, fmt.Errorf("Limited to %s %s", hard.String(), a.GetResource()))
return false, fmt.Errorf("Limited to %s %s", hard.String(), resourceName)
} else {
status.Used[resourceName] = *resource.NewQuantity(used.Value()+int64(1), resource.DecimalSI)
dirty = true
@@ -173,7 +159,7 @@ func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, cli
if a.GetOperation() == "UPDATE" {
oldPod, err := client.Pods(a.GetNamespace()).Get(pod.Name)
if err != nil {
return false, apierrors.NewForbidden(resourceName, name, err)
return false, err
}
oldCPU := resourcequota.PodCPU(oldPod)
oldMemory := resourcequota.PodMemory(oldPod)
@@ -185,10 +171,10 @@ func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, cli
if hardMemFound {
used, usedFound := status.Used[api.ResourceMemory]
if !usedFound {
return false, apierrors.NewForbidden(resourceName, name, fmt.Errorf("Quota usage stats are not yet known, unable to admit resource until an accurate count is completed."))
return false, fmt.Errorf("Quota usage stats are not yet known, unable to admit resource until an accurate count is completed.")
}
if used.Value()+deltaMemory.Value() > hardMem.Value() {
return false, apierrors.NewForbidden(resourceName, name, fmt.Errorf("Limited to %s memory", hardMem.String()))
return false, fmt.Errorf("Limited to %s memory", hardMem.String())
} else {
status.Used[api.ResourceMemory] = *resource.NewQuantity(used.Value()+deltaMemory.Value(), resource.DecimalSI)
dirty = true
@@ -198,10 +184,10 @@ func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, cli
if hardCPUFound {
used, usedFound := status.Used[api.ResourceCPU]
if !usedFound {
return false, apierrors.NewForbidden(resourceName, name, fmt.Errorf("Quota usage stats are not yet known, unable to admit resource until an accurate count is completed."))
return false, fmt.Errorf("Quota usage stats are not yet known, unable to admit resource until an accurate count is completed.")
}
if used.MilliValue()+deltaCPU.MilliValue() > hardCPU.MilliValue() {
return false, apierrors.NewForbidden(resourceName, name, fmt.Errorf("Limited to %s CPU", hardCPU.String()))
return false, fmt.Errorf("Limited to %s CPU", hardCPU.String())
} else {
status.Used[api.ResourceCPU] = *resource.NewMilliQuantity(used.MilliValue()+deltaCPU.MilliValue(), resource.DecimalSI)
dirty = true

View File

@@ -41,7 +41,7 @@ func getResourceRequirements(cpu, memory string) api.ResourceRequirements {
func TestAdmissionIgnoresDelete(t *testing.T) {
namespace := "default"
handler := NewResourceQuota(&testclient.Fake{})
err := handler.Admit(admission.NewAttributesRecord(nil, namespace, "pods", "DELETE"))
err := handler.Admit(admission.NewAttributesRecord(nil, "Pod", namespace, "pods", "DELETE"))
if err != nil {
t.Errorf("ResourceQuota should admit all deletes", err)
}
@@ -67,7 +67,7 @@ func TestIncrementUsagePods(t *testing.T) {
r := api.ResourcePods
status.Hard[r] = resource.MustParse("2")
status.Used[r] = resource.MustParse("1")
dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, namespace, "pods", "CREATE"), status, client)
dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", namespace, "pods", "CREATE"), status, client)
if err != nil {
t.Errorf("Unexpected error", err)
}
@@ -107,7 +107,7 @@ func TestIncrementUsageMemory(t *testing.T) {
Volumes: []api.Volume{{Name: "vol"}},
Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}},
}}
dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, namespace, "pods", "CREATE"), status, client)
dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE"), status, client)
if err != nil {
t.Errorf("Unexpected error", err)
}
@@ -148,7 +148,7 @@ func TestExceedUsageMemory(t *testing.T) {
Volumes: []api.Volume{{Name: "vol"}},
Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "3Gi")}},
}}
_, err := IncrementUsage(admission.NewAttributesRecord(newPod, namespace, "pods", "CREATE"), status, client)
_, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE"), status, client)
if err == nil {
t.Errorf("Expected memory usage exceeded error")
}
@@ -181,7 +181,7 @@ func TestIncrementUsageCPU(t *testing.T) {
Volumes: []api.Volume{{Name: "vol"}},
Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}},
}}
dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, namespace, "pods", "CREATE"), status, client)
dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE"), status, client)
if err != nil {
t.Errorf("Unexpected error", err)
}
@@ -222,7 +222,7 @@ func TestExceedUsageCPU(t *testing.T) {
Volumes: []api.Volume{{Name: "vol"}},
Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("500m", "1Gi")}},
}}
_, err := IncrementUsage(admission.NewAttributesRecord(newPod, namespace, "pods", "CREATE"), status, client)
_, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE"), status, client)
if err == nil {
t.Errorf("Expected CPU usage exceeded error")
}
@@ -248,7 +248,7 @@ func TestExceedUsagePods(t *testing.T) {
r := api.ResourcePods
status.Hard[r] = resource.MustParse("1")
status.Used[r] = resource.MustParse("1")
_, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, namespace, "pods", "CREATE"), status, client)
_, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", namespace, "pods", "CREATE"), status, client)
if err == nil {
t.Errorf("Expected error because this would exceed your quota")
}
@@ -270,7 +270,7 @@ func TestIncrementUsageServices(t *testing.T) {
r := api.ResourceServices
status.Hard[r] = resource.MustParse("2")
status.Used[r] = resource.MustParse("1")
dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Service{}, namespace, "services", "CREATE"), status, client)
dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Service{}, "Service", namespace, "services", "CREATE"), status, client)
if err != nil {
t.Errorf("Unexpected error", err)
}
@@ -299,7 +299,7 @@ func TestExceedUsageServices(t *testing.T) {
r := api.ResourceServices
status.Hard[r] = resource.MustParse("1")
status.Used[r] = resource.MustParse("1")
_, err := IncrementUsage(admission.NewAttributesRecord(&api.Service{}, namespace, "services", "CREATE"), status, client)
_, err := IncrementUsage(admission.NewAttributesRecord(&api.Service{}, "Service", namespace, "services", "CREATE"), status, client)
if err == nil {
t.Errorf("Expected error because this would exceed usage")
}
@@ -321,7 +321,7 @@ func TestIncrementUsageReplicationControllers(t *testing.T) {
r := api.ResourceReplicationControllers
status.Hard[r] = resource.MustParse("2")
status.Used[r] = resource.MustParse("1")
dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.ReplicationController{}, namespace, "replicationControllers", "CREATE"), status, client)
dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.ReplicationController{}, "ReplicationController", namespace, "replicationControllers", "CREATE"), status, client)
if err != nil {
t.Errorf("Unexpected error", err)
}
@@ -350,7 +350,7 @@ func TestExceedUsageReplicationControllers(t *testing.T) {
r := api.ResourceReplicationControllers
status.Hard[r] = resource.MustParse("1")
status.Used[r] = resource.MustParse("1")
_, err := IncrementUsage(admission.NewAttributesRecord(&api.ReplicationController{}, namespace, "replicationControllers", "CREATE"), status, client)
_, err := IncrementUsage(admission.NewAttributesRecord(&api.ReplicationController{}, "ReplicationController", namespace, "replicationControllers", "CREATE"), status, client)
if err == nil {
t.Errorf("Expected error for exceeding hard limits")
}