Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem serializing some Kueue types for logging #4137

Open
gabesaba opened this issue Feb 3, 2025 · 8 comments · May be fixed by #4420
Open

Problem serializing some Kueue types for logging #4137

gabesaba opened this issue Feb 3, 2025 · 8 comments · May be fixed by #4420
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@gabesaba
Copy link
Contributor

gabesaba commented Feb 3, 2025

What happened:
Observing error messages like the following in logs:

"nominatedAssignmentError": "json: unsupported type: resources.FlavorResourceQuantities"
"resourcesError": "json: unsupported type: resources.FlavorResourceQuantities"
"resourcesRequiringPreemptionError": "json: unsupported type: sets.Set[[sigs.k8s.io/kueue/pkg/resources.FlavorResource](http://sigs.k8s.io/kueue/pkg/resources.FlavorResource)]"
"usageError": "json: unsupported type: resources.FlavorResourceQuantities"

here are the lines, corresponding to v0.10.0

cache/snapshot.go:64
cache/snapshot.go:73
preemption/preemption.go:346
scheduler/logging.go:43

What you expected to happen:
These types should be serialized properly

How to reproduce it (as minimally and precisely as possible):
Run integration tests with higher log verbosity, grep for json: unsupported type

Anything else we need to know?:
The scope of this bug includes identifying other types we fail to serialize - the list above is not necessarily exhaustive - and fixing those too

Environment:

  • Kueue version (use git describe --tags --dirty --always): v0.10.0
@gabesaba gabesaba added the kind/bug Categorizes issue or PR as related to a bug. label Feb 3, 2025
@nasedil
Copy link

nasedil commented Feb 26, 2025

/assign

@nasedil
Copy link

nasedil commented Feb 26, 2025

I've changed verbosity for integration tests in Makefile to '-vv' and ran make test-integration | grep "unsupported" but output is empty. Could it be that I search for logs in a wrong place or run the tests incorrectly?

@mimowo
Copy link
Contributor

mimowo commented Feb 26, 2025

Maybe try to adjust log level at the framework level (eg. -3 -> -5):

ctrl.SetLogger(util.NewTestingLogger(ginkgo.GinkgoWriter, -3))

@nasedil
Copy link

nasedil commented Feb 26, 2025

Thanks @mimowo that worked.

I can see only "resources.FlavorResourceQuantities" and "sets.Set[sigs.k8s.io/kueue/pkg/resources.FlavorResource]" appear in logs. In code I see there is no serialization for any types.

For "sets.Set[sigs.k8s.io/kueue/pkg/resources.FlavorResource]" to be serializebale the type k8s.io/apimachinery/pkg/util/sets.Set should be serializable, from the upstream repository: https://github.com/kubernetes/apimachinery/blob/master/pkg/util/sets/set.go

I've created an issue there to sync: kubernetes/apimachinery#187

@mimowo
Copy link
Contributor

mimowo commented Feb 26, 2025

Ok, but until done upstream we can probably just convert the set to slice easily.

EDIT: I other words I think we can just have the String function on our type which will do the custom serialization. I think logger checks on the object if its type implements the custom String.

@nasedil
Copy link

nasedil commented Feb 26, 2025

For example, here is a place where it's logged:
https://github.com/kubernetes-sigs/kueue/blob/main/pkg/scheduler/preemption/preemption.go#L362 — here it unsuccessfully tries to serialize preemptionCtx.frsNeedPreemption

And here is how the preemptionCtx.frsNeedPreemption defined:
https://github.com/kubernetes-sigs/kueue/blob/main/pkg/scheduler/preemption/preemption.go#L73

Do you suggest passing to logger something like preemptionCtx.getFrsNeedPreemptionString() which converts preemptionCtx.frsNeedPreemption to a serializable format?

Ok, but until done upstream we can probably just convert the set to slice easily.

EDIT: I other words I think we can just have the String function on our type which will do the custom serialization. I think logger checks on the object if its type implements the custom String.

@mimowo
Copy link
Contributor

mimowo commented Feb 26, 2025

Do you suggest passing to logger something like preemptionCtx.getFrsNeedPreemptionString() which converts preemptionCtx.frsNeedPreemption to a serializable format?

No, I was thinking to wrap the set into a type with "String" function, something like:

type sets.Set[resources.FlavorResource] FlavorResourceSet

func (s *FlavorResourceSet) String() string {
return "xyz"
}

would this work? I haven't tried, so it might not actually work, dunno.

@tenzen-y
Copy link
Member

Thanks @mimowo that worked.

I can see only "resources.FlavorResourceQuantities" and "sets.Set[sigs.k8s.io/kueue/pkg/resources.FlavorResource]" appear in logs. In code I see there is no serialization for any types.

For "sets.Set[sigs.k8s.io/kueue/pkg/resources.FlavorResource]" to be serializebale the type k8s.io/apimachinery/pkg/util/sets.Set should be serializable, from the upstream repository: https://github.com/kubernetes/apimachinery/blob/master/pkg/util/sets/set.go

I've created an issue there to sync: kubernetes/apimachinery#187

NOTE: The library issue was migrated to kubernetes/kubernetes#130452

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
4 participants