cluster-api-provider-linode icon indicating copy to clipboard operation
cluster-api-provider-linode copied to clipboard

[fix] [improvement] move apiserver loadbalancing logic from linodemachine controller to linodecluster controller

Open amold1 opened this issue 1 year ago • 1 comments

What this PR does / why we need it: For APIServer LoadBalancing, we are currently creating NB or DNS entries from the LinodeMachine controller causing inconsistencies during the creation/deletion process leading to either failed attempts to create a cluster or some DNS Entries being left behind. We had also turned off concurrency to mitigate these issues but that hasn't proven to be a good solution.

This PR now moves all of the logic to the LinodeCluster controller allowing it to accurately access what DNS entries or NB backends need to be added or deleted.

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 #

Special notes for your reviewer:

TODOs:

  • [ ] squashed commits
  • [ ] includes documentation
  • [ ] adds unit tests
  • [ ] adds or updates e2e tests

amold1 avatar Aug 15 '24 17:08 amold1

Codecov Report

Attention: Patch coverage is 61.26761% with 110 lines in your changes missing coverage. Please review.

Project coverage is 64.53%. Comparing base (86d3a25) to head (2dda21f). Report is 1 commits behind head on main.

Files Patch % Lines
controller/linodecluster_controller.go 21.53% 47 Missing and 4 partials :warning:
cloud/services/domains.go 75.00% 20 Missing and 5 partials :warning:
controller/linodecluster_controller_helpers.go 40.00% 9 Missing and 3 partials :warning:
cloud/services/loadbalancers.go 82.45% 6 Missing and 4 partials :warning:
cloud/scope/cluster.go 78.94% 2 Missing and 2 partials :warning:
controller/linodemachine_controller.go 0.00% 2 Missing and 2 partials :warning:
cmd/main.go 0.00% 2 Missing :warning:
controller/linodemachine_controller_helpers.go 0.00% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #457      +/-   ##
==========================================
- Coverage   65.33%   64.53%   -0.81%     
==========================================
  Files          80       81       +1     
  Lines        4192     4196       +4     
==========================================
- Hits         2739     2708      -31     
- Misses       1243     1278      +35     
  Partials      210      210              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 15 '24 17:08 codecov[bot]

Changes look good to me, can you rebase off main and squash @amold1 ? 🙏

AshleyDumaine avatar Aug 21 '24 19:08 AshleyDumaine

Changes look good to me, can you rebase off main and squash @amold1 ? 🙏

rebased and checks running now - I'll "squash and merge" on approval

amold1 avatar Aug 21 '24 19:08 amold1