Skip to content

Commit f3ba287

Browse files
authored
Merge pull request #5214 from PanSpagetka/rosa-initial-integration-test
🌱Add initial Rosa machine pool integration tests
2 parents 0d61f5b + b926856 commit f3ba287

File tree

10 files changed

+1069
-22
lines changed

10 files changed

+1069
-22
lines changed

controlplane/rosa/controllers/rosacontrolplane_controller.go

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131

3232
stsv2 "github.com/aws/aws-sdk-go-v2/service/sts"
3333
sts "github.com/aws/aws-sdk-go/service/sts"
34+
"github.com/aws/aws-sdk-go/service/sts/stsiface"
3435
"github.com/google/go-cmp/cmp"
3536
idputils "github.com/openshift-online/ocm-common/pkg/idp/utils"
3637
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
@@ -40,6 +41,7 @@ import (
4041
corev1 "k8s.io/api/core/v1"
4142
apierrors "k8s.io/apimachinery/pkg/api/errors"
4243
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
44+
"k8s.io/apimachinery/pkg/runtime"
4345
"k8s.io/apimachinery/pkg/types"
4446
kerrors "k8s.io/apimachinery/pkg/util/errors"
4547
"k8s.io/apiserver/pkg/storage/names"
@@ -58,6 +60,7 @@ import (
5860
rosacontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/rosa/api/v1beta2"
5961
expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
6062
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/annotations"
63+
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud"
6164
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
6265
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
6366
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/rosa"
@@ -89,11 +92,15 @@ type ROSAControlPlaneReconciler struct {
8992
WatchFilterValue string
9093
WaitInfraPeriod time.Duration
9194
Endpoints []scope.ServiceEndpoint
95+
NewStsClient func(cloud.ScopeUsage, cloud.Session, logger.Wrapper, runtime.Object) stsiface.STSAPI
96+
NewOCMClient func(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (rosa.OCMClient, error)
9297
}
9398

9499
// SetupWithManager is used to setup the controller.
95100
func (r *ROSAControlPlaneReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
96101
log := logger.FromContext(ctx)
102+
r.NewOCMClient = rosa.NewWrappedOCMClient
103+
r.NewStsClient = scope.NewSTSClient
97104

98105
rosaControlPlane := &rosacontrolplanev1.ROSAControlPlane{}
99106
c, err := ctrl.NewControllerManagedBy(mgr).
@@ -173,6 +180,7 @@ func (r *ROSAControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.Req
173180
ControllerName: strings.ToLower(rosaControlPlaneKind),
174181
Endpoints: r.Endpoints,
175182
Logger: log,
183+
NewStsClient: r.NewStsClient,
176184
})
177185
if err != nil {
178186
return ctrl.Result{}, fmt.Errorf("failed to create scope: %w", err)
@@ -202,9 +210,12 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc
202210
return ctrl.Result{}, err
203211
}
204212
}
213+
if r.NewOCMClient == nil {
214+
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: NewOCMClient is nil")
215+
}
205216

206-
ocmClient, err := rosa.NewOCMClient(ctx, rosaScope)
207-
if err != nil {
217+
ocmClient, err := r.NewOCMClient(ctx, rosaScope)
218+
if err != nil || ocmClient == nil {
208219
// TODO: need to expose in status, as likely the credentials are invalid
209220
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: %w", err)
210221
}
@@ -336,7 +347,7 @@ func (r *ROSAControlPlaneReconciler) reconcileDelete(ctx context.Context, rosaSc
336347
}
337348

338349
ocmClient, err := rosa.NewOCMClient(ctx, rosaScope)
339-
if err != nil {
350+
if err != nil || ocmClient == nil {
340351
// TODO: need to expose in status, as likely the credentials are invalid
341352
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: %w", err)
342353
}
@@ -410,7 +421,7 @@ func (r *ROSAControlPlaneReconciler) deleteMachinePools(ctx context.Context, ros
410421
return len(machinePools) == 0, nil
411422
}
412423

413-
func (r *ROSAControlPlaneReconciler) reconcileClusterVersion(rosaScope *scope.ROSAControlPlaneScope, ocmClient *ocm.Client, cluster *cmv1.Cluster) error {
424+
func (r *ROSAControlPlaneReconciler) reconcileClusterVersion(rosaScope *scope.ROSAControlPlaneScope, ocmClient rosa.OCMClient, cluster *cmv1.Cluster) error {
414425
version := rosaScope.ControlPlane.Spec.Version
415426
if version == rosa.RawVersionID(cluster.Version()) {
416427
conditions.MarkFalse(rosaScope.ControlPlane, rosacontrolplanev1.ROSAControlPlaneUpgradingCondition, "upgraded", clusterv1.ConditionSeverityInfo, "")
@@ -428,14 +439,15 @@ func (r *ROSAControlPlaneReconciler) reconcileClusterVersion(rosaScope *scope.RO
428439
return nil
429440
}
430441

431-
scheduledUpgrade, err := rosa.CheckExistingScheduledUpgrade(ocmClient, cluster)
442+
rosaOCMClient := ocmClient.(*ocm.Client)
443+
scheduledUpgrade, err := rosa.CheckExistingScheduledUpgrade(rosaOCMClient, cluster)
432444
if err != nil {
433445
return fmt.Errorf("failed to get existing scheduled upgrades: %w", err)
434446
}
435447

436448
if scheduledUpgrade == nil {
437449
ack := (rosaScope.ControlPlane.Spec.VersionGate == rosacontrolplanev1.Acknowledge || rosaScope.ControlPlane.Spec.VersionGate == rosacontrolplanev1.AlwaysAcknowledge)
438-
scheduledUpgrade, err = rosa.ScheduleControlPlaneUpgrade(ocmClient, cluster, version, time.Now(), ack)
450+
scheduledUpgrade, err = rosa.ScheduleControlPlaneUpgrade(rosaOCMClient, cluster, version, time.Now(), ack)
439451
if err != nil {
440452
condition := &clusterv1.Condition{
441453
Type: rosacontrolplanev1.ROSAControlPlaneUpgradingCondition,
@@ -465,7 +477,7 @@ func (r *ROSAControlPlaneReconciler) reconcileClusterVersion(rosaScope *scope.RO
465477
return nil
466478
}
467479

468-
func (r *ROSAControlPlaneReconciler) updateOCMCluster(rosaScope *scope.ROSAControlPlaneScope, ocmClient *ocm.Client, cluster *cmv1.Cluster, creator *rosaaws.Creator) error {
480+
func (r *ROSAControlPlaneReconciler) updateOCMCluster(rosaScope *scope.ROSAControlPlaneScope, ocmClient rosa.OCMClient, cluster *cmv1.Cluster, creator *rosaaws.Creator) error {
469481
ocmClusterSpec, updated := r.updateOCMClusterSpec(rosaScope.ControlPlane, cluster)
470482

471483
if updated {
@@ -764,7 +776,7 @@ func (r *ROSAControlPlaneReconciler) reconcileExternalAuthBootstrapKubeconfig(ct
764776
return nil
765777
}
766778

767-
func (r *ROSAControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope, ocmClient *ocm.Client, cluster *cmv1.Cluster) error {
779+
func (r *ROSAControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope, ocmClient rosa.OCMClient, cluster *cmv1.Cluster) error {
768780
rosaScope.Debug("Reconciling ROSA kubeconfig for cluster", "cluster-name", rosaScope.RosaClusterName())
769781

770782
clusterRef := client.ObjectKeyFromObject(rosaScope.Cluster)
@@ -785,8 +797,9 @@ func (r *ROSAControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, ro
785797
userName := fmt.Sprintf("%s-capi-admin", clusterName)
786798
apiServerURL := cluster.API().URL()
787799

800+
c := ocmClient.(*ocm.Client)
788801
// create new user with admin privileges in the ROSA cluster if 'userName' doesn't already exist.
789-
err = rosa.CreateAdminUserIfNotExist(ocmClient, cluster.ID(), userName, password)
802+
err = rosa.CreateAdminUserIfNotExist(c, cluster.ID(), userName, password)
790803
if err != nil {
791804
return err
792805
}
@@ -876,7 +889,7 @@ func (r *ROSAControlPlaneReconciler) reconcileClusterAdminPassword(ctx context.C
876889
return password, nil
877890
}
878891

879-
func validateControlPlaneSpec(ocmClient *ocm.Client, rosaScope *scope.ROSAControlPlaneScope) (string, error) {
892+
func validateControlPlaneSpec(ocmClient rosa.OCMClient, rosaScope *scope.ROSAControlPlaneScope) (string, error) {
880893
version := rosaScope.ControlPlane.Spec.Version
881894
channelGroup := rosaScope.ControlPlane.Spec.ChannelGroup
882895
valid, err := ocmClient.ValidateHypershiftVersion(version, channelGroup)

exp/controllers/rosamachinepool_controller.go

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/aws/aws-sdk-go/aws"
99
"github.com/aws/aws-sdk-go/service/ec2"
10+
"github.com/aws/aws-sdk-go/service/sts/stsiface"
1011
"github.com/blang/semver"
1112
"github.com/google/go-cmp/cmp"
1213
"github.com/google/go-cmp/cmp/cmpopts"
@@ -16,6 +17,7 @@ import (
1617
corev1 "k8s.io/api/core/v1"
1718
apierrors "k8s.io/apimachinery/pkg/api/errors"
1819
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
20+
"k8s.io/apimachinery/pkg/runtime"
1921
"k8s.io/apimachinery/pkg/runtime/schema"
2022
"k8s.io/apimachinery/pkg/util/intstr"
2123
"k8s.io/client-go/tools/record"
@@ -31,6 +33,7 @@ import (
3133

3234
rosacontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/rosa/api/v1beta2"
3335
expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
36+
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud"
3437
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
3538
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
3639
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/rosa"
@@ -48,11 +51,15 @@ type ROSAMachinePoolReconciler struct {
4851
Recorder record.EventRecorder
4952
WatchFilterValue string
5053
Endpoints []scope.ServiceEndpoint
54+
NewStsClient func(cloud.ScopeUsage, cloud.Session, logger.Wrapper, runtime.Object) stsiface.STSAPI
55+
NewOCMClient func(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (rosa.OCMClient, error)
5156
}
5257

5358
// SetupWithManager is used to setup the controller.
5459
func (r *ROSAMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
5560
log := logger.FromContext(ctx)
61+
r.NewOCMClient = rosa.NewWrappedOCMClient
62+
r.NewStsClient = scope.NewSTSClient
5663

5764
gvk, err := apiutil.GVKForObject(new(expinfrav1.ROSAMachinePool), mgr.GetScheme())
5865
if err != nil {
@@ -148,6 +155,7 @@ func (r *ROSAMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Requ
148155
ControlPlane: controlPlane,
149156
ControllerName: "rosaControlPlane",
150157
Endpoints: r.Endpoints,
158+
NewStsClient: r.NewStsClient,
151159
})
152160
if err != nil {
153161
return ctrl.Result{}, errors.Wrap(err, "failed to create rosaControlPlane scope")
@@ -185,9 +193,12 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context,
185193
return ctrl.Result{}, err
186194
}
187195
}
196+
if r.NewOCMClient == nil {
197+
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: NewOCMClient is nil")
198+
}
188199

189-
ocmClient, err := rosa.NewOCMClient(ctx, rosaControlPlaneScope)
190-
if err != nil {
200+
ocmClient, err := r.NewOCMClient(ctx, rosaControlPlaneScope)
201+
if err != nil || ocmClient == nil {
191202
// TODO: need to expose in status, as likely the credentials are invalid
192203
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: %w", err)
193204
}
@@ -197,7 +208,6 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context,
197208
return ctrl.Result{}, fmt.Errorf("failed to validate ROSAMachinePool.spec: %w", err)
198209
}
199210
if failureMessage != nil {
200-
machinePoolScope.RosaMachinePool.Status.FailureMessage = failureMessage
201211
// dont' requeue because input is invalid and manual intervention is needed.
202212
return ctrl.Result{}, nil
203213
}
@@ -220,7 +230,6 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context,
220230
if err != nil {
221231
return ctrl.Result{}, err
222232
}
223-
224233
if found {
225234
if rosaMachinePool.Spec.AvailabilityZone == "" {
226235
// reflect the current AvailabilityZone in the spec if not set.
@@ -298,8 +307,8 @@ func (r *ROSAMachinePoolReconciler) reconcileDelete(
298307
) error {
299308
machinePoolScope.Info("Reconciling deletion of RosaMachinePool")
300309

301-
ocmClient, err := rosa.NewOCMClient(ctx, rosaControlPlaneScope)
302-
if err != nil {
310+
ocmClient, err := r.NewOCMClient(ctx, rosaControlPlaneScope)
311+
if err != nil || ocmClient == nil {
303312
// TODO: need to expose in status, as likely the credentials are invalid
304313
return fmt.Errorf("failed to create OCM client: %w", err)
305314
}
@@ -320,7 +329,7 @@ func (r *ROSAMachinePoolReconciler) reconcileDelete(
320329
return nil
321330
}
322331

323-
func (r *ROSAMachinePoolReconciler) reconcileMachinePoolVersion(machinePoolScope *scope.RosaMachinePoolScope, ocmClient *ocm.Client, nodePool *cmv1.NodePool) error {
332+
func (r *ROSAMachinePoolReconciler) reconcileMachinePoolVersion(machinePoolScope *scope.RosaMachinePoolScope, ocmClient rosa.OCMClient, nodePool *cmv1.NodePool) error {
324333
version := machinePoolScope.RosaMachinePool.Spec.Version
325334
if version == "" || version == rosa.RawVersionID(nodePool.Version()) {
326335
machinePoolScope.RosaMachinePool.Status.AvailableUpgrades = nodePool.Version().AvailableUpgrades()
@@ -335,7 +344,8 @@ func (r *ROSAMachinePoolReconciler) reconcileMachinePoolVersion(machinePoolScope
335344
}
336345

337346
if scheduledUpgrade == nil {
338-
scheduledUpgrade, err = rosa.ScheduleNodePoolUpgrade(ocmClient, clusterID, nodePool, version, time.Now())
347+
rosaOCMClient := ocmClient.(*ocm.Client)
348+
scheduledUpgrade, err = rosa.ScheduleNodePoolUpgrade(rosaOCMClient, clusterID, nodePool, version, time.Now())
339349
if err != nil {
340350
return fmt.Errorf("failed to schedule nodePool upgrade to version %s: %w", version, err)
341351
}
@@ -357,7 +367,7 @@ func (r *ROSAMachinePoolReconciler) reconcileMachinePoolVersion(machinePoolScope
357367
return nil
358368
}
359369

360-
func (r *ROSAMachinePoolReconciler) updateNodePool(machinePoolScope *scope.RosaMachinePoolScope, ocmClient *ocm.Client, nodePool *cmv1.NodePool) (*cmv1.NodePool, error) {
370+
func (r *ROSAMachinePoolReconciler) updateNodePool(machinePoolScope *scope.RosaMachinePoolScope, ocmClient rosa.OCMClient, nodePool *cmv1.NodePool) (*cmv1.NodePool, error) {
361371
machinePool := machinePoolScope.RosaMachinePool.DeepCopy()
362372
// default all fields before comparing, so that nil/unset fields don't cause an unnecessary update call.
363373
machinePool.Default()

0 commit comments

Comments
 (0)