Skip to content

Commit a7a0aca

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

9 files changed

+220
-184
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

+38-12
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+
"old_appwrapper", appwrapper,
152+
"deletion_message", deletionMessage,
153+
"new_appwrapper", 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+
"deletion_message", 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+
"deletion_message", 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+
"deletion_message", 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,7 +214,7 @@ 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

@@ -210,7 +227,8 @@ func (r *AppWrapperReconciler) getOCMSecret(ctx context.Context, secretRef *core
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+
"acquired_machine_types", existingAcquiredMachineTypes,
294+
)
269295
break
270296
}
271297
}

‎controllers/machinepools.go

+27-9
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
1313

1414
"k8s.io/apimachinery/pkg/types"
15-
"k8s.io/klog"
1615
ctrl "sigs.k8s.io/controller-runtime"
1716
)
1817

@@ -44,6 +43,7 @@ func hasAwLabel(machinePool *cmv1.MachinePool, aw *arbv1.AppWrapper) bool {
4443
}
4544

4645
func (r *AppWrapperReconciler) scaleMachinePool(ctx context.Context, aw *arbv1.AppWrapper, demandPerInstanceType map[string]int) (ctrl.Result, error) {
46+
logger := ctrl.LoggerFrom(ctx)
4747
connection, err := r.createOCMConnection()
4848
if err != nil {
4949
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
@@ -72,25 +72,36 @@ func (r *AppWrapperReconciler) scaleMachinePool(ctx context.Context, aw *arbv1.A
7272
if numberOfMachines != replicas {
7373
m := make(map[string]string)
7474
m[aw.Name] = aw.Name
75-
klog.Infof("The instanceRequired array: %v", userRequestedInstanceType)
7675

7776
machinePoolID := strings.ReplaceAll(aw.Name+"-"+userRequestedInstanceType, ".", "-")
7877
createMachinePool, err := cmv1.NewMachinePool().ID(machinePoolID).InstanceType(userRequestedInstanceType).Replicas(replicas).Labels(m).Build()
7978
if err != nil {
80-
klog.Errorf(`Error building MachinePool: %v`, err)
79+
logger.Error(
80+
err, "Error building MachinePool",
81+
"user_requested_instance_type", userRequestedInstanceType,
82+
)
8183
}
82-
klog.Infof("Built MachinePool with instance type %v and name %v", userRequestedInstanceType, createMachinePool.ID())
84+
logger.Info(
85+
"Sending MachinePool creation request",
86+
"instance_type", userRequestedInstanceType,
87+
"machinepool_name", createMachinePool.ID(),
88+
)
8389
response, err := clusterMachinePools.Add().Body(createMachinePool).SendContext(ctx)
8490
if err != nil {
85-
klog.Errorf(`Error creating MachinePool: %v`, err)
91+
logger.Error(err, "Error creating MachinePool")
8692
}
87-
klog.Infof("Created MachinePool: %v", response)
93+
logger.Info(
94+
"Successfully created MachinePool",
95+
"machinepool_name", createMachinePool.ID(),
96+
"response", response,
97+
)
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 {
96107
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
@@ -107,9 +118,15 @@ 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+
)
111125
}
112-
klog.Infof("Successfully Scaled down target machinepool %v", id)
126+
logger.Info(
127+
"Successfully scaled down target machinepool",
128+
"machinepool", targetMachinePool,
129+
)
113130
}
114131
return true
115132
})
@@ -129,6 +146,7 @@ func (r *AppWrapperReconciler) machinePoolExists() (bool, error) {
129146

130147
// getOCMClusterID determines the internal clusterID to be used for OCM API calls
131148
func (r *AppWrapperReconciler) getOCMClusterID(ctx context.Context) error {
149+
logger := ctrl.LoggerFrom(ctx)
132150
cv := &configv1.ClusterVersion{}
133151
err := r.Get(ctx, types.NamespacedName{Name: "version"}, cv)
134152
if err != nil {
@@ -148,7 +166,7 @@ func (r *AppWrapperReconciler) getOCMClusterID(ctx context.Context) error {
148166

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

154172
response.Items().Each(func(cluster *cmv1.Cluster) bool {

0 commit comments

Comments
 (0)