Fix: kubeadm_join: Correct node check for Kubernetes 1.32+

[teasonia]

Summary

This commit corrects the node check in kubeadm_join.pp to work correctly with Kubernetes 1.32 and later versions. The previous check relied on kubelet having permissions to list all nodes, which is no longer the case. The updated check uses kubectl get nodes <node_name> instead.

Additional Context

After upgrading kubernetes to version 1.32, applying puppet on existing nodes results in the node being added again. This happens because getting all nodes causes an error.

# KUBECONFIG=/etc/kubernetes/kubelet.conf kubectl get nodes
Error from server (Forbidden): nodes is forbidden: User "system:node:k8s-test" cannot list resource "nodes" in API group "" at the cluster scope: node 'k8s-test' cannot read all nodes, only its own Node object

Getting its own node happens without errors

# KUBECONFIG=/etc/kubernetes/kubelet.conf kubectl get nodes k8s-test
NAME       STATUS   ROLES    AGE    VERSION
k8s-test   Ready    <none>   461d   v1.32.1

[CLAassistant]

All committers have signed the CLA.

[deric]

@teasonia Thanks for your contribution! The modification needs to be backwards compatible. You should check $kubernetes_version and modify the command accordingly. Also such change should be covered by tests.

[cekstam]

Basically, this is what's needed;

  if versioncmp($kubernetes_version, '1.32.0') >= 0 {
    $kubeadm_get_node_command = "kubectl get nodes ${node_name}"
  } else {
    $kubeadm_get_node_command = "kubectl get nodes | grep ${node_name}"
  }

  exec { 'kubeadm join':
    command     => "kubeadm join ${kubeadm_join_flags}",
    environment => $env,
    path        => $path,
    logoutput   => true,
    timeout     => 0,
    unless      => $kubeadm_get_node_command,
  }
}

How do I get that into this PR?

[asusk7m550]

As far as I know the versioncmp is not needed. This command should work from at least version 1.18 and is a basic command. So it should work for all version.

Regarding the change, it would be a good idea to update the manifests/kubeadm_init.pp also.

--- a/manifests/kubeadm_init.pp
+++ b/manifests/kubeadm_init.pp
@@ -36,7 +36,7 @@ define kubernetes::kubeadm_init (
     path        => $path,
     logoutput   => true,
     timeout     => 0,
-    unless      => "kubectl get nodes | grep ${node_name}",
+    unless      => "kubectl get nodes ${node_name}",
   }

[yoshz]

This module is broken on >=1.32 without the changes in this MR. Can we merge this MR?

[deric]

@yoshz No, we can't merge it. The code needs to be rebased.

[jst-cyr]

Hello all! I just rebased this again and ran the basic CI. Unfortunately, the acceptance tests seem to be broken with an underlying provisioning issue (unrelated to this PR) so testing can't go further. That underlying provisioning issue will need to be resolved so we can get acceptance tests working again before being able to move further.

That said, there was some discussion here about needing a version check for which command to run. Am I correct in understanding that the current change is backwards compatible?