Skip to content

Commit 2d8b068

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4822 from vincepri/fixup-cluster-cp
🌱 Expose Cluster Control Plane ref through cluster scope
2 parents 91941d9 + 591e196 commit 2d8b068

14 files changed

+79
-54
lines changed

‎controllers/awsmachine_controller.go

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
"github.com/pkg/errors"
3333
corev1 "k8s.io/api/core/v1"
3434
apierrors "k8s.io/apimachinery/pkg/api/errors"
35-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3635
kerrors "k8s.io/apimachinery/pkg/util/errors"
3736
"k8s.io/client-go/tools/record"
3837
"k8s.io/klog/v2"
@@ -61,7 +60,6 @@ import (
6160
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/userdata"
6261
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
6362
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
64-
"sigs.k8s.io/cluster-api/controllers/external"
6563
capierrors "sigs.k8s.io/cluster-api/errors"
6664
"sigs.k8s.io/cluster-api/util"
6765
"sigs.k8s.io/cluster-api/util/annotations"
@@ -147,6 +145,7 @@ func (r *AWSMachineReconciler) getObjectStoreService(scope scope.S3Scope) servic
147145

148146
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=awsmachines,verbs=get;list;watch;update;patch;delete
149147
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=awsmachines/status,verbs=get;update;patch
148+
// +kubebuilder:rbac:groups=controlplane.cluster.x-k8s.io,resources=*,verbs=get;list;watch
150149
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines;machines/status,verbs=get;list;watch
151150
// +kubebuilder:rbac:groups="",resources=secrets;,verbs=get;list;watch
152151
// +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch
@@ -202,16 +201,10 @@ func (r *AWSMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request)
202201

203202
infrav1.SetDefaults_AWSMachineSpec(&awsMachine.Spec)
204203

205-
cp, err := r.getControlPlane(ctx, log, cluster)
206-
if err != nil {
207-
return ctrl.Result{}, err
208-
}
209-
210204
// Create the machine scope
211205
machineScope, err := scope.NewMachineScope(scope.MachineScopeParams{
212206
Client: r.Client,
213207
Cluster: cluster,
214-
ControlPlane: cp,
215208
Machine: machine,
216209
InfraCluster: infraCluster,
217210
AWSMachine: awsMachine,
@@ -1225,22 +1218,3 @@ func (r *AWSMachineReconciler) ensureInstanceMetadataOptions(ec2svc services.EC2
12251218

12261219
return ec2svc.ModifyInstanceMetadataOptions(instance.ID, machine.Spec.InstanceMetadataOptions)
12271220
}
1228-
1229-
// +kubebuilder:rbac:groups=controlplane.cluster.x-k8s.io,resources=*,verbs=get;list;watch
1230-
1231-
func (r *AWSMachineReconciler) getControlPlane(ctx context.Context, log *logger.Logger, cluster *clusterv1.Cluster) (*unstructured.Unstructured, error) {
1232-
var ns string
1233-
1234-
if ns = cluster.Spec.ControlPlaneRef.Namespace; ns == "" {
1235-
ns = cluster.Namespace
1236-
}
1237-
1238-
controlPlane, err := external.Get(ctx, r.Client, cluster.Spec.ControlPlaneRef, ns)
1239-
if err != nil {
1240-
log.Error(err, "unable to get ControlPlane referenced in the given cluster", "cluster", fmt.Sprintf("%s/%s", cluster.Namespace, cluster.Name))
1241-
1242-
return nil, err
1243-
}
1244-
1245-
return controlPlane, nil
1246-
}

‎controllers/awsmachine_controller_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
"github.com/pkg/errors"
3131
corev1 "k8s.io/api/core/v1"
3232
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
33-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3433
"k8s.io/client-go/tools/record"
3534
"k8s.io/utils/ptr"
3635
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -418,7 +417,6 @@ func getMachineScope(cs *scope.ClusterScope, awsMachine *infrav1.AWSMachine) (*s
418417
InfrastructureReady: true,
419418
},
420419
},
421-
ControlPlane: &unstructured.Unstructured{},
422420
Machine: &clusterv1.Machine{
423421
ObjectMeta: metav1.ObjectMeta{
424422
Name: "test",

‎controllers/awsmachine_controller_unit_test.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import (
3333
"github.com/pkg/errors"
3434
corev1 "k8s.io/api/core/v1"
3535
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
36-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3736
"k8s.io/apimachinery/pkg/types"
3837
"k8s.io/client-go/tools/record"
3938
"k8s.io/klog/v2"
@@ -131,7 +130,6 @@ func TestAWSMachineReconciler(t *testing.T) {
131130
},
132131
},
133132
InfraCluster: cs,
134-
ControlPlane: &unstructured.Unstructured{},
135133
AWSMachine: awsMachine,
136134
},
137135
)
@@ -160,7 +158,6 @@ func TestAWSMachineReconciler(t *testing.T) {
160158
InfrastructureReady: true,
161159
},
162160
},
163-
ControlPlane: &unstructured.Unstructured{},
164161
Machine: &clusterv1.Machine{
165162
Spec: clusterv1.MachineSpec{
166163
ClusterName: "capi-test",

‎pkg/cloud/interfaces.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package cloud
1818

1919
import (
2020
awsclient "github.com/aws/aws-sdk-go/aws/client"
21+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2122
"sigs.k8s.io/controller-runtime/pkg/client"
2223

2324
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
@@ -67,6 +68,8 @@ type ClusterScoper interface {
6768

6869
// Cluster returns the cluster object.
6970
ClusterObj() ClusterObject
71+
// UnstructuredControlPlane returns the unstructured control plane object.
72+
UnstructuredControlPlane() (*unstructured.Unstructured, error)
7073

7174
// IdentityRef returns the AWS infrastructure cluster identityRef.
7275
IdentityRef() *infrav1.AWSIdentityReference

‎pkg/cloud/scope/cluster.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
awsclient "github.com/aws/aws-sdk-go/aws/client"
2424
"github.com/pkg/errors"
25+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2526
"k8s.io/klog/v2"
2627
"sigs.k8s.io/controller-runtime/pkg/client"
2728

@@ -395,3 +396,9 @@ func (s *ClusterScope) Partition() string {
395396
func (s *ClusterScope) AdditionalControlPlaneIngressRules() []infrav1.IngressRule {
396397
return s.AWSCluster.Spec.NetworkSpec.DeepCopy().AdditionalControlPlaneIngressRules
397398
}
399+
400+
// UnstructuredControlPlane returns the unstructured object for the control plane, if any.
401+
// When the reference is not set, it returns an empty object.
402+
func (s *ClusterScope) UnstructuredControlPlane() (*unstructured.Unstructured, error) {
403+
return getUnstructuredControlPlane(context.TODO(), s.client, s.Cluster)
404+
}

‎pkg/cloud/scope/machine.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323

2424
"github.com/pkg/errors"
2525
corev1 "k8s.io/api/core/v1"
26-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2726
"k8s.io/apimachinery/pkg/types"
2827
"k8s.io/klog/v2"
2928
"k8s.io/utils/ptr"
@@ -44,7 +43,6 @@ import (
4443
type MachineScopeParams struct {
4544
Client client.Client
4645
Logger *logger.Logger
47-
ControlPlane *unstructured.Unstructured
4846
Cluster *clusterv1.Cluster
4947
Machine *clusterv1.Machine
5048
InfraCluster EC2Scope
@@ -69,9 +67,6 @@ func NewMachineScope(params MachineScopeParams) (*MachineScope, error) {
6967
if params.InfraCluster == nil {
7068
return nil, errors.New("aws cluster is required when creating a MachineScope")
7169
}
72-
if params.ControlPlane == nil {
73-
return nil, errors.New("cluster control plane is required when creating a MachineScope")
74-
}
7570

7671
if params.Logger == nil {
7772
log := klog.Background()
@@ -86,7 +81,6 @@ func NewMachineScope(params MachineScopeParams) (*MachineScope, error) {
8681
Logger: *params.Logger,
8782
client: params.Client,
8883
patchHelper: helper,
89-
ControlPlane: params.ControlPlane,
9084
Cluster: params.Cluster,
9185
Machine: params.Machine,
9286
InfraCluster: params.InfraCluster,
@@ -102,7 +96,6 @@ type MachineScope struct {
10296

10397
Cluster *clusterv1.Cluster
10498
Machine *clusterv1.Machine
105-
ControlPlane *unstructured.Unstructured
10699
InfraCluster EC2Scope
107100
AWSMachine *infrav1.AWSMachine
108101
}
@@ -377,8 +370,22 @@ func (m *MachineScope) IsEKSManaged() bool {
377370
return m.InfraCluster.InfraCluster().GetObjectKind().GroupVersionKind().Kind == ekscontrolplanev1.AWSManagedControlPlaneKind
378371
}
379372

373+
// IsControlPlaneExternallyManaged checks if the control plane is externally managed.
374+
//
375+
// This is determined by the kind of the control plane object (EKS for example),
376+
// or if the control plane referenced object is reporting as externally managed.
380377
func (m *MachineScope) IsControlPlaneExternallyManaged() bool {
381-
return util.IsExternalManagedControlPlane(m.ControlPlane)
378+
if m.IsEKSManaged() {
379+
return true
380+
}
381+
382+
// Check if the control plane is externally managed.
383+
u, err := m.InfraCluster.UnstructuredControlPlane()
384+
if err != nil {
385+
m.Error(err, "failed to get unstructured control plane")
386+
return false
387+
}
388+
return util.IsExternalManagedControlPlane(u)
382389
}
383390

384391
// IsExternallyManaged checks if the machine is externally managed.

‎pkg/cloud/scope/machine_test.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222

2323
corev1 "k8s.io/api/core/v1"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2625
"k8s.io/apimachinery/pkg/runtime"
2726
"k8s.io/utils/ptr"
2827
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -133,8 +132,7 @@ func setupMachineScope() (*MachineScope, error) {
133132
InfraCluster: &ClusterScope{
134133
AWSCluster: awsCluster,
135134
},
136-
ControlPlane: &unstructured.Unstructured{},
137-
AWSMachine: awsMachine,
135+
AWSMachine: awsMachine,
138136
},
139137
)
140138
}
@@ -225,10 +223,9 @@ func TestGetRawBootstrapDataWithFormat(t *testing.T) {
225223

226224
machineScope, err := NewMachineScope(
227225
MachineScopeParams{
228-
Client: client,
229-
Machine: machine,
230-
Cluster: cluster,
231-
ControlPlane: &unstructured.Unstructured{},
226+
Client: client,
227+
Machine: machine,
228+
Cluster: cluster,
232229
InfraCluster: &ClusterScope{
233230
AWSCluster: awsCluster,
234231
},

‎pkg/cloud/scope/managedcontrolplane.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
appsv1 "k8s.io/api/apps/v1"
2828
corev1 "k8s.io/api/core/v1"
2929
rbacv1 "k8s.io/api/rbac/v1"
30+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3031
"k8s.io/apimachinery/pkg/runtime"
3132
"k8s.io/klog/v2"
3233
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -440,3 +441,9 @@ func (s *ManagedControlPlaneScope) Partition() string {
440441
func (s *ManagedControlPlaneScope) AdditionalControlPlaneIngressRules() []infrav1.IngressRule {
441442
return nil
442443
}
444+
445+
// UnstructuredControlPlane returns the unstructured object for the control plane, if any.
446+
// When the reference is not set, it returns an empty object.
447+
func (s *ManagedControlPlaneScope) UnstructuredControlPlane() (*unstructured.Unstructured, error) {
448+
return getUnstructuredControlPlane(context.TODO(), s.Client, s.Cluster)
449+
}

‎pkg/cloud/scope/shared.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,18 @@ limitations under the License.
1717
package scope
1818

1919
import (
20+
"context"
2021
"fmt"
2122

2223
"github.com/pkg/errors"
24+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
25+
"sigs.k8s.io/controller-runtime/pkg/client"
2326

2427
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
2528
expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
2629
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
30+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
31+
"sigs.k8s.io/cluster-api/controllers/external"
2732
)
2833

2934
var (
@@ -127,3 +132,24 @@ func (p *defaultSubnetPlacementStrategy) getSubnetsForAZs(azs []string, controlP
127132

128133
return subnetIDs, nil
129134
}
135+
136+
// getUnstructuredControlPlane returns the unstructured object for the control plane, if any.
137+
// When the reference is not set, it returns an empty object.
138+
func getUnstructuredControlPlane(ctx context.Context, client client.Client, cluster *clusterv1.Cluster) (*unstructured.Unstructured, error) {
139+
if cluster.Spec.ControlPlaneRef == nil {
140+
// If the control plane ref is not set, return an empty object.
141+
// Not having a control plane ref is valid given API contracts.
142+
return &unstructured.Unstructured{}, nil
143+
}
144+
145+
namespace := cluster.Spec.ControlPlaneRef.Namespace
146+
if namespace == "" {
147+
namespace = cluster.Namespace
148+
}
149+
150+
u, err := external.Get(ctx, client, cluster.Spec.ControlPlaneRef, namespace)
151+
if err != nil {
152+
return nil, errors.Wrapf(err, "failed to retrieve control plane object %s/%s", cluster.Spec.ControlPlaneRef.Namespace, cluster.Spec.ControlPlaneRef.Name)
153+
}
154+
return u, nil
155+
}

‎pkg/cloud/services/ec2/instances.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ func (s *Service) CreateInstance(scope *scope.MachineScope, userData []byte, use
183183

184184
if !scope.IsControlPlaneExternallyManaged() && !scope.IsExternallyManaged() && !scope.IsEKSManaged() && s.scope.Network().APIServerELB.DNSName == "" {
185185
record.Eventf(s.scope.InfraCluster(), "FailedCreateInstance", "Failed to run controlplane, APIServer ELB not available")
186-
187186
return nil, awserrors.NewFailedDependency("failed to run controlplane, APIServer ELB not available")
188187
}
189188

0 commit comments

Comments
 (0)