Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱Add initial Rosa machine pool integration tests #5214

Merged
merged 1 commit into from
Jan 31, 2025
Merged

🌱Add initial Rosa machine pool integration tests #5214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions controlplane/rosa/controllers/rosacontrolplane_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (

stsv2 "github.com/aws/aws-sdk-go-v2/service/sts"
sts "github.com/aws/aws-sdk-go/service/sts"
"github.com/aws/aws-sdk-go/service/sts/stsiface"
"github.com/google/go-cmp/cmp"
idputils "github.com/openshift-online/ocm-common/pkg/idp/utils"
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
Expand All @@ -40,6 +41,7 @@ import (
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
kerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apiserver/pkg/storage/names"
Expand All @@ -58,6 +60,7 @@ import (
rosacontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/rosa/api/v1beta2"
expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/annotations"
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud"
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/rosa"
Expand Down Expand Up @@ -89,11 +92,15 @@ type ROSAControlPlaneReconciler struct {
WatchFilterValue string
WaitInfraPeriod time.Duration
Endpoints []scope.ServiceEndpoint
NewStsClient func(cloud.ScopeUsage, cloud.Session, logger.Wrapper, runtime.Object) stsiface.STSAPI
NewOCMClient func(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (rosa.OCMClient, error)
}

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

rosaControlPlane := &rosacontrolplanev1.ROSAControlPlane{}
c, err := ctrl.NewControllerManagedBy(mgr).
Expand Down Expand Up @@ -173,6 +180,7 @@ func (r *ROSAControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.Req
ControllerName: strings.ToLower(rosaControlPlaneKind),
Endpoints: r.Endpoints,
Logger: log,
NewStsClient: r.NewStsClient,
})
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to create scope: %w", err)
Expand Down Expand Up @@ -202,9 +210,12 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc
return ctrl.Result{}, err
}
}
if r.NewOCMClient == nil {
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: NewOCMClient is nil")
}

ocmClient, err := rosa.NewOCMClient(ctx, rosaScope)
if err != nil {
ocmClient, err := r.NewOCMClient(ctx, rosaScope)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check for nil, r.NewOCMClient could be nil

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you forget this, if r.NewOCMClient != nil

if err != nil || ocmClient == nil {
// TODO: need to expose in status, as likely the credentials are invalid
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: %w", err)
}
Expand Down Expand Up @@ -336,7 +347,7 @@ func (r *ROSAControlPlaneReconciler) reconcileDelete(ctx context.Context, rosaSc
}

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

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

scheduledUpgrade, err := rosa.CheckExistingScheduledUpgrade(ocmClient, cluster)
rosaOCMClient := ocmClient.(*ocm.Client)
scheduledUpgrade, err := rosa.CheckExistingScheduledUpgrade(rosaOCMClient, cluster)
if err != nil {
return fmt.Errorf("failed to get existing scheduled upgrades: %w", err)
}

if scheduledUpgrade == nil {
ack := (rosaScope.ControlPlane.Spec.VersionGate == rosacontrolplanev1.Acknowledge || rosaScope.ControlPlane.Spec.VersionGate == rosacontrolplanev1.AlwaysAcknowledge)
scheduledUpgrade, err = rosa.ScheduleControlPlaneUpgrade(ocmClient, cluster, version, time.Now(), ack)
scheduledUpgrade, err = rosa.ScheduleControlPlaneUpgrade(rosaOCMClient, cluster, version, time.Now(), ack)
if err != nil {
condition := &clusterv1.Condition{
Type: rosacontrolplanev1.ROSAControlPlaneUpgradingCondition,
Expand Down Expand Up @@ -465,7 +477,7 @@ func (r *ROSAControlPlaneReconciler) reconcileClusterVersion(rosaScope *scope.RO
return nil
}

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

if updated {
Expand Down Expand Up @@ -764,7 +776,7 @@ func (r *ROSAControlPlaneReconciler) reconcileExternalAuthBootstrapKubeconfig(ct
return nil
}

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

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

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

func validateControlPlaneSpec(ocmClient *ocm.Client, rosaScope *scope.ROSAControlPlaneScope) (string, error) {
func validateControlPlaneSpec(ocmClient rosa.OCMClient, rosaScope *scope.ROSAControlPlaneScope) (string, error) {
version := rosaScope.ControlPlane.Spec.Version
channelGroup := rosaScope.ControlPlane.Spec.ChannelGroup
valid, err := ocmClient.ValidateHypershiftVersion(version, channelGroup)
Expand Down
28 changes: 19 additions & 9 deletions exp/controllers/rosamachinepool_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/sts/stsiface"
"github.com/blang/semver"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
Expand All @@ -16,6 +17,7 @@ import (
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/client-go/tools/record"
Expand All @@ -31,6 +33,7 @@ import (

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

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

gvk, err := apiutil.GVKForObject(new(expinfrav1.ROSAMachinePool), mgr.GetScheme())
if err != nil {
Expand Down Expand Up @@ -148,6 +155,7 @@ func (r *ROSAMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Requ
ControlPlane: controlPlane,
ControllerName: "rosaControlPlane",
Endpoints: r.Endpoints,
NewStsClient: r.NewStsClient,
})
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "failed to create rosaControlPlane scope")
Expand Down Expand Up @@ -185,9 +193,12 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context,
return ctrl.Result{}, err
}
}
if r.NewOCMClient == nil {
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: NewOCMClient is nil")
}

ocmClient, err := rosa.NewOCMClient(ctx, rosaControlPlaneScope)
if err != nil {
ocmClient, err := r.NewOCMClient(ctx, rosaControlPlaneScope)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same, check for nil

if err != nil || ocmClient == nil {
// TODO: need to expose in status, as likely the credentials are invalid
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: %w", err)
}
Expand All @@ -197,7 +208,6 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context,
return ctrl.Result{}, fmt.Errorf("failed to validate ROSAMachinePool.spec: %w", err)
}
if failureMessage != nil {
machinePoolScope.RosaMachinePool.Status.FailureMessage = failureMessage
// dont' requeue because input is invalid and manual intervention is needed.
return ctrl.Result{}, nil
}
Expand All @@ -220,7 +230,6 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context,
if err != nil {
return ctrl.Result{}, err
}

if found {
if rosaMachinePool.Spec.AvailabilityZone == "" {
// reflect the current AvailabilityZone in the spec if not set.
Expand Down Expand Up @@ -298,8 +307,8 @@ func (r *ROSAMachinePoolReconciler) reconcileDelete(
) error {
machinePoolScope.Info("Reconciling deletion of RosaMachinePool")

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

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

if scheduledUpgrade == nil {
scheduledUpgrade, err = rosa.ScheduleNodePoolUpgrade(ocmClient, clusterID, nodePool, version, time.Now())
rosaOCMClient := ocmClient.(*ocm.Client)
scheduledUpgrade, err = rosa.ScheduleNodePoolUpgrade(rosaOCMClient, clusterID, nodePool, version, time.Now())
if err != nil {
return fmt.Errorf("failed to schedule nodePool upgrade to version %s: %w", version, err)
}
Expand All @@ -357,7 +367,7 @@ func (r *ROSAMachinePoolReconciler) reconcileMachinePoolVersion(machinePoolScope
return nil
}

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