Skip to content

Commit e027cf5

Browse files
PanSpagetkaPanSpagetka
committed
Assert ocm.Client for exported functions. So we dont create public API changes
1 parent adac640 commit e027cf5

File tree

6 files changed

+47
-21
lines changed

6 files changed

+47
-21
lines changed

‎controlplane/rosa/controllers/rosacontrolplane_controller.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ type ROSAControlPlaneReconciler struct {
9999
// SetupWithManager is used to setup the controller.
100100
func (r *ROSAControlPlaneReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
101101
log := logger.FromContext(ctx)
102-
r.NewOCMClient = rosa.NewOCMClient
102+
r.NewOCMClient = rosa.NewOCMClient2
103103
r.NewStsClient = scope.NewSTSClient
104104

105105
rosaControlPlane := &rosacontrolplanev1.ROSAControlPlane{}
@@ -210,6 +210,9 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc
210210
return ctrl.Result{}, err
211211
}
212212
}
213+
if r.NewOCMClient == nil {
214+
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: NewOCMClient is nil")
215+
}
213216

214217
ocmClient, err := r.NewOCMClient(ctx, rosaScope)
215218
if err != nil || ocmClient == nil {
@@ -432,14 +435,15 @@ func (r *ROSAControlPlaneReconciler) reconcileClusterVersion(rosaScope *scope.RO
432435
return nil
433436
}
434437

435-
scheduledUpgrade, err := rosa.CheckExistingScheduledUpgrade(ocmClient, cluster)
438+
rosaOCMClient := ocmClient.(*ocm.Client)
439+
scheduledUpgrade, err := rosa.CheckExistingScheduledUpgrade(rosaOCMClient, cluster)
436440
if err != nil {
437441
return fmt.Errorf("failed to get existing scheduled upgrades: %w", err)
438442
}
439443

440444
if scheduledUpgrade == nil {
441445
ack := (rosaScope.ControlPlane.Spec.VersionGate == rosacontrolplanev1.Acknowledge || rosaScope.ControlPlane.Spec.VersionGate == rosacontrolplanev1.AlwaysAcknowledge)
442-
scheduledUpgrade, err = rosa.ScheduleControlPlaneUpgrade(ocmClient, cluster, version, time.Now(), ack)
446+
scheduledUpgrade, err = rosa.ScheduleControlPlaneUpgrade(rosaOCMClient, cluster, version, time.Now(), ack)
443447
if err != nil {
444448
condition := &clusterv1.Condition{
445449
Type: rosacontrolplanev1.ROSAControlPlaneUpgradingCondition,
@@ -787,8 +791,9 @@ func (r *ROSAControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, ro
787791
userName := fmt.Sprintf("%s-capi-admin", clusterName)
788792
apiServerURL := cluster.API().URL()
789793

794+
c := ocmClient.(*ocm.Client)
790795
// create new user with admin privileges in the ROSA cluster if 'userName' doesn't already exist.
791-
err = rosa.CreateAdminUserIfNotExist(ocmClient, cluster.ID(), userName, password)
796+
err = rosa.CreateAdminUserIfNotExist(c, cluster.ID(), userName, password)
792797
if err != nil {
793798
return err
794799
}

‎exp/controllers/rosamachinepool_controller.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ type ROSAMachinePoolReconciler struct {
5858
// SetupWithManager is used to setup the controller.
5959
func (r *ROSAMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
6060
log := logger.FromContext(ctx)
61-
r.NewOCMClient = rosa.NewOCMClient
61+
r.NewOCMClient = rosa.NewOCMClient2
6262
r.NewStsClient = scope.NewSTSClient
6363

6464
gvk, err := apiutil.GVKForObject(new(expinfrav1.ROSAMachinePool), mgr.GetScheme())
@@ -193,6 +193,9 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context,
193193
return ctrl.Result{}, err
194194
}
195195
}
196+
if r.NewOCMClient == nil {
197+
return ctrl.Result{}, fmt.Errorf("failed to create OCM client: NewOCMClient is nil")
198+
}
196199

197200
ocmClient, err := r.NewOCMClient(ctx, rosaControlPlaneScope)
198201
if err != nil || ocmClient == nil {
@@ -340,7 +343,8 @@ func (r *ROSAMachinePoolReconciler) reconcileMachinePoolVersion(machinePoolScope
340343
}
341344

342345
if scheduledUpgrade == nil {
343-
scheduledUpgrade, err = rosa.ScheduleNodePoolUpgrade(ocmClient, clusterID, nodePool, version, time.Now())
346+
rosaOCMClient := ocmClient.(*ocm.Client)
347+
scheduledUpgrade, err = rosa.ScheduleNodePoolUpgrade(rosaOCMClient, clusterID, nodePool, version, time.Now())
344348
if err != nil {
345349
return fmt.Errorf("failed to schedule nodePool upgrade to version %s: %w", version, err)
346350
}

‎exp/controllers/rosamachinepool_controller_test.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -422,11 +422,12 @@ func TestRosaMachinePoolReconcile(t *testing.T) {
422422
mockCtrl := gomock.NewController(t)
423423
recorder := record.NewFakeRecorder(10)
424424
ctx := context.TODO()
425+
controlPlaneName := "rosa-control-plane-9"
425426
mp := &expinfrav1.ROSAMachinePool{
426427
ObjectMeta: metav1.ObjectMeta{
427-
Name: "rosa-machinepool-9",
428+
Name: controlPlaneName,
428429
Namespace: ns.Name,
429-
UID: "rosa-machinepool-9",
430+
UID: types.UID(controlPlaneName),
430431
Finalizers: []string{expinfrav1.RosaMachinePoolFinalizer},
431432
},
432433
TypeMeta: metav1.TypeMeta{
@@ -452,19 +453,21 @@ func TestRosaMachinePoolReconcile(t *testing.T) {
452453

453454
cpPh, err := patch.NewHelper(cp, testEnv)
454455
cp.Status.Ready = true
455-
cp.Status.ID = "rosa-control-plane-9"
456+
cp.Status.ID = controlPlaneName
456457
g.Expect(cpPh.Patch(ctx, cp)).To(Succeed())
457458
g.Expect(err).ShouldNot(HaveOccurred())
458459

459460
ocmMock := mocks.NewMockOCMClient(mockCtrl)
461+
nodePoolName := "node-pool-1"
460462
expect := func(m *mocks.MockOCMClientMockRecorder) {
461463
m.GetNodePool(gomock.Any(), gomock.Any()).DoAndReturn(func(clusterId string, nodePoolID string) (*cmv1.NodePool, bool, error) {
462464
nodePoolBuilder := nodePoolBuilder(mp.Spec, omp.Spec)
463-
nodePool, err := nodePoolBuilder.ID("node-pool-1").Build()
465+
nodePool, err := nodePoolBuilder.ID(nodePoolName).Build()
464466
g.Expect(err).NotTo(HaveOccurred())
465467
return nodePool, true, nil
466468
}).Times(1)
467-
m.DeleteNodePool("rosa-control-plane-9", "node-pool-1").DoAndReturn(func(clusterId string, nodePoolID string) error {
469+
m.DeleteNodePool(controlPlaneName, nodePoolName).DoAndReturn(func(clusterId string, nodePoolID string) error {
470+
testEnv.Delete(ctx, mp)
468471
return nil
469472
}).Times(1)
470473
}
@@ -512,11 +515,11 @@ func TestRosaMachinePoolReconcile(t *testing.T) {
512515

513516
machinePoolScope.Close()
514517
time.Sleep(50 * time.Millisecond)
515-
m := &expinfrav1.ROSAMachinePool{}
518+
rosaMachinePool := &expinfrav1.ROSAMachinePool{}
516519
key := client.ObjectKey{Name: mp.Name, Namespace: ns.Name}
517-
err4 := testEnv.Get(ctx, key, m)
518-
g.Expect(err4).ToNot(HaveOccurred())
519-
g.Expect(m.Finalizers).To(BeNil())
520+
err4 := testEnv.Get(ctx, key, rosaMachinePool)
521+
g.Expect(err4).To(HaveOccurred())
522+
g.Expect(rosaMachinePool.Finalizers).To(BeNil())
520523

521524
for _, obj := range objects {
522525
cleanupObject(g, obj)

‎pkg/rosa/client.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,19 @@ const (
2121
)
2222

2323
// NewOCMClient creates a new OCM client.
24-
func NewOCMClient(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (OCMClient, error) {
24+
func NewOCMClient(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (*ocm.Client, error) {
25+
token, url, err := ocmCredentials(ctx, rosaScope)
26+
if err != nil {
27+
return nil, err
28+
}
29+
return ocm.NewClient().Logger(logrus.New()).Config(&ocmcfg.Config{
30+
AccessToken: token,
31+
URL: url,
32+
}).Build()
33+
}
34+
35+
// NewOCMClient2 creates a new OCM client.
36+
func NewOCMClient2(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (OCMClient, error) {
2537
token, url, err := ocmCredentials(ctx, rosaScope)
2638
if err != nil {
2739
return nil, err
@@ -36,6 +48,7 @@ func NewOCMClient(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (
3648
}
3749
return &c, err
3850
}
51+
3952
func newOCMRawConnection(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (*sdk.Connection, error) {
4053
logger, err := sdk.NewGoLoggerBuilder().
4154
Debug(false).

‎pkg/rosa/idps.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55

66
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
7+
"github.com/openshift/rosa/pkg/ocm"
78
)
89

910
const (
@@ -13,7 +14,7 @@ const (
1314

1415
// CreateAdminUserIfNotExist creates a new admin user withe username/password in the cluster if username doesn't already exist.
1516
// the user is granted admin privileges by being added to a special IDP called `cluster-admin` which will be created if it doesn't already exist.
16-
func CreateAdminUserIfNotExist(client OCMClient, clusterID, username, password string) error {
17+
func CreateAdminUserIfNotExist(client *ocm.Client, clusterID, username, password string) error {
1718
existingClusterAdminIDP, userList, err := findExistingClusterAdminIDP(client, clusterID)
1819
if err != nil {
1920
return fmt.Errorf("failed to find existing cluster admin IDP: %w", err)
@@ -74,7 +75,7 @@ func CreateAdminUserIfNotExist(client OCMClient, clusterID, username, password s
7475
}
7576

7677
// CreateUserIfNotExist creates a new user with `username` and adds it to the group if it doesn't already exist.
77-
func CreateUserIfNotExist(client OCMClient, clusterID string, group, username string) (*cmv1.User, error) {
78+
func CreateUserIfNotExist(client *ocm.Client, clusterID string, group, username string) (*cmv1.User, error) {
7879
user, err := client.GetUser(clusterID, group, username)
7980
if user != nil || err != nil {
8081
return user, err

‎pkg/rosa/versions.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
var MinSupportedVersion = semver.MustParse("4.14.0")
1414

1515
// CheckExistingScheduledUpgrade checks and returns the current upgrade schedule if any.
16-
func CheckExistingScheduledUpgrade(client OCMClient, cluster *cmv1.Cluster) (*cmv1.ControlPlaneUpgradePolicy, error) {
16+
func CheckExistingScheduledUpgrade(client *ocm.Client, cluster *cmv1.Cluster) (*cmv1.ControlPlaneUpgradePolicy, error) {
1717
upgradePolicies, err := client.GetControlPlaneUpgradePolicies(cluster.ID())
1818
if err != nil {
1919
return nil, err
@@ -27,7 +27,7 @@ func CheckExistingScheduledUpgrade(client OCMClient, cluster *cmv1.Cluster) (*cm
2727
}
2828

2929
// ScheduleControlPlaneUpgrade schedules a new control plane upgrade to the specified version at the specified time.
30-
func ScheduleControlPlaneUpgrade(client OCMClient, cluster *cmv1.Cluster, version string, nextRun time.Time, ack bool) (*cmv1.ControlPlaneUpgradePolicy, error) {
30+
func ScheduleControlPlaneUpgrade(client *ocm.Client, cluster *cmv1.Cluster, version string, nextRun time.Time, ack bool) (*cmv1.ControlPlaneUpgradePolicy, error) {
3131
// earliestNextRun is set to at least 5 min from now by the OCM API.
3232
// Set our next run request to something slightly longer than 5min to make sure we account for the latency between when we send this
3333
// request and when the server processes it.
@@ -71,7 +71,7 @@ func ScheduleControlPlaneUpgrade(client OCMClient, cluster *cmv1.Cluster, versio
7171
}
7272

7373
// ScheduleNodePoolUpgrade schedules a new nodePool upgrade to the specified version at the specified time.
74-
func ScheduleNodePoolUpgrade(client OCMClient, clusterID string, nodePool *cmv1.NodePool, version string, nextRun time.Time) (*cmv1.NodePoolUpgradePolicy, error) {
74+
func ScheduleNodePoolUpgrade(client *ocm.Client, clusterID string, nodePool *cmv1.NodePool, version string, nextRun time.Time) (*cmv1.NodePoolUpgradePolicy, error) {
7575
// earliestNextRun is set to at least 5 min from now by the OCM API.
7676
// Set our next run request to something slightly longer than 5min to make sure we account for the latency between when we send this
7777
// request and when the server processes it.

0 commit comments

Comments
 (0)