bug: StatefulSets are not reevaluated after rescheduling #67
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
First commit is #66 and will be removed once that PR is merged. |
carlosrejano
approved these changes
Jan 4, 2024
joaoqalves
requested changes
Jan 8, 2024
# Context Noe caches the Pods it has already processed to avoid reprocessing them unless there are changes. We do this by storing the `NamespacedName` of the Pod, which includes the namespace and Pod name. For Deployments, rescheduling the Pod actually changes its name as the termination is generated. For StatefulSets, the name is kept the same no matter how many reschedules happen. Our caching approach prevents us form reevaluating the location of StatefulSet pods if they were previously scheduled, and we've seen this allocate StatefulSet pods to nodes whose architectures where not supported by the Pod. # Bugfix approach The intention of the cache is to reduce load over Registries and Noe itself. In our case so far StatefulSets are much less common than Deployments, so we choose to avoid the cache for StatefulSets. This is a stopgap measure to prevent misscheduling of current workloads until a better approach to prevent reprocessing of Pods can be devised that supports StatefulSets. # What we change We add a `skipCache(*v1.Pod)` function that checks if the Pod is owned by a StatefulSet. If so, the caching mechanism is skipped for this workload. Change-Id: Ie7bdf2d863f433e7998b1b92cea129c6bda4dd8f
miguelbernadi
force-pushed
the
fix-statefulset-reschedule-bug
branch
from
c5e2458 to
33ae035
Compare
Co-authored-by: João Alves <[email protected]>
joaoqalves
approved these changes
Jan 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
Noe caches the Pods it has already processed to avoid reprocessing
them unless there are changes. We do this by storing the
NamespacedNameof the Pod, which includes the namespace and Podname.
For Deployments, rescheduling the Pod actually changes its name as the
termination is generated. For StatefulSets, the name is kept the same
no matter how many reschedules happen.
Our caching approach prevents us form reevaluating the location of
StatefulSet pods if they were previously scheduled, and we've seen
this allocate StatefulSet pods to nodes whose architectures where not
supported by the Pod.
Bugfix approach
The intention of the cache is to reduce load over Registries and Noe
itself. In our case so far StatefulSets are much less common than
Deployments, so we choose to avoid the cache for StatefulSets.
This is a stopgap measure to prevent misscheduling of current
workloads until a better approach to prevent reprocessing of Pods can
be devised that supports StatefulSets.
What we change
We add a
skipCache(*v1.Pod)function that checks if the Pod is ownedby a StatefulSet. If so, the caching mechanism is skipped for this
workload.