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

Ensure GetControlPlaneEndpoint can still return an IP #177

Conversation

flawedmatrix
Copy link
Contributor

Even if the cluster-controlplane-endpoint annotation or ClusterClass topology variable for apiServerEndpoint are not set, it can still retrieve the control plane endpoint from the cluster.Spec.ControlPlaneEndpoint.Host.

What this PR does / why we need it:
This change mitigates the situation in which a user might accidentally delete the control plane HA service for the cluster and for some reason the cluster-controlplane-endpoint annotation or ClusterClass topology variable for apiServerEndpoint are not set.

Which issue(s) this PR fixes:

Fixes #

Describe testing done for PR:
Manually verified on a testbed with Avi + AKO.

  1. Scaled down deployment of AKOO to 0
  2. Edited the cluster and deleted the apiServerEndpoint topology variable and deleted the control plane HA service
  3. Scaled deployment of AKOO back to 1
  4. Saw that the control plane HA service was created again with the correct ExternalIP.

Special notes for your reviewer:

Release note:


New PR Checklist

  • [ x ] Ensure PR contains only public links or terms
  • [ x ] Use good commit messages
  • [ x ] Squash the commits in this branch before merge to preserve our git history
  • [ x ] If this PR is just an idea or POC, use a Draft PR instead of a full PR
  • [ x ] Add appropriate labels according to what type of issue is being addressed.

Even if the cluster-controlplane-endpoint annotation or ClusterClass
topology variable for apiServerEndpoint are not set, it can still
retrieve the control plane endpoint from the
cluster.Spec.ControlPlaneEndpoint.Host.

Co-authored-by: Aidan Obley <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Dec 7, 2023

Codecov Report

Merging #177 (7f72fde) into main (ce32f01) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #177      +/-   ##
==========================================
+ Coverage   36.27%   36.34%   +0.07%     
==========================================
  Files          20       20              
  Lines        2721     2724       +3     
==========================================
+ Hits          987      990       +3     
  Misses       1675     1675              
  Partials       59       59              
Files Coverage Δ
pkg/ako-operator/lib.go 100.00% <100.00%> (ø)

- Modifying the cluster object directly in Golang also somehow modifies
  it on the fake API side
Copy link
Contributor

@lubronzhan lubronzhan left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Thanks

Copy link
Member

@XudongLiuHarold XudongLiuHarold left a comment

Choose a reason for hiding this comment

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

LGTM

@lubronzhan lubronzhan merged commit 3046d4a into vmware-tanzu:main Dec 8, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants