tests: Prom client testing#1818
Conversation
… a dedicated promClientController The underlying implementation is the same. Only moving the code under a separate controller that can be unit test independently of the descheduler type implementation.
There was a problem hiding this comment.
Pull request overview
Adds unit tests around the Prometheus client reconciliation logic and refactors the descheduler to encapsulate Prometheus client/token reconciliation into a dedicated controller.
Changes:
- Refactor: move Prometheus client/token reconciliation logic from
deschedulerintopromClientController. - Wire
runProfilesand token reconciliation runners to usepromClientCtrl. - Add unit tests covering invalid config, client recreation, informer event handling, and in-cluster SA token reconciliation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/descheduler/descheduler.go | Introduces promClientController and updates descheduler wiring to use it for Prometheus client access and reconciliation. |
| pkg/descheduler/descheduler_test.go | Adds unit tests validating promClientController behavior and updates existing Prometheus client access test for the refactor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/descheduler/descheduler_test.go
Outdated
|
|
||
| // Start the reconciler to process queue items | ||
| go setup.ctrl.runAuthenticationSecretReconciler(ctx) | ||
| defer cancel() |
There was a problem hiding this comment.
cancel() is deferred twice in this test (once right after context.WithCancel, and again after starting the reconciler goroutine). The second defer cancel() is redundant and can be removed to avoid confusion.
| defer cancel() |
| var promClient promapi.Client | ||
| if d.promClientCtrl != nil { | ||
| promClient = d.promClientCtrl.prometheusClient() | ||
| } |
There was a problem hiding this comment.
promClientCtrl.prometheusClient() reads a field that is concurrently mutated by the secret reconciler / in-cluster token reconciler goroutines. This introduces a data race when runProfiles is executing concurrently with reconciliation. Consider protecting promClient, currentPrometheusAuthToken, and previousPrometheusClientTransport with a mutex (or using atomic.Value for the client) and exposing a concurrency-safe accessor for use here.
There was a problem hiding this comment.
Fixing defects is part of the upcoming broader refactoring
| var createdClients []promapi.Client | ||
| setup.ctrl.createPrometheusClient = func(url, token string) (promapi.Client, *http.Transport, error) { | ||
| client := &mockPrometheusClient{name: "client-" + token} | ||
| createdClients = append(createdClients, client) | ||
| return client, &http.Transport{}, nil |
There was a problem hiding this comment.
createdClients is appended to from setup.ctrl.createPrometheusClient (invoked by the reconciler goroutine) while the test concurrently reads len(createdClients) and indexes into the slice. This will trip the race detector and can be flaky. Use synchronization (e.g., a mutex around the slice or replace it with an atomic counter / channel) to make the assertions concurrency-safe.
33ccc85 to
a8de54e
Compare
a8de54e to
bdd59e4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if metricProviderTokenReconciliation == secretReconciliation { | ||
| go descheduler.runAuthenticationSecretReconciler(ctx) | ||
| go descheduler.promClientCtrl.runAuthenticationSecretReconciler(ctx) | ||
| } |
There was a problem hiding this comment.
In DryRun mode, descheduler is re-created after setupPrometheusProvider is called. That means the Secrets informer event handler/lister get wired to the first promClientCtrl, but the reconciler started here uses the second promClientCtrl, so the queue never receives events and the Prometheus client/token won’t be reconciled. Fix by reusing the original promClientCtrl when swapping to the dry-run descheduler, or re-running setupPrometheusProvider after the dry-run descheduler is created so the handler/lister target the same controller instance.
There was a problem hiding this comment.
The dry run mode has not been extended to prometheus client case. Yet, the tests are there to avoid potential regressions here.
| expectClientCreated bool | ||
| expectCurrentToken string | ||
| expectPreviousTransportCleared bool | ||
| expectPromClientCleared bool | ||
| }{ |
There was a problem hiding this comment.
The expectPromClientCleared field is never set to true in any test case and reconcileInClusterSAToken currently doesn’t clear promClient on any path, so this adds dead/unused test complexity. Either remove this field/branch, or add a concrete scenario (and production behavior) where promClient is expected to be cleared and assert it.
42bcec2 to
fe2523f
Compare
Currently, there's a single prometheus client reconciler for both in
cluster and secret based strategies. The in cluster reconciling is run in
sync with each descheduling cycle. An in file token either changes or it
does not. If changed a new prometheus client is created. The secret
based reconciling is run async and watches for secret object changes. If a
secret changes a new client is created. The internal state of the
reconciler keeps previous connection data for clearing and checks.
The current reconciler implementation lacks mutually exclusive access.
So data races are possible. The prometheus configuration validation is
performed during every sync. The future refactorings is expected to move
the validation to the creation phase of the reconciler.
The extra unit testing is expected to cover the following scenarios:
- in cluster:
- in file token is unchanged: no-op
- in file token is changed: client is created or fails to be created
- secret:
- no secret is not found: no client creation, internal state cleared
- secret is found: if token changed a new client created, otherwise
no-op
- prometheus config validation
- prometheus client injection
Any error during new prom client creation should be followed by closing
the previous connection and reseting the internal state. Yet, the error
handling is not that strict currently. So the current extra unit testing
keeps the incomplete testing cases as they are.
Other use of the tests is to make sure every time a new prometheus
client is created a descheduling cycle injects a new profile with the
updated prometheus clients. So the future refactoring does not introduce
a regression.
fe2523f to
5a53f16
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
… (#4098) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [kubernetes-sigs/descheduler](https://github.com/kubernetes-sigs/descheduler) | minor | `0.34.0` → `v0.35.0` | --- ### Release Notes <details> <summary>kubernetes-sigs/descheduler (kubernetes-sigs/descheduler)</summary> ### [`v0.35.0`](https://github.com/kubernetes-sigs/descheduler/releases/tag/v0.35.0): Descheduler v0.35.0 [Compare Source](kubernetes-sigs/descheduler@v0.34.0...v0.35.0) #### What's Changed - feat: enable pod protection based on storage classes by [@​ricardomaraschini](https://github.com/ricardomaraschini) in [#​1752](kubernetes-sigs/descheduler#1752) - fix: pod resource calculation to consider native sidecars by [@​a7i](https://github.com/a7i) in [#​1771](kubernetes-sigs/descheduler#1771) - docs: fix incorrect gracePeriodSeconds default in README.md by [@​petersalas](https://github.com/petersalas) in [#​1773](kubernetes-sigs/descheduler#1773) - docs: fix README.md link to kubernetes bot commands by [@​Sycrosity](https://github.com/Sycrosity) in [#​1772](kubernetes-sigs/descheduler#1772) - Fix "Current requires cgo or $USER set in environment" error by [@​abelfodil](https://github.com/abelfodil) in [#​1764](kubernetes-sigs/descheduler#1764) - refactor(TestPodLifeTime): remove ineffective owner references assignments by [@​ingvagabund](https://github.com/ingvagabund) in [#​1781](kubernetes-sigs/descheduler#1781) - refactor(TestPodLifeTime): have a pod fully created through BuildTestPod without any edits by [@​ingvagabund](https://github.com/ingvagabund) in [#​1782](kubernetes-sigs/descheduler#1782) - refactor(TestPodLifeTime): consolidations, simplifications and node instance for each unit test by [@​ingvagabund](https://github.com/ingvagabund) in [#​1783](kubernetes-sigs/descheduler#1783) - refactor(TestPodLifeTime): inline pod creation in each unit test to avoid accidental pod spec updates by [@​ingvagabund](https://github.com/ingvagabund) in [#​1784](kubernetes-sigs/descheduler#1784) - refactor(TestPodLifeTime): update unit test names and simplify pod creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1785](kubernetes-sigs/descheduler#1785) - feat(TestPodLifeTime): check only expected pods are evicted by [@​ingvagabund](https://github.com/ingvagabund) in [#​1787](kubernetes-sigs/descheduler#1787) - feat(PodLifeTime): document the plugin with details that can be used for reasoning during reviews and design discussions by [@​ingvagabund](https://github.com/ingvagabund) in [#​1789](kubernetes-sigs/descheduler#1789) - refactor(TestPodLifeTime): split the unit tests into smaller semantically close groups by [@​ingvagabund](https://github.com/ingvagabund) in [#​1790](kubernetes-sigs/descheduler#1790) - refactor(TestFindDuplicatePods): have a pod fully created through BuildTestPod without any edits by [@​ingvagabund](https://github.com/ingvagabund) in [#​1791](kubernetes-sigs/descheduler#1791) - refactor(TestFindDuplicatePods): reduce duplicates and inline by [@​ingvagabund](https://github.com/ingvagabund) in [#​1792](kubernetes-sigs/descheduler#1792) - refactor(TestRemoveDuplicates): reduce test code duplication by [@​ingvagabund](https://github.com/ingvagabund) in [#​1793](kubernetes-sigs/descheduler#1793) - refactor(TestRemovePodsHavingTooManyRestarts): inline object creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1794](kubernetes-sigs/descheduler#1794) - refactor(TestPodAntiAffinity): inline object creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1795](kubernetes-sigs/descheduler#1795) - refactor(TestRemovePodsViolatingNodeAffinity): inline object creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1796](kubernetes-sigs/descheduler#1796) - refactor(TestDeletePodsViolatingNodeTaints): inline object creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1797](kubernetes-sigs/descheduler#1797) - doc: introduce contributing guidelines specific to the project by [@​ingvagabund](https://github.com/ingvagabund) in [#​1798](kubernetes-sigs/descheduler#1798) - refactor(TestDefaultEvictor): de-dup code and use helpers by [@​ingvagabund](https://github.com/ingvagabund) in [#​1803](kubernetes-sigs/descheduler#1803) - refactor(plugins): simplify the way pods are created by [@​ingvagabund](https://github.com/ingvagabund) in [#​1804](kubernetes-sigs/descheduler#1804) - fix(TestReadyNodesWithNodeSelector): make sure nodeLister.List always returns a non-empty list so the lister is always tested by [@​ingvagabund](https://github.com/ingvagabund) in [#​1800](kubernetes-sigs/descheduler#1800) - refactor(pkg/framework/profile): dedup unit test code by [@​ingvagabund](https://github.com/ingvagabund) in [#​1806](kubernetes-sigs/descheduler#1806) - doc(Design Decisions FAQ): Why doesn't the framework provide helpers for registering and retrieving indexers for plugins by [@​ingvagabund](https://github.com/ingvagabund) in [#​1807](kubernetes-sigs/descheduler#1807) - feat(profile): inject a plugin instance ID to each built plugin by [@​ingvagabund](https://github.com/ingvagabund) in [#​1808](kubernetes-sigs/descheduler#1808) - feat: register a node indexer for the global node selector instead of listing nodes with the selector by [@​ingvagabund](https://github.com/ingvagabund) in [#​1802](kubernetes-sigs/descheduler#1802) - chore(pkg/descheduler): make TestPodEvictorReset table driven by [@​ingvagabund](https://github.com/ingvagabund) in [#​1810](kubernetes-sigs/descheduler#1810) - refactor(pkg/operator): replace informerResource with a kubeClientSandbox by [@​ingvagabund](https://github.com/ingvagabund) in [#​1811](kubernetes-sigs/descheduler#1811) - refactor(pkg/descheduler): more handlers and dropping unused code by [@​ingvagabund](https://github.com/ingvagabund) in [#​1813](kubernetes-sigs/descheduler#1813) - refactor(pkg/descheduler): create fake shared informer factory only once by [@​ingvagabund](https://github.com/ingvagabund) in [#​1812](kubernetes-sigs/descheduler#1812) - fix(kubeClientSandbox): do not wait for pods in the fake indexers if they are already deleted by [@​ingvagabund](https://github.com/ingvagabund) in [#​1814](kubernetes-sigs/descheduler#1814) - test(pkg/descheduler): test a prometheus client update propagates to a plugin profile handle by [@​ingvagabund](https://github.com/ingvagabund) in [#​1816](kubernetes-sigs/descheduler#1816) - Add namespace label selector by [@​W1seKappa](https://github.com/W1seKappa) in [#​1786](kubernetes-sigs/descheduler#1786) - tests: Prom client testing by [@​ingvagabund](https://github.com/ingvagabund) in [#​1818](kubernetes-sigs/descheduler#1818) - Deduplicate descheduler initialization code so unit tests test more of the production code by [@​ingvagabund](https://github.com/ingvagabund) in [#​1819](kubernetes-sigs/descheduler#1819) - test(token reconciling): have tests initialize the prom client reconciling through the descheduler's bootstraping entry too by [@​ingvagabund](https://github.com/ingvagabund) in [#​1820](kubernetes-sigs/descheduler#1820) - refactor(promClientController): split it into two prom client controllers by [@​ingvagabund](https://github.com/ingvagabund) in [#​1821](kubernetes-sigs/descheduler#1821) - feat(pkg/descheduler): create profiles outside the descheduling cycle by [@​ingvagabund](https://github.com/ingvagabund) in [#​1815](kubernetes-sigs/descheduler#1815) - refactor: move prometheus client controller related code under a seperate file by [@​ingvagabund](https://github.com/ingvagabund) in [#​1823](kubernetes-sigs/descheduler#1823) - Update go dependecies to fix vulnerabilities by [@​sammedsingalkar09](https://github.com/sammedsingalkar09) in [#​1822](kubernetes-sigs/descheduler#1822) - chore: extend the list of supported Go versions by [@​ingvagabund](https://github.com/ingvagabund) in [#​1828](kubernetes-sigs/descheduler#1828) - bump(golangci-lint): update and migrate by [@​ingvagabund](https://github.com/ingvagabund) in [#​1829](kubernetes-sigs/descheduler#1829) - \[v0.35.0] bump to kubernetes 1.35 deps by [@​a7i](https://github.com/a7i) in [#​1827](kubernetes-sigs/descheduler#1827) - chore: upgrade github.com/gomarkdown/markdown to latest version by [@​a7i](https://github.com/a7i) in [#​1831](kubernetes-sigs/descheduler#1831) - \[v0.35.0] update docs and manifests by [@​a7i](https://github.com/a7i) in [#​1832](kubernetes-sigs/descheduler#1832) - Change annotations condition to deploymentAnnotations for Deployment object annotations by [@​davidandreoletti](https://github.com/davidandreoletti) in [#​1830](kubernetes-sigs/descheduler#1830) #### New Contributors - [@​petersalas](https://github.com/petersalas) made their first contribution in [#​1773](kubernetes-sigs/descheduler#1773) - [@​Sycrosity](https://github.com/Sycrosity) made their first contribution in [#​1772](kubernetes-sigs/descheduler#1772) - [@​abelfodil](https://github.com/abelfodil) made their first contribution in [#​1764](kubernetes-sigs/descheduler#1764) - [@​W1seKappa](https://github.com/W1seKappa) made their first contribution in [#​1786](kubernetes-sigs/descheduler#1786) - [@​sammedsingalkar09](https://github.com/sammedsingalkar09) made their first contribution in [#​1822](kubernetes-sigs/descheduler#1822) - [@​davidandreoletti](https://github.com/davidandreoletti) made their first contribution in [#​1830](kubernetes-sigs/descheduler#1830) **Full Changelog**: <kubernetes-sigs/descheduler@v0.34.0...v0.35.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNS43IiwidXBkYXRlZEluVmVyIjoiNDMuMjUuNyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiaW1hZ2UiXX0=--> Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/4098 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [descheduler](https://github.com/kubernetes-sigs/descheduler) | minor | `0.34.0` → `0.35.0` | --- ### Release Notes <details> <summary>kubernetes-sigs/descheduler (descheduler)</summary> ### [`v0.35.0`](https://github.com/kubernetes-sigs/descheduler/releases/tag/v0.35.0): Descheduler v0.35.0 [Compare Source](kubernetes-sigs/descheduler@v0.34.0...v0.35.0) ##### What's Changed - feat: enable pod protection based on storage classes by [@​ricardomaraschini](https://github.com/ricardomaraschini) in [#​1752](kubernetes-sigs/descheduler#1752) - fix: pod resource calculation to consider native sidecars by [@​a7i](https://github.com/a7i) in [#​1771](kubernetes-sigs/descheduler#1771) - docs: fix incorrect gracePeriodSeconds default in README.md by [@​petersalas](https://github.com/petersalas) in [#​1773](kubernetes-sigs/descheduler#1773) - docs: fix README.md link to kubernetes bot commands by [@​Sycrosity](https://github.com/Sycrosity) in [#​1772](kubernetes-sigs/descheduler#1772) - Fix "Current requires cgo or $USER set in environment" error by [@​abelfodil](https://github.com/abelfodil) in [#​1764](kubernetes-sigs/descheduler#1764) - refactor(TestPodLifeTime): remove ineffective owner references assignments by [@​ingvagabund](https://github.com/ingvagabund) in [#​1781](kubernetes-sigs/descheduler#1781) - refactor(TestPodLifeTime): have a pod fully created through BuildTestPod without any edits by [@​ingvagabund](https://github.com/ingvagabund) in [#​1782](kubernetes-sigs/descheduler#1782) - refactor(TestPodLifeTime): consolidations, simplifications and node instance for each unit test by [@​ingvagabund](https://github.com/ingvagabund) in [#​1783](kubernetes-sigs/descheduler#1783) - refactor(TestPodLifeTime): inline pod creation in each unit test to avoid accidental pod spec updates by [@​ingvagabund](https://github.com/ingvagabund) in [#​1784](kubernetes-sigs/descheduler#1784) - refactor(TestPodLifeTime): update unit test names and simplify pod creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1785](kubernetes-sigs/descheduler#1785) - feat(TestPodLifeTime): check only expected pods are evicted by [@​ingvagabund](https://github.com/ingvagabund) in [#​1787](kubernetes-sigs/descheduler#1787) - feat(PodLifeTime): document the plugin with details that can be used for reasoning during reviews and design discussions by [@​ingvagabund](https://github.com/ingvagabund) in [#​1789](kubernetes-sigs/descheduler#1789) - refactor(TestPodLifeTime): split the unit tests into smaller semantically close groups by [@​ingvagabund](https://github.com/ingvagabund) in [#​1790](kubernetes-sigs/descheduler#1790) - refactor(TestFindDuplicatePods): have a pod fully created through BuildTestPod without any edits by [@​ingvagabund](https://github.com/ingvagabund) in [#​1791](kubernetes-sigs/descheduler#1791) - refactor(TestFindDuplicatePods): reduce duplicates and inline by [@​ingvagabund](https://github.com/ingvagabund) in [#​1792](kubernetes-sigs/descheduler#1792) - refactor(TestRemoveDuplicates): reduce test code duplication by [@​ingvagabund](https://github.com/ingvagabund) in [#​1793](kubernetes-sigs/descheduler#1793) - refactor(TestRemovePodsHavingTooManyRestarts): inline object creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1794](kubernetes-sigs/descheduler#1794) - refactor(TestPodAntiAffinity): inline object creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1795](kubernetes-sigs/descheduler#1795) - refactor(TestRemovePodsViolatingNodeAffinity): inline object creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1796](kubernetes-sigs/descheduler#1796) - refactor(TestDeletePodsViolatingNodeTaints): inline object creation by [@​ingvagabund](https://github.com/ingvagabund) in [#​1797](kubernetes-sigs/descheduler#1797) - doc: introduce contributing guidelines specific to the project by [@​ingvagabund](https://github.com/ingvagabund) in [#​1798](kubernetes-sigs/descheduler#1798) - refactor(TestDefaultEvictor): de-dup code and use helpers by [@​ingvagabund](https://github.com/ingvagabund) in [#​1803](kubernetes-sigs/descheduler#1803) - refactor(plugins): simplify the way pods are created by [@​ingvagabund](https://github.com/ingvagabund) in [#​1804](kubernetes-sigs/descheduler#1804) - fix(TestReadyNodesWithNodeSelector): make sure nodeLister.List always returns a non-empty list so the lister is always tested by [@​ingvagabund](https://github.com/ingvagabund) in [#​1800](kubernetes-sigs/descheduler#1800) - refactor(pkg/framework/profile): dedup unit test code by [@​ingvagabund](https://github.com/ingvagabund) in [#​1806](kubernetes-sigs/descheduler#1806) - doc(Design Decisions FAQ): Why doesn't the framework provide helpers for registering and retrieving indexers for plugins by [@​ingvagabund](https://github.com/ingvagabund) in [#​1807](kubernetes-sigs/descheduler#1807) - feat(profile): inject a plugin instance ID to each built plugin by [@​ingvagabund](https://github.com/ingvagabund) in [#​1808](kubernetes-sigs/descheduler#1808) - feat: register a node indexer for the global node selector instead of listing nodes with the selector by [@​ingvagabund](https://github.com/ingvagabund) in [#​1802](kubernetes-sigs/descheduler#1802) - chore(pkg/descheduler): make TestPodEvictorReset table driven by [@​ingvagabund](https://github.com/ingvagabund) in [#​1810](kubernetes-sigs/descheduler#1810) - refactor(pkg/operator): replace informerResource with a kubeClientSandbox by [@​ingvagabund](https://github.com/ingvagabund) in [#​1811](kubernetes-sigs/descheduler#1811) - refactor(pkg/descheduler): more handlers and dropping unused code by [@​ingvagabund](https://github.com/ingvagabund) in [#​1813](kubernetes-sigs/descheduler#1813) - refactor(pkg/descheduler): create fake shared informer factory only once by [@​ingvagabund](https://github.com/ingvagabund) in [#​1812](kubernetes-sigs/descheduler#1812) - fix(kubeClientSandbox): do not wait for pods in the fake indexers if they are already deleted by [@​ingvagabund](https://github.com/ingvagabund) in [#​1814](kubernetes-sigs/descheduler#1814) - test(pkg/descheduler): test a prometheus client update propagates to a plugin profile handle by [@​ingvagabund](https://github.com/ingvagabund) in [#​1816](kubernetes-sigs/descheduler#1816) - Add namespace label selector by [@​W1seKappa](https://github.com/W1seKappa) in [#​1786](kubernetes-sigs/descheduler#1786) - tests: Prom client testing by [@​ingvagabund](https://github.com/ingvagabund) in [#​1818](kubernetes-sigs/descheduler#1818) - Deduplicate descheduler initialization code so unit tests test more of the production code by [@​ingvagabund](https://github.com/ingvagabund) in [#​1819](kubernetes-sigs/descheduler#1819) - test(token reconciling): have tests initialize the prom client reconciling through the descheduler's bootstraping entry too by [@​ingvagabund](https://github.com/ingvagabund) in [#​1820](kubernetes-sigs/descheduler#1820) - refactor(promClientController): split it into two prom client controllers by [@​ingvagabund](https://github.com/ingvagabund) in [#​1821](kubernetes-sigs/descheduler#1821) - feat(pkg/descheduler): create profiles outside the descheduling cycle by [@​ingvagabund](https://github.com/ingvagabund) in [#​1815](kubernetes-sigs/descheduler#1815) - refactor: move prometheus client controller related code under a seperate file by [@​ingvagabund](https://github.com/ingvagabund) in [#​1823](kubernetes-sigs/descheduler#1823) - Update go dependecies to fix vulnerabilities by [@​sammedsingalkar09](https://github.com/sammedsingalkar09) in [#​1822](kubernetes-sigs/descheduler#1822) - chore: extend the list of supported Go versions by [@​ingvagabund](https://github.com/ingvagabund) in [#​1828](kubernetes-sigs/descheduler#1828) - bump(golangci-lint): update and migrate by [@​ingvagabund](https://github.com/ingvagabund) in [#​1829](kubernetes-sigs/descheduler#1829) - \[v0.35.0] bump to kubernetes 1.35 deps by [@​a7i](https://github.com/a7i) in [#​1827](kubernetes-sigs/descheduler#1827) - chore: upgrade github.com/gomarkdown/markdown to latest version by [@​a7i](https://github.com/a7i) in [#​1831](kubernetes-sigs/descheduler#1831) - \[v0.35.0] update docs and manifests by [@​a7i](https://github.com/a7i) in [#​1832](kubernetes-sigs/descheduler#1832) - Change annotations condition to deploymentAnnotations for Deployment object annotations by [@​davidandreoletti](https://github.com/davidandreoletti) in [#​1830](kubernetes-sigs/descheduler#1830) ##### New Contributors - [@​petersalas](https://github.com/petersalas) made their first contribution in [#​1773](kubernetes-sigs/descheduler#1773) - [@​Sycrosity](https://github.com/Sycrosity) made their first contribution in [#​1772](kubernetes-sigs/descheduler#1772) - [@​abelfodil](https://github.com/abelfodil) made their first contribution in [#​1764](kubernetes-sigs/descheduler#1764) - [@​W1seKappa](https://github.com/W1seKappa) made their first contribution in [#​1786](kubernetes-sigs/descheduler#1786) - [@​sammedsingalkar09](https://github.com/sammedsingalkar09) made their first contribution in [#​1822](kubernetes-sigs/descheduler#1822) - [@​davidandreoletti](https://github.com/davidandreoletti) made their first contribution in [#​1830](kubernetes-sigs/descheduler#1830) **Full Changelog**: <kubernetes-sigs/descheduler@v0.34.0...v0.35.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNS43IiwidXBkYXRlZEluVmVyIjoiNDMuMjUuNyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiY2hhcnQiXX0=--> Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/4099 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>
Description
Currently, there's a single prometheus client reconciler for both in cluster and secret based strategies. The in cluster reconciling is run in sync with each descheduling cycle. An in file token either changes or it does not. If changed a new prometheus client is created. The secret based reconciling is run async and watches for secret object changes. If a secret changes a new client is created. The internal state of the reconciler keeps previous connection data for clearing and checks.
The current reconciler implementation lacks mutually exclusive access. So data races are possible. The prometheus configuration validation is performed during every sync. The future refactorings is expected to move the validation to the creation phase of the reconciler.
The extra unit testing is expected to cover the following scenarios:
Any error during new prom client creation should be followed by closing the previous connection and reseting the internal state. Yet, the error handling is not that strict currently. So the current extra unit testing keeps the incomplete testing cases as they are.
Other use of the tests is to make sure every time a new prometheus client is created a descheduling cycle injects a new profile with the updated prometheus clients. So the future refactoring does not introduce a regression
Checklist
Please ensure your pull request meets the following criteria before submitting
for review, these items will be used by reviewers to assess the quality and
completeness of your changes: