Skip to content

Commit d2efee1

Browse files
VanillaSpoonVanillaSpoon
committed
update: logging to match current method
1 parent dc84b9e commit d2efee1

File tree

6 files changed

+90
-104
lines changed

6 files changed

+90
-104
lines changed

‎controllers/appWrapper_controller_test.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
func (r *AppWrapperReconciler) TestDiscoverInstanceTypes(t *testing.T) {
1313
g := gomega.NewGomegaWithT(t)
14-
ctx := context.Background()
1514
tests := []struct {
1615
name string
1716
input *arbv1.AppWrapper

‎controllers/appwrapper_controller.go‎

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package controllers
1818

1919
import (
2020
"context"
21-
"fmt"
2221
ocmsdk "github.com/openshift-online/ocm-sdk-go"
2322
"github.com/project-codeflare/instascale/pkg/config"
2423
arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
@@ -152,14 +151,16 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
152151
}
153152

154153
func (r *AppWrapperReconciler) setMachineType(ctx context.Context) error {
154+
logger := ctrl.LoggerFrom(ctx)
155155
if r.MachineType != MachineTypeMachineSet && r.ocmClusterID == "" {
156-
if err := r.getOCMClusterID(); err != nil {
156+
if err := r.getOCMClusterID(ctx); err != nil {
157157
return err
158158
}
159159
}
160160
hypershiftEnabled, err := r.checkHypershiftEnabled(ctx)
161161
if err != nil {
162-
return fmt.Errorf("error checking if hypershift is enabled: %w", err)
162+
logger.Error(err, "error checking if hypershift is enabled")
163+
return err
163164
}
164165
if hypershiftEnabled {
165166
r.MachineType = MachineTypeNodePool
@@ -189,7 +190,9 @@ func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context,
189190
"newAppWrapper", matchedAw,
190191
)
191192
if err := r.swapNodeLabels(ctx, appwrapper, matchedAw); err != nil {
192-
klog.Errorf("Error swapping node labels for AppWrapper %s: %v", appwrapper.Name, err)
193+
logger.Error(err, "Error swapping node labels for AppWrapper",
194+
"appwrapper", appwrapper,
195+
)
193196
return err
194197
}
195198
} else {
@@ -199,7 +202,9 @@ func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context,
199202
"deletionMessage", deletionMessage,
200203
)
201204
if err := r.annotateToDeleteMachine(ctx, appwrapper); err != nil {
202-
klog.Errorf("Error annotating to delete machine for AppWrapper %s: %v", appwrapper.Name, err)
205+
logger.Error(err, "Error annotating to delete machine for AppWrapper",
206+
"appwrapper", appwrapper,
207+
)
203208
return err
204209
}
205210
}
@@ -210,29 +215,33 @@ func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context,
210215
"deletionMessage", deletionMessage,
211216
)
212217
if err := r.deleteMachineSet(ctx, appwrapper); err != nil {
213-
klog.Errorf("Error deleting MachineSet for AppWrapper %s: %v", appwrapper.Name, err)
218+
logger.Error(err, "Error deleting MachineSet for AppWrapper",
219+
"appwrapper", appwrapper)
214220
return err
215221
}
216222
}
217-
} else {
223+
224+
case MachineTypeNodePool:
218225
logger.Info(
219-
"AppWrapper deleted, scaling down machine pool",
226+
"AppWrapper deleted, scaling down nodepool",
220227
"appWrapper", appwrapper,
221228
"deletionMessage", deletionMessage,
222229
)
223-
}
224-
225-
case MachineTypeNodePool:
226-
klog.Infof("Appwrapper %s scale-down node pool: %s ", deletionMessage, appwrapper.Name)
227230
if _, err := r.deleteNodePool(ctx, appwrapper); err != nil {
228-
klog.Errorf("Error deleting NodePool for AppWrapper %s: %v", appwrapper.Name, err)
231+
logger.Error(err, "Error deleting NodePool for AppWrapper",
232+
"appwrapper", appwrapper)
229233
return err
230234
}
231235

232236
case MachineTypeMachinePool:
233-
klog.Infof("Appwrapper %s scale-down machine pool: %s ", deletionMessage, appwrapper.Name)
237+
logger.Info(
238+
"AppWrapper deleted, scaling down machine pool",
239+
"appWrapper", appwrapper,
240+
"deletionMessage", deletionMessage,
241+
)
234242
if _, err := r.deleteMachinePool(ctx, appwrapper); err != nil {
235-
klog.Errorf("Error deleting MachinePool for AppWrapper %s: %v", appwrapper.Name, err)
243+
logger.Error(err, "Error deleting MachinePool for AppWrapper",
244+
"appwrapper", appwrapper)
236245
return err
237246
}
238247
}
@@ -255,17 +264,16 @@ func (r *AppWrapperReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Ma
255264
r.MachineType = MachineTypeMachineSet // default to MachineSet
256265
if ocmSecretRef := r.Config.OCMSecretRef; ocmSecretRef != nil {
257266
if ocmSecret, err := r.getOCMSecret(ctx, ocmSecretRef); err != nil {
258-
return fmt.Errorf("error reading OCM Secret from ref %q: %w", ocmSecretRef, err)
267+
logger.Error(err, "error reading OCM Secret from ref",
268+
"ref", ocmSecretRef)
269+
return err
259270
} else if token := ocmSecret.Data["token"]; len(token) > 0 {
260271
r.ocmToken = string(token)
261272
r.MachineType = MachineTypeMachinePool
262273
} else {
263-
return fmt.Errorf("token is missing from OCM Secret %q", ocmSecretRef)
264-
}
265-
if ok, err := r.machinePoolExists(); err != nil {
274+
logger.Error(err, "token is missing from OCM Secret %q",
275+
"ref", ocmSecretRef)
266276
return err
267-
} else if ok {
268-
logger.Info("Using machine pools for cluster auto-scaling")
269277
}
270278
}
271279

@@ -288,10 +296,12 @@ func (r *AppWrapperReconciler) discoverInstanceTypes(ctx context.Context, aw *ar
288296
"appWrapper", aw,
289297
)
290298
if _, exists := aw.Annotations["loggedNoInstances"]; !exists {
291-
klog.Infof("Found AW %s that cannot be scaled due to missing orderedinstance label", aw.Name)
299+
logger.Info("Found AW that cannot be scaled due to missing orderedinstance label",
300+
"appwrapper", aw)
292301
r.setAnnotation(aw, "loggedNoInstances", "true")
293302
if err := r.Update(ctx, aw); err != nil {
294-
klog.Errorf("Error adding annotation to AW %s: %v", aw.Name, err)
303+
logger.Error(err, "Error adding annotation to AW",
304+
"appwrapper", aw)
295305
}
296306
}
297307
return demandMapPerInstanceType

‎controllers/machinepools.go‎

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,11 @@ package controllers
22

33
import (
44
"context"
5-
"fmt"
65
"strings"
76

87
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
98
arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
109

11-
"k8s.io/apimachinery/pkg/types"
1210
ctrl "sigs.k8s.io/controller-runtime"
1311
)
1412

@@ -104,52 +102,3 @@ func (r *AppWrapperReconciler) deleteMachinePool(ctx context.Context, aw *arbv1.
104102
})
105103
return ctrl.Result{Requeue: false}, nil
106104
}
107-
108-
func (r *AppWrapperReconciler) machinePoolExists() (bool, error) {
109-
connection, err := r.createOCMConnection()
110-
if err != nil {
111-
return false, fmt.Errorf("error creating OCM connection: %w", err)
112-
}
113-
defer connection.Close()
114-
115-
machinePools := connection.ClustersMgmt().V1().Clusters().Cluster(r.ocmClusterID).MachinePools()
116-
return machinePools != nil, nil
117-
}
118-
119-
// getOCMClusterID determines the internal clusterID to be used for OCM API calls
120-
func (r *AppWrapperReconciler) getOCMClusterID(ctx context.Context) error {
121-
logger := ctrl.LoggerFrom(ctx)
122-
cv := &configv1.ClusterVersion{}
123-
err := r.Get(ctx, types.NamespacedName{Name: "version"}, cv)
124-
if err != nil {
125-
return fmt.Errorf("can't get clusterversion: %v", err)
126-
}
127-
128-
internalClusterID := string(cv.Spec.ClusterID)
129-
130-
connection, err := r.createOCMConnection()
131-
if err != nil {
132-
logger.Error(err, "Error creating OCM connection")
133-
}
134-
defer connection.Close()
135-
136-
// Get the client for the resource that manages the collection of clusters:
137-
collection := connection.ClustersMgmt().V1().Clusters()
138-
139-
response, err := collection.List().Search(fmt.Sprintf("external_id = '%s'", internalClusterID)).Size(1).Page(1).SendContext(ctx)
140-
if err != nil {
141-
logger.Error(err, "Error getting cluster id")
142-
}
143-
144-
response.Items().Each(func(cluster *cmv1.Cluster) bool {
145-
r.ocmClusterID = cluster.ID()
146-
logger.Info(
147-
"Cluster Info",
148-
"clusterId", cluster.ID(),
149-
"clusterName", cluster.Name(),
150-
"clusterState", cluster.State(),
151-
)
152-
return true
153-
})
154-
return nil
155-
}

‎controllers/nodepools.go‎

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,19 @@ package controllers
22

33
import (
44
"context"
5-
"fmt"
6-
"os"
75
"strings"
86

97
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
108
arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
119

12-
"k8s.io/klog"
1310
ctrl "sigs.k8s.io/controller-runtime"
1411
)
1512

1613
func (r *AppWrapperReconciler) scaleNodePool(ctx context.Context, aw *arbv1.AppWrapper, demandPerInstanceType map[string]int) (ctrl.Result, error) {
14+
logger := ctrl.LoggerFrom(ctx)
1715
connection, err := r.createOCMConnection()
1816
if err != nil {
19-
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
17+
logger.Error(err, "Error creating OCM connection")
2018
return ctrl.Result{}, err
2119
}
2220
defer connection.Close()
@@ -32,7 +30,6 @@ func (r *AppWrapperReconciler) scaleNodePool(ctx context.Context, aw *arbv1.AppW
3230

3331
numberOfMachines := 0
3432
response.Items().Each(func(nodePool *cmv1.NodePool) bool {
35-
3633
if nodePool.AWSNodePool().InstanceType() == userRequestedInstanceType && hasAwLabel(nodePool.Labels(), aw) {
3734
numberOfMachines = nodePool.Replicas()
3835
return false
@@ -43,29 +40,43 @@ func (r *AppWrapperReconciler) scaleNodePool(ctx context.Context, aw *arbv1.AppW
4340
if numberOfMachines != replicas {
4441
m := make(map[string]string)
4542
m[aw.Name] = aw.Name
46-
klog.Infof("The instanceRequired array: %v", userRequestedInstanceType)
43+
logger.Info("The instanceRequired array",
44+
"InstanceRequired", userRequestedInstanceType)
4745

4846
nodePoolID := strings.ReplaceAll(aw.Name+"-"+userRequestedInstanceType, ".", "-")
4947

5048
createNodePool, err := cmv1.NewNodePool().AWSNodePool(cmv1.NewAWSNodePool().InstanceType(userRequestedInstanceType)).ID(nodePoolID).Replicas(replicas).Labels(m).Build()
5149
if err != nil {
52-
klog.Errorf(`Error building NodePool: %v`, err)
50+
logger.Error(
51+
err, "Error building NodePool",
52+
"userRequestedInstanceType", userRequestedInstanceType,
53+
)
5354
}
54-
klog.Infof("Built NodePool with instance type %v and name %v", userRequestedInstanceType, createNodePool.ID())
55+
logger.Info(
56+
"Sending NodePool creation request",
57+
"instanceType", userRequestedInstanceType,
58+
"nodePoolName", createNodePool.ID(),
59+
)
5560
response, err := clusterNodePools.Add().Body(createNodePool).SendContext(ctx)
5661
if err != nil {
57-
klog.Errorf(`Error creating NodePool: %v`, err)
62+
logger.Error(err, "Error creating NodePool")
63+
} else {
64+
logger.Info(
65+
"Successfully created NodePool",
66+
"nodePoolName", createNodePool.ID(),
67+
"response", response,
68+
)
5869
}
59-
klog.Infof("Created NodePool: %v", response)
6070
}
6171
}
6272
return ctrl.Result{Requeue: false}, nil
6373
}
6474

6575
func (r *AppWrapperReconciler) deleteNodePool(ctx context.Context, aw *arbv1.AppWrapper) (ctrl.Result, error) {
76+
logger := ctrl.LoggerFrom(ctx)
6677
connection, err := r.createOCMConnection()
6778
if err != nil {
68-
fmt.Fprintf(os.Stderr, "Error creating OCM connection: %v", err)
79+
logger.Error(err, "Error creating OCM connection")
6980
return ctrl.Result{}, err
7081
}
7182
defer connection.Close()
@@ -79,39 +90,50 @@ func (r *AppWrapperReconciler) deleteNodePool(ctx context.Context, aw *arbv1.App
7990
if strings.Contains(id, aw.Name) {
8091
targetNodePool, err := connection.ClustersMgmt().V1().Clusters().Cluster(r.ocmClusterID).NodePools().NodePool(id).Delete().SendContext(ctx)
8192
if err != nil {
82-
klog.Infof("Error deleting target nodepool %v", targetNodePool)
93+
logger.Error(
94+
err, "Error deleting nodepool",
95+
"nodePool", targetNodePool,
96+
)
97+
} else {
98+
logger.Info(
99+
"Successfully scaled down target nodepool",
100+
"nodePool", targetNodePool,
101+
)
83102
}
84-
klog.Infof("Successfully Scaled down target nodepool %v", id)
85103
}
86104
return true
87105
})
88106
return ctrl.Result{Requeue: false}, nil
89107
}
90108

91109
func (r *AppWrapperReconciler) checkHypershiftEnabled(ctx context.Context) (bool, error) {
110+
logger := ctrl.LoggerFrom(ctx)
92111
connection, err := r.createOCMConnection()
93112
if err != nil {
94-
return false, fmt.Errorf("error creating OCM connection: %w", err)
113+
logger.Error(err, "Error creating OCM connection")
114+
return false, err
95115
}
96116
defer connection.Close()
97117

98118
clusterResource := connection.ClustersMgmt().V1().Clusters().Cluster(r.ocmClusterID)
99119

100120
response, err := clusterResource.Get().SendContext(ctx)
101121
if err != nil {
102-
return false, fmt.Errorf("error fetching cluster details: %w", err)
122+
logger.Error(err, "error fetching cluster details")
123+
return false, err
103124
}
104125

105126
body := response.Body()
106127
if body == nil {
107-
return false, fmt.Errorf("empty response body")
128+
logger.Error(err, "Empty resource body when checking Hypershift enabled status")
129+
return false, err
108130
}
109131

110132
hypershiftEnabled := false
111133
if body.Hypershift() != nil {
112134
hypershiftEnabled = body.Hypershift().Enabled()
113135
}
114136

115-
fmt.Printf("Hypershift enabled status: %v\n", hypershiftEnabled)
137+
logger.Info("Checked Hypershift enabled status", "status", hypershiftEnabled)
116138
return hypershiftEnabled, nil
117139
}

0 commit comments

Comments
 (0)