Skip to content

Commit 854ca8d

Browse files
Bobbins228Bobbins228
committed
Review changes, improved annotateToDeleteMachine function
1 parent 3948c52 commit 854ca8d

File tree

5 files changed

+279
-144
lines changed

5 files changed

+279
-144
lines changed

‎controllers/appwrapper_controller.go

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,16 @@ import (
4141
// AppWrapperReconciler reconciles a AppWrapper object
4242
type AppWrapperReconciler struct {
4343
client.Client
44-
Scheme *runtime.Scheme
45-
Config config.InstaScaleConfiguration
46-
kubeClient *kubernetes.Clientset
44+
Scheme *runtime.Scheme
45+
Config config.InstaScaleConfiguration
46+
kubeClient *kubernetes.Clientset
47+
ocmClusterID string
48+
ocmToken string
49+
useMachineSets bool
4750
}
4851

4952
var (
5053
deletionMessage string
51-
ocmClusterID string
52-
ocmToken string
5354
maxScaleNodesAllowed int
5455
)
5556

@@ -85,9 +86,9 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
8586
// todo: Move the getOCMClusterID call out of reconcile loop.
8687
// Only reason we are calling it here is that the client is not able to make
8788
// calls until it is started, so SetupWithManager is not working.
88-
if ocmSecretRef := r.Config.OCMSecretRef; ocmSecretRef != nil && ocmClusterID == "" {
89+
if !r.useMachineSets && r.ocmClusterID == "" {
8990
if err := r.getOCMClusterID(ctx); err != nil {
90-
return ctrl.Result{}, err
91+
return ctrl.Result{Requeue: true, RequeueAfter: timeFiveSeconds}, err
9192
}
9293
}
9394
var appwrapper arbv1.AppWrapper
@@ -105,7 +106,7 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
105106
if !controllerutil.ContainsFinalizer(&appwrapper, finalizerName) {
106107
controllerutil.AddFinalizer(&appwrapper, finalizerName)
107108
if err := r.Update(ctx, &appwrapper); err != nil {
108-
return ctrl.Result{}, err
109+
return ctrl.Result{RequeueAfter: timeFiveSeconds}, nil
109110
}
110111
return ctrl.Result{}, nil
111112
}
@@ -122,31 +123,28 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
122123
}
123124

124125
demandPerInstanceType := r.discoverInstanceTypes(&appwrapper)
125-
//for userRequestedInstanceType := range demandPerInstanceType {
126126
if ocmSecretRef := r.Config.OCMSecretRef; ocmSecretRef != nil {
127127
return r.scaleMachinePool(ctx, &appwrapper, demandPerInstanceType)
128128
} else {
129-
if r.Config.Reuse {
129+
switch strings.ToLower(r.Config.MachineSetsStrategy) {
130+
case "reuse":
130131
return r.reconcileReuseMachineSet(ctx, &appwrapper, demandPerInstanceType)
131-
} else {
132+
case "duplicate":
132133
return r.reconcileCreateMachineSet(ctx, &appwrapper, demandPerInstanceType)
133134
}
134135
}
136+
return ctrl.Result{}, nil
135137
}
136138

137139
func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context, appwrapper *arbv1.AppWrapper) error {
138-
if appwrapper.Status.State == "Completed" {
140+
if appwrapper.Status.State == arbv1.AppWrapperStateCompleted {
139141
deletionMessage = "completed"
140142
} else {
141143
deletionMessage = "deleted"
142144
}
143-
if ocmSecretRef := r.Config.OCMSecretRef; ocmSecretRef != nil {
144-
klog.Infof("Appwrapper %s scale-down machine pool: %s ", deletionMessage, appwrapper.Name)
145-
if _, err := r.deleteMachinePool(ctx, appwrapper); err != nil {
146-
return err
147-
}
148-
} else {
149-
if r.Config.Reuse {
145+
if r.useMachineSets {
146+
switch strings.ToLower(r.Config.MachineSetsStrategy) {
147+
case "reuse":
150148
matchedAw := r.findExactMatch(ctx, appwrapper)
151149
if matchedAw != nil {
152150
klog.Infof("Appwrapper %s %s, swapping machines to %s", appwrapper.Name, deletionMessage, matchedAw.Name)
@@ -159,18 +157,23 @@ func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context,
159157
return err
160158
}
161159
}
162-
} else {
160+
case "duplicate":
163161
klog.Infof("Appwrapper %s scale-down machineset: %s ", deletionMessage, appwrapper.Name)
164162
if err := r.deleteMachineSet(ctx, appwrapper); err != nil {
165163
return err
166164
}
167165
}
166+
} else {
167+
klog.Infof("Appwrapper %s scale-down machine pool: %s ", deletionMessage, appwrapper.Name)
168+
if _, err := r.deleteMachinePool(ctx, appwrapper); err != nil {
169+
return err
170+
}
168171
}
169172
return nil
170173
}
171174

172175
// SetupWithManager sets up the controller with the Manager.
173-
func (r *AppWrapperReconciler) SetupWithManager(mgr ctrl.Manager) error {
176+
func (r *AppWrapperReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
174177

175178
restConfig := mgr.GetConfig()
176179

@@ -181,16 +184,17 @@ func (r *AppWrapperReconciler) SetupWithManager(mgr ctrl.Manager) error {
181184
}
182185

183186
maxScaleNodesAllowed = int(r.Config.MaxScaleoutAllowed)
184-
187+
r.useMachineSets = true
185188
if ocmSecretRef := r.Config.OCMSecretRef; ocmSecretRef != nil {
186-
if ocmSecret, err := r.getOCMSecret(context.Background(), ocmSecretRef); err != nil {
189+
r.useMachineSets = false
190+
if ocmSecret, err := r.getOCMSecret(ctx, ocmSecretRef); err != nil {
187191
return fmt.Errorf("error reading OCM Secret from ref %q: %w", ocmSecretRef, err)
188192
} else if token := ocmSecret.Data["token"]; len(token) > 0 {
189-
ocmToken = string(token)
193+
r.ocmToken = string(token)
190194
} else {
191195
return fmt.Errorf("token is missing from OCM Secret %q", ocmSecretRef)
192196
}
193-
if ok, err := machinePoolExists(); err != nil {
197+
if ok, err := r.machinePoolExists(); err != nil {
194198
return err
195199
} else if ok {
196200
klog.Info("Using machine pools for cluster auto-scaling")

‎controllers/machinepools.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
ctrl "sigs.k8s.io/controller-runtime"
1717
)
1818

19-
func createOCMConnection() (*ocmsdk.Connection, error) {
19+
func (r *AppWrapperReconciler) createOCMConnection() (*ocmsdk.Connection, error) {
2020
logger, err := ocmsdk.NewGoLoggerBuilder().
2121
Debug(false).
2222
Build()
@@ -26,7 +26,7 @@ func createOCMConnection() (*ocmsdk.Connection, error) {
2626

2727
connection, err := ocmsdk.NewConnectionBuilder().
2828
Logger(logger).
29-
Tokens(ocmToken).
29+
Tokens(r.ocmToken).
3030
Build()
3131
if err != nil {
3232
return nil, fmt.Errorf("can't build connection: %v", err)
@@ -46,14 +46,14 @@ func hasAwLabel(machinePool *cmv1.MachinePool, aw *arbv1.AppWrapper) bool {
4646
func (r *AppWrapperReconciler) scaleMachinePool(ctx context.Context, aw *arbv1.AppWrapper, demandPerInstanceType map[string]int) (ctrl.Result, error) {
4747
for userRequestedInstanceType := range demandPerInstanceType {
4848
replicas := demandPerInstanceType[userRequestedInstanceType]
49-
connection, err := createOCMConnection()
49+
connection, err := r.createOCMConnection()
5050
if err != nil {
5151
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
5252
return ctrl.Result{}, err
5353
}
5454
defer connection.Close()
5555

56-
clusterMachinePools := connection.ClustersMgmt().V1().Clusters().Cluster(ocmClusterID).MachinePools()
56+
clusterMachinePools := connection.ClustersMgmt().V1().Clusters().Cluster(r.ocmClusterID).MachinePools()
5757

5858
response, err := clusterMachinePools.List().SendContext(ctx)
5959
if err != nil {
@@ -91,21 +91,21 @@ func (r *AppWrapperReconciler) scaleMachinePool(ctx context.Context, aw *arbv1.A
9191
}
9292

9393
func (r *AppWrapperReconciler) deleteMachinePool(ctx context.Context, aw *arbv1.AppWrapper) (ctrl.Result, error) {
94-
connection, err := createOCMConnection()
94+
connection, err := r.createOCMConnection()
9595
if err != nil {
9696
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
9797
return ctrl.Result{}, err
9898
}
9999
defer connection.Close()
100100

101-
machinePoolsConnection := connection.ClustersMgmt().V1().Clusters().Cluster(ocmClusterID).MachinePools().List()
101+
machinePoolsConnection := connection.ClustersMgmt().V1().Clusters().Cluster(r.ocmClusterID).MachinePools().List()
102102

103103
machinePoolsListResponse, _ := machinePoolsConnection.Send()
104104
machinePoolsList := machinePoolsListResponse.Items()
105105
machinePoolsList.Range(func(index int, item *cmv1.MachinePool) bool {
106106
id, _ := item.GetID()
107107
if strings.Contains(id, aw.Name) {
108-
targetMachinePool, err := connection.ClustersMgmt().V1().Clusters().Cluster(ocmClusterID).MachinePools().MachinePool(id).Delete().SendContext(ctx)
108+
targetMachinePool, err := connection.ClustersMgmt().V1().Clusters().Cluster(r.ocmClusterID).MachinePools().MachinePool(id).Delete().SendContext(ctx)
109109
if err != nil {
110110
klog.Infof("Error deleting target machinepool %v", targetMachinePool)
111111
}
@@ -116,14 +116,14 @@ func (r *AppWrapperReconciler) deleteMachinePool(ctx context.Context, aw *arbv1.
116116
return ctrl.Result{Requeue: false}, nil
117117
}
118118

119-
func machinePoolExists() (bool, error) {
120-
connection, err := createOCMConnection()
119+
func (r *AppWrapperReconciler) machinePoolExists() (bool, error) {
120+
connection, err := r.createOCMConnection()
121121
if err != nil {
122122
return false, fmt.Errorf("error creating OCM connection: %w", err)
123123
}
124124
defer connection.Close()
125125

126-
machinePools := connection.ClustersMgmt().V1().Clusters().Cluster(ocmClusterID).MachinePools()
126+
machinePools := connection.ClustersMgmt().V1().Clusters().Cluster(r.ocmClusterID).MachinePools()
127127
return machinePools != nil, nil
128128
}
129129

@@ -137,7 +137,7 @@ func (r *AppWrapperReconciler) getOCMClusterID(ctx context.Context) error {
137137

138138
internalClusterID := string(cv.Spec.ClusterID)
139139

140-
connection, err := createOCMConnection()
140+
connection, err := r.createOCMConnection()
141141
if err != nil {
142142
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
143143
}
@@ -152,7 +152,7 @@ func (r *AppWrapperReconciler) getOCMClusterID(ctx context.Context) error {
152152
}
153153

154154
response.Items().Each(func(cluster *cmv1.Cluster) bool {
155-
ocmClusterID = cluster.ID()
155+
r.ocmClusterID = cluster.ID()
156156
fmt.Printf("%s - %s - %s\n", cluster.ID(), cluster.Name(), cluster.State())
157157
return true
158158
})

0 commit comments

Comments
 (0)