refactor: client.go file helper methods #360

dkoshkin · 2024-01-10T20:49:02Z

What this PR does / why we need it:
Refactored the existing methods and functions to be unit testable.
Also made some methods that do not use the struct as generic functions.
The changes were primarily an effort to add unit test coverage.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

How Has This Been Tested?:
New unit tests and ran through the dev doc to create a cluster with management creds.

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

nutanix-cn-prow-bot · 2024-01-10T20:49:41Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2024-01-10T20:52:17Z

Codecov Report

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

Comparison is base (37a3def) 13.70% compared to head (6bbd2e8) 20.37%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
pkg/client/client.go	78.33%	13 Missing ⚠️
controllers/helpers.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #360      +/-   ##
==========================================
+ Coverage   13.70%   20.37%   +6.67%     
==========================================
  Files          17       17              
  Lines        1197     1212      +15     
==========================================
+ Hits          164      247      +83     
+ Misses       1033      965      -68

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

api/v1beta1/nutanixcluster_types.go

controllers/helpers.go

controllers/nutanixcluster_controller.go

pkg/client/client.go

thunderboltsid · 2024-01-12T09:50:06Z

/retest

thunderboltsid · 2024-01-12T12:20:10Z

/test e2e-k8s-upgrade
/test e2e-capx-controller-upgrade

dkoshkin · 2024-01-12T17:24:54Z

Thanks for triggering the tests @thunderboltsid, do you think these test failures are related, from the errors it looks like it couldn't delete the old Nodes, is that a known flake?

thunderboltsid · 2024-01-16T17:41:23Z

Thanks for triggering the tests @thunderboltsid, do you think these test failures are related, from the errors it looks like it couldn't delete the old Nodes, is that a known flake?

The issues stemmed from some older resource from another failed test that were not cleaned up properly that ended up causing IP contention between two separate clusters.

thunderboltsid · 2024-01-16T17:43:37Z

Let's rebase this PR against the latest changes; I've manually cleaned up the problematic resources so it shouldn't be a problem now.

api/v1beta1/nutanixcluster_types_test.go

pkg/client/client_test.go

pkg/client/client.go

thunderboltsid · 2024-01-26T11:31:40Z

/retest

thunderboltsid · 2024-01-26T14:28:17Z

/retest

thunderboltsid · 2024-01-26T15:06:18Z

/lgtm

Refactored the existing methods and functions to be unit testable. Also made some methods that do not use the struct as generic functions. The changes were primarily an effort to add unit test coverage.

thunderboltsid · 2024-02-08T08:52:51Z

/retest

thunderboltsid · 2024-02-08T20:44:41Z

/test e2e-capx-controller-upgrade

* refactor: client.go file helper methods Refactored the existing methods and functions to be unit testable. Also made some methods that do not use the struct as generic functions. The changes were primarily an effort to add unit test coverage. * refactor: more testable read file function * test: new nutanixcluster types unit tests * test: additional test cases for errors * fixup! refactor: client.go file helper methods * fixup! refactor: client.go file helper methods * fixup! refactor: more testable read file function * fixup! refactor: client.go file helper methods

* refactor: client.go file helper methods (#360) * refactor: client.go file helper methods Refactored the existing methods and functions to be unit testable. Also made some methods that do not use the struct as generic functions. The changes were primarily an effort to add unit test coverage. * refactor: more testable read file function * test: new nutanixcluster types unit tests * test: additional test cases for errors * fixup! refactor: client.go file helper methods * fixup! refactor: client.go file helper methods * fixup! refactor: more testable read file function * fixup! refactor: client.go file helper methods * Ensure fallback config is only read when prismCentral is absent (#403) Skip reading fallback config file from /etc/nutanix/config/prismCentral if NutanixCluster has prismCentral set. Co-authored-by: Sid Shukla <sid.shukla@nutanix.com> * Update build-dev.yaml add codecov token --------- Co-authored-by: Dimitri Koshkin <dimitri.koshkin@nutanix.com> Co-authored-by: Deepak Muley <deepak.muley@nutanix.com>

* refactor: client.go file helper methods Refactored the existing methods and functions to be unit testable. Also made some methods that do not use the struct as generic functions. The changes were primarily an effort to add unit test coverage. * refactor: more testable read file function * test: new nutanixcluster types unit tests * test: additional test cases for errors * fixup! refactor: client.go file helper methods * fixup! refactor: client.go file helper methods * fixup! refactor: more testable read file function * fixup! refactor: client.go file helper methods

* fix: improve error handling for Prism Client (#354) * fix: use wrapper errors to clearly denote issues in client building * fix: adds a function to properly sanitize the address * fix: adds tests for ip address case given * fix: uses a defined type for port error * fix: clean up variable naming * fix: remove validation here to be moved into prism-client * refactor: task status file (#355) * test: add unit tests for pkg/client/state * refactor: use wait.Poll function waiting for task state * refactor: use consistent task status names * fixup! test: add unit tests for pkg/client/state * fix: revert to previous behaviod polling forever The ctx passed into WaitForTaskToSucceed is only used to cancel HTTP reqests and not to cancel the wait. * chore: add license headers * fix: better function name * refactor: client.go file helper methods (#360) * refactor: client.go file helper methods Refactored the existing methods and functions to be unit testable. Also made some methods that do not use the struct as generic functions. The changes were primarily an effort to add unit test coverage. * refactor: more testable read file function * test: new nutanixcluster types unit tests * test: additional test cases for errors * fixup! refactor: client.go file helper methods * fixup! refactor: client.go file helper methods * fixup! refactor: more testable read file function * fixup! refactor: client.go file helper methods * Switch Nutanix Client to using Session Auth (#398) This will ensure we make fewer basic auth requests to Prism Central IAM Services. --------- Co-authored-by: Faiq <faiq.raza@nutanix.com> Co-authored-by: Dimitri Koshkin <dimitri.koshkin@nutanix.com>

nutanix-cn-prow-bot bot added the do-not-merge/work-in-progress label Jan 10, 2024

nutanix-cn-prow-bot bot requested review from deepakm-ntnx and tuxtof January 10, 2024 20:49

dkoshkin commented Jan 10, 2024

View reviewed changes

dkoshkin force-pushed the refactor-client-file branch from dc31efd to 12a8c67 Compare January 10, 2024 21:10

dkoshkin marked this pull request as ready for review January 10, 2024 21:58

dkoshkin force-pushed the refactor-client-file branch from c2a3bdc to 3c13bb0 Compare January 11, 2024 16:17

faiq reviewed Jan 11, 2024

View reviewed changes

pkg/client/client.go Show resolved Hide resolved

dkoshkin force-pushed the refactor-client-file branch 3 times, most recently from 97cef1c to d4f2b41 Compare January 17, 2024 21:20