vsphere-csi-driver
vsphere-csi-driver copied to clipboard
kubernetes-sigs
Fix NewInformer() to return Informer instance with appropriate client associated
What this PR does / why we need it: NewInformer() function in kubernetes package does not check if the informer instance returned is based out of the client given in the request or not. Currently in cases like TKG cluster deployment, 2 different k8s clients can be provided as input to this function, which include in-cluster config k8s client (vanilla k8s client/guest cluster k8s client) and supervisor cluster config k8s client, to provide informer instance on objects in two different cluster configs. Hence it is necessary to return informer instance against appropriate client, that is passed as input, instead of using same instance for all clients. This PR addresses this code-correctness issue and creates two separate instances of informer against each k8s cluster config and returns them as per request by the caller.
Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #
Testing done: Manually tested the CSI driver deployment with some testlogs added, volume provisioning and pod deployment with the change
CSI syncer container logs:
2022-09-26T06:06:53.282Z INFO kubernetes/informers.go:76 NewInformer(): Found informer instance for %!s(bool=true) as <nil> {"TraceId": "23db0c2b-871a-4e9c-ab94-c1f296a9cbd1"}
2022-09-26T06:06:53.283Z INFO kubernetes/informers.go:86 NewInformer(): Created informer instance for in-cluster config {"TraceId": "23db0c2b-871a-4e9c-ab94-c1f296a9cbd1"}
...
2022-09-26T06:06:53.421Z INFO kubernetes/informers.go:76 NewInformer(): Found informer instance for %!s(bool=true) as &{0xc000571380 0xc000172460 0xc0006208a0 <nil> 0xc000638a00 0x1a0f4c0 <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil>}
...
2022-09-26T06:06:53.487Z INFO kubernetes/informers.go:76 NewInformer(): Found informer instance for %!s(bool=true) as &{0xc000571380 0xc000172460 0xc0006208a0 <nil> 0xc000638a00 0x1a0f4c0 0xc0006b21e0 0x1a0f4c0 0xc0006b2140 0x1a0f4c0 <nil> <nil> 0xc0006b2280 0x1a0f4c0 <nil>}
CSI node plugin logs:
2022-09-26T06:06:29.469Z INFO kubernetes/informers.go:76 NewInformer(): Found informer instance for %!s(bool=true) as <nil> {"TraceId": "69f91afe-cdeb-4d52-b6e9-06b7cf09c6b3", "TraceId": "c731fbbc-246d-4d7d-bb9c-f160431a2c0b"}
2022-09-26T06:06:29.471Z INFO kubernetes/informers.go:86 NewInformer(): Created informer instance for in-cluster config {"TraceId": "69f91afe-cdeb-4d52-b6e9-06b7cf09c6b3", "TraceId": "c731fbbc-246d-4d7d-bb9c-f160431a2c0b"}
Special notes for your reviewer:
Release note:
Fix NewInformer() to return Informer instance created against given client
Hi @akankshapanse. Thanks for your PR.
I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
Build ID: 1389
Block vanilla build status: FAILURE
Stage before exit: e2e-tests
Jenkins E2E Test Results:
------------------------------
Ran 1 of 638 Specs in 358.556 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 637 Skipped
PASS
Ginkgo ran 1 suite in 6m52.288043306s
Test Suite Passed
--
------------------------------
Ran 12 of 638 Specs in 4484.643 seconds
SUCCESS! -- 12 Passed | 0 Failed | 0 Pending | 626 Skipped
PASS
Ginkgo ran 1 suite in 1h15m1.395244304s
Test Suite Passed
--
Ran 40 of 638 Specs in 3239.087 seconds
FAIL! -- 24 Passed | 16 Failed | 0 Pending | 598 Skipped
Ginkgo ran 1 suite in 54m17.198686166s
Test Suite Failed
Build ID: 1389 Block vanilla build status: FAILURE Stage before exit: e2e-tests Jenkins E2E Test Results: ------------------------------ Ran 1 of 638 Specs in 358.556 seconds SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 637 Skipped PASS Ginkgo ran 1 suite in 6m52.288043306s Test Suite Passed -- ------------------------------ Ran 12 of 638 Specs in 4484.643 seconds SUCCESS! -- 12 Passed | 0 Failed | 0 Pending | 626 Skipped PASS Ginkgo ran 1 suite in 1h15m1.395244304s Test Suite Passed -- Ran 40 of 638 Specs in 3239.087 seconds FAIL! -- 24 Passed | 16 Failed | 0 Pending | 598 Skipped Ginkgo ran 1 suite in 54m17.198686166s Test Suite Failed
Please ignore this test result since the image used for running pipeline was old. Will re-start the testrun with updated image.
block vanilla pipeline results
------------------------------
Ran 1 of 638 Specs in 319.876 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 637 Skipped
PASS
Ginkgo ran 1 suite in 6m20.451361939s
Test Suite Passed
--
------------------------------
Ran 12 of 638 Specs in 4268.948 seconds
SUCCESS! -- 12 Passed | 0 Failed | 0 Pending | 626 Skipped
PASS
Ginkgo ran 1 suite in 1h11m27.096550777s
Test Suite Passed
--
Ran 40 of 638 Specs in 954.049 seconds
FAIL! -- 38 Passed | 2 Failed | 0 Pending | 598 Skipped
Ginkgo ran 1 suite in 16m14.849860071s Test Suite Failed
testcases that failed in last run got passed when re-run
File vanilla build status: FAILURE
Stage before exit: e2e-tests
Jenkins E2E Test Results:
Ran 38 of 638 Specs in 9899.013 seconds
FAIL! -- 37 Passed | 1 Failed | 0 Pending | 600 Skipped
--- FAIL: TestE2E (9899.06s)
FAIL
Ginkgo ran 1 suite in 2h46m22.549667499s
Test Suite Failed
File vanilla build status: FAILURE Stage before exit: e2e-tests Jenkins E2E Test Results: Ran 38 of 638 Specs in 9899.013 seconds FAIL! -- 37 Passed | 1 Failed | 0 Pending | 600 Skipped --- FAIL: TestE2E (9899.06s) FAIL Ginkgo ran 1 suite in 2h46m22.549667499s Test Suite Failed
Testcase failure observed is due to setup problem (File Server connectivity issue), not related to the changes here
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: akankshapanse, chethanv28, divyenpatel
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [chethanv28,divyenpatel]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment