Skip to content

Stable sort for RuntimeHandlers#12

Merged
EricMountain merged 1 commit into
v2.1.0-ddfrom
em/stable_runtime_handlers
Jun 27, 2025
Merged

Stable sort for RuntimeHandlers#12
EricMountain merged 1 commit into
v2.1.0-ddfrom
em/stable_runtime_handlers

Conversation

@EricMountain
Copy link
Copy Markdown
Member

@EricMountain EricMountain commented Jun 27, 2025

The runtimeHandlers list in the response to crictl info has unstable ordering
since commit 97eb1cd that uses a map instead of a list.

This causes the kubelet to update node status subresources every few seconds
leading to excessive API server load.

This change enforces stable ordering based on the runtime handler name.

The runtimeHandlers list in the response to `crictl info` has unstable ordering
since commit 97eb1cd that uses a map instead of a list.

This causes the kubelet to update node status subresources every few seconds
leading to excessive API server load.

This change enforces stable ordering based on the runtime handler name.
@EricMountain EricMountain force-pushed the em/stable_runtime_handlers branch from ddce8dd to 5f00061 Compare June 27, 2025 12:15
@EricMountain EricMountain marked this pull request as ready for review June 27, 2025 12:15
}

// Get runtime handlers in a stable order
resp.RuntimeHandlers = slices.Collect(maps.Values(c.runtimeHandlers))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could have a maps.Values that sorts directly.

It's a nit more than anything else

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn’t exist AFAIK. Go intentionally makes maps have unstable ordering, so this seems to be the idiomatic way of doing things.

@nyodas
Copy link
Copy Markdown

nyodas commented Jun 27, 2025

Also don't know if there is one.
But we should probably have a test to check that it stays sorted over time.

Especially in case of a refactor like what just happened.

@EricMountain
Copy link
Copy Markdown
Member Author

Also don't know if there is one. But we should probably have a test to check that it stays sorted over time.

Especially in case of a refactor like what just happened.

I’ll make a test as part of the upstream PR

@EricMountain EricMountain merged commit 39b2cf2 into v2.1.0-dd Jun 27, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants