search

kubernetes-sigs / kube-scheduler-wasm-extension

All the things to make the scheduler extendable with wasm.
Apache License 2.0
110 stars 22 forks source link

feature: always assign the same instance to the same Pod #21

Closed sanposhiho closed 1 year ago

sanposhiho commented 1 year ago

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR changes the host to always assign the same guest instance to the same Pod. Two motivations here.

1. support cycle state #16

This change simplifies the problem how to support the cycle state. That's difficult because users can put any data into cycle state, and we don't know what'll be put. After this PR, we don't need to pass the cycle state between host and guest; the guest initializes the cycle state inside of it and uses the same cycle state in the same scheduling.

Obviously, this idea means that users cannot pass the data from one plugin to other plugin through the cycle state, but that usecase is rare. (Actually, AFAIK, no in-tree plugins do that) Not saying we never support it in the future though, I'd say we don't need to prioritize it.

2. cache resource data

Thanks to the snapshot, the NodeInfo and Pod passed to each extension point aren't get changed during the same scheduling cycle. We don't need to pass them to guest every time we call the guest. The guest can cache them during the same scheduling cycle.

Which issue(s) this PR fixes:

Part of #16

Special notes for your reviewer:

Detailed description: https://github.com/kubernetes-sigs/kube-scheduler-wasm-extension/issues/16#issuecomment-1566352602

Does this PR introduce a user-facing change?

NONE
sanposhiho commented 1 year ago

cc @codefromthecrypt

kerthcet commented 1 year ago

pass the data from one plugin to other plugin

I heard of such case in the community meeting for performance, but I can't recall it. I think it's ok because all the plugins up-to-now are independent, that's why we need plugin.

sanposhiho commented 1 year ago

just rebased.

sanposhiho commented 1 year ago

pass the data from one plugin to other plugin

Actually my company also does that actually. 😅 But, yes we could say that's very minor usecase that we can give a lower priority.

kerthcet commented 1 year ago

Actually my company also does that actually.

What's your scenario?

sanposhiho commented 1 year ago

What's your scenario?

We have a plugin very similar to coscheduling. And we want to notify the post filter plugin (which is also a custom one.) if the Pod during scheduling is the first Pod among the pod group.

sanposhiho commented 1 year ago

@codefromthecrypt Thanks, this one right? Sorry forgot to reply, but cookie design looks very good.

codefromthecrypt commented 1 year ago

Thanks, https://github.com/kubernetes-sigs/kube-scheduler-wasm-extension/pull/21#pullrequestreview-1448527569 right? Sorry forgot to reply, but cookie design looks very good.

yes except maybe I will call it etag not cookie. In any case we need to extract or derive 32-bits from the UUID.

sanposhiho commented 1 year ago

rebase + have a change for feedback from @kerthcet

sanposhiho commented 1 year ago

rebased

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: codefromthecrypt, sanposhiho

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/kube-scheduler-wasm-extension/blob/main/OWNERS)~~ [sanposhiho] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
kerthcet commented 1 year ago

/lgtm