[
https://issues.apache.org/jira/browse/YUNIKORN-1998?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17782171#comment-17782171
]
Peter Bacsko commented on YUNIKORN-1998:
----------------------------------------
Merged to master, thanks [~Yu-Lin Chen] for the fix.
> Stale AdmissionControllerConf was used in e2e test
> --------------------------------------------------
>
> Key: YUNIKORN-1998
> URL: https://issues.apache.org/jira/browse/YUNIKORN-1998
> Project: Apache YuniKorn
> Issue Type: Bug
> Components: test - e2e
> Reporter: Yu-Lin Chen
> Assignee: Yu-Lin Chen
> Priority: Minor
> Labels: pull-request-available
> Fix For: 1.4.0
>
>
> In e2e test for "user_group_limit", we updated Yunikorn ConfigMap before
> submit sleep pods.
> [https://github.com/apache/yunikorn-k8shim/blob/master/test/e2e/user_group_limit/user_group_limit_test.go#L67-L70]
> However, the AdmissionControllerConf in Admission controller doesn't
> immediately reflect the changes. The time gap leads following error in CI
> flow:
> [https://github.com/apache/yunikorn-k8shim/actions/runs/6282144385/job/17062987755?pr=677#step:5:2346]
> We need to find a way to ensure AdmissionControllerConf has been updated
> before e2e test submits a new pods.
> This issue can be reproduced if we introduce 1 second delay when the
> admission controller updates the AdmissionControllerConf. Here is the example
> code.
> {code:java}
> func (h *configMapUpdateHandler) OnUpdate(_, newObj interface{}) {
> cm := utils.Convert2ConfigMap(newObj)
>
> // sleep 1 to delay AdmissionControllerConf update
> time.Sleep(1 * time.Second)
>
> if idx, ok := h.configMapIndex(cm); ok {
> h.conf.configUpdated(idx, cm)
> }
> } {code}
> [https://github.com/apache/yunikorn-k8shim/blob/master/pkg/admission/conf/am_conf.go#L237-L238]
> Below are the before/after AM logs when we added 1 sec delay:
> Success without delay 1 sec:
> * (AM configMapUpdateHandler) 05:36:39.184Z.
> AdmissionControllerConf trying to upgrade config
> * (AM configMapUpdateHandler)
> {color:#de350b}05:36:39.185Z{color} . AdmissionControllerConf config upgraded
> * (AM Webhook) 05:36:39.218Z. AM
> receive AdmissionReview request
> * (AM Webhook)
> {color:#de350b}05:36:39.221Z{color}. AM check Pods with new config, E2E
> Test Passed
> Failed with delay 1 sec:
> * (AM configMapUpdateHandler) 08:19:31.025Z.
> AdmissionControllerConf trying to upgrade config
> * (AM Webhook) 08:19:31.067Z. AM
> receive AdmissionReview request
> * (AM Webhook) 08:19:31.069Z
> {color:#ff0000}*AM check Pods with stale config*{color}, {color:#ff0000}*E2E
> Test failed* {color}
> * (AM configMapUpdateHandler) 08:19:32.026Z.
> AdmissionControllerConf config upgraded
> In my kind cluster(v1.24.15), there is only 0.036 sec gap
> ({color:#de350b}05:36:39.185Z~ {color}{color:#de350b}05:36:39.221Z{color}).
> It's possible that the admission controller in CI flow runs those steps in
> different order.
> The possible Solution:
> 1. In the e2e test, sleep for 1 seconds between updating the configmap and
> submitting new sleep pods. (It's quick fix, I assumed the time gap is always
> less than 1 sec )
> 2. In the e2e test, check current AdmissionControllerConf's value before
> submit a new pod. (How do client dump current AdmissionControllerConf? Need
> to seek for more advice.)
> Since this issue only impact e2e test for now, we can go with solution #1 as
> a quick fix. But it'll be better if we allow client to check whether
> AdmissionControllerConf is up-to-date.
> Please kindly let me know if I have any misunderstandings.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: issues-h...@yunikorn.apache.org
