[Bug] use kube-vip-iptables image when kube_vip_lb_fwdmethod is masquerade

[aviral-agarwal]

What type of PR is this?
/kind bug

What this PR does / why we need it:

• kube-vip maintains two images ghcr.io/kube-vip/kube-vip and ghcr.io/kube-vip/kube-vip-iptables https://github.com/kube-vip/kube-vip/pkgs/container/kube-vip-iptables, which needs to be selected based on lb_fwdmethod flag in kube-vip manifest yml.
  If lb_fwdmethod is masquerade, then kube-vip-iptables image needs to be used
• In Kubespray, this kube-vip manifest flag is mapped to or is picked from kube_vip_lb_fwdmethod variable
• But currently, in Kubepsray, the image for kube-vip is fixed to ghcr.io/kube-vip/kube-vip irrespective of lb_fwdmethod or kube_vip_lb_fwdmethod, which causes kube-vip to not work as expected
• The following error can be observed in kube-vip pod logs

time="2025-02-28T15:52:37Z" level=info msg="Node [vm1-private] is assuming leadership of the cluster"
time="2025-02-28T15:52:37Z" level=fatal msg="could not add iptables rules for masquerade: could not ge
t iptables version: run cmd 'iptables --version' wtith error: exec: \"iptables\": executable file not
found in $PATH"

Which issue(s) this PR fixes:
Fixes #

• Though I could not find issues in Kubespray that this PR fixes, I am adding the following links to issues from kube-vip where this issue has been discussed and closed with this solution
  kube-vip issue 1065
  kube-vip issue 874
• Unfortunately, kube-vip documentation does not cover this well enough (also being discussed in one of the above issue)
• For reference, I see an old PR use kube-vip image with iptables command #11838 which, I believe, was not merged. I have tried to incorporate the feedback from the same in this PR

Special notes for your reviewer:

• add a new variable kube_vip_iptables_image_repo in roles\kubespray-defaults\defaults\main\download.yml for kube-vip-iptables image of kube-vip
• changes made in roles\kubernetes\node\templates\manifests\kube-vip.manifest.j2 to use ghcr.io/kube-vip/kube-vip-iptables instead of ghcr.io/kube-vip/kube-vip if kube_vip_lb_fwdmethod is masquerade
• I am following kube-vip issue 1065 to make sure kube-vip-iptables image is only required for kube_vip_lb_fwdmethod==masquerade. If required for any other mode, I will update.

Does this PR introduce a user-facing change?:

• only one change
• kube_vip_lb_fwdmethod defined with defaults in roles\kubernetes\node\defaults\main.yml but not in inventory\sample\group_vars\k8s_cluster\addons.yml
• adding kube_vip_lb_fwdmethod in inventory\sample\group_vars\k8s_cluster\addons.yml as it is frequently used by kube-vip users
• It is only added as a comment/hint so that folks don't need to search for it in code, and are aware
• It is added with already existing kube-vip related settings, which are expected to be updated by users

Fixed kube-vip to use `kube-vip/kube-vip-iptables` image instead of `kube-vip/kube-vip` when `lb_fwdmethod` or `kube_vip_lb_fwdmethod` is set to `masquerade`

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: aviral-agarwal / name: Aviral Agarwal (933f320)

[k8s-ci-robot]

Welcome @aviral-agarwal!

It looks like this is your first PR to kubernetes-sigs/kubespray 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubespray has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @aviral-agarwal. 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-sigs/prow repository.

[tico88612]

I think you don't need create another variable, you just need to modify it according to kube_vip_lb_fwdmethod on kube_vip_image_repo. (like add the condition)

[tico88612]

Maybe we don't need to change that.

[tico88612]

Please reserve the release-note block and write some description in your PR content.

[aviral-agarwal]

Added release-note block in the PR, I believe it is supposed to be crip and clear
Also, committed the change as per earlier review comments, not using the second variable, just updating the already existing kube_vip_image_repo variable conditionally in roles/kubespray-defaults/defaults/main/download.yml

[tico88612]

Squash your commit to one.

[tico88612]

Squash your commit to one without merge commit. (maybe you need force to push)

[aviral-agarwal]

please check now, 1 single commit

[tico88612]

/ok-to-test

[tico88612]

/retest

[aviral-agarwal]

Hi,
What could be the reason for failed tests?
Not sure about what I am looking at in the failure logs

From logs, seems to be when
task Download_file | Show url of file to download in roles\download\tasks\download_file.yml
is used

[tico88612]

Probably, I guess it's kube_vip_lb_fwdmethod not defined.

[aviral-agarwal]

ah, yes, that is the case, I tested and confirmed the same at my end
It seems if kube_vip_lb_fwdmethod is commented or not defined in inventory\sample\group_vars\k8s_cluster\addons.yml (I was defining it for testing),
in the code flow, the default value is not being picked up when defining defaults in roles\kubespray-defaults\defaults\main\download.yml

So, to fix (both approaches tested and working)

Approach 1:

• Uncomment the # kube_vip_lb_fwdmethod: local in inventory\sample\group_vars\k8s_cluster\addons.yml as default
• Though simpler and cleaner, this seems to create minor redundancy in code (defining kube_vip_lb_fwdmethod twice with defaults) and feels like a hack/trick (in rare cases, the user may comment kube_vip_lb_fwdmethod in addons.yml, presuming the default value from download.yml will take over)

Approach 2:

• As was being done in the earlier commit, move the conditional selection of image to kube-vip manifest jinja template roles\kubernetes\node\templates\manifests\kube-vip.manifest.j2
• I can avoid using the extra variable in roles\kubespray-defaults\defaults\main\download.yml, though it seems cleaner
• This approach seems to be more robust

I am updating the code according to Approach 2, to be reviewed
Please let me know your thoughts

Also, can I initiate a retest as well?

[VannTen]

You should instead move the default definitions of kube_vip_lb_fwdmethod to kubespray-defaults.
The reason for failure is that is defined in kubernetes/node/defaults, which is not in scope when kubespray download images.

Thus it's undefined at this point.
We should avoid doing this at jinja level because this will be a bad UX for folks using a custom image, for instance.
Modifying only the default let users override it if needed.

[aviral-agarwal]

Understood. I was also using a custom image, and I realized earlier that the image manipulation in the Jinja template would have messed with that.

And moving kube_vip_lb_fwdmethod from roles\kubernetes\node\defaults\main.yml to kubespray-defaults brings the variable in scope as expected, without defining in addons.yml
In roles\kubespray-defaults\defaults\main\main.yml, there was config for kube-vip already. Keeping them together in code file and provided a comment for the same

I tested by explicitly providing a custom image by defining kube_vip_image_repo in addons.yml, and it is correctly populated in the template

If an image is not explicitly provided, the correct image basis kube_vip_lb_fwdmethod is selected

The latest commit contains code as per above

I am not sure why the 3 tests keep failing, though

[VannTen]

Thanks, that should do. /approve /lgtm /retest-failed There is a flake on Ubuntu with apt, it's known (though the root cause is not).

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aviral-agarwal, VannTen

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 [VannTen]

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