Properly format windows env-vars, redeploy windows plans if they fail…

… up to 5 times
HarrisonWAffel · Oct 10, 2024 · 2c0869a · 2c0869a
1 parent bf39f57
commit 2c0869a
Show file tree

Hide file tree

Showing 10 changed files with 228 additions and 28 deletions.
diff --git a/pkg/capr/common.go b/pkg/capr/common.go
@@ -674,3 +674,54 @@ func PreBootstrap(mgmtCluster *v3.Cluster) bool {
 
 	return !v3.ClusterConditionPreBootstrapped.IsTrue(mgmtCluster)
 }
+
+// FormatWindowsEnvVar accepts a corev1.EnvVar and returns a string to be used in either
+// a Powershell script or the Rancher planner, indicated by the isPlanVariable parameter.
+// This function automatically configures the '$env:' prefix for a given environment variable,
+// automatically prefixes boolean values with '$', and surrounds string variables with double quotes as
+// needed. If the provided variable name incorrectly uses either '$env:' or '$' for the given isPlanVariable
+// value, it will be removed.
+func FormatWindowsEnvVar(envVar corev1.EnvVar, isPlanVariable bool) string {
+	lowerValue := strings.ToLower(envVar.Value)
+	isBool := lowerValue == "$true" || lowerValue == "$false" ||
+		lowerValue == "true" || lowerValue == "false"
+
+	// remove any user provided prefixes and suffixes
+	if strings.HasPrefix(envVar.Name, "$env:") {
+		envVar.Name = strings.TrimPrefix(envVar.Name, "$env:")
+	}
+
+	if strings.HasPrefix(envVar.Value, "\"") {
+		envVar.Value = strings.TrimPrefix(envVar.Value, "\"")
+	}
+
+	if strings.HasSuffix(envVar.Value, "\"") {
+		envVar.Value = strings.TrimSuffix(envVar.Value, "\"")
+	}
+
+	if !isBool {
+		format := ""
+		if isPlanVariable {
+			format = "%s=%s"
+		} else {
+			// Non-boolean variables are always treated as strings,
+			// even numbers
+			format = "$env:%s=\"%s\""
+		}
+		return fmt.Sprintf(format, envVar.Name, envVar.Value)
+	}
+
+	if !strings.HasPrefix(envVar.Value, "$") && !isPlanVariable {
+		envVar.Value = "$" + envVar.Value
+	}
+
+	if strings.HasPrefix(envVar.Value, "$") && isPlanVariable {
+		envVar.Value = strings.TrimPrefix(envVar.Value, "$")
+	}
+
+	if isPlanVariable {
+		return fmt.Sprintf("%s=%s", envVar.Name, envVar.Value)
+	}
+
+	return fmt.Sprintf("$env:%s=%s", envVar.Name, envVar.Value)
+}
diff --git a/pkg/capr/common_test.go b/pkg/capr/common_test.go
@@ -1,6 +1,7 @@
 package capr
 
 import (
+	corev1 "k8s.io/api/core/v1"
 	"reflect"
 	"testing"
 
@@ -313,3 +314,76 @@ func TestCompressInterface(t *testing.T) {
 		})
 	}
 }
+
+func TestFormatWindowsEnvVar(t *testing.T) {
+	tests := []struct {
+		Name           string
+		EnvVar         corev1.EnvVar
+		IsPlanVar      bool
+		ExpectedString string
+	}{
+		{
+			Name: "Basic String",
+			EnvVar: corev1.EnvVar{
+				Name:  "BASIC_STRING",
+				Value: "ABC123",
+			},
+			IsPlanVar:      false,
+			ExpectedString: "$env:BASIC_STRING=\"ABC123\"",
+		},
+		{
+			Name: "Basic Bool",
+			EnvVar: corev1.EnvVar{
+				Name:  "BASIC_BOOL",
+				Value: "true",
+			},
+			IsPlanVar:      false,
+			ExpectedString: "$env:BASIC_BOOL=$true",
+		},
+		{
+			Name: "Basic Plan String",
+			EnvVar: corev1.EnvVar{
+				Name:  "PLAN_STRING",
+				Value: "VALUE",
+			},
+			IsPlanVar:      true,
+			ExpectedString: "PLAN_STRING=VALUE",
+		},
+		{
+			Name: "Basic Plan Bool",
+			EnvVar: corev1.EnvVar{
+				Name:  "PLAN_BOOL",
+				Value: "true",
+			},
+			IsPlanVar:      true,
+			ExpectedString: "PLAN_BOOL=true",
+		},
+		{
+			Name: "Plan name Mistakenly includes $env:",
+			EnvVar: corev1.EnvVar{
+				Name:  "$env:PLAN_BOOL",
+				Value: "true",
+			},
+			IsPlanVar:      true,
+			ExpectedString: "PLAN_BOOL=true",
+		},
+		{
+			Name: "Plan value Mistakenly Includes $",
+			EnvVar: corev1.EnvVar{
+				Name:  "PLAN_BOOL",
+				Value: "$true",
+			},
+			IsPlanVar:      true,
+			ExpectedString: "PLAN_BOOL=true",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.Name, func(t *testing.T) {
+			out := FormatWindowsEnvVar(tc.EnvVar, tc.IsPlanVar)
+			if out != tc.ExpectedString {
+				t.Fatalf("Expected %s, got %s", tc.ExpectedString, out)
+			}
+		})
+	}
+}
diff --git a/pkg/capr/installer/installer.go b/pkg/capr/installer/installer.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"strings"
 
+	"github.com/rancher/rancher/pkg/capr"
 	"github.com/rancher/rancher/pkg/settings"
 	"github.com/rancher/rancher/pkg/systemtemplate"
 	"github.com/rancher/rancher/pkg/tls"
@@ -135,9 +136,15 @@ func WindowsInstallScript(ctx context.Context, token string, envVars []corev1.En
 	binaryURL := ""
 	if settings.WinsAgentVersion.Get() != "" {
 		if settings.ServerURL.Get() != "" {
-			binaryURL = fmt.Sprintf("$env:CATTLE_AGENT_BINARY_BASE_URL=\"%s/assets\"", settings.ServerURL.Get())
+			binaryURL = capr.FormatWindowsEnvVar(corev1.EnvVar{
+				Name:  "CATTLE_AGENT_BINARY_BASE_URL",
+				Value: fmt.Sprintf("%s/assets", settings.ServerURL.Get()),
+			}, false)
 		} else if defaultHost != "" {
-			binaryURL = fmt.Sprintf("$env:CATTLE_AGENT_BINARY_BASE_URL=\"https://%s/assets\"", defaultHost)
+			binaryURL = capr.FormatWindowsEnvVar(corev1.EnvVar{
+				Name:  "CATTLE_AGENT_BINARY_BASE_URL",
+				Value: fmt.Sprintf("https://%s/assets", defaultHost),
+			}, false)
 		}
 	}
 
@@ -156,22 +163,38 @@ func WindowsInstallScript(ctx context.Context, token string, envVars []corev1.En
 	if v, ok := ctx.Value(tls.InternalAPI).(bool); ok && v {
 		ca = systemtemplate.InternalCAChecksum()
 	}
+
 	if ca != "" {
-		ca = "$env:CATTLE_CA_CHECKSUM=\"" + ca + "\""
+		ca = capr.FormatWindowsEnvVar(corev1.EnvVar{
+			Name:  "CATTLE_CA_CHECKSUM",
+			Value: ca,
+		}, false)
 	}
+
+	var tokenEnvVar string
 	if token != "" {
-		token = "$env:CATTLE_ROLE_NONE=\"true\"\n$env:CATTLE_TOKEN=\"" + token + "\""
+		tokenEnvVar = capr.FormatWindowsEnvVar(corev1.EnvVar{
+			Name:  "CATTLE_TOKEN",
+			Value: token,
+		}, false)
 	}
+
 	envVarBuf := &strings.Builder{}
 	for _, envVar := range envVars {
 		if envVar.Value == "" {
 			continue
 		}
-		envVarBuf.WriteString(fmt.Sprintf("$env:%s=\"%s\"\n", envVar.Name, envVar.Value))
+		envVarBuf.WriteString(capr.FormatWindowsEnvVar(corev1.EnvVar{
+			Name:  envVar.Name,
+			Value: envVar.Value,
+		}, false))
 	}
 	server := ""
 	if settings.ServerURL.Get() != "" {
-		server = fmt.Sprintf("$env:CATTLE_SERVER=\"%s\"", settings.ServerURL.Get())
+		server = capr.FormatWindowsEnvVar(corev1.EnvVar{
+			Name:  "CATTLE_SERVER",
+			Value: settings.ServerURL.Get(),
+		}, false)
 	}
 
 	strictVerify := "false"
@@ -192,8 +215,9 @@ $env:CSI_PROXY_URL = "%s"
 $env:CSI_PROXY_VERSION = "%s"
 $env:CSI_PROXY_KUBELET_PATH = "C:%s/bin/kubelet.exe"
 $env:STRICT_VERIFY = "%s"
+$env:CATTLE_ROLE_NONE = "true"
 
 Invoke-WinsInstaller @PSBoundParameters
 exit 0
-`, data, envVarBuf.String(), binaryURL, server, ca, token, csiProxyURL, csiProxyVersion, dataDir, strictVerify)), nil
+`, data, envVarBuf.String(), binaryURL, server, ca, tokenEnvVar, csiProxyURL, csiProxyVersion, dataDir, strictVerify)), nil
 }
diff --git a/pkg/capr/planner/etcdcreate.go b/pkg/capr/planner/etcdcreate.go
@@ -67,7 +67,7 @@ func (p *Planner) runEtcdSnapshotManagementServiceStart(controlPlane *rkev1.RKEC
 	// Generate and deliver desired plan for the bootstrap/init node first.
 	if err := p.reconcile(controlPlane, tokensSecret, clusterPlan, true, bootstrapTier, isEtcd, isNotInitNodeOrIsDeleting,
 		"1", "",
-		controlPlane.Spec.UpgradeStrategy.ControlPlaneDrainOptions); err != nil {
+		controlPlane.Spec.UpgradeStrategy.ControlPlaneDrainOptions, -1, 1); err != nil {
 		return err
 	}
 

diff --git a/pkg/capr/planner/instructions.go b/pkg/capr/planner/instructions.go
@@ -8,6 +8,7 @@ import (
 	rkev1 "github.com/rancher/rancher/pkg/apis/rke.cattle.io/v1"
 	"github.com/rancher/rancher/pkg/apis/rke.cattle.io/v1/plan"
 	"github.com/rancher/rancher/pkg/capr"
+	corev1 "k8s.io/api/core/v1"
 )
 
 const (
@@ -26,14 +27,22 @@ func (p *Planner) generateInstallInstruction(controlPlane *rkev1.RKEControlPlane
 		}
 		switch cattleOS {
 		case capr.WindowsMachineOS:
-			env = append(env, fmt.Sprintf("$env:%s=\"%s\"", arg.Name, arg.Value))
+			env = append(env, capr.FormatWindowsEnvVar(corev1.EnvVar{
+				Name:  arg.Name,
+				Value: arg.Value,
+			}, true))
 		default:
 			env = append(env, fmt.Sprintf("%s=%s", arg.Name, arg.Value))
 		}
 	}
 	switch cattleOS {
 	case capr.WindowsMachineOS:
+		// TODO: Properly format the data dir when adding full support for Windows nodes
 		env = append(env, fmt.Sprintf("$env:%s_DATA_DIR=\"c:%s\"", strings.ToUpper(capr.GetRuntime(controlPlane.Spec.KubernetesVersion)), capr.GetDistroDataDir(controlPlane)))
+		env = append(env, capr.FormatWindowsEnvVar(corev1.EnvVar{
+			Name:  "INSTALL_RKE2_VERSION",
+			Value: strings.ReplaceAll(controlPlane.Spec.KubernetesVersion, "+", "-"),
+		}, true))
 	default:
 		env = append(env, fmt.Sprintf("%s_DATA_DIR=%s", strings.ToUpper(capr.GetRuntime(controlPlane.Spec.KubernetesVersion)), capr.GetDistroDataDir(controlPlane)))
 	}
@@ -60,7 +69,10 @@ func (p *Planner) generateInstallInstruction(controlPlane *rkev1.RKEControlPlane
 	if isOnlyWorker(entry) {
 		switch cattleOS {
 		case capr.WindowsMachineOS:
-			instruction.Env = append(instruction.Env, fmt.Sprintf("$env:INSTALL_%s_EXEC=\"agent\"", capr.GetRuntimeEnv(controlPlane.Spec.KubernetesVersion)))
+			instruction.Env = append(instruction.Env, capr.FormatWindowsEnvVar(corev1.EnvVar{
+				Name:  fmt.Sprintf("INSTALL_%s_EXEC", capr.GetRuntimeEnv(controlPlane.Spec.KubernetesVersion)),
+				Value: "agent",
+			}, true))
 		default:
 			instruction.Env = append(instruction.Env, fmt.Sprintf("INSTALL_%s_EXEC=agent", capr.GetRuntimeEnv(controlPlane.Spec.KubernetesVersion)))
 		}
@@ -77,7 +89,10 @@ func (p *Planner) addInstallInstructionWithRestartStamp(nodePlan plan.NodePlan,
 	stamp := restartStamp(nodePlan, controlPlane, p.getInstallerImage(controlPlane))
 	switch entry.Metadata.Labels[capr.CattleOSLabel] {
 	case capr.WindowsMachineOS:
-		restartStampEnv = "$env:RESTART_STAMP=\"" + stamp + "\""
+		restartStampEnv = capr.FormatWindowsEnvVar(corev1.EnvVar{
+			Name:  "WINS_RESTART_STAMP",
+			Value: stamp,
+		}, true)
 	default:
 		restartStampEnv = "RESTART_STAMP=" + stamp
 	}
@@ -93,7 +108,10 @@ func (p *Planner) generateInstallInstructionWithSkipStart(controlPlane *rkev1.RK
 	var skipStartEnv string
 	switch entry.Metadata.Labels[capr.CattleOSLabel] {
 	case capr.WindowsMachineOS:
-		skipStartEnv = fmt.Sprintf("$env:INSTALL_%s_SKIP_START=\"true\"", strings.ToUpper(capr.GetRuntime(controlPlane.Spec.KubernetesVersion)))
+		skipStartEnv = capr.FormatWindowsEnvVar(corev1.EnvVar{
+			Name:  fmt.Sprintf("INSTALL_%s_SKIP_START", strings.ToUpper(capr.GetRuntime(controlPlane.Spec.KubernetesVersion))),
+			Value: "true",
+		}, true)
 	default:
 		skipStartEnv = fmt.Sprintf("INSTALL_%s_SKIP_START=true", strings.ToUpper(capr.GetRuntime(controlPlane.Spec.KubernetesVersion)))
 	}

diff --git a/pkg/capr/planner/planentry.go b/pkg/capr/planner/planentry.go
@@ -136,6 +136,14 @@ func isOnlyWorker(entry *planEntry) bool {
 	return !isEtcd(entry) && !isControlPlane(entry) && isWorker(entry)
 }
 
+func isOnlyWindowsWorker(entry *planEntry) bool {
+	return isOnlyWorker(entry) && windows(entry)
+}
+
+func isOnlyLinuxWorker(entry *planEntry) bool {
+	return isOnlyWorker(entry) && !windows(entry)
+}
+
 func windows(entry *planEntry) bool {
 	if entry == nil || entry.Metadata == nil {
 		return false