[YUNIKORN-2083] Shim: Handle missing applicationID cleanly in standard mode #708
Conversation
| @@ -73,19 +74,24 @@ const ( | |||
| CMKubeQPS = PrefixKubernetes + "qps" | |||
| CMKubeBurst = PrefixKubernetes + "burst" | |||
|
|
|||
| // admissioncontroller | |||
| PrefixAMFiltering = PrefixAdmissionController + "filtering." | |||
| AMFilteringGenerateUniqueAppIds = PrefixAMFiltering + "generateUniqueAppId" | |||
There was a problem hiding this comment.
NOTE: This is needed as the specific algorithm used for generating the appID is controlled by an admission-controller specific property. We use that here as well to maintain backwards compatibility. The value is duplicated form am_conf.go as we would create a circular package reference otherwise.
| @@ -208,6 +216,7 @@ func handleNonReloadableConfig(old *SchedulerConf, new *SchedulerConf) { | |||
| checkNonReloadableBool(CMSvcDisableGangScheduling, &old.DisableGangScheduling, &new.DisableGangScheduling) | |||
| checkNonReloadableString(CMSvcPlaceholderImage, &old.PlaceHolderImage, &new.PlaceHolderImage) | |||
| checkNonReloadableString(CMSvcNodeInstanceTypeNodeLabelKey, &old.InstanceTypeNodeLabelKey, &new.InstanceTypeNodeLabelKey) | |||
| checkNonReloadableBool(AMFilteringGenerateUniqueAppIds, &old.GenerateUniqueAppIds, &new.GenerateUniqueAppIds) | |||
There was a problem hiding this comment.
NOTE: We mark the generateUniqueAppIds property as non-reloadable since changing this value would cause the computed applicationIDs of existing Pods to change. This would completely mess up internal state tracking.
craigcondit
requested review from
wilfred-s,
pbacsko,
manirajv06 and
sunilgovind
Codecov Report
@@ Coverage Diff @@
## master #708 +/- ##
==========================================
- Coverage 71.98% 71.87% -0.12%
==========================================
Files 49 49
Lines 7949 7956 +7
==========================================
- Hits 5722 5718 -4
- Misses 2031 2041 +10
- Partials 196 197 +1
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…d mode If a Pod with schedulerName: yunikorn is encountered with missing applicationID metadata, generate an applicationID using the same algorithm as the admission controller would have. This allows these Pods to be scheduled successfully.
craigcondit
force-pushed
the
YUNIKORN-2083
branch
from
75c5578 to
f83dcb1
Compare
What is this PR for?
If a Pod with
schedulerName: yunikornis encountered with missing applicationID metadata, generate an applicationID using the same algorithm as the admission controller would have. This allows these Pods to be scheduled successfully.What type of PR is it?
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-2083
How should this be tested?
Updated unit tests to handle new logic (both plugin / standard mode). Also manually created a Pod with the necessary conditions and verified that it scheduled properly and was assigned the appropriate applicationID by YuniKorn.
Screenshots (if appropriate)
Questions: