From 900457c09bac0023e5e3454f6ace0105f233c99c Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 25 Jun 2024 15:36:23 +0200 Subject: [PATCH 1/3] ktesting: improve per-test log output format.Object adds some white space in front of the value and a type identifier in angle brackets. Both is distracting when printing simple values and can be avoided by picking fmt.Sprintf for those types, plus trimming the result of format.Object. Before: allocator.go:483: I0625 15:35:31.946980] Allocating one device currentClaim= : 0 totalClaims= : 1 currentRequest= : 0 totalRequestsPerClaim= : 1 currentDevice= : 0 devicesPerRequest= : 1 allDevices= : false adminAccess= : false After: allocator.go:483: I0625 15:35:04.371441] Allocating one device currentClaim=0 totalClaims=1 currentRequest=0 totalRequestsPerClaim=1 currentDevice=0 devicesPerRequest=1 allDevices=false adminAccess=false --- test/utils/ktesting/tcontext.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/utils/ktesting/tcontext.go b/test/utils/ktesting/tcontext.go index 055ced48a87..277a739e420 100644 --- a/test/utils/ktesting/tcontext.go +++ b/test/utils/ktesting/tcontext.go @@ -20,6 +20,7 @@ import ( "context" "flag" "fmt" + "strings" "time" "github.com/onsi/gomega" @@ -241,7 +242,19 @@ func Init(tb TB, opts ...InitOption) TContext { if c.PerTestOutput { config := ktesting.NewConfig( ktesting.AnyToString(func(v interface{}) string { - return format.Object(v, 1) + // For basic types where the string + // representation is "obvious" we use + // fmt.Sprintf because format.Object always + // adds a <"type"> prefix, which is too long + // for simple values. + switch v := v.(type) { + case int, int32, int64, uint, uint32, uint64, float32, float64, bool: + return fmt.Sprintf("%v", v) + case string: + return v + default: + return strings.TrimSpace(format.Object(v, 1)) + } }), ktesting.VerbosityFlagName("v"), ktesting.VModuleFlagName("vmodule"), From 7f87629a3fcad5891d5f0570f41917ccd25fd13f Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 25 Jun 2024 15:53:01 +0200 Subject: [PATCH 2/3] DRA e2e: fix error reporting in test driver Dropping the error that is returned by allocateOne hides the reason *why* allocation failed. Including the UID is "too much information" for an error message (usually the user doesn't care about the exact identity, just the name) and the claim name can and will be added by the caller. Before: controller.go:373: E0625 16:04:12.140953] test-driver.cdi.k8s.io/resource controller: processing failed err="claim test-dramq9jv-resource-h72pg: failed allocating claim 8551afba-3c9a-4a8a-8633-6fad6c4b9e42" key="schedulingCtx:test/test-dramq9jv" event.go:377: I0625 16:04:12.141031] test-driver.cdi.k8s.io/resource controller: Event(v1.ObjectReference{Kind:"PodSchedulingContext", Namespace:"test", Name:"test-dra65gfw", UID:"6be9ba57-31da-4fef-b61d-b0468d71afcf", APIVersion:"resource.k8s.io/v1alpha3", ResourceVersion:"197", FieldPath:""}): type: 'Warning' reason: 'Failed' claim test-dra65gfw-resource-zpzrj: failed allocating claim f98a32e1-ab7d-4b34-a258-6d8224aa9006 After: controller.go:373: E0625 16:02:54.248059] test-driver.cdi.k8s.io/resource controller: processing failed err="claim test-dram98ll-resource-nvsbj: device selectors are not supported" key="schedulingCtx:test/test-dram98ll" event.go:377: I0625 16:02:54.248163] test-driver.cdi.k8s.io/resource controller: Event(v1.ObjectReference{Kind:"PodSchedulingContext", Namespace:"test", Name:"test-dratpt77", UID:"24010402-b026-4fe4-a535-e1dab69db8c0", APIVersion:"resource.k8s.io/v1alpha3", ResourceVersion:"298", FieldPath:""}): type: 'Warning' reason: 'Failed' claim test-dratpt77-resource-vlgrv: device selectors are not supported --- test/e2e/dra/test-driver/app/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/dra/test-driver/app/controller.go b/test/e2e/dra/test-driver/app/controller.go index c647796678f..e094ac7894f 100644 --- a/test/e2e/dra/test-driver/app/controller.go +++ b/test/e2e/dra/test-driver/app/controller.go @@ -241,7 +241,7 @@ func (c *ExampleController) allocateOneByOne(ctx context.Context, claimAllocatio for _, ca := range claimAllocations { allocationResult, err := c.allocateOne(ctx, ca.Claim, ca.ClaimParameters, ca.Class, ca.ClassParameters, selectedNode) if err != nil { - ca.Error = fmt.Errorf("failed allocating claim %v", ca.Claim.UID) + ca.Error = err continue } ca.Allocation = allocationResult From 2da9e660e3b4227a6f97de58a44b9a88c2fe3d40 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Sat, 15 Jun 2024 14:28:27 +0200 Subject: [PATCH 3/3] resourceclaim controller: add missing log output The logging was fairly complete about *not* doing something, but the actual ResourceClaim creation was not logged. --- pkg/controller/resourceclaim/controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/resourceclaim/controller.go b/pkg/controller/resourceclaim/controller.go index e8ca41e973b..9c27ca7ab24 100644 --- a/pkg/controller/resourceclaim/controller.go +++ b/pkg/controller/resourceclaim/controller.go @@ -657,6 +657,7 @@ func (ec *Controller) handleClaim(ctx context.Context, pod *v1.Pod, podClaim v1. metrics.ResourceClaimCreateFailures.Inc() return fmt.Errorf("create ResourceClaim %s: %v", claimName, err) } + logger.V(4).Info("Created ResourceClaim", "claim", klog.KObj(claim), "pod", klog.KObj(pod)) ec.claimCache.Mutation(claim) }