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

Jump to bottom

🌱Add initial Rosa machine pool integration tests #5214

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

🌱Add initial Rosa machine pool integration tests #5214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3294a57
Add initial Rosa machine pool integration tests
PanSpagetka Nov 12, 2024
975b9c4
tmp
PanSpagetka Nov 28, 2024
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
22 changes: 15 additions & 7 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.NewOCMClient
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 @@ -203,8 +211,8 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc
}
}

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

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 @@ -332,7 +340,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 @@ -406,7 +414,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 Down Expand Up @@ -461,7 +469,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 @@ -758,7 +766,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 Down Expand Up @@ -870,7 +878,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
valid, err := ocmClient.ValidateHypershiftVersion(version, ocm.DefaultChannelGroup)
if err != nil {
Expand Down
40 changes: 33 additions & 7 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 @@

"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 @@
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 @@

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 @@
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.NewOCMClient
r.NewStsClient = scope.NewSTSClient

gvk, err := apiutil.GVKForObject(new(expinfrav1.ROSAMachinePool), mgr.GetScheme())
if err != nil {
Expand Down Expand Up @@ -148,6 +155,7 @@
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 @@ -186,18 +194,36 @@
}
}

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)
}

failureMessage, err := validateMachinePoolSpec(machinePoolScope)
if err != nil {

Check failure on line 204 in exp/controllers/rosamachinepool_controller.go

View workflow job for this annotation

GitHub Actions / lint 

unnecessary leading newline (whitespace)

return ctrl.Result{}, fmt.Errorf("failed to validate ROSAMachinePool.spec: %w", err)
}
if failureMessage != nil {
machinePoolScope.RosaMachinePool.Status.FailureMessage = failureMessage
// fmt.Println("FAIL", machinePoolScope.RosaMachinePool.Name)

Check failure on line 209 in exp/controllers/rosamachinepool_controller.go

View workflow job for this annotation

GitHub Actions / lint 

commentedOutCode: may want to remove commented-out code (gocritic)

// machinePoolScope.RosaMachinePool.Status.FailureMessage = failureMessage
// conditions.MarkFalse(machinePoolScope.RosaMachinePool,
// expinfrav1.RosaMachinePoolReadyCondition,
// expinfrav1.RosaMachinePoolReconciliationFailedReason,
// clusterv1.ConditionSeverityError,
// "failed to create ROSAMachinePool: %s", *failureMessage)
// machinePoolScope.Close()
// annotations.AddAnnotations(machinePoolScope.RosaMachinePool, map[string]string{
// clusterv1.ReplicasManagedByAnnotation: "rosa",
// })
// if err := machinePoolScope.PatchRosaMachinePoolObject(ctx); err != nil {
// fmt.Println("ERRRRR PatchRosaMachinePoolObject", err.Error())
// return ctrl.Result{}, err
// }
// machinePoolScope.Patch(ctx, machinePoolScope.RosaMachinePool)
// r.Status().Update(ctx, machinePoolScope.RosaMachinePool)
// dont' requeue because input is invalid and manual intervention is needed.
return ctrl.Result{}, nil
}
Expand Down Expand Up @@ -298,8 +324,8 @@
) 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 +346,7 @@
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()) {
conditions.MarkFalse(machinePoolScope.RosaMachinePool, expinfrav1.RosaMachinePoolUpgradingCondition, "upgraded", clusterv1.ConditionSeverityInfo, "")
Expand Down Expand Up @@ -356,7 +382,7 @@
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