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

Add ReplicaCount to AKODeploymentConfig #248

Merged

Conversation

christianang
Copy link
Contributor

@christianang christianang commented May 7, 2024

What this PR does / why we need it:

This adds spec.extraConfigs.replicaCount to AKODeploymentConfig, which will set the loadBalancerAndIngressService.config.replica_count field in the load-balancer-and-ingress-service-data-values secret.
I also left the default if the field is unset at 1.

Which issue(s) this PR fixes:

Fixes #247

Describe testing done for PR:

Created an AKODeploymentConfig with a replica count of two and see that the data values was configured correctly. I also see two ako instances on my cluster.

Special notes for your reviewer:

Release note:

Add `spec.extraConfigs.replicaCount` to `AKODeploymentConfig` to configure the number of AKO replicas.

New PR Checklist

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

@christianang christianang marked this pull request as ready for review May 7, 2024 22:50
@christianang christianang force-pushed the configurable-ako-replicaset branch from 1b61e4b to 5026df8 Compare May 7, 2024 23:01
@codecov-commenter
Copy link

codecov-commenter commented May 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 31.42%. Comparing base (aeacc93) to head (1b1516e).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #248      +/-   ##
==========================================
+ Coverage   31.01%   31.42%   +0.40%     
==========================================
  Files          31       31              
  Lines        2879     2896      +17     
==========================================
+ Hits          893      910      +17     
  Misses       1916     1916              
  Partials       70       70              
Files Coverage Δ
api/v1alpha1/akodeploymentconfig_types.go 100.00% <ø> (ø)
api/v1alpha1/akodeploymentconfig_webhook.go 79.27% <100.00%> (+1.39%) ⬆️
pkg/ako/values.go 63.84% <100.00%> (+0.51%) ⬆️

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.

Changes overall look good. Could you add a validation inside the validation webhook?
https://github.com/vmware-tanzu/load-balancer-operator-for-kubernetes/blob/main/api/v1alpha1/akodeploymentconfig_webhook.go

@christianang christianang force-pushed the configurable-ako-replicaset branch 2 times, most recently from f1fa253 to 1b1516e Compare May 7, 2024 23:27
@christianang
Copy link
Contributor Author

Changes overall look good. Could you add a validation inside the validation webhook?
https://github.com/vmware-tanzu/load-balancer-operator-for-kubernetes/blob/main/api/v1alpha1/akodeploymentconfig_webhook.go

Added.

@christianang christianang force-pushed the configurable-ako-replicaset branch from 1b1516e to 09db096 Compare May 8, 2024 16:39
Copy link
Contributor

@flawedmatrix flawedmatrix left a comment

Choose a reason for hiding this comment

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

Looks good to me

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

@christianang christianang merged commit c9e2988 into vmware-tanzu:main May 8, 2024
4 checks passed
@christianang christianang deleted the configurable-ako-replicaset branch May 8, 2024 18:30
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.

High Availability of AKO
5 participants