Skip to content

[WIP] Refactor MetalLB Installation #12126

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
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[WIP] Refactor MetalLB Installation #12126

wants to merge 5 commits into from

Conversation

tico88612
Copy link
Member

What type of PR is this?

/kind design
/kind feature

What this PR does / why we need it:

Deprecate the old template installation.
This is proof of concept for the application installation via Kubespray.

Special notes for your reviewer:

This PR can be delayed until the next release (2.29), which is important but not an emergency.

Does this PR introduce a user-facing change?:

MetalLB installation replaces Jinja template with Helm

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/design Categorizes issue or PR as related to design. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 11, 2025
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 11, 2025
@tico88612
Copy link
Member Author

/label tide/merge-method-merge
/ok-to-test

@k8s-ci-robot k8s-ci-robot added tide/merge-method-merge Denotes a PR that should use a standard merge by tide when it merges. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Apr 11, 2025
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 7, 2025
@tico88612 tico88612 force-pushed the refactor/metallb-install branch from c2da1a6 to 46c5cc3 Compare May 18, 2025 08:48
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 18, 2025
tico88612 added 5 commits May 22, 2025 21:43
@tico88612
Add metallb values template
088d0be 
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
@tico88612
Delete unused task
2123f44 
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
@tico88612
Replace template with metallb helm install
7efc650 
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
@tico88612
Delete metallb template file
56a18cd 
Signed-off-by: ChengHao Yang
<17496418+tico88612@users.noreply.github.com>
@tico88612
Upgrade metallb 0.14.9
f12437b 
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
@tico88612 tico88612 force-pushed the refactor/metallb-install branch from 46c5cc3 to f12437b Compare May 22, 2025 13:44
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tico88612
Once this PR has been reviewed and has the lgtm label, please assign mzaian for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tmurakam
tmurakam reviewed May 23, 2025
View reviewed changes
roles/kubernetes-apps/metallb/tasks/main.yml
when:
- inventory_hostname == groups['kube_control_plane'][0]

- name: Kubernetes Apps | Install and configure MetalLB
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you need to remove these older resources before install the helm chart when upgrading cluster? (or out of scope of this role?)

Copy link
Member Author

@tico88612 tico88612 May 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If you delete the old resources directly, it will cause a service interruption, which I have seen someone else ask about upgrade earlier.
My idea is to add label and annotation to the existing MetalLB resources so that helm can take over.

wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood.
No problem, if the charts can overwrite all of the old resources.

@tico88612 tico88612 changed the title Refactor MetalLB Installation [WIP] Refactor MetalLB Installation May 23, 2025
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmurakam tmurakam tmurakam left review comments

@cyclinder cyclinder Awaiting requested review from cyclinder

@yankay yankay Awaiting requested review from yankay

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/design Categorizes issue or PR as related to design. kind/feature Categorizes issue or PR as related to a new feature. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tide/merge-method-merge Denotes a PR that should use a standard merge by tide when it merges.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tico88612 @k8s-ci-robot @tmurakam