depreacte post cycle from scheduling framework #1392
Conversation
Signed-off-by: Nir Rozenbaum <[email protected]>
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nirrozenbaum 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
k8s-ci-robot
added
the
size/L
|
cc @liu-cong |
Signed-off-by: Nir Rozenbaum <[email protected]>
Signed-off-by: Nir Rozenbaum <[email protected]>
| primaryProfileResult := schedulingResult.ProfileResults[schedulingResult.PrimaryProfileName] | ||
| targetPod := primaryProfileResult.TargetPods[0].GetPod() // get the first pod of the primary profile | ||
|
|
||
| state, err := plugins.ReadPluginStateKey[*SchedulingContextState](p.pluginState, request.RequestId, PrefixCachePluginType) |
There was a problem hiding this comment.
We should also clean up the per-request state after reading, right?
There was a problem hiding this comment.
yes, fixed it. good catch!
my original intention was to do explicitly delete on successful request handling and let the cleanup go routine clean entries only of requests that failed in the middle.
I think it would be useful to add a generic ReadAndDeletePluginStateKey function to avoid this confusion, but will leave that for a follow up to keep this PR tightly scoped on the PostCycle deprecation.
Signed-off-by: Nir Rozenbaum <[email protected]>
|
/lgtm |
k8s-ci-robot
added
the
lgtm
3 participants
This PR deprecates PostCycle extension point from scheduling framework.
instead of calling PostCycle in prefix plugin, this was changed to PreRequest.
Additionally, the use of CycleState was replaced with PluginState, since PreRequest is not part of the scheduling cycle and therefore the state cannot be shared between the extension point with that mechanism.
tests were updated accordingly.
please pay attention that I do plan on having all the extension points defined in the same plugins package, which is currently not the case (I don't like the fact that prefix plugin imports requestcontrol).
due to the fact that a release is close, this change will be pushed at the beginning of the next release cycle (after first RC of current release) to avoid big changes in plugins before the release.