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

refactor: helm action in componentplan controller #248

Merged
merged 5 commits into from
Aug 23, 2023

Conversation

Abirdcfly
Copy link
Member

@Abirdcfly Abirdcfly commented Aug 15, 2023

What type of PR is this?

/kind cleanup

What this PR does / why we need it

There are 5 commit:

  • 792ca6a: abstracts the retry time into 2 variables, one for 1 minute and one for 3 seconds. (No code logic changes)
  • 1b877dd: New feature, add the corresponding cpl's ns,name,uid,generation to the description of the helm release to facilitate the subsequent identification of duplicate installation issues.
  • d1450be: Refactor componentplan's controller to abstract operations on helm release into a release work pool. By that we simplifies the logic of the controller and makes it easier to test the controller and helm relese work pool independently.
  • fc341c8: Before adding the test, we found that the onsi/ginkgo version we used was too old and migrated to the new version.
  • 4fc5597: Simply init a test of componentplan's controller.

Post-sequence tests will be added in the new pr.

Which issue(s) this PR fixes

For #194

Special notes for your reviewer

@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #248 (09b2056) into main (6c0f0f3) will decrease coverage by 0.02%.
The diff coverage is 7.90%.

@@            Coverage Diff             @@
##             main     #248      +/-   ##
==========================================
- Coverage   17.61%   17.60%   -0.02%     
==========================================
  Files          34       35       +1     
  Lines        3452     3562     +110     
==========================================
+ Hits          608      627      +19     
- Misses       2811     2898      +87     
- Partials       33       37       +4     
Files Changed Coverage Δ
pkg/helm/helm_release_workpool.go 0.00% <0.00%> (ø)
controllers/componentplan_controller.go 6.17% <7.89%> (+6.17%) ⬆️
pkg/helm/helm.go 5.41% <68.00%> (+5.41%) ⬆️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Abirdcfly Abirdcfly force-pushed the cplcontroller branch 23 times, most recently from 10b5110 to 4fc5597 Compare August 17, 2023 05:54
@Abirdcfly Abirdcfly marked this pull request as ready for review August 17, 2023 05:58
@Abirdcfly Abirdcfly force-pushed the cplcontroller branch 2 times, most recently from 27740df to a46b11e Compare August 18, 2023 09:43
bjwswang
bjwswang previously approved these changes Aug 18, 2023
@Abirdcfly
Copy link
Member Author

test failed reported by #274, maybe fixed by #279

@Abirdcfly Abirdcfly force-pushed the cplcontroller branch 2 times, most recently from dbf417e to e134f2b Compare August 22, 2023 14:13
@Abirdcfly Abirdcfly merged commit d73d61f into kubebb:main Aug 23, 2023
@Abirdcfly Abirdcfly deleted the cplcontroller branch August 23, 2023 02:31
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.

3 participants