Merge pull request #213 from Vacant2333/fix-replace-attach-script

fix: replace the attach-script's runtime configurations

Author: jwcesign

pkg/providers/ack/ack.go

@@ -24,6 +24,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"regexp"
 	"sort"
 	"strings"
 	"sync"
@@ -106,21 +107,23 @@ func (p *DefaultProvider) GetClusterCNI(_ context.Context) (string, error) {
 	return p.clusterCNI, nil
 }
 
-// Get the ID of the target nodepool id when DescribeClusterAttachScriptsRequest.
-// If there is no default nodepool, select the nodepool with the most HealthyNodes.
+// We need to manually retrieve the runtime configuration of the nodepool, with the default node pool prioritized.
+// If there is no default node pool, we will choose the runtime configuration of the node pool with the most HealthyNodes.
+// In some cases, the DescribeClusterAttachScripts interface may return the Docker runtime in clusters running version 1.24,
+// even though Docker runtime only supports clusters of version 1.22 and below.
 //
 //nolint:gocyclo
-func (p *DefaultProvider) getTargetNodePoolID(ctx context.Context) (*string, error) {
+func (p *DefaultProvider) getClusterAttachRuntimeConfiguration(ctx context.Context) (string, string, error) {
 	resp, err := p.ackClient.DescribeClusterNodePools(tea.String(p.clusterID), &ackclient.DescribeClusterNodePoolsRequest{})
 	if err != nil {
 		log.FromContext(ctx).Error(err, "Failed to describe cluster nodepools")
-		return nil, err
+		return "", "", err
 	}
 	if resp == nil || resp.Body == nil || resp.Body.Nodepools == nil {
-		return nil, fmt.Errorf("empty describe cluster nodepools response")
+		return "", "", fmt.Errorf("empty describe cluster nodepools response")
 	}
 	if len(resp.Body.Nodepools) == 0 {
-		return nil, fmt.Errorf("no nodepool found")
+		return "", "", fmt.Errorf("no nodepool found")
 	}
 
 	nodepools := resp.Body.Nodepools
@@ -145,10 +148,12 @@ func (p *DefaultProvider) getTargetNodePoolID(ctx context.Context) (*string, err
 	})
 
 	targetNodepool := nodepools[0]
-	if targetNodepool.NodepoolInfo == nil {
-		return nil, fmt.Errorf("target describe cluster nodepool is empty")
+	if targetNodepool.KubernetesConfig == nil || targetNodepool.KubernetesConfig.Runtime == nil ||
+		targetNodepool.KubernetesConfig.RuntimeVersion == nil {
+		return "", "", fmt.Errorf("target describe cluster nodepool is empty")
 	}
-	return targetNodepool.NodepoolInfo.NodepoolId, nil
+	return tea.StringValue(targetNodepool.KubernetesConfig.Runtime),
+		tea.StringValue(targetNodepool.KubernetesConfig.RuntimeVersion), nil
 }
 
 func (p *DefaultProvider) GetNodeRegisterScript(ctx context.Context,
@@ -161,34 +166,34 @@ func (p *DefaultProvider) GetNodeRegisterScript(ctx context.Context,
 		return p.resolveUserData(cachedScript.(string), labels, nodeClaim, kubeletCfg, userData), nil
 	}
 
-	nodepoolID, err := p.getTargetNodePoolID(ctx)
-	if err != nil {
-		// Don't return here, we can process when there is no default cluster id.
-		// We need to try to obtain a usable nodepool ID in order to get the cluster attach scripts.
-		// One known scenario is on an ACK cluster with version 1.24, where the user deleted the default nodepool and
-		// created a nodepool with a containerd runtime. The DescribeClusterAttachScriptsRequest api will use the
-		// CRI configuration of the deleted default nodepool, which might be using the Docker runtime.
-		// This could result in nodes failing to register to the new cluster.
-		log.FromContext(ctx).Error(err, "Failed to get default nodepool id")
-	}
 	reqPara := &ackclient.DescribeClusterAttachScriptsRequest{
 		KeepInstanceName: tea.Bool(true),
-		NodepoolId:       nodepoolID,
 	}
 	resp, err := p.ackClient.DescribeClusterAttachScripts(tea.String(p.clusterID), reqPara)
 	if err != nil {
 		log.FromContext(ctx).Error(err, "Failed to get node registration script")
 		return "", err
 	}
-	s := tea.StringValue(resp.Body)
-	if s == "" {
+	respStr := tea.StringValue(resp.Body)
+	if respStr == "" {
 		err := errors.New("empty node registration script")
 		log.FromContext(ctx).Error(err, "")
 		return "", err
 	}
+	respStr = strings.ReplaceAll(respStr, "\r\n", "")
+
+	runtime, runtimeVersion, err := p.getClusterAttachRuntimeConfiguration(ctx)
+	if err != nil {
+		// If error happen, we do nothing here
+		log.FromContext(ctx).Error(err, "Failed to get cluster attach runtime configuration")
+	} else {
+		// Replace the runtime and runtime-version parameter
+		respStr = regexp.MustCompile(`\s+--runtime\s+\S+\s+--runtime-version\s+\S+`).ReplaceAllString(
+			respStr, fmt.Sprintf(" --runtime %s --runtime-version %s", runtime, runtimeVersion))
+	}
 
-	p.cache.SetDefault(p.clusterID, s)
-	return p.resolveUserData(s, labels, nodeClaim, kubeletCfg, userData), nil
+	p.cache.SetDefault(p.clusterID, respStr)
+	return p.resolveUserData(respStr, labels, nodeClaim, kubeletCfg, userData), nil
 }
 
 func (p *DefaultProvider) resolveUserData(respStr string, labels map[string]string, nodeClaim *karpv1.NodeClaim,
@@ -207,8 +212,7 @@ func (p *DefaultProvider) resolveUserData(respStr string, labels map[string]stri
 	}
 
 	// Clean up the input string
-	cleanupStr := strings.ReplaceAll(respStr, "\r\n", "")
-	script.WriteString(cleanupStr + " ")
+	script.WriteString(respStr + " ")
 	// Add labels
 	script.WriteString(fmt.Sprintf("--labels %s ", p.formatLabels(labels)))
 	// Add kubelet config