Update tests and condition messaging

Author: AcidLeroy

api/bootstrap/kubeadm/v1beta1/kubeadmconfig_types.go

@@ -221,9 +221,6 @@ func (c *KubeadmConfigSpec) validateFiles(pathPrefix *field.Path) field.ErrorLis
 				),
 			)
 		}
-		// n.b.: if we ever add types besides Secret as a ContentFrom
-		// Source, we must add webhook validation here for one of the
-		// sources being non-nil.
 		if file.ContentFormat != "" && file.ContentFormat != FileContentFormatGoTemplate {
 			allErrs = append(
 				allErrs,
@@ -234,6 +231,9 @@ func (c *KubeadmConfigSpec) validateFiles(pathPrefix *field.Path) field.ErrorLis
 				),
 			)
 		}
+		// n.b.: if we ever add types besides Secret as a ContentFrom
+		// Source, we must add webhook validation here for one of the
+		// sources being non-nil.
 		if file.ContentFrom != nil {
 			if file.ContentFrom.Secret.Name == "" {
 				allErrs = append(

api/bootstrap/kubeadm/v1beta1/v1beta2_condition_consts.go

@@ -68,8 +68,11 @@ const (
 	// could be resolved from the Cluster's control plane reference when applicable.
 	KubeadmConfigControlPlaneKubernetesVersionAvailableV1Beta2Condition = "ControlPlaneKubernetesVersionAvailable"
 
-	// KubeadmConfigControlPlaneKubernetesVersionAvailableV1Beta2Reason surfaces when join version resolution succeeded.
-	KubeadmConfigControlPlaneKubernetesVersionAvailableV1Beta2Reason = clusterv1beta1.AvailableV1Beta2Reason
+	// KubeadmConfigControlPlaneKubernetesVersionFromControlPlaneV1Beta2Reason surfaces when join version was read from the control plane.
+	KubeadmConfigControlPlaneKubernetesVersionFromControlPlaneV1Beta2Reason = "ControlPlaneKubernetesVersionFromControlPlane"
+
+	// KubeadmConfigControlPlaneKubernetesVersionFromMachineV1Beta2Reason surfaces when join version uses the Machine.
+	KubeadmConfigControlPlaneKubernetesVersionFromMachineV1Beta2Reason = "ControlPlaneKubernetesVersionFromMachine"
 
 	// KubeadmConfigControlPlaneKubernetesVersionResolutionFailedV1Beta2Reason surfaces when resolution failed.
 	KubeadmConfigControlPlaneKubernetesVersionResolutionFailedV1Beta2Reason = "ControlPlaneKubernetesVersionResolutionFailed"

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

@@ -73,15 +73,21 @@ const (
 )
 
 // KubeadmConfig's ControlPlaneKubernetesVersionAvailable condition and corresponding reasons.
-// This condition is set when reconciling worker join bootstrap data: it reflects whether the controller
-// could resolve the Kubernetes version from the Cluster's control plane reference (when present).
+// This condition is set when reconciling worker join bootstrap data: it reflects how the controller chose
+// the Kubernetes version for join (from the control plane reference when possible, otherwise from the Machine).
 const (
-	// KubeadmConfigControlPlaneKubernetesVersionAvailableCondition is true when the control plane Kubernetes
-	// version for join could be resolved or is not required (no controlPlaneRef or control plane has no spec.version).
+	// KubeadmConfigControlPlaneKubernetesVersionAvailableCondition is true when the Kubernetes version for worker
+	// join was chosen successfully (from the control plane or from the Machine).
 	KubeadmConfigControlPlaneKubernetesVersionAvailableCondition = "ControlPlaneKubernetesVersionAvailable"
 
-	// KubeadmConfigControlPlaneKubernetesVersionAvailableReason surfaces when join version resolution succeeded.
-	KubeadmConfigControlPlaneKubernetesVersionAvailableReason = clusterv1.AvailableReason
+	// KubeadmConfigControlPlaneKubernetesVersionFromControlPlaneReason surfaces when the Kubernetes version for
+	// worker join was read from the referenced control plane's spec.version.
+	KubeadmConfigControlPlaneKubernetesVersionFromControlPlaneReason = "FromControlPlane"
+
+	// KubeadmConfigControlPlaneKubernetesVersionFromMachineReason surfaces when the Kubernetes version for worker
+	// join uses the Machine's spec.version because the Cluster has no controlPlaneRef or the referenced control
+	// plane does not expose a version.
+	KubeadmConfigControlPlaneKubernetesVersionFromMachineReason = "FromMachine"
 
 	// KubeadmConfigControlPlaneKubernetesVersionResolutionFailedReason surfaces when the controller could not
 	// read the control plane object or its Kubernetes version while a controlPlaneRef is set.

api/bootstrap/kubeadm/v1beta2/kubeadmconfig_types.go

@@ -218,9 +218,6 @@ func (c *KubeadmConfigSpec) validateFiles(pathPrefix *field.Path) field.ErrorLis
 				),
 			)
 		}
-		// n.b.: if we ever add types besides Secret as a ContentFrom
-		// Source, we must add webhook validation here for one of the
-		// sources being non-nil.
 		if file.ContentFormat != "" && file.ContentFormat != FileContentFormatGoTemplate {
 			allErrs = append(
 				allErrs,
@@ -231,6 +228,9 @@ func (c *KubeadmConfigSpec) validateFiles(pathPrefix *field.Path) field.ErrorLis
 				),
 			)
 		}
+		// n.b.: if we ever add types besides Secret as a ContentFrom
+		// Source, we must add webhook validation here for one of the
+		// sources being non-nil.
 		if file.ContentFrom.IsDefined() {
 			if file.ContentFrom.Secret.Name == "" {
 				allErrs = append(

api/bootstrap/kubeadm/v1beta2/v1beta1_condition_consts.go

@@ -47,6 +47,14 @@ const (
 	// ControlPlaneKubernetesVersionResolutionFailedV1Beta1Reason (Severity=Warning) documents a failure to read
 	// the control plane object or its Kubernetes version while resolving join bootstrap data.
 	ControlPlaneKubernetesVersionResolutionFailedV1Beta1Reason = "ControlPlaneKubernetesVersionResolutionFailed"
+
+	// ControlPlaneKubernetesVersionFromControlPlaneV1Beta1Reason documents that the Kubernetes version for worker
+	// join was read from the Cluster's control plane reference.
+	ControlPlaneKubernetesVersionFromControlPlaneV1Beta1Reason = "ControlPlaneKubernetesVersionFromControlPlane"
+
+	// ControlPlaneKubernetesVersionFromMachineV1Beta1Reason documents that the Kubernetes version for worker join
+	// uses the Machine's spec.version because the control plane reference is unset or does not expose a version.
+	ControlPlaneKubernetesVersionFromMachineV1Beta1Reason = "ControlPlaneKubernetesVersionFromMachine"
 )
 
 const (

bootstrap/kubeadm/internal/bootstrapfiles/template_test.go

@@ -59,4 +59,14 @@ func TestRenderTemplates(t *testing.T) {
 		_, err := RenderTemplates(in, data)
 		g.Expect(err).To(HaveOccurred())
 	})
+
+	t.Run("template execution errors on missing field", func(t *testing.T) {
+		g := NewWithT(t)
+		in := []bootstrapv1.File{
+			{Path: "/d", ContentFormat: bootstrapv1.FileContentFormatGoTemplate, Content: "{{ .NonExistentField }}"},
+		}
+		_, err := RenderTemplates(in, data)
+		g.Expect(err).To(HaveOccurred())
+		g.Expect(err.Error()).To(ContainSubstring(`failed to execute go-template for file "/d"`))
+	})
 }

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go

@@ -721,14 +721,29 @@ func (r *KubeadmConfigReconciler) joinWorker(ctx context.Context, scope *Scope)
 		})
 		return ctrl.Result{}, err
 	}
+	var cpVersionReasonV1Beta1, cpVersionReason string
+	var cpVersionMsg string
 	if kubernetesVersion == "" {
 		kubernetesVersion = scope.ConfigOwner.KubernetesVersion()
-	}
-	v1beta1conditions.MarkTrue(scope.Config, bootstrapv1.ControlPlaneKubernetesVersionAvailableV1Beta1Condition)
+		cpVersionReasonV1Beta1 = bootstrapv1.ControlPlaneKubernetesVersionFromMachineV1Beta1Reason
+		cpVersionReason = bootstrapv1.KubeadmConfigControlPlaneKubernetesVersionFromMachineReason
+		cpVersionMsg = "Kubernetes version for join uses the Machine's spec.version because the control plane reference is unset or does not expose a version."
+	} else {
+		cpVersionReasonV1Beta1 = bootstrapv1.ControlPlaneKubernetesVersionFromControlPlaneV1Beta1Reason
+		cpVersionReason = bootstrapv1.KubeadmConfigControlPlaneKubernetesVersionFromControlPlaneReason
+		cpVersionMsg = "Kubernetes version for join was read from the Cluster's control plane reference."
+	}
+	v1beta1conditions.Set(scope.Config, &clusterv1.Condition{
+		Type:    bootstrapv1.ControlPlaneKubernetesVersionAvailableV1Beta1Condition,
+		Status:  corev1.ConditionTrue,
+		Reason:  cpVersionReasonV1Beta1,
+		Message: cpVersionMsg,
+	})
 	conditions.Set(scope.Config, metav1.Condition{
-		Type:   bootstrapv1.KubeadmConfigControlPlaneKubernetesVersionAvailableCondition,
-		Status: metav1.ConditionTrue,
-		Reason: bootstrapv1.KubeadmConfigControlPlaneKubernetesVersionAvailableReason,
+		Type:    bootstrapv1.KubeadmConfigControlPlaneKubernetesVersionAvailableCondition,
+		Status:  metav1.ConditionTrue,
+		Reason:  cpVersionReason,
+		Message: cpVersionMsg,
 	})
 	parsedVersion, err := semver.ParseTolerant(kubernetesVersion)
 	if err != nil {

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller_test.go

@@ -25,8 +25,10 @@ import (
 
 	ignition "github.com/flatcar/ignition/config/v2_3"
 	. "github.com/onsi/gomega"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	bootstrapapi "k8s.io/cluster-bootstrap/token/api"
@@ -2787,8 +2789,23 @@ func TestKubeadmConfigReconciler_Reconcile_v1beta2_conditions(t *testing.T) {
 				condition := conditions.Get(newConfig, conditionType)
 				g.Expect(condition).ToNot(BeNil(), "condition %s is missing", conditionType)
 				g.Expect(condition.Status).To(Equal(metav1.ConditionTrue))
-				g.Expect(condition.Message).To(BeEmpty())
+				if conditionType == bootstrapv1.KubeadmConfigControlPlaneKubernetesVersionAvailableCondition {
+					g.Expect(condition.Reason).To(Equal(bootstrapv1.KubeadmConfigControlPlaneKubernetesVersionFromMachineReason))
+					g.Expect(condition.Message).To(ContainSubstring("Machine"))
+				} else {
+					g.Expect(condition.Message).To(BeEmpty())
+				}
 			}
+			foundCPKubeVer := false
+			for _, c := range newConfig.GetV1Beta1Conditions() {
+				if c.Type == bootstrapv1.ControlPlaneKubernetesVersionAvailableV1Beta1Condition {
+					g.Expect(c.Status).To(Equal(corev1.ConditionTrue))
+					g.Expect(c.Reason).To(Equal(bootstrapv1.ControlPlaneKubernetesVersionFromMachineV1Beta1Reason))
+					foundCPKubeVer = true
+					break
+				}
+			}
+			g.Expect(foundCPKubeVer).To(BeTrue())
 			for _, conditionType := range []string{clusterv1.PausedCondition} {
 				condition := conditions.Get(newConfig, conditionType)
 				g.Expect(condition).ToNot(BeNil(), "condition %s is missing", conditionType)
@@ -2798,3 +2815,72 @@ func TestKubeadmConfigReconciler_Reconcile_v1beta2_conditions(t *testing.T) {
 		})
 	}
 }
+
+func TestKubeadmConfigReconciler_Reconcile_v1beta2_conditions_ControlPlaneKubernetesVersionFromControlPlaneRef(t *testing.T) {
+	g := NewWithT(t)
+	scheme := runtime.NewScheme()
+	g.Expect(apiextensionsv1.AddToScheme(scheme)).To(Succeed())
+	g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
+	g.Expect(bootstrapv1.AddToScheme(scheme)).To(Succeed())
+
+	clusterName := "my-cluster-cp"
+	cluster := builder.Cluster(metav1.NamespaceDefault, clusterName).Build()
+	cluster.Status.Conditions = []metav1.Condition{{Type: clusterv1.ClusterControlPlaneInitializedCondition, Status: metav1.ConditionTrue}}
+	cluster.Status.Initialization.InfrastructureProvisioned = ptr.To(true)
+	cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{Host: "example.com", Port: 6443}
+	cluster.Spec.ControlPlaneRef = clusterv1.ContractVersionedObjectReference{
+		APIGroup: builder.ControlPlaneGroupVersion.Group,
+		Kind:     builder.TestControlPlaneKind,
+		Name:     "cp",
+	}
+	cp := builder.TestControlPlane(metav1.NamespaceDefault, "cp").WithVersion(testSkewK8sVersion).Build()
+	crd := builder.TestControlPlaneCRD.DeepCopy()
+
+	machine := builder.Machine(metav1.NamespaceDefault, "my-machine").
+		WithVersion(testK8sVersion).
+		WithClusterName(cluster.Name).
+		WithBootstrapTemplate(bootstrapbuilder.KubeadmConfig(metav1.NamespaceDefault, "").Unstructured()).
+		Build()
+	kubeadmConfig := newKubeadmConfig(metav1.NamespaceDefault, "kubeadmconfig")
+	kubeadmConfig.SetOwnerReferences([]metav1.OwnerReference{{
+		APIVersion: clusterv1.GroupVersion.String(),
+		Kind:       "Machine",
+		Name:       machine.Name,
+	}})
+
+	objects := []client.Object{cluster, machine, kubeadmConfig, cp, crd}
+	objects = append(objects, createSecrets(t, cluster, kubeadmConfig)...)
+
+	myclient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).WithStatusSubresource(&bootstrapv1.KubeadmConfig{}).Build()
+
+	r := &KubeadmConfigReconciler{
+		Client:              myclient,
+		SecretCachingClient: myclient,
+		ClusterCache:        clustercache.NewFakeClusterCache(myclient, client.ObjectKey{Name: cluster.Name, Namespace: cluster.Namespace}),
+		KubeadmInitLock:     &myInitLocker{},
+	}
+
+	key := client.ObjectKeyFromObject(kubeadmConfig)
+	_, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: key})
+	g.Expect(err).ToNot(HaveOccurred())
+
+	newConfig := &bootstrapv1.KubeadmConfig{}
+	g.Expect(myclient.Get(ctx, key, newConfig)).To(Succeed())
+
+	c := conditions.Get(newConfig, bootstrapv1.KubeadmConfigControlPlaneKubernetesVersionAvailableCondition)
+	g.Expect(c).ToNot(BeNil())
+	g.Expect(c.Status).To(Equal(metav1.ConditionTrue))
+	g.Expect(c.Reason).To(Equal(bootstrapv1.KubeadmConfigControlPlaneKubernetesVersionFromControlPlaneReason))
+	g.Expect(c.Message).To(ContainSubstring("control plane reference"))
+
+	foundCPKubeVer := false
+	for _, cond := range newConfig.GetV1Beta1Conditions() {
+		if cond.Type == bootstrapv1.ControlPlaneKubernetesVersionAvailableV1Beta1Condition {
+			g.Expect(cond.Status).To(Equal(corev1.ConditionTrue))
+			g.Expect(cond.Reason).To(Equal(bootstrapv1.ControlPlaneKubernetesVersionFromControlPlaneV1Beta1Reason))
+			foundCPKubeVer = true
+			break
+		}
+	}
+	g.Expect(foundCPKubeVer).To(BeTrue())
+}