Merge pull request #4822 from vincepri/fixup-cluster-cp

🌱 Expose Cluster Control Plane ref through cluster scope

Author: k8s-ci-robot

controllers/awsmachine_controller.go

@@ -32,7 +32,6 @@ import (
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/klog/v2"
@@ -61,7 +60,6 @@ import (
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/userdata"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
-	"sigs.k8s.io/cluster-api/controllers/external"
 	capierrors "sigs.k8s.io/cluster-api/errors"
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
@@ -147,6 +145,7 @@ func (r *AWSMachineReconciler) getObjectStoreService(scope scope.S3Scope) servic
 
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=awsmachines,verbs=get;list;watch;update;patch;delete
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=awsmachines/status,verbs=get;update;patch
+// +kubebuilder:rbac:groups=controlplane.cluster.x-k8s.io,resources=*,verbs=get;list;watch
 // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines;machines/status,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=secrets;,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch
@@ -202,16 +201,10 @@ func (r *AWSMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	infrav1.SetDefaults_AWSMachineSpec(&awsMachine.Spec)
 
-	cp, err := r.getControlPlane(ctx, log, cluster)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-
 	// Create the machine scope
 	machineScope, err := scope.NewMachineScope(scope.MachineScopeParams{
 		Client:       r.Client,
 		Cluster:      cluster,
-		ControlPlane: cp,
 		Machine:      machine,
 		InfraCluster: infraCluster,
 		AWSMachine:   awsMachine,
@@ -1225,22 +1218,3 @@ func (r *AWSMachineReconciler) ensureInstanceMetadataOptions(ec2svc services.EC2
 
 	return ec2svc.ModifyInstanceMetadataOptions(instance.ID, machine.Spec.InstanceMetadataOptions)
 }
-
-// +kubebuilder:rbac:groups=controlplane.cluster.x-k8s.io,resources=*,verbs=get;list;watch
-
-func (r *AWSMachineReconciler) getControlPlane(ctx context.Context, log *logger.Logger, cluster *clusterv1.Cluster) (*unstructured.Unstructured, error) {
-	var ns string
-
-	if ns = cluster.Spec.ControlPlaneRef.Namespace; ns == "" {
-		ns = cluster.Namespace
-	}
-
-	controlPlane, err := external.Get(ctx, r.Client, cluster.Spec.ControlPlaneRef, ns)
-	if err != nil {
-		log.Error(err, "unable to get ControlPlane referenced in the given cluster", "cluster", fmt.Sprintf("%s/%s", cluster.Namespace, cluster.Name))
-
-		return nil, err
-	}
-
-	return controlPlane, nil
-}

controllers/awsmachine_controller_test.go

@@ -30,7 +30,6 @@ import (
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -418,7 +417,6 @@ func getMachineScope(cs *scope.ClusterScope, awsMachine *infrav1.AWSMachine) (*s
 					InfrastructureReady: true,
 				},
 			},
-			ControlPlane: &unstructured.Unstructured{},
 			Machine: &clusterv1.Machine{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",

controllers/awsmachine_controller_unit_test.go

@@ -33,7 +33,6 @@ import (
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/klog/v2"
@@ -131,7 +130,6 @@ func TestAWSMachineReconciler(t *testing.T) {
 					},
 				},
 				InfraCluster: cs,
-				ControlPlane: &unstructured.Unstructured{},
 				AWSMachine:   awsMachine,
 			},
 		)
@@ -160,7 +158,6 @@ func TestAWSMachineReconciler(t *testing.T) {
 						InfrastructureReady: true,
 					},
 				},
-				ControlPlane: &unstructured.Unstructured{},
 				Machine: &clusterv1.Machine{
 					Spec: clusterv1.MachineSpec{
 						ClusterName: "capi-test",

pkg/cloud/interfaces.go

@@ -18,6 +18,7 @@ package cloud
 
 import (
 	awsclient "github.com/aws/aws-sdk-go/aws/client"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
@@ -67,6 +68,8 @@ type ClusterScoper interface {
 
 	// Cluster returns the cluster object.
 	ClusterObj() ClusterObject
+	// UnstructuredControlPlane returns the unstructured control plane object.
+	UnstructuredControlPlane() (*unstructured.Unstructured, error)
 
 	// IdentityRef returns the AWS infrastructure cluster identityRef.
 	IdentityRef() *infrav1.AWSIdentityReference

pkg/cloud/scope/cluster.go

@@ -22,6 +22,7 @@ import (
 
 	awsclient "github.com/aws/aws-sdk-go/aws/client"
 	"github.com/pkg/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -395,3 +396,9 @@ func (s *ClusterScope) Partition() string {
 func (s *ClusterScope) AdditionalControlPlaneIngressRules() []infrav1.IngressRule {
 	return s.AWSCluster.Spec.NetworkSpec.DeepCopy().AdditionalControlPlaneIngressRules
 }
+
+// UnstructuredControlPlane returns the unstructured object for the control plane, if any.
+// When the reference is not set, it returns an empty object.
+func (s *ClusterScope) UnstructuredControlPlane() (*unstructured.Unstructured, error) {
+	return getUnstructuredControlPlane(context.TODO(), s.client, s.Cluster)
+}

pkg/cloud/scope/machine.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
 	"k8s.io/utils/ptr"
@@ -44,7 +43,6 @@ import (
 type MachineScopeParams struct {
 	Client       client.Client
 	Logger       *logger.Logger
-	ControlPlane *unstructured.Unstructured
 	Cluster      *clusterv1.Cluster
 	Machine      *clusterv1.Machine
 	InfraCluster EC2Scope
@@ -69,9 +67,6 @@ func NewMachineScope(params MachineScopeParams) (*MachineScope, error) {
 	if params.InfraCluster == nil {
 		return nil, errors.New("aws cluster is required when creating a MachineScope")
 	}
-	if params.ControlPlane == nil {
-		return nil, errors.New("cluster control plane is required when creating a MachineScope")
-	}
 
 	if params.Logger == nil {
 		log := klog.Background()
@@ -86,7 +81,6 @@ func NewMachineScope(params MachineScopeParams) (*MachineScope, error) {
 		Logger:       *params.Logger,
 		client:       params.Client,
 		patchHelper:  helper,
-		ControlPlane: params.ControlPlane,
 		Cluster:      params.Cluster,
 		Machine:      params.Machine,
 		InfraCluster: params.InfraCluster,
@@ -102,7 +96,6 @@ type MachineScope struct {
 
 	Cluster      *clusterv1.Cluster
 	Machine      *clusterv1.Machine
-	ControlPlane *unstructured.Unstructured
 	InfraCluster EC2Scope
 	AWSMachine   *infrav1.AWSMachine
 }
@@ -377,8 +370,22 @@ func (m *MachineScope) IsEKSManaged() bool {
 	return m.InfraCluster.InfraCluster().GetObjectKind().GroupVersionKind().Kind == ekscontrolplanev1.AWSManagedControlPlaneKind
 }
 
+// IsControlPlaneExternallyManaged checks if the control plane is externally managed.
+//
+// This is determined by the kind of the control plane object (EKS for example),
+// or if the control plane referenced object is reporting as externally managed.
 func (m *MachineScope) IsControlPlaneExternallyManaged() bool {
-	return util.IsExternalManagedControlPlane(m.ControlPlane)
+	if m.IsEKSManaged() {
+		return true
+	}
+
+	// Check if the control plane is externally managed.
+	u, err := m.InfraCluster.UnstructuredControlPlane()
+	if err != nil {
+		m.Error(err, "failed to get unstructured control plane")
+		return false
+	}
+	return util.IsExternalManagedControlPlane(u)
 }
 
 // IsExternallyManaged checks if the machine is externally managed.

pkg/cloud/scope/machine_test.go

@@ -22,7 +22,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -133,8 +132,7 @@ func setupMachineScope() (*MachineScope, error) {
 			InfraCluster: &ClusterScope{
 				AWSCluster: awsCluster,
 			},
-			ControlPlane: &unstructured.Unstructured{},
-			AWSMachine:   awsMachine,
+			AWSMachine: awsMachine,
 		},
 	)
 }
@@ -225,10 +223,9 @@ func TestGetRawBootstrapDataWithFormat(t *testing.T) {
 
 		machineScope, err := NewMachineScope(
 			MachineScopeParams{
-				Client:       client,
-				Machine:      machine,
-				Cluster:      cluster,
-				ControlPlane: &unstructured.Unstructured{},
+				Client:  client,
+				Machine: machine,
+				Cluster: cluster,
 				InfraCluster: &ClusterScope{
 					AWSCluster: awsCluster,
 				},

pkg/cloud/scope/managedcontrolplane.go

@@ -27,6 +27,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -440,3 +441,9 @@ func (s *ManagedControlPlaneScope) Partition() string {
 func (s *ManagedControlPlaneScope) AdditionalControlPlaneIngressRules() []infrav1.IngressRule {
 	return nil
 }
+
+// UnstructuredControlPlane returns the unstructured object for the control plane, if any.
+// When the reference is not set, it returns an empty object.
+func (s *ManagedControlPlaneScope) UnstructuredControlPlane() (*unstructured.Unstructured, error) {
+	return getUnstructuredControlPlane(context.TODO(), s.Client, s.Cluster)
+}

pkg/cloud/scope/shared.go

@@ -17,13 +17,18 @@ limitations under the License.
 package scope
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/pkg/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
 	expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/cluster-api/controllers/external"
 )
 
 var (
@@ -127,3 +132,24 @@ func (p *defaultSubnetPlacementStrategy) getSubnetsForAZs(azs []string, controlP
 
 	return subnetIDs, nil
 }
+
+// getUnstructuredControlPlane returns the unstructured object for the control plane, if any.
+// When the reference is not set, it returns an empty object.
+func getUnstructuredControlPlane(ctx context.Context, client client.Client, cluster *clusterv1.Cluster) (*unstructured.Unstructured, error) {
+	if cluster.Spec.ControlPlaneRef == nil {
+		// If the control plane ref is not set, return an empty object.
+		// Not having a control plane ref is valid given API contracts.
+		return &unstructured.Unstructured{}, nil
+	}
+
+	namespace := cluster.Spec.ControlPlaneRef.Namespace
+	if namespace == "" {
+		namespace = cluster.Namespace
+	}
+
+	u, err := external.Get(ctx, client, cluster.Spec.ControlPlaneRef, namespace)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to retrieve control plane object %s/%s", cluster.Spec.ControlPlaneRef.Namespace, cluster.Spec.ControlPlaneRef.Name)
+	}
+	return u, nil
+}

pkg/cloud/services/ec2/instances.go

@@ -183,7 +183,6 @@ func (s *Service) CreateInstance(scope *scope.MachineScope, userData []byte, use
 
 	if !scope.IsControlPlaneExternallyManaged() && !scope.IsExternallyManaged() && !scope.IsEKSManaged() && s.scope.Network().APIServerELB.DNSName == "" {
 		record.Eventf(s.scope.InfraCluster(), "FailedCreateInstance", "Failed to run controlplane, APIServer ELB not available")
-
 		return nil, awserrors.NewFailedDependency("failed to run controlplane, APIServer ELB not available")
 	}