Repository: rancher/cluster-api-provider-rke2
Pull request #418 (2024-08-27 12:20:57)

Make control plane endpoint default registration method
<!--
Label the PR with the kind of change this for:

kind/feature
kind/bug
kind/documentation
kind/regression
kind/*
-->

**What this PR does / why we need it**:
Fixes: #343 

From the list of available registration methods, control plane one seems to be the most suitable for a default. It will not work for all cases, but for those which do, namely clusters without restrictions on accessing public API and address of CP endpoint, this should work out of the box as a quickstart method.

Users may decide to define a different registrationMethod, where one edit of the value is intentionally allowed.

<!-- Enter a description of the change and why this change is needed -->

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

**Checklist**:
<!-- Put an "X" character inside the brackets of each completed task. Some may be optional depending on the PR in which case these can be deleted -->

- [ ] squashed commits into logical changes
- [ ] includes documentation
- [x] adds unit tests
- [ ] adds or updates e2e tests


furkatgofurov7: This needs an update as discussed offline with @Danil-Grigorev, please hold off merging it manually. 

Patch:
diff --git a/controlplane/api/v1beta1/rke2controlplane_types.go b/controlplane/api/v1beta1/rke2controlplane_types.go
index 6c56308..b853d32 100644
--- a/controlplane/api/v1beta1/rke2controlplane_types.go
+++ b/controlplane/api/v1beta1/rke2controlplane_types.go
@@ -77,9 +77,9 @@ type RKE2ControlPlaneSpec struct {
 	NodeDrainTimeout *metav1.Duration `json:"nodeDrainTimeout,omitempty"`
 
 	// RegistrationMethod is the method to use for registering nodes into the RKE2 cluster.
-	// +kubebuilder:validation:Enum=internal-first;internal-only-ips;external-only-ips;address;control-plane-endpoint
+	// +kubebuilder:validation:Enum=internal-first;internal-only-ips;external-only-ips;address;control-plane-endpoint;""
 	// +optional
-	RegistrationMethod RegistrationMethod `json:"registrationMethod"`
+	RegistrationMethod RegistrationMethod `json:"registrationMethod,omitempty"`
 
 	// RegistrationAddress is an explicit address to use when registering a node. This is required if
 	// the registration type is "address". Its for scenarios where a load-balancer or VIP is used.
diff --git a/controlplane/api/v1beta1/rke2controlplane_webhook.go b/controlplane/api/v1beta1/rke2controlplane_webhook.go
index 475ed14..7a9d110 100644
--- a/controlplane/api/v1beta1/rke2controlplane_webhook.go
+++ b/controlplane/api/v1beta1/rke2controlplane_webhook.go
@@ -85,9 +85,10 @@ func (r *RKE2ControlPlane) ValidateUpdate(old runtime.Object) (admission.Warning
 	allErrs = append(allErrs, bootstrapv1.ValidateRKE2ConfigSpec(r.Name, &r.Spec.RKE2ConfigSpec)...)
 	allErrs = append(allErrs, r.validateCNI()...)
 
-	if r.Spec.RegistrationMethod != oldControlplane.Spec.RegistrationMethod {
+	oldSet := oldControlplane.Spec.RegistrationMethod != ""
+	if oldSet && r.Spec.RegistrationMethod != oldControlplane.Spec.RegistrationMethod {
 		allErrs = append(allErrs,
-			field.Invalid(field.NewPath("spec", "registrationMethod"), r.Spec.RegistrationMethod, "field is immutable"),
+			field.Invalid(field.NewPath("spec", "registrationMethod"), r.Spec.RegistrationMethod, "field value is immutable once set"),
 		)
 	}
 
diff --git a/controlplane/api/v1beta1/rke2controlplanetemplate_webhook.go b/controlplane/api/v1beta1/rke2controlplanetemplate_webhook.go
index 0da528b..be18d0a 100644
--- a/controlplane/api/v1beta1/rke2controlplanetemplate_webhook.go
+++ b/controlplane/api/v1beta1/rke2controlplanetemplate_webhook.go
@@ -81,9 +81,10 @@ func (r *RKE2ControlPlaneTemplate) ValidateUpdate(old runtime.Object) (admission
 	allErrs = append(allErrs, bootstrapv1.ValidateRKE2ConfigSpec(r.Name, &r.Spec.Template.Spec.RKE2ConfigSpec)...)
 	allErrs = append(allErrs, r.validateCNI()...)
 
-	if r.Spec.Template.Spec.RegistrationMethod != oldControlplane.Spec.Template.Spec.RegistrationMethod {
+	oldSet := oldControlplane.Spec.Template.Spec.RegistrationMethod != ""
+	if oldSet && r.Spec.Template.Spec.RegistrationMethod != oldControlplane.Spec.Template.Spec.RegistrationMethod {
 		allErrs = append(allErrs,
-			field.Invalid(field.NewPath("spec", "registrationMethod"), r.Spec.Template.Spec.RegistrationMethod, "field is immutable"),
+			field.Invalid(field.NewPath("spec", "registrationMethod"), r.Spec.Template.Spec.RegistrationMethod, "field value is immutable once set"),
 		)
 	}
 
diff --git a/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_rke2controlplanes.yaml b/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_rke2controlplanes.yaml
index 7d8f7e0..5449122 100644
--- a/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_rke2controlplanes.yaml
+++ b/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_rke2controlplanes.yaml
@@ -1949,6 +1949,7 @@ spec:
                 - external-only-ips
                 - address
                 - control-plane-endpoint
+                - ""
                 type: string
               replicas:
                 description: Replicas is the number of replicas for the Control Plane.
diff --git a/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_rke2controlplanetemplates.yaml b/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_rke2controlplanetemplates.yaml
index 5de035a..8adafa1 100644
--- a/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_rke2controlplanetemplates.yaml
+++ b/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_rke2controlplanetemplates.yaml
@@ -782,6 +782,7 @@ spec:
                         - external-only-ips
                         - address
                         - control-plane-endpoint
+                        - ""
                         type: string
                       replicas:
                         description: Replicas is the number of replicas for the Control
diff --git a/pkg/registration/registration.go b/pkg/registration/registration.go
index 8c23510..dccc004 100644
--- a/pkg/registration/registration.go
+++ b/pkg/registration/registration.go
@@ -43,7 +43,7 @@ func NewRegistrationMethod(method string) (GetRegistrationAddresses, error) {
 		return registrationMethodWithFilter(filterExternalOnly), nil
 	case "address":
 		return registrationMethodAddress, nil
-	case "control-plane-endpoint":
+	case "control-plane-endpoint", "":
 		return registrationMethodControlPlaneEndpoint, nil
 	default:
 		return nil, fmt.Errorf("unsupported registration method: %s", method)
diff --git a/samples/aws/cluster-template.yaml b/samples/aws/cluster-template.yaml
index 5775b61..2b6ef09 100644
--- a/samples/aws/cluster-template.yaml
+++ b/samples/aws/cluster-template.yaml
@@ -136,7 +136,6 @@ spec:
       backupConfig:
         retention: "10"
         scheduleCron: "*/15 * * * *"
-  registrationMethod: control-plane-endpoint
   rolloutStrategy:
     type: "RollingUpdate"
     rollingUpdate:
diff --git a/samples/docker/clusterclass/clusterclass-quick-start.yaml b/samples/docker/clusterclass/clusterclass-quick-start.yaml
index 07d092b..ab665f1 100644
--- a/samples/docker/clusterclass/clusterclass-quick-start.yaml
+++ b/samples/docker/clusterclass/clusterclass-quick-start.yaml
@@ -159,7 +159,6 @@ spec:
         disableComponents:
           kubernetesComponents: [ "cloudController"]
       nodeDrainTimeout: 2m
-      registrationMethod: "control-plane-endpoint"
       rolloutStrategy:
         type: "RollingUpdate"
         rollingUpdate:


Test patch:
diff --git a/pkg/registration/registration_test.go b/pkg/registration/registration_test.go
index 9f8055b..b28d412 100644
--- a/pkg/registration/registration_test.go
+++ b/pkg/registration/registration_test.go
@@ -53,6 +53,10 @@ func TestNewRegistrationMethod(t *testing.T) {
 			name:        "control-plane-endpoint",
 			expectError: false,
 		},
+		{
+			name:        "",
+			expectError: false,
+		},
 		{
 			name:        "unknownmethod",
 			expectError: true,
diff --git a/test/e2e/config/e2e_conf.yaml b/test/e2e/config/e2e_conf.yaml
index 1b855c6..f1774ca 100644
--- a/test/e2e/config/e2e_conf.yaml
+++ b/test/e2e/config/e2e_conf.yaml
@@ -44,6 +44,23 @@ providers:
           new: "--leader-elect=false"
         - old: --metrics-addr=127.0.0.1:8080
           new: --metrics-addr=:8080
+  - name: docker-updated
+    type: InfrastructureProvider
+    versions:
+    - name: "v1.7.2"
+      value: "https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.7.2/infrastructure-components-development.yaml"
+      type: "url"
+      contract: v1beta1
+      files:
+        - sourcePath: "../data/infrastructure/cluster-template-docker-updated.yaml"
+        - sourcePath: "../data/shared/v1beta1/metadata.yaml"
+      replacements:
+        - old: "imagePullPolicy: Always"
+          new: "imagePullPolicy: IfNotPresent"
+        - old: "--leader-elect"
+          new: "--leader-elect=false"
+        - old: --metrics-addr=127.0.0.1:8080
+          new: --metrics-addr=:8080
   - name: rke2-control-plane
     type: ControlPlaneProvider
     versions:
diff --git a/test/e2e/data/infrastructure/cluster-template-docker-updated.yaml b/test/e2e/data/infrastructure/cluster-template-docker-updated.yaml
new file mode 100644
index 0000000..6057cf9
--- /dev/null
+++ b/test/e2e/data/infrastructure/cluster-template-docker-updated.yaml
@@ -0,0 +1,173 @@
+apiVersion: v1
+data:
+  value: |-
+    # generated by kind
+    global
+      log /dev/log local0
+      log /dev/log local1 notice
+      daemon
+      # limit memory usage to approximately 18 MB
+      # (see https://github.com/kubernetes-sigs/kind/pull/3115)
+      maxconn 100000
+
+    resolvers docker
+      nameserver dns 127.0.0.11:53
+
+    defaults
+      log global
+      mode tcp
+      option dontlognull
+      # TODO: tune these
+      timeout connect 5000
+      timeout client 50000
+      timeout server 50000
+      # allow to boot despite dns don't resolve backends
+      default-server init-addr none
+
+    frontend stats
+      bind *:8404
+      stats enable
+      stats uri /
+      stats refresh 10s
+
+    frontend control-plane
+      bind *:{{ .FrontendControlPlanePort }}
+      {{ if .IPv6 -}}
+      bind :::{{ .FrontendControlPlanePort }};
+      {{- end }}
+      default_backend kube-apiservers
+
+    backend kube-apiservers
+      option httpchk GET /healthz
+      http-check expect status 401
+      # TODO: we should be verifying (!)
+      {{range $server, $address := .BackendServers}}
+      server {{ $server }} {{ JoinHostPort $address $.BackendControlPlanePort }} check check-ssl verify none resolvers docker resolve-prefer {{ if $.IPv6 -}} ipv6 {{- else -}} ipv4 {{- end }}
+      {{- end}}
+
+    frontend rke2-join
+      bind *:9345
+      {{ if .IPv6 -}}
+      bind :::9345;
+      {{- end }}
+      default_backend rke2-servers
+
+    backend rke2-servers
+      option httpchk GET /v1-rke2/readyz
+      http-check expect status 403
+      {{range $server, $address := .BackendServers}}
+      server {{ $server }} {{ $address }}:9345 check check-ssl verify none
+      {{- end}}
+kind: ConfigMap
+metadata:
+  name: ${CLUSTER_NAME}-lb-config
+---
+apiVersion: cluster.x-k8s.io/v1beta1
+kind: Cluster 
+metadata:
+  name: ${CLUSTER_NAME} 
+spec:
+  clusterNetwork:
+    pods:
+      cidrBlocks:
+      - 10.45.0.0/16
+    services:
+      cidrBlocks:
+      - 10.46.0.0/16
+    serviceDomain: cluster.local
+  controlPlaneRef:
+    apiVersion: controlplane.cluster.x-k8s.io/v1beta1
+    kind: RKE2ControlPlane
+    name: ${CLUSTER_NAME}-control-plane
+  infrastructureRef:
+    apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+    kind: DockerCluster
+    name: ${CLUSTER_NAME}
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: DockerCluster
+metadata:
+  name: ${CLUSTER_NAME}
+spec:
+  loadBalancer:
+    customHAProxyConfigTemplateRef:
+      name: ${CLUSTER_NAME}-lb-config
+---
+apiVersion: controlplane.cluster.x-k8s.io/v1beta1
+kind: RKE2ControlPlane
+metadata:
+  name: ${CLUSTER_NAME}-control-plane
+spec: 
+  replicas: ${CONTROL_PLANE_MACHINE_COUNT}
+  version: ${KUBERNETES_VERSION}+rke2r1
+  rolloutStrategy:
+    type: "RollingUpdate"
+    rollingUpdate:
+      maxSurge: 1
+  agentConfig:
+    nodeAnnotations:
+      test: "true"
+  serverConfig:
+    disableComponents:
+      kubernetesComponents:
+      - cloudController
+  infrastructureRef:
+    apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+    kind: DockerMachineTemplate
+    name:  "${CLUSTER_NAME}-control-plane"
+  nodeDrainTimeout: 30s
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: DockerMachineTemplate
+metadata:
+  name:  "${CLUSTER_NAME}-control-plane"
+spec:
+  template:
+    spec:
+      customImage: kindest/node:${KIND_IMAGE_VERSION}
+      bootstrapTimeout: 15m
+---
+apiVersion: cluster.x-k8s.io/v1beta1
+kind: MachineDeployment
+metadata:
+  name: ${CLUSTER_NAME}-md-0
+spec:
+  clusterName: ${CLUSTER_NAME}
+  replicas: ${WORKER_MACHINE_COUNT}
+  selector:
+    matchLabels:
+      cluster.x-k8s.io/cluster-name: ${CLUSTER_NAME}
+  template:
+    spec:
+      version: ${KUBERNETES_VERSION}+rke2r1
+      clusterName: ${CLUSTER_NAME}
+      bootstrap:
+        configRef:
+          apiVersion: bootstrap.cluster.x-k8s.io/v1beta1
+          kind: RKE2ConfigTemplate
+          name: ${CLUSTER_NAME}-md-0
+      infrastructureRef:
+        apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+        kind: DockerMachineTemplate
+        name: ${CLUSTER_NAME}-md-0
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: DockerMachineTemplate
+metadata:
+  name: ${CLUSTER_NAME}-md-0
+spec:
+  template:
+    spec:
+      customImage: kindest/node:${KIND_IMAGE_VERSION}
+      bootstrapTimeout: 15m
+---
+apiVersion: bootstrap.cluster.x-k8s.io/v1beta1
+kind: RKE2ConfigTemplate
+metadata:
+  name: ${CLUSTER_NAME}-md-0
+spec: 
+  template:
+    spec:
+      agentConfig:
+        nodeAnnotations:
+          test: "true"
diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go
index 85067f1..1a4c04f 100644
--- a/test/e2e/e2e_test.go
+++ b/test/e2e/e2e_test.go
@@ -96,8 +96,8 @@ var _ = Describe("Workload cluster creation", func() {
 					LogFolder:                clusterctlLogFolder,
 					ClusterctlConfigPath:     clusterctlConfigPath,
 					KubeconfigPath:           bootstrapClusterProxy.GetKubeconfigPath(),
-					InfrastructureProvider:   "docker",
-					Flavor:                   "docker",
+					InfrastructureProvider:   "docker-updated",
+					Flavor:                   "docker-updated",
 					Namespace:                namespace.Name,
 					ClusterName:              clusterName,
 					KubernetesVersion:        e2eConfig.GetVariable(KubernetesVersion),
@@ -117,8 +117,8 @@ var _ = Describe("Workload cluster creation", func() {
 					LogFolder:                clusterctlLogFolder,
 					ClusterctlConfigPath:     clusterctlConfigPath,
 					KubeconfigPath:           bootstrapClusterProxy.GetKubeconfigPath(),
-					InfrastructureProvider:   "docker",
-					Flavor:                   "docker",
+					InfrastructureProvider:   "docker-updated",
+					Flavor:                   "docker-updated",
 					Namespace:                namespace.Name,
 					ClusterName:              clusterName,
 					KubernetesVersion:        e2eConfig.GetVariable(KubernetesVersion),
@@ -137,8 +137,8 @@ var _ = Describe("Workload cluster creation", func() {
 					LogFolder:                clusterctlLogFolder,
 					ClusterctlConfigPath:     clusterctlConfigPath,
 					KubeconfigPath:           bootstrapClusterProxy.GetKubeconfigPath(),
-					InfrastructureProvider:   "docker",
-					Flavor:                   "docker",
+					InfrastructureProvider:   "docker-updated",
+					Flavor:                   "docker-updated",
 					Namespace:                namespace.Name,
 					ClusterName:              clusterName,
 					KubernetesVersion:        e2eConfig.GetVariable(KubernetesVersionUpgradeTo),
@@ -170,8 +170,8 @@ var _ = Describe("Workload cluster creation", func() {
 					LogFolder:                clusterctlLogFolder,
 					ClusterctlConfigPath:     clusterctlConfigPath,
 					KubeconfigPath:           bootstrapClusterProxy.GetKubeconfigPath(),
-					InfrastructureProvider:   "docker",
-					Flavor:                   "docker",
+					InfrastructureProvider:   "docker-updated",
+					Flavor:                   "docker-updated",
 					Namespace:                namespace.Name,
 					ClusterName:              clusterName,
 					KubernetesVersion:        e2eConfig.GetVariable(KubernetesVersionUpgradeTo),
@@ -196,8 +196,8 @@ var _ = Describe("Workload cluster creation", func() {
 					LogFolder:                clusterctlLogFolder,
 					ClusterctlConfigPath:     clusterctlConfigPath,
 					KubeconfigPath:           bootstrapClusterProxy.GetKubeconfigPath(),
-					InfrastructureProvider:   "docker",
-					Flavor:                   "docker",
+					InfrastructureProvider:   "docker-updated",
+					Flavor:                   "docker-updated",
 					Namespace:                namespace.Name,
 					ClusterName:              clusterName,
 					KubernetesVersion:        e2eConfig.GetVariable(KubernetesVersionUpgradeTo),
@@ -222,8 +222,8 @@ var _ = Describe("Workload cluster creation", func() {
 					LogFolder:                clusterctlLogFolder,
 					ClusterctlConfigPath:     clusterctlConfigPath,
 					KubeconfigPath:           bootstrapClusterProxy.GetKubeconfigPath(),
-					InfrastructureProvider:   "docker",
-					Flavor:                   "docker",
+					InfrastructureProvider:   "docker-updated",
+					Flavor:                   "docker-updated",
 					Namespace:                namespace.Name,
 					ClusterName:              clusterName,
 					KubernetesVersion:        e2eConfig.GetVariable(KubernetesVersionUpgradeTo),