Update MetalLB deployment, wait for resource. #9995

Jeroen0494 · 2023-04-17T12:24:45Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change

/kind bug

/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

Fixes #10003

What this PR does / why we need it:

Continuation of PR #9120.

Does this PR introduce a user-facing change?:

Update MetalLB deployment, wait for resource.

k8s-ci-robot · 2023-04-17T12:24:54Z

Hi @Jeroen0494. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Jeroen0494 · 2023-04-17T12:26:41Z

Also see my comment here:
#9120 (comment)

floryut

@Jeroen0494 Thanks for the followup fix 🙇
/ok-to-test

liupeng0518 · 2023-04-19T08:20:38Z

#9990

pls rebase master branch.

Jeroen0494 · 2023-04-20T09:09:10Z

#9990

pls rebase master branch.

Hi, I rebased, pushed some more fixes and squashed my commits.

eugene-marchanka · 2023-04-20T16:18:06Z

Hey! I have an issue running your branch:

TASK [kubernetes-apps/metallb : Kubernetes Apps | Install and configure MetalLB] **********************************************************************************************************
fatal: [test00-00]: FAILED! => 
  msg: '''dict object'' has no attribute ''results'''
skipping: [test00-01]
skipping: [test00-02]

Could you, please, take a look?
Thanks!

Jeroen0494 · 2023-04-20T17:08:24Z

Hey! I have an issue running your branch:

TASK [kubernetes-apps/metallb : Kubernetes Apps | Install and configure MetalLB] **********************************************************************************************************
fatal: [test00-00]: FAILED! => 
  msg: '''dict object'' has no attribute ''results'''
skipping: [test00-01]
skipping: [test00-02]

Could you, please, take a look? Thanks!

Hi, I'm sorry this is happening. I'll look into it tomorrow afternoon when I'm at work, and give the whole thing a more thorough testing.

I'm also a bit surprised this wasn't noticed during CI/CD testing. @floryut is MetalLB included in the Kubespray CI tests?

eugene-marchanka · 2023-04-20T17:14:46Z

I fixed it modifying this:

- name: Kubernetes Apps | Lay Down MetalLB
  become: true
  template:
    src: "{{ item }}.j2"
    dest: "{{ kube_config_dir }}/{{ item }}"
    mode: 0644
  with_items:
  - metallb.yml
  register: metallb_rendering
  when:
  - "inventory_hostname == groups['kube_control_plane'][0]"

but faced another problem:

  msg: |-
    error running kubectl (/usr/local/bin/kubectl apply --force --wait --filename=/etc/kubernetes/metallb.yml) command (rc=1), out='customresourcedefinition.apiextensions.k8s.io/addresspools.metallb.io created
    customresourcedefinition.apiextensions.k8s.io/bfdprofiles.metallb.io created
    customresourcedefinition.apiextensions.k8s.io/bgpadvertisements.metallb.io created
    customresourcedefinition.apiextensions.k8s.io/bgppeers.metallb.io created
    customresourcedefinition.apiextensions.k8s.io/communities.metallb.io created
    customresourcedefinition.apiextensions.k8s.io/ipaddresspools.metallb.io created
    customresourcedefinition.apiextensions.k8s.io/l2advertisements.metallb.io created
    clusterrole.rbac.authorization.k8s.io/metallb-system:controller created
    clusterrole.rbac.authorization.k8s.io/metallb-system:speaker created
    clusterrolebinding.rbac.authorization.k8s.io/metallb-system:controller created
    clusterrolebinding.rbac.authorization.k8s.io/metallb-system:speaker created
    ', err='error parsing /etc/kubernetes/metallb.yml: error converting YAML to JSON: yaml: line 133: mapping values are not allowed in this context
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found
    Error from server (NotFound): error when creating "/etc/kubernetes/metallb.yml": namespaces "metallb-system" not found

eugene-marchanka

some refactoring for error:

TASK [kubernetes-apps/metallb : MetalLB | Create address pools configuration] *************************************************************************************************************
fatal: [test00-00]: FAILED! =>                                                                                                                                                             
  msg: |-                                                                                                                                                                                  
    The task includes an option with an undefined variable. The error was: 'item' is undefined                                                                                             
                                                                                                                                                                                           
    The error appears to be in '..kubespray/roles/kubernetes-apps/metallb/tasks/main.yml': line 6
8, column 5, but may                                                                                                                                                                       
    be elsewhere in the file depending on the exact syntax problem.                                                                                                                        
                                                                                                                                                                                           
    The offending line appears to be:                                                                                                                                                      
                                                                                                                                                                                                                                                                                                                                                                                      
      - name: MetalLB | Create address pools configuration                                                                                                                                 
        ^ here                                                                                                                                                                             
skipping: [test00-01]                                                                                                                                                                      
skipping: [test00-02]

roles/kubernetes-apps/metallb/tasks/main.yml

eugene-marchanka · 2023-04-20T18:28:32Z

another one:

TASK [kubernetes-apps/metallb : Kubernetes Apps | Delete MetalLB ConfigMap] ***************************************************************************************************************
skipping: [test00-01]
skipping: [test00-02]
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ModuleNotFoundError: No module named 'kubernetes'
fatal: [test00-00]: FAILED! => changed=false 
  error: No module named 'kubernetes'

eugene-marchanka · 2023-04-20T18:42:52Z

another one:

TASK [kubernetes-apps/metallb : Kubernetes Apps | Delete MetalLB ConfigMap] ***************************************************************************************************************
skipping: [test00-01]
skipping: [test00-02]
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ModuleNotFoundError: No module named 'kubernetes'
fatal: [test00-00]: FAILED! => changed=false 
  error: No module named 'kubernetes'

This works fine with me:

- name: Kubernetes Apps | Delete MetalLB ConfigMap
  command: "{{ bin_dir }}/kubectl delete configmap -n metallb-system config --ignore-not-found=true"
  when: inventory_hostname == groups['kube_control_plane'][0]
  no_log: "{{ not (unsafe_show_logs|bool) }}"

floryut · 2023-04-21T13:29:28Z

Hey! I have an issue running your branch:
TASK [kubernetes-apps/metallb : Kubernetes Apps | Install and configure MetalLB] **********************************************************************************************************
fatal: [test00-00]: FAILED! => 
  msg: '''dict object'' has no attribute ''results'''
skipping: [test00-01]
skipping: [test00-02]
Could you, please, take a look? Thanks!
Hi, I'm sorry this is happening. I'll look into it tomorrow afternoon when I'm at work, and give the whole thing a more thorough testing.

I'm also a bit surprised this wasn't noticed during CI/CD testing. @floryut is MetalLB included in the Kubespray CI tests?

Now that you are mentioning this, after a quick search we don't have any CI coverage for MetalLB use cases, not even a simple deployment :/

Jeroen0494 · 2023-04-21T14:14:47Z

@eugene-marchanka Thank you for the help! I've updated the installation, should be quite a bit better now. Could you give it a spin?

eugene-marchanka · 2023-04-21T15:50:55Z

@eugene-marchanka Thank you for the help! I've updated the installation, should be quite a bit better now. Could you give it a spin?

Hey!
Just did another run and got an error:

TASK [kubernetes-apps/metallb : MetalLB | Create address pools configuration] *************************************************************************************************************
skipping: [test00-01]
skipping: [test00-02]
fatal: [test00-00]: FAILED! => changed=false 
  msg: |-
    error running kubectl (/usr/local/bin/kubectl apply --force --filename=/etc/kubernetes/pools.yaml) command (rc=1), out='', err='Error from server (InternalError): error when creating "/etc/kubernetes/pools.yaml": Internal error occurred: failed calling webhook "ipaddresspoolvalidationwebhook.metallb.io": failed to call webhook: Post "https://webhook-service.metallb-system.svc:443/validate-metallb-io-v1beta1-ipaddresspool?timeout=10s": dial tcp 10.233.14.18:443: connect: connection refused
    '

Jeroen0494 · 2023-04-21T16:08:24Z

@eugene-marchanka Thank you for the help! I've updated the installation, should be quite a bit better now. Could you give it a spin?

Hey! Just did another run and got an error:

TASK [kubernetes-apps/metallb : MetalLB | Create address pools configuration] *************************************************************************************************************
skipping: [test00-01]
skipping: [test00-02]
fatal: [test00-00]: FAILED! => changed=false 
  msg: |-
    error running kubectl (/usr/local/bin/kubectl apply --force --filename=/etc/kubernetes/pools.yaml) command (rc=1), out='', err='Error from server (InternalError): error when creating "/etc/kubernetes/pools.yaml": Internal error occurred: failed calling webhook "ipaddresspoolvalidationwebhook.metallb.io": failed to call webhook: Post "https://webhook-service.metallb-system.svc:443/validate-metallb-io-v1beta1-ipaddresspool?timeout=10s": dial tcp 10.233.14.18:443: connect: connection refused
    '

Yea, so this is the reason I want to use the kubernetes.core.k8s module. Because the playbook needs to wait until the controller pod is properly running before resources can be created.

@floryut any change I can switch from using the self-written kube module to the kubernetes.core collection, and add this as a requirement in requirements.txt? Because the kube module doesn't have what I need to prevent any further errors in this regard. Without a proper wait-until-ready, this isn't going to work.

eugene-marchanka · 2023-04-21T17:12:53Z

@eugene-marchanka Thank you for the help! I've updated the installation, should be quite a bit better now. Could you give it a spin?

Hey! Just did another run and got an error:

TASK [kubernetes-apps/metallb : MetalLB | Create address pools configuration] *************************************************************************************************************
skipping: [test00-01]
skipping: [test00-02]
fatal: [test00-00]: FAILED! => changed=false 
  msg: |-
    error running kubectl (/usr/local/bin/kubectl apply --force --filename=/etc/kubernetes/pools.yaml) command (rc=1), out='', err='Error from server (InternalError): error when creating "/etc/kubernetes/pools.yaml": Internal error occurred: failed calling webhook "ipaddresspoolvalidationwebhook.metallb.io": failed to call webhook: Post "https://webhook-service.metallb-system.svc:443/validate-metallb-io-v1beta1-ipaddresspool?timeout=10s": dial tcp 10.233.14.18:443: connect: connection refused
    '

A workaround is to add simple sleep task before deploying pools. Finally deployed without issues 🚀 :

- name: Kubernetes Apps | Install and configure MetalLB
  kube:
    name: "MetalLB"
    kubectl: "{{ bin_dir }}/kubectl"
    filename: "{{ kube_config_dir }}/metallb.yml"
    state: "{{ metallb_rendering.changed | ternary('latest','present') }}"
    wait: true
  become: true
  when:
  - "inventory_hostname == groups['kube_control_plane'][0]"

- name: Sleep
  command: "sleep 300"
  when: inventory_hostname == groups['kube_control_plane'][0]
  no_log: "{{ not (unsafe_show_logs|bool) }}"

- name: MetalLB | Address pools
  block:
  - name: MetalLB | Layout address pools template
    ansible.builtin.template:
      src: pools.yaml.j2
      dest: "{{ kube_config_dir }}/pools.yaml"
      mode: 0644
    register: pools_rendering

Jeroen0494 · 2023-05-09T08:50:28Z

Thank you so much for working hard to fix the issue @Jeroen0494 @eugene-marchanka !
@Jeroen0494 Could you apply @eugene-marchanka's workaround into this pull request? That would help many people I guess.

I certainly can, but this would implement a wait of 300, even if the controller is already running. What's against using the kubernetes.core.k8s module? I already have working code for this.

@oomichi kind reminder.

yankay · 2023-05-18T02:31:00Z

HI @Jeroen0494

The v2.22 will release the next week.
So the PR can be combined into v2.22 if it is merged before it.

Jeroen0494 · 2023-05-23T19:02:42Z

HI @Jeroen0494

The v2.22 will release the next week. So the PR can be combined into v2.22 if it is merged before it.

Thanks. I'm still waiting for an answer for my question, so I guess it won't be ready before the release.

Nathanael-Mtd · 2023-05-24T15:29:46Z

Hi !

It seems Kubespray v2.22.0 release broke upgrade/install for metallb users because that PR isn't merged :/

Maybe it should a good idea to add an advertissement to tell users to skip the metallb upgrade for the v2.22.0 release ?

cosandr

Thanks for your effort, I've tried this PR out a bit and found some issues.

I'd also like to comment on the usage of kubernetes.core modules, these require the Python kubernetes library on the target host, which is likely not so trivial to setup considering the broad OS compatibility of kubespray. You could attempt to delegate these tasks to localhost, but personally I don't think that's a great solution either.

Oh yeah, and I suggest you look into kubectl wait instead of an arbitrary sleep, i.e. kubectl -n metallb-system wait --for condition=established --timeout=300s crd/addresspools.metallb.io.

roles/kubernetes-apps/metallb/templates/metallb.yml.j2

Jeroen0494 · 2023-05-26T11:38:25Z

Thanks for your effort, I've tried this PR out a bit and found some issues.

I'd also like to comment on the usage of kubernetes.core modules, these require the Python kubernetes library on the target host, which is likely not so trivial to setup considering the broad OS compatibility of kubespray. You could attempt to delegate these tasks to localhost, but personally I don't think that's a great solution either.

Oh yeah, and I suggest you look into kubectl wait instead of an arbitrary sleep, i.e. kubectl -n metallb-system wait --for condition=established --timeout=300s crd/addresspools.metallb.io.

Indeed we use connection: local for our internal Kubernetes deployments, the only change this would require for Kubespray is to add the following to the requirements.txt:

ansible-galaxy collection install kubernetes.core

That's really everything.

But I will take another look at the kubectl wait command to see if there is something this can offer. And I will review your requested changes.

Jeroen0494 · 2023-05-26T12:21:14Z

@cosandr @yankay I've added a simple wait for the controller to be running.

MrFreezeex

A few comments but looks good overall, thanks :). Could you also add a test similarly to the other PR?

roles/kubernetes-apps/metallb/tasks/main.yml

MrFreezeex · 2023-05-26T13:44:55Z

Ah and could you also please cleanup (or squash) you commit history?

Signed-off-by: Jeroen Rijken <[email protected]>

Jeroen0494 · 2023-05-30T07:21:22Z

@MrFreezeex fixes applied and commits squashed.

MrFreezeex · 2023-05-30T08:09:46Z

@MrFreezeex fixes applied and commits squashed.

It's missing a test like in here https://github.com/kubernetes-sigs/kubespray/pull/10135/files#diff-bd336fb2e0cd1a14218d71fb76a197149c77d677109d2415562c9c53fe774e76

Signed-off-by: Jeroen Rijken <[email protected]>

Jeroen0494 · 2023-05-30T08:59:00Z

I've added a basic test.

MrFreezeex

Thanks for your contribution and the extra miles of adding tests and docs!
/lgtm

k8s-ci-robot · 2023-05-30T09:58:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floryut, Jeroen0494, MrFreezeex

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [floryut]

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

* Update MetalLB deployment, wait for resource. Signed-off-by: Jeroen Rijken <[email protected]> * yml to yaml, add basic test for metallb Signed-off-by: Jeroen Rijken <[email protected]> --------- Signed-off-by: Jeroen Rijken <[email protected]>

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 17, 2023

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 17, 2023

k8s-ci-robot requested review from jayonlau and oomichi April 17, 2023 12:24

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 17, 2023

floryut approved these changes Apr 18, 2023

View reviewed changes

Jeroen0494 force-pushed the feat/metallbv0.13.4 branch 2 times, most recently from ae9598a to 1beb29f Compare April 20, 2023 09:08

oomichi mentioned this pull request Apr 20, 2023

[MetalLB] Remove unused resources #10004

Merged

eugene-marchanka suggested changes Apr 20, 2023

View reviewed changes

roles/kubernetes-apps/metallb/tasks/main.yml Outdated Show resolved Hide resolved

roles/kubernetes-apps/metallb/tasks/main.yml Outdated Show resolved Hide resolved

roles/kubernetes-apps/metallb/tasks/main.yml Outdated Show resolved Hide resolved

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 21, 2023

Jeroen0494 mentioned this pull request May 15, 2023

Release Proposal v2.22 #10075

Closed

cosandr reviewed May 25, 2023

View reviewed changes

roles/kubernetes-apps/metallb/templates/metallb.yml.j2 Outdated Show resolved Hide resolved

roles/kubernetes-apps/metallb/templates/metallb.yml.j2 Outdated Show resolved Hide resolved

roles/kubernetes-apps/metallb/templates/metallb.yml.j2 Outdated Show resolved Hide resolved

Jeroen0494 mentioned this pull request May 26, 2023

Metallb fix #10135

Closed

Jeroen0494 force-pushed the feat/metallbv0.13.4 branch from 9760fe9 to 720b393 Compare May 26, 2023 11:52

MrFreezeex reviewed May 26, 2023

View reviewed changes

roles/kubernetes-apps/metallb/tasks/main.yml Outdated Show resolved Hide resolved

roles/kubernetes-apps/metallb/tasks/main.yml Outdated Show resolved Hide resolved

Jeroen0494 mentioned this pull request May 30, 2023

Avoid creating metallb config if unnecessary #10152

Closed

Update MetalLB deployment, wait for resource.

d74d1b6

Signed-off-by: Jeroen Rijken <[email protected]>

Jeroen0494 force-pushed the feat/metallbv0.13.4 branch from a145ff4 to d74d1b6 Compare May 30, 2023 07:20

yml to yaml, add basic test for metallb

ee7f2bb

Signed-off-by: Jeroen Rijken <[email protected]>

MrFreezeex approved these changes May 30, 2023

View reviewed changes

k8s-ci-robot assigned MrFreezeex May 30, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 30, 2023

k8s-ci-robot merged commit ea7dcd4 into kubernetes-sigs:master May 30, 2023

Jeroen0494 mentioned this pull request May 31, 2023

[2.22] MetalLB backport #10164

Merged

yankay mentioned this pull request Jun 8, 2023

Release Proposal v2.22.1 #10179

Closed

Jeroen0494 deleted the feat/metallbv0.13.4 branch June 8, 2023 06:53

yankay mentioned this pull request Aug 24, 2023

Release Proposal v2.23 #10389

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update MetalLB deployment, wait for resource. #9995

Update MetalLB deployment, wait for resource. #9995

Jeroen0494 commented Apr 17, 2023 •

edited by floryut

Loading

k8s-ci-robot commented Apr 17, 2023

Jeroen0494 commented Apr 17, 2023

floryut left a comment

liupeng0518 commented Apr 19, 2023

Jeroen0494 commented Apr 20, 2023

eugene-marchanka commented Apr 20, 2023

Jeroen0494 commented Apr 20, 2023

eugene-marchanka commented Apr 20, 2023

eugene-marchanka left a comment •

edited

Loading

eugene-marchanka commented Apr 20, 2023

eugene-marchanka commented Apr 20, 2023

floryut commented Apr 21, 2023

Jeroen0494 commented Apr 21, 2023

eugene-marchanka commented Apr 21, 2023

Jeroen0494 commented Apr 21, 2023

eugene-marchanka commented Apr 21, 2023

Jeroen0494 commented May 9, 2023

yankay commented May 18, 2023 •

edited by floryut

Loading

Jeroen0494 commented May 23, 2023

Nathanael-Mtd commented May 24, 2023

cosandr left a comment •

edited

Loading

Jeroen0494 commented May 26, 2023

Jeroen0494 commented May 26, 2023

MrFreezeex left a comment

MrFreezeex commented May 26, 2023 •

edited

Loading

Jeroen0494 commented May 30, 2023

MrFreezeex commented May 30, 2023

Jeroen0494 commented May 30, 2023

MrFreezeex left a comment •

edited

Loading

k8s-ci-robot commented May 30, 2023

Update MetalLB deployment, wait for resource. #9995

Update MetalLB deployment, wait for resource. #9995

Conversation

Jeroen0494 commented Apr 17, 2023 • edited by floryut Loading

k8s-ci-robot commented Apr 17, 2023

Jeroen0494 commented Apr 17, 2023

floryut left a comment

Choose a reason for hiding this comment

liupeng0518 commented Apr 19, 2023

Jeroen0494 commented Apr 20, 2023

eugene-marchanka commented Apr 20, 2023

Jeroen0494 commented Apr 20, 2023

eugene-marchanka commented Apr 20, 2023

eugene-marchanka left a comment • edited Loading

Choose a reason for hiding this comment

eugene-marchanka commented Apr 20, 2023

eugene-marchanka commented Apr 20, 2023

floryut commented Apr 21, 2023

Jeroen0494 commented Apr 21, 2023

eugene-marchanka commented Apr 21, 2023

Jeroen0494 commented Apr 21, 2023

eugene-marchanka commented Apr 21, 2023

Jeroen0494 commented May 9, 2023

yankay commented May 18, 2023 • edited by floryut Loading

Jeroen0494 commented May 23, 2023

Nathanael-Mtd commented May 24, 2023

cosandr left a comment • edited Loading

Choose a reason for hiding this comment

Jeroen0494 commented May 26, 2023

Jeroen0494 commented May 26, 2023

MrFreezeex left a comment

Choose a reason for hiding this comment

MrFreezeex commented May 26, 2023 • edited Loading

Jeroen0494 commented May 30, 2023

MrFreezeex commented May 30, 2023

Jeroen0494 commented May 30, 2023

MrFreezeex left a comment • edited Loading

Choose a reason for hiding this comment

k8s-ci-robot commented May 30, 2023

Jeroen0494 commented Apr 17, 2023 •

edited by floryut

Loading

eugene-marchanka left a comment •

edited

Loading

yankay commented May 18, 2023 •

edited by floryut

Loading

cosandr left a comment •

edited

Loading

MrFreezeex commented May 26, 2023 •

edited

Loading

MrFreezeex left a comment •

edited

Loading