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.

Sign up for GitHub

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

Jump to bottom

feat: Add the ServiceLoadbalancer Addon, with MetalLB as first provider #592

Merged
merged 4 commits into from
May 2, 2024
Merged

feat: Add the ServiceLoadbalancer Addon, with MetalLB as first provider #592

merged 4 commits into from
May 2, 2024

Conversation

dlipovetsky
Copy link
Contributor

@dlipovetsky dlipovetsky commented Apr 29, 2024 •
edited
Loading

What problem does this PR solve?:
Adds the ServiceLoadbalancer addon, and support for MetalLB as the first provider.

TODO:

  • API type
  • Generic handler
  • MetalLB handler
  • Deploy MetalLB Helm chart
  • Default configuration for MetalLB Helm chart
  • Document the ServiceLoadbalancer Addon.

Which issue(s) this PR fixes:
Fixes https://jira.nutanix.com/browse/D2IQ-100444

How Has This Been Tested?:

Special notes for your reviewer:

@jimmidyson
Copy link
Member

jimmidyson commented Apr 30, 2024 •
edited
Loading

When you find time, please don't forget the docs 😉

@github-actions github-actions bot added feature and removed feature labels Apr 30, 2024
@dlipovetsky dlipovetsky force-pushed the serviceloadbalancer branch from 374ba57 to dc65afd Compare April 30, 2024 21:58
@dlipovetsky dlipovetsky marked this pull request as ready for review April 30, 2024 22:02
@dlipovetsky dlipovetsky force-pushed the serviceloadbalancer branch from dc65afd to 649a21b Compare April 30, 2024 22:14
@dlipovetsky dlipovetsky requested review from faiq and dkoshkin April 30, 2024 22:18
dkoshkin
dkoshkin approved these changes May 1, 2024
View reviewed changes
Copy link
Contributor

@dkoshkin dkoshkin left a comment

Choose a reason for hiding this comment

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

Looks great, just the one comment about Loadbalancer vs LoadBalancer in the API, and other vars and output strings.

api/v1alpha1/addon_types.go Outdated Show resolved Hide resolved
@dlipovetsky dlipovetsky mentioned this pull request May 1, 2024
Merged
@jimmidyson jimmidyson enabled auto-merge (squash) May 2, 2024 08:17
@jimmidyson jimmidyson merged commit e42d887 into nutanix-cloud-native:main May 2, 2024
16 checks passed
@github-actions github-actions bot mentioned this pull request May 2, 2024
Merged
jimmidyson added a commit that referenced this pull request May 8, 2024
@dlipovetsky @dkoshkin @jimmidyson
fix: Correct the CSI handler logic (#603)
f091343 
**What problem does this PR solve?**:
- Return an error when the CSI variable is defined, but the
  Providers list is empty.
- Return an error when the CSI provider is defined, but its
  StorageClassConfig list is empty.
- Return an error when any CSI provider is unknown.
- Return an error when DefaultStorage is empty.
- Return an error when the DefaultStorage Provider Name does not match a
set provider.
- Return an error when the DefaultStorage StorageClassConfig Name does
not match the name of a StorageClassConfig for the default provider.

Adds unit tests to verify this logic, and more. Covers the majority of
the generic CSI handler `AfterControlPlaneInitialized` hook.

**Which issue(s) this PR fixes**:
Fixes #

**How Has This Been Tested?**:
<!--
Please describe the tests that you ran to verify your changes.
Provide output from the tests and any manual steps needed to replicate
the tests.
-->
Adds a unit test; I think this is the first unit test in CAREN to
exercise a handler request-response flow.
 
**Special notes for your reviewer**:
<!--
Use this to provide any additional information to the reviewers.
This may include:
- Best way to review the PR.
- Where the author wants the most review attention on.
- etc.
-->
I noticed these issues as I was studying the CSI handler in order to
write the ServiceLoadbalancer handler. I'll be adding a unit test to
#592 as well.

---------

Co-authored-by: Dimitri Koshkin <[email protected]>
Co-authored-by: Jimmi Dyson <[email protected]>
jimmidyson added a commit that referenced this pull request May 21, 2024
@jimmidyson @github-actions
release(main): v0.9.0 (#597)
0ce44c2 
🤖 I have created a release *beep* *boop*
---


## 0.9.0 (2024-05-21)

<!-- Release notes generated using configuration in .github/release.yaml
at main -->

## What's Changed
### Exciting New Features 🎉
* feat: expose GenerateNoProxy func by @mhrabovcin in
#594
* feat: Add the ServiceLoadbalancer Addon, with MetalLB as first
provider by @dlipovetsky in
#592
* feat: adds GPU mutation by @faiq in
#591
* feat: Add GenericClusterConfig and add docs on usage with own CC by
@jimmidyson in
#606
* feat: Enable unprivileged ports sysctl in containerd config by
@jimmidyson in
#645
* feat: API for encryption at-rest by @supershal in
#610
* feat: Bump sigs.k8s.io/cluster-api to v1.7.2 by @jimmidyson in
#661
* feat: Pull calico images from quay.io instead of docker hub by
@jimmidyson in
#676
* feat: update cluster autoscaler to v1.30.0 by @dkoshkin in
#681
### Fixes 🔧
* fix: Fix error messages returned by HelmChartGetter by @dlipovetsky in
#598
* fix: use a consistent MachineDeployment class name by @dkoshkin in
#612
* fix: Do not return error if serviceLoadBalancer field is not set by
@dlipovetsky in
#611
* fix: use provided options for serverside apply by @supershal in
#627
* fix: Correct the CSI handler logic by @dlipovetsky in
#603
* fix: Fix the internal ClusterConfig type used for provider-agnostic
logic by @jimmidyson in
#607
* fix: log mutation failure errors by @supershal in
#649
* fix: Always apply containerd patches by @jimmidyson in
#644
* fix: cluster-autoscaler Helm values for workload clusters by @dkoshkin
in
#658
* fix: Make Cluster the owner of image registry credential secret by
@dlipovetsky in
#648
* fix: Upgrade dynamic-credential-provider to v0.5.3 by @jimmidyson in
#677
### Other Changes
* build: Add v0.8 release metadata by @jimmidyson in
#595
* refactor: Clean up API constants, and explain usage by @dlipovetsky in
#588
* docs: Add how to deploy CAREN by @jimmidyson in
#599
* docs: Upgrade hugo to latest by @jimmidyson in
#601
* docs: Update addons docs and tweak release doc by @jimmidyson in
#596
* build: Ensure provider metadata is up to date when releasing by
@jimmidyson in
#600
* docs: Add how to create clusters by @jimmidyson in
#602
* docs: Update docsy module by @jimmidyson in
#605
* refactor: Apply kubebuilder annotations for required/optional
everywhere by @jimmidyson in
#604
* docs: Cluster Autoscaler is deployed on the management cluster by
@dkoshkin in
#608
* docs: Fix missing placeholder in "create nutanix cluster" doc by
@dlipovetsky in
#609
* refactor: Remove unused api/variables package by @dlipovetsky in
#623
* refactor: move label helper functions to utils package by @supershal
in
#626
* build: Use go1.22.3 toolchain to mitigate vulnerabilties by
@jimmidyson in
#628
* build: Temporary lint config fix until next golangci-lint release by
@jimmidyson in
#629
* build: Update license for Nutanix by @jimmidyson in
#456
* test(e2e): Consistent core/bootstrap/control-plane provider versions
by @jimmidyson in
#639
* ci: free up disk space before running tests by @dkoshkin in
#643
* test: Add more context to panic in envtest helper by @dlipovetsky in
#641
* refactor: Use colon to separate context from wrapped error by
@dlipovetsky in
#642
* refactor: Remove unused test helper function by @dlipovetsky in
#647
* test: Add even more context to panic in envtest helper by @dlipovetsky
in
#650
* build: Make module-relative "go list -m" compatible with GOWORK by
@dlipovetsky in
#651
* test: Match cluster namespace to cluster name by @dlipovetsky in
#652
* refactor: Write configuration under /etc/caren by @dlipovetsky in
#656
* build: use a shorter namespace caren-system by @dkoshkin in
#662
* refactor: Use a Credentials struct consistently by @dlipovetsky in
#663
* test: add encryptionAtRest config in capi-quick-start by @supershal in
#659
* test(e2e): Fix up secret ownership checks by @jimmidyson in
#665
* test: Remove hard-coded text focus and label for e2e tests by
@dlipovetsky in
#667
* ci: Use new dependabot multimodule capabilities by @jimmidyson in
#664
* refactor: aggregate types to be used by clients by @dkoshkin in
#672
* test: Add E2E_DRYRUN and E2E_VERBOSE make vars by @dlipovetsky in
#666
* build: Ignore all gitlint rules for dependabot commits by @jimmidyson
in
#675
* build: Update all tools by @jimmidyson in
#678
* test(e2e): Use upstream CRS helpers by @jimmidyson in
#680
* build: Correct dry-run output by @jimmidyson in
#679
* build: Use k8s v1.29.4 as default Kubernetes version by @jimmidyson in
#646

## New Contributors
* @prajnutanix made their first contribution in
#638

**Full Changelog**:
v0.8.1...v0.9.0

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yannickstruyf3 yannickstruyf3 yannickstruyf3 approved these changes

@dkoshkin dkoshkin dkoshkin approved these changes

@jimmidyson jimmidyson jimmidyson approved these changes

@faiq faiq Awaiting requested review from faiq

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dlipovetsky @jimmidyson @dkoshkin @yannickstruyf3