Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: generate k8s client only once #750

Conversation

MathieuCesbron
Copy link
Contributor

@MathieuCesbron MathieuCesbron commented Jan 13, 2024

Description

I realised that we create a new k8s client every time we need it. It's quite consuming, we should create it at the start of the controller and use it for his lifetime. We then pass the client down to each functions.

I realised this because we cannot add integration tests with EnvTest if we create a new client every time we need one #748

PS: It looks like the coverage test fail because some code removed was tested so it technically lowers the coverage.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • Functionality/bugs have been confirmed to be unchanged or fixed.
  • I have performed a self-review of my own code.

Copy link

codecov bot commented Jan 13, 2024

Codecov Report

Attention: 110 lines in your changes are missing coverage. Please review.

Comparison is base (e57531a) 28.80% compared to head (aa7042b) 29.25%.

Files Patch % Lines
k8sutils/poddisruption.go 0.00% 22 Missing ⚠️
k8sutils/redis-cluster.go 5.00% 19 Missing ⚠️
k8sutils/statefulset.go 0.00% 16 Missing ⚠️
k8sutils/services.go 0.00% 12 Missing ⚠️
k8sutils/redis-sentinel.go 0.00% 10 Missing ⚠️
controllers/rediscluster_controller.go 0.00% 8 Missing ⚠️
k8sutils/redis-replication.go 0.00% 6 Missing ⚠️
k8sutils/redis-standalone.go 0.00% 6 Missing ⚠️
controllers/redisreplication_controller.go 0.00% 3 Missing ⚠️
controllers/redissentinel_controller.go 0.00% 3 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #750      +/-   ##
==========================================
+ Coverage   28.80%   29.25%   +0.44%     
==========================================
  Files          19       19              
  Lines        3291     3241      -50     
==========================================
  Hits          948      948              
+ Misses       2308     2258      -50     
  Partials       35       35              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MathieuCesbron MathieuCesbron force-pushed the Generate_k8s_client_only_once branch 3 times, most recently from 4fe0305 to f47fc67 Compare January 13, 2024 19:44
Signed-off-by: Mathieu Cesbron <[email protected]>
@MathieuCesbron MathieuCesbron marked this pull request as ready for review January 13, 2024 20:44
@drivebyer drivebyer changed the title Generate k8s client only once feat: generate k8s client only once Jan 14, 2024
@drivebyer drivebyer enabled auto-merge (squash) January 14, 2024 07:23
@drivebyer drivebyer self-requested a review January 14, 2024 07:23
Copy link
Collaborator

@drivebyer drivebyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@drivebyer drivebyer merged commit 7665d4a into OT-CONTAINER-KIT:master Jan 14, 2024
28 of 29 checks passed
mattrobinsonsre pushed a commit to mattrobinsonsre/redis-operator that referenced this pull request Jul 11, 2024
Signed-off-by: Mathieu Cesbron <[email protected]>
Signed-off-by: Matt Robinson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants