refactor: unify inconsistent variables in apiserver endpoint by kajal-jotwani · Pull Request #12897 · kubernetes-sigs/kubespray

kajal-jotwani · 2026-01-24T08:00:05Z

What type of PR is this?
/kind cleanup

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

What this PR does / why we need it:
This PR replaces several overlapping and inconsistently named API server
load balancer variables with two clear endpoint variables.

It updates defaults, templates, and documentation to consistently use these
endpoints, making it easier to understand which address is used externally and
which is used internally within the cluster, while keeping existing setups
working.

kube_apiserver_endpoint (external access)
kube_apiserver_cluster_internal_endpoint (internal cluster access)

Which issue(s) this PR fixes:

Fixes #12883

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

k8s-ci-robot · 2026-01-24T08:00:12Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

linux-foundation-easycla · 2026-01-24T08:00:13Z

The committers listed above are authorized under a signed CLA.

✅ login: kajal-jotwani / name: Kajal Jotwani (17a6a0b, 1da232e, 463a157, 4995162, 4e461ba, 6fd3191, 80fb7b6, 8448a3e, 8f30867, 97326cd, e62fc93, f01689c, f8c845b, ff949d3)

k8s-ci-robot · 2026-01-24T08:00:15Z

Hi @kajal-jotwani. 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.

Details

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.

VannTen

Thanks for your work !
/ok-to-test

Lots of comments but it's just because the current code is messy (not yours, the current kubespray codebase I mean), it's easier to elaborate how we want to do it with proposed changes 👍

docs/operations/ha-mode.md

roles/kubernetes/client/tasks/main.yml

roles/kubernetes/control-plane/tasks/kubeadm-setup.yml

roles/kubernetes/control-plane/templates/kubeadm-config.v1beta3.yaml.j2

roles/kubernetes/control-plane/templates/kubeadm-config.v1beta4.yaml.j2

VannTen · 2026-01-27T10:51:12Z

roles/kubernetes/kubeadm/tasks/main.yml

- name: Update server field in kubelet kubeconfig - external lb
+- name: Update server field in kubelet kubeconfig - when using control plane endpoint
  lineinfile:
    dest: "{{ kube_config_dir }}/kubelet.conf"
    regexp: '^    server: https'
-    line: '    server: {{ kube_apiserver_endpoint }}'
+    line: '    server: {{ kube_apiserver_cluster_internal_endpoint }}'
    backup: true
  when:
    - ('kube_control_plane' not in group_names)
-    - loadbalancer_apiserver is defined
+    - kubeadm_use_control_plane_endpoint | default(false)
  notify: Kubeadm | restart kubelet


Won't be necessary after #12870

Got it! So should I remove this task entirely?

You can leave it, if the other PR merge before that will need a rebase, no big deal.

Sorry I was confused regarding the other PR. We can delete both theses tasks ("Update server field"), with the correct variables into kubeadm configuration this should not be needed anymore.

VannTen · 2026-01-27T10:54:52Z

roles/kubespray_defaults/defaults/main/main.yml

+      https://{{ loadbalancer_apiserver.address | ansible.utils.ipwrap }}:{{ loadbalancer_apiserver.port | default(kube_apiserver_port) }}
  {%- elif ('kube_control_plane' not in group_names) and loadbalancer_apiserver_localhost -%}
      https://localhost:{{ loadbalancer_apiserver_port | default(kube_apiserver_port) }}
  {%- elif 'kube_control_plane' in group_names -%}


This won't be necessary because kubeadm does it by itself see #12870

roles/kubespray_defaults/defaults/main/main.yml

kajal-jotwani · 2026-01-29T18:15:54Z

Thanks a lot for the detailed review!
I’ve addressed most of the comments and I’m currently validating the changes to make sure everything is consistent.
I’ll push any remaining fixes and ping once it’s ready for final review.

docs/operations/ha-mode.md

VannTen · 2026-02-02T07:55:35Z

docs/operations/ha-mode.md

 Access API endpoints are evaluated automatically, as the following:

 | Endpoint type                | kube_control_plane                       | non-master              | external              |
 |------------------------------|------------------------------------------|-------------------------|-----------------------|
 | Local LB (default)           | `https://dbip:sp`                        | `https://lc:nsp`        | `https://m[0].aip:sp` |
 | Local LB (default) + cbip    | `https://cbip:sp` and `https://lc:nsp`   | `https://lc:nsp`        | `https://m[0].aip:sp` |
 | Local LB + Unmanaged here LB | `https://dbip:sp`                        | `https://lc:nsp`        | `https://ext`         |
 | External LB, no internal     | `https://dbip:sp`                        | `<https://lb:lp>`       | `https://lb:lp`       |
 | No ext/int LB                | `https://dbip:sp`                        | `<https://m[0].aip:sp>` | `https://m[0].aip:sp` |

 Where:

 * `m[0]` - the first node in the `kube_control_plane` group;
-* `lb` - LB FQDN, `apiserver_loadbalancer_domain_name`;
+* `lb` - External loadbalancer address from `kube_apiserver_endpoint`;
 * `ext` - Externally load balanced VIP:port and FQDN, not managed by Kubespray;
 * `lc` - localhost;
 * `cbip` - a custom bind IP, `kube_apiserver_bind_address`;
 * `dbip` - localhost for the default bind IP '0.0.0.0';
 * `nsp` - nginx secure port, `loadbalancer_apiserver_port`, defers to `sp`;
 * `sp` - secure port, `kube_apiserver_port`;
-* `lp` - LB port, `loadbalancer_apiserver.port`, defers to the secure port;
+* `lp` - External loadbalancer port from `kube_apiserver_endpoint`;
 * `ip` - the node IP, defers to the ansible IP;
 * `aip` - `access_ip`, defers to the ip.

+The `kube_apiserver_endpoint` and `kube_apiserver_cluster_internal_endpoint` variables
+automatically resolve to the appropriate values based on your loadbalancer configuration.
+


I've been of the impression this whole section was a bit too much complicated to be clear (way too much variables).
Also, since we're going towards more alignement with kubeadm (see #12870 ) it will be wrong for control plane (always using localhost).

I would maybe simplify this and just mention both variables (kube_**endpoint) and maybe link to the defaults file where we would put comment explaining the computed default ?

@tico88612 what do you think ? I kinda struggle to know what's the right amount of details ?

(It's kinda out of scope of the PR though so it's okay if we don't fix this here).

roles/kubernetes/client/tasks/main.yml

roles/kubernetes/control-plane/tasks/kubeadm-setup.yml

VannTen · 2026-02-02T08:45:57Z

roles/kubernetes/kubeadm/tasks/main.yml

- name: Update server field in kubelet kubeconfig - external lb
+- name: Update server field in kubelet kubeconfig - when using control plane endpoint
  lineinfile:
    dest: "{{ kube_config_dir }}/kubelet.conf"
    regexp: '^    server: https'
-    line: '    server: {{ kube_apiserver_endpoint }}'
+    line: '    server: {{ kube_apiserver_cluster_internal_endpoint }}'
    backup: true
  when:
    - ('kube_control_plane' not in group_names)
-    - loadbalancer_apiserver is defined
+    - kubeadm_use_control_plane_endpoint | default(false)
  notify: Kubeadm | restart kubelet


You can leave it, if the other PR merge before that will need a rebase, no big deal.

roles/kubernetes/preinstall/tasks/0040-verify-settings.yml

VannTen · 2026-02-02T09:27:28Z

roles/kubespray_defaults/defaults/main/main.yml

 first_kube_control_plane_address: "{{ hostvars[groups['kube_control_plane'][0]]['main_access_ip'] }}"
-loadbalancer_apiserver_localhost: "{{ loadbalancer_apiserver is not defined }}"
+# Loadbalancer configuration
+loadbalancer_apiserver_localhost: true


This is kinda tricky to get right if we want to keep the current behavior, roughly.
(aka, defaulting to LB localhost if we don't explicitely define a LB) 🤔 .

Possible solution:
Use an internal var (in /vars, which can't be overriden from inventory.) for the actual value, with something like kube_apiserver_endpoint | d(first_master_endpoint).
This then means we can check if the user level variable is defined with is defined.

Wdyt ?

Yep, that makes sense. I considered a true/false user option, but that would lose the distinction between explicit intent and defaults. Keeping the user var optional and resolving an internal effective value preserves current behavior and lets us detect explicit LB config via is defined. should I go ahead and implement this?

Yeah, I think so. We should still have the variable in defaults but with "{{ undef() }}" which should have the same effect as not defining it but be more explicit.

roles/kubespray_defaults/defaults/main/main.yml

roles/network_facts/tasks/no_proxy.yml

VannTen

Incomplete review of some stuff:
But the test failure might be related to the parenthese stuff, not sure.

VannTen · 2026-02-05T14:35:00Z

roles/validate_inventory/tasks/main.yml

+# TODO: remove after release 2.31
+- name: Stop if legacy apiserver LB variables are used
+  assert:
+    that:
+      - loadbalancer_apiserver is not defined
+      - apiserver_loadbalancer_domain_name is not defined
+      - kube_apiserver_global_endpoint is not defined
+      - kubeadm_config_api_fqdn is not defined
+    fail_msg: |-
+      API server loadbalancer variables have been deprecated.
+
+      Please update your inventory to use:
+        - kube_apiserver_endpoint
+        - kube_apiserver_cluster_internal_endpoint
+
+      And optionally:
+        - loadbalancer_apiserver_localhost (true/false)
+        - loadbalancer_apiserver_port
+  run_once: true
+  when: not ignore_assert_errors
+


Now that #12942 landed you can use it directly 👍

VannTen · 2026-02-05T14:38:36Z

roles/kubespray_defaults/defaults/main/main.yml

+
+# Internal endpoint for cluster traffic.
+# Defaults to the global endpoint, unless we are using a localhost API load balancer.
+kube_apiserver_cluster_internal_endpoint: "{{ 'https://127.0.0.1:' ~ loadbalancer_apiserver_port if loadbalancer_apiserver_localhost else kube_apiserver_endpoint }}"


I think you need some

Suggested change

kube_apiserver_cluster_internal_endpoint: "{{ 'https://127.0.0.1:' ~ loadbalancer_apiserver_port if loadbalancer_apiserver_localhost else kube_apiserver_endpoint }}"

kube_apiserver_cluster_internal_endpoint: "{{ ('https://127.0.0.1:' ~ loadbalancer_apiserver_port) if loadbalancer_apiserver_localhost else kube_apiserver_endpoint }}"

I'm not sure but the if else might be more closely binding than ~ which would probably not work.

kajal-jotwani · 2026-02-13T07:14:56Z

/test pull-kubespray-yamllint

VannTen · 2026-02-13T13:16:28Z

roles/kubernetes/kubeadm/templates/kubeadm-client.conf.j2

-{% else %}
-    apiServerEndpoint: "{{ kubeadm_discovery_address }}"
-{% endif %}
+    apiServerEndpoint: "{{ _kube_apiserver_cluster_internal_endpoint | urlsplit('netloc') }}"


This should be the _kube_apiserver_endpoint here, not the internal one.
The reason for this is that this is used for discovery, aka, for the control plane, before there is a local apiserver. I suspect this causes the CI failures.

Changed this to _kube_apiserver_endpoint, thanks for pointing that!!

I tried to fix the CI failures as well but I’m still not able to resolve them. I’m working on it and trying to figure out what’s breaking.

VannTen · 2026-02-13T13:18:24Z

roles/kubernetes/control-plane/tasks/kubeadm-secondary.yml

    kubeadm_discovery_address: >-
-      {%- if "127.0.0.1" in kube_apiserver_endpoint or "localhost" in kube_apiserver_endpoint -%}
+      {%- if "127.0.0.1" in _kube_apiserver_endpoint or "localhost" in _kube_apiserver_endpoint -%}
      {{ first_kube_control_plane_address | ansible.utils.ipwrap }}:{{ kube_apiserver_port }}
      {%- else -%}
-      {{ kube_apiserver_endpoint | regex_replace('https://', '') }}
+      {{ _kube_apiserver_endpoint | regex_replace('https://', '') }}
      {%- endif %}


I don't think we need this at all, that variable should just go and we'll use _kube_apiserver_endpoint instead.
There could be an argument to expose it to the user for a endpoint only for discovery, but that's out of scope.

Let's delete the kubeadm_discovery_address variable.
From my reading, it's replaceable with _kube_apiserver_endpoint without issue.

kajal-jotwani · 2026-02-14T06:31:05Z

@VannTen CI is now passing after the latest fixes and I have also incorporated the suggested changes!!

roles/kubernetes/control-plane/templates/kubeadm-controlplane.yaml.j2

VannTen · 2026-02-16T08:38:05Z

roles/kubernetes/control-plane/tasks/kubeadm-secondary.yml

    kubeadm_discovery_address: >-
-      {%- if "127.0.0.1" in kube_apiserver_endpoint or "localhost" in kube_apiserver_endpoint -%}
+      {%- if "127.0.0.1" in _kube_apiserver_endpoint or "localhost" in _kube_apiserver_endpoint -%}
      {{ first_kube_control_plane_address | ansible.utils.ipwrap }}:{{ kube_apiserver_port }}
      {%- else -%}
-      {{ kube_apiserver_endpoint | regex_replace('https://', '') }}
+      {{ _kube_apiserver_endpoint | regex_replace('https://', '') }}
      {%- endif %}


Let's delete the kubeadm_discovery_address variable.
From my reading, it's replaceable with _kube_apiserver_endpoint without issue.

VannTen · 2026-02-16T08:42:29Z

roles/kubernetes/control-plane/templates/kubeadm-config.v1beta3.yaml.j2

-{% else %}
-controlPlaneEndpoint: "{{ main_ip | ansible.utils.ipwrap }}:{{ kube_apiserver_port }}"
-{% endif %}
+controlPlaneEndpoint: "{{ _kube_apiserver_endpoint | urlsplit('netloc') }}"


I think these ones should still use _kube_apiserver_cluster_internal_endpoint, or did that break CI ?
This is different from the JoinConfiguration (which is used when adding a node/control plane), as it should be for normal operations, so for example the localhost loadbalancer should be there (which should only be internal)

If that doesn't work it probably means we have and ordering problems somewhere 🤔

Yes CI was failing when using _kube_apiserver_cluster_internal_endpoint, which is why I changed it to _kube_apiserver_endpoint. I'll dig into it to understand what’s causing the issue

VannTen · 2026-02-16T08:43:52Z

roles/kubernetes/control-plane/templates/kubeadm-config.v1beta4.yaml.j2

-{% else %}
-controlPlaneEndpoint: "{{ main_ip | ansible.utils.ipwrap }}:{{ kube_apiserver_port }}"
-{% endif %}
+controlPlaneEndpoint: "{{ _kube_apiserver_endpoint | urlsplit('netloc') }}"


VannTen · 2026-02-16T08:49:35Z

roles/kubernetes/kubeadm/tasks/main.yml

- name: Update server field in kubelet kubeconfig - external lb
+- name: Update server field in kubelet kubeconfig - when using control plane endpoint
  lineinfile:
    dest: "{{ kube_config_dir }}/kubelet.conf"
    regexp: '^    server: https'
-    line: '    server: {{ kube_apiserver_endpoint }}'
+    line: '    server: {{ kube_apiserver_cluster_internal_endpoint }}'
    backup: true
  when:
    - ('kube_control_plane' not in group_names)
-    - loadbalancer_apiserver is defined
+    - kubeadm_use_control_plane_endpoint | default(false)
  notify: Kubeadm | restart kubelet


Sorry I was confused regarding the other PR. We can delete both theses tasks ("Update server field"), with the correct variables into kubeadm configuration this should not be needed anymore.

roles/kubernetes/kubeadm/tasks/main.yml

VannTen · 2026-02-16T08:57:28Z

roles/kubernetes/node/templates/node-kubeconfig.yaml.j2

See #13012 , this template isn't used anywhere.

Noted, thanks! I’ll rebase once #13012 is merged.

roles/kubespray_defaults/defaults/main/main.yml

roles/validate_inventory/tasks/main.yml

…ectly

k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jan 24, 2026

k8s-ci-robot requested review from cyclinder and tico88612 January 24, 2026 08:00

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jan 24, 2026

VannTen reviewed Jan 27, 2026

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 27, 2026

kajal-jotwani force-pushed the unify-apiserver-endpoints branch from 359dc46 to 19417e2 Compare January 29, 2026 16:37

kajal-jotwani requested a review from VannTen February 1, 2026 13:10

VannTen reviewed Feb 2, 2026

View reviewed changes

kajal-jotwani requested a review from VannTen February 5, 2026 14:31

VannTen reviewed Feb 5, 2026

View reviewed changes

kajal-jotwani force-pushed the unify-apiserver-endpoints branch 2 times, most recently from 3dc1e99 to 0072dcc Compare February 6, 2026 13:38

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 9, 2026

VannTen mentioned this pull request Feb 12, 2026

proxy: Fix the no_proxy variable #12981

Open

kajal-jotwani force-pushed the unify-apiserver-endpoints branch from 85c7ebd to a36237b Compare February 13, 2026 07:34

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 13, 2026

VannTen mentioned this pull request Feb 13, 2026

Refactor(defaults): centralize remove node defaults #13004

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 13, 2026

VannTen reviewed Feb 13, 2026

View reviewed changes

kajal-jotwani force-pushed the unify-apiserver-endpoints branch from fbb09c2 to 34b8923 Compare February 13, 2026 14:33

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 13, 2026

Srishti-j18 mentioned this pull request Feb 13, 2026

eliminate all instances of default filter in roles => var should have a default defined only once #11822

Open

chadswen mentioned this pull request Feb 13, 2026

Undefined check for apiserver_loadbalancer_domain_name in apiserver_sans #13009

Merged

kajal-jotwani force-pushed the unify-apiserver-endpoints branch from 922c608 to 7a56c8e Compare February 13, 2026 20:34

VannTen reviewed Feb 16, 2026

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 16, 2026

kajal-jotwani added 11 commits February 16, 2026 19:18

refactor: unify inconsistent variables in apiserver endpoint

f01689c

Remove legacy apiserver LB docs and add safeguard check

8f30867

refactor: unify and simplify apiserver loadbalancer variables

f8c845b

fix: resolve CI failures

17a6a0b

refactor: unify and simplify apiserver loadbalancer variables

463a157

refactor: unify and simplify apiserver loadbalancer variables

1da232e

refactor: unify and simplify apiserver loadbalancer variables

e62fc93

refactor: resolve apiserver endpoints via internal vars

80fb7b6

fix: resolve ci failure

6fd3191

fix: use kube_apiserver_endpoint for kubeadm discovery

4995162

fix: use kube_apiserver_endpoint for kubeadm discovery

4e461ba

kajal-jotwani force-pushed the unify-apiserver-endpoints branch from 7a56c8e to 4e461ba Compare February 16, 2026 13:48

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 16, 2026

kajal-jotwani added 3 commits February 16, 2026 22:06

remove loadbalancer handling and simplify validation message

8448a3e

remove kubelet and kube-proxy endpoint patching

97326cd

remove kubeadm_discovery_address and use _kube_apiserver_endpoint dir…

ff949d3

…ectly

kajal-jotwani force-pushed the unify-apiserver-endpoints branch 2 times, most recently from 175fa57 to ff949d3 Compare February 16, 2026 19:39

Conversation

kajal-jotwani commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jan 24, 2026

Uh oh!

linux-foundation-easycla bot commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jan 24, 2026

Uh oh!

VannTen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kajal-jotwani commented Jan 29, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VannTen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kajal-jotwani commented Feb 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kajal-jotwani commented Feb 14, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kajal-jotwani commented Jan 24, 2026 •

edited

Loading

linux-foundation-easycla bot commented Jan 24, 2026 •

edited

Loading

VannTen left a comment •

edited

Loading