Skip to content

Commit 88f43d6

Browse files
KPostOfficeKPostOffice
committed
use context to create new loggers
Signed-off-by: Kevin <[email protected]>
1 parent a7bb744 commit 88f43d6

9 files changed

+232
-190
lines changed

‎controllers/appWrapper_controller_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package controllers
22

33
import (
4+
"context"
45
"testing"
56

67
"github.com/onsi/gomega"
@@ -99,7 +100,7 @@ func (r *AppWrapperReconciler) TestDiscoverInstanceTypes(t *testing.T) {
99100

100101
for _, test := range tests {
101102
t.Run(test.name, func(t *testing.T) {
102-
result := r.discoverInstanceTypes(test.input)
103+
result := r.discoverInstanceTypes(context.TODO(), test.input)
103104
g.Expect(result).To(gomega.Equal(test.expected))
104105
})
105106
}

‎controllers/appwrapper_controller.go

+39-13
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3030
"k8s.io/apimachinery/pkg/runtime"
3131
"k8s.io/client-go/kubernetes"
32-
"k8s.io/klog"
3332

3433
"k8s.io/apimachinery/pkg/labels"
3534
ctrl "sigs.k8s.io/controller-runtime"
@@ -81,7 +80,6 @@ const (
8180
// For more details, check Reconcile and its Result here:
8281
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
8382
func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
84-
8583
_ = log.FromContext(ctx)
8684
// todo: Move the getOCMClusterID call out of reconcile loop.
8785
// Only reason we are calling it here is that the client is not able to make
@@ -122,7 +120,7 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
122120
return ctrl.Result{}, nil
123121
}
124122

125-
demandPerInstanceType := r.discoverInstanceTypes(&appwrapper)
123+
demandPerInstanceType := r.discoverInstanceTypes(ctx, &appwrapper)
126124
if ocmSecretRef := r.Config.OCMSecretRef; ocmSecretRef != nil {
127125
return r.scaleMachinePool(ctx, &appwrapper, demandPerInstanceType)
128126
} else {
@@ -137,6 +135,7 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
137135
}
138136

139137
func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context, appwrapper *arbv1.AppWrapper) error {
138+
logger := ctrl.LoggerFrom(ctx)
140139
if appwrapper.Status.State == arbv1.AppWrapperStateCompleted {
141140
deletionMessage = "completed"
142141
} else {
@@ -147,24 +146,41 @@ func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context,
147146
case "reuse":
148147
matchedAw := r.findExactMatch(ctx, appwrapper)
149148
if matchedAw != nil {
150-
klog.Infof("Appwrapper %s %s, swapping machines to %s", appwrapper.Name, deletionMessage, matchedAw.Name)
149+
logger.Info(
150+
"AppWrapper deleted transferring machines",
151+
"oldAppWrapper", appwrapper,
152+
"deletionMessage", deletionMessage,
153+
"newAppWrapper", matchedAw,
154+
)
151155
if err := r.swapNodeLabels(ctx, appwrapper, matchedAw); err != nil {
152156
return err
153157
}
154158
} else {
155-
klog.Infof("Appwrapper %s %s, scaling down machines", appwrapper.Name, deletionMessage)
159+
logger.Info(
160+
"Scaling down machines associated with deleted AppWrapper",
161+
"appWrapper", appwrapper,
162+
"deletionMessage", deletionMessage,
163+
)
156164
if err := r.annotateToDeleteMachine(ctx, appwrapper); err != nil {
157165
return err
158166
}
159167
}
160168
case "duplicate":
161-
klog.Infof("Appwrapper %s scale-down machineset: %s ", deletionMessage, appwrapper.Name)
169+
logger.Info(
170+
"AppWrapper deleted, scaling down machineset",
171+
"appWrapper", appwrapper,
172+
"deletionMessage", deletionMessage,
173+
)
162174
if err := r.deleteMachineSet(ctx, appwrapper); err != nil {
163175
return err
164176
}
165177
}
166178
} else {
167-
klog.Infof("Appwrapper %s scale-down machine pool: %s ", deletionMessage, appwrapper.Name)
179+
logger.Info(
180+
"AppWrapper deleted, scaling down machine pool",
181+
"appWrapper", appwrapper,
182+
"deletionMessage", deletionMessage,
183+
)
168184
if _, err := r.deleteMachinePool(ctx, appwrapper); err != nil {
169185
return err
170186
}
@@ -175,6 +191,7 @@ func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context,
175191
// SetupWithManager sets up the controller with the Manager.
176192
func (r *AppWrapperReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
177193

194+
logger := ctrl.LoggerFrom(ctx)
178195
restConfig := mgr.GetConfig()
179196

180197
var err error
@@ -197,20 +214,21 @@ func (r *AppWrapperReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Ma
197214
if ok, err := r.machinePoolExists(); err != nil {
198215
return err
199216
} else if ok {
200-
klog.Info("Using machine pools for cluster auto-scaling")
217+
logger.Info("Using machine pools for cluster auto-scaling")
201218
}
202219
}
203220

204221
return ctrl.NewControllerManagedBy(mgr).
205-
For(&arbv1.AppWrapper{}).
222+
For(&arbv1.AppWrapper{}).Named("instascale").
206223
Complete(r)
207224
}
208225

209226
func (r *AppWrapperReconciler) getOCMSecret(ctx context.Context, secretRef *corev1.SecretReference) (*corev1.Secret, error) {
210227
return r.kubeClient.CoreV1().Secrets(secretRef.Namespace).Get(ctx, secretRef.Name, metav1.GetOptions{})
211228
}
212229

213-
func (r *AppWrapperReconciler) discoverInstanceTypes(aw *arbv1.AppWrapper) map[string]int {
230+
func (r *AppWrapperReconciler) discoverInstanceTypes(ctx context.Context, aw *arbv1.AppWrapper) map[string]int {
231+
logger := ctrl.LoggerFrom(ctx)
214232
demandMapPerInstanceType := make(map[string]int)
215233
var instanceRequired []string
216234
for k, v := range aw.Labels {
@@ -220,7 +238,10 @@ func (r *AppWrapperReconciler) discoverInstanceTypes(aw *arbv1.AppWrapper) map[s
220238
}
221239

222240
if len(instanceRequired) < 1 {
223-
klog.Infof("Found AW %s that cannot be scaled due to missing orderedinstance label", aw.ObjectMeta.Name)
241+
logger.Info(
242+
"AppWrapper cannot be scaled out due to missing orderedinstance label",
243+
"appWrapper", aw,
244+
)
224245
return demandMapPerInstanceType
225246
}
226247

@@ -237,6 +258,7 @@ func (r *AppWrapperReconciler) discoverInstanceTypes(aw *arbv1.AppWrapper) map[s
237258
}
238259

239260
func (r *AppWrapperReconciler) findExactMatch(ctx context.Context, aw *arbv1.AppWrapper) *arbv1.AppWrapper {
261+
logger := ctrl.LoggerFrom(ctx)
240262
var match *arbv1.AppWrapper = nil
241263
appwrappers := arbv1.AppWrapperList{}
242264

@@ -250,7 +272,7 @@ func (r *AppWrapperReconciler) findExactMatch(ctx context.Context, aw *arbv1.App
250272

251273
err := r.List(ctx, &appwrappers, listOptions)
252274
if err != nil {
253-
klog.Error("Cannot list queued appwrappers, associated machines will be deleted")
275+
logger.Error(err, "Cannot list queued appwrappers, associated machines will be deleted")
254276
return match
255277
}
256278
var existingAcquiredMachineTypes = ""
@@ -265,7 +287,11 @@ func (r *AppWrapperReconciler) findExactMatch(ctx context.Context, aw *arbv1.App
265287
if eachAw.Status.State != arbv1.AppWrapperStateEnqueued {
266288
if eachAw.Labels["orderedinstance"] == existingAcquiredMachineTypes {
267289
match = &eachAw
268-
klog.Infof("Found exact match, %v appwrapper has acquired machinetypes %v", eachAw.Name, existingAcquiredMachineTypes)
290+
logger.Info(
291+
"AppWrapper has successfully acquired requested machine types",
292+
"appWrapper", eachAw,
293+
"acquiredMachineTypes", existingAcquiredMachineTypes,
294+
)
269295
break
270296
}
271297
}

‎controllers/machinepools.go

+38-14
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package controllers
33
import (
44
"context"
55
"fmt"
6-
"os"
76
"strings"
87

98
ocmsdk "github.com/openshift-online/ocm-sdk-go"
@@ -12,7 +11,6 @@ import (
1211
arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
1312

1413
"k8s.io/apimachinery/pkg/types"
15-
"k8s.io/klog"
1614
ctrl "sigs.k8s.io/controller-runtime"
1715
)
1816

@@ -44,9 +42,10 @@ func hasAwLabel(machinePool *cmv1.MachinePool, aw *arbv1.AppWrapper) bool {
4442
}
4543

4644
func (r *AppWrapperReconciler) scaleMachinePool(ctx context.Context, aw *arbv1.AppWrapper, demandPerInstanceType map[string]int) (ctrl.Result, error) {
45+
logger := ctrl.LoggerFrom(ctx)
4746
connection, err := r.createOCMConnection()
4847
if err != nil {
49-
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
48+
logger.Error(err, "Error creating OCM connection")
5049
return ctrl.Result{}, err
5150
}
5251
defer connection.Close()
@@ -72,28 +71,40 @@ func (r *AppWrapperReconciler) scaleMachinePool(ctx context.Context, aw *arbv1.A
7271
if numberOfMachines != replicas {
7372
m := make(map[string]string)
7473
m[aw.Name] = aw.Name
75-
klog.Infof("The instanceRequired array: %v", userRequestedInstanceType)
7674

7775
machinePoolID := strings.ReplaceAll(aw.Name+"-"+userRequestedInstanceType, ".", "-")
7876
createMachinePool, err := cmv1.NewMachinePool().ID(machinePoolID).InstanceType(userRequestedInstanceType).Replicas(replicas).Labels(m).Build()
7977
if err != nil {
80-
klog.Errorf(`Error building MachinePool: %v`, err)
78+
logger.Error(
79+
err, "Error building MachinePool",
80+
"userRequestedInstanceType", userRequestedInstanceType,
81+
)
8182
}
82-
klog.Infof("Built MachinePool with instance type %v and name %v", userRequestedInstanceType, createMachinePool.ID())
83+
logger.Info(
84+
"Sending MachinePool creation request",
85+
"instanceType", userRequestedInstanceType,
86+
"machinePoolName", createMachinePool.ID(),
87+
)
8388
response, err := clusterMachinePools.Add().Body(createMachinePool).SendContext(ctx)
8489
if err != nil {
85-
klog.Errorf(`Error creating MachinePool: %v`, err)
90+
logger.Error(err, "Error creating MachinePool")
91+
} else {
92+
logger.Info(
93+
"Successfully created MachinePool",
94+
"machinePoolName", createMachinePool.ID(),
95+
"response", response,
96+
)
8697
}
87-
klog.Infof("Created MachinePool: %v", response)
8898
}
8999
}
90100
return ctrl.Result{Requeue: false}, nil
91101
}
92102

93103
func (r *AppWrapperReconciler) deleteMachinePool(ctx context.Context, aw *arbv1.AppWrapper) (ctrl.Result, error) {
104+
logger := ctrl.LoggerFrom(ctx)
94105
connection, err := r.createOCMConnection()
95106
if err != nil {
96-
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
107+
logger.Error(err, "Error creating OCM connection")
97108
return ctrl.Result{}, err
98109
}
99110
defer connection.Close()
@@ -107,9 +118,16 @@ func (r *AppWrapperReconciler) deleteMachinePool(ctx context.Context, aw *arbv1.
107118
if strings.Contains(id, aw.Name) {
108119
targetMachinePool, err := connection.ClustersMgmt().V1().Clusters().Cluster(r.ocmClusterID).MachinePools().MachinePool(id).Delete().SendContext(ctx)
109120
if err != nil {
110-
klog.Infof("Error deleting target machinepool %v", targetMachinePool)
121+
logger.Error(
122+
err, "Error deleting machinepool",
123+
"machinePool", targetMachinePool,
124+
)
125+
} else {
126+
logger.Info(
127+
"Successfully scaled down target machinepool",
128+
"machinePool", targetMachinePool,
129+
)
111130
}
112-
klog.Infof("Successfully Scaled down target machinepool %v", id)
113131
}
114132
return true
115133
})
@@ -129,6 +147,7 @@ func (r *AppWrapperReconciler) machinePoolExists() (bool, error) {
129147

130148
// getOCMClusterID determines the internal clusterID to be used for OCM API calls
131149
func (r *AppWrapperReconciler) getOCMClusterID(ctx context.Context) error {
150+
logger := ctrl.LoggerFrom(ctx)
132151
cv := &configv1.ClusterVersion{}
133152
err := r.Get(ctx, types.NamespacedName{Name: "version"}, cv)
134153
if err != nil {
@@ -139,7 +158,7 @@ func (r *AppWrapperReconciler) getOCMClusterID(ctx context.Context) error {
139158

140159
connection, err := r.createOCMConnection()
141160
if err != nil {
142-
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
161+
logger.Error(err, "Error creating OCM connection")
143162
}
144163
defer connection.Close()
145164

@@ -148,12 +167,17 @@ func (r *AppWrapperReconciler) getOCMClusterID(ctx context.Context) error {
148167

149168
response, err := collection.List().Search(fmt.Sprintf("external_id = '%s'", internalClusterID)).Size(1).Page(1).SendContext(ctx)
150169
if err != nil {
151-
klog.Errorf(`Error getting cluster id: %v`, err)
170+
logger.Error(err, "Error getting cluster id")
152171
}
153172

154173
response.Items().Each(func(cluster *cmv1.Cluster) bool {
155174
r.ocmClusterID = cluster.ID()
156-
fmt.Printf("%s - %s - %s\n", cluster.ID(), cluster.Name(), cluster.State())
175+
logger.Info(
176+
"Cluster Info",
177+
"clusterId", cluster.ID(),
178+
"clusterName", cluster.Name(),
179+
"clusterState", cluster.State(),
180+
)
157181
return true
158182
})
159183
return nil

0 commit comments

Comments
 (0)