Skip to content

Commit 9b2908d

Browse files
mohamed-essammohamed-essam
committed
Unify controller logging
1 parent 0e068ed commit 9b2908d

File tree

5 files changed

+145
-148
lines changed

5 files changed

+145
-148
lines changed

‎internal/controller/nbpolicy_controller.go‎

Lines changed: 35 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import (
1111
"k8s.io/apimachinery/pkg/types"
1212
ctrl "sigs.k8s.io/controller-runtime"
1313
"sigs.k8s.io/controller-runtime/pkg/client"
14-
"sigs.k8s.io/controller-runtime/pkg/log"
1514

15+
"github.com/go-logr/logr"
1616
netbirdiov1 "github.com/netbirdio/kubernetes-operator/api/v1"
1717
"github.com/netbirdio/kubernetes-operator/internal/util"
1818
netbird "github.com/netbirdio/netbird/management/client/rest"
@@ -35,15 +35,15 @@ var (
3535
errNetBirdAPI = fmt.Errorf("netbird API error")
3636
)
3737

38-
func (r *NBPolicyReconciler) getResources(ctx context.Context, nbPolicy *netbirdiov1.NBPolicy) ([]netbirdiov1.NBResource, error) {
38+
func (r *NBPolicyReconciler) getResources(ctx context.Context, nbPolicy *netbirdiov1.NBPolicy, logger logr.Logger) ([]netbirdiov1.NBResource, error) {
3939
var resourceList []netbirdiov1.NBResource
4040
var updatedManagedServiceList []string
4141
for _, rss := range nbPolicy.Status.ManagedServiceList {
4242
var resource netbirdiov1.NBResource
4343
namespacedName := types.NamespacedName{Namespace: strings.Split(rss, "/")[0], Name: strings.Split(rss, "/")[1]}
4444
err := r.Client.Get(ctx, namespacedName, &resource)
4545
if err != nil && !errors.IsNotFound(err) {
46-
ctrl.Log.Error(errKubernetesAPI, "Error getting NBResource", "namespace", namespacedName.Namespace, "name", namespacedName.Name)
46+
logger.Error(errKubernetesAPI, "Error getting NBResource", "namespace", namespacedName.Namespace, "name", namespacedName.Name)
4747
nbPolicy.Status.Conditions = netbirdiov1.NBConditionFalse("internalError", fmt.Sprintf("Error getting NBResource: %v", err))
4848
return nil, err
4949
}
@@ -58,12 +58,12 @@ func (r *NBPolicyReconciler) getResources(ctx context.Context, nbPolicy *netbird
5858
return resourceList, nil
5959
}
6060

61-
func (r *NBPolicyReconciler) mapResources(ctx context.Context, req ctrl.Request, nbPolicy *netbirdiov1.NBPolicy, resources []netbirdiov1.NBResource) (map[string][]int32, []string, error) {
61+
func (r *NBPolicyReconciler) mapResources(ctx context.Context, nbPolicy *netbirdiov1.NBPolicy, resources []netbirdiov1.NBResource, logger logr.Logger) (map[string][]int32, []string, error) {
6262
portMapping := map[string]map[int32]interface{}{
6363
"tcp": make(map[int32]interface{}),
6464
"udp": make(map[int32]interface{}),
6565
}
66-
groups, err := r.groupNamesToIDs(ctx, req, nbPolicy.Spec.DestinationGroups)
66+
groups, err := r.groupNamesToIDs(ctx, nbPolicy.Spec.DestinationGroups, logger)
6767
if err != nil {
6868
return nil, nil, err
6969
}
@@ -92,9 +92,9 @@ func (r *NBPolicyReconciler) mapResources(ctx context.Context, req ctrl.Request,
9292
return ports, groups, nil
9393
}
9494

95-
func (r *NBPolicyReconciler) createPolicy(ctx context.Context, req ctrl.Request, nbPolicy *netbirdiov1.NBPolicy, protocol string, sourceGroupIDs, destinationGroupIDs, ports []string) (*string, error) {
95+
func (r *NBPolicyReconciler) createPolicy(ctx context.Context, nbPolicy *netbirdiov1.NBPolicy, protocol string, sourceGroupIDs, destinationGroupIDs, ports []string, logger logr.Logger) (*string, error) {
9696
policyName := fmt.Sprintf("%s %s", nbPolicy.Spec.Name, strings.ToUpper(protocol))
97-
ctrl.Log.Info("Creating NetBird Policy", "name", policyName, "description", nbPolicy.Spec.Description, "protocol", protocol, "sources", sourceGroupIDs, "destinations", destinationGroupIDs, "ports", ports, "bidirectional", nbPolicy.Spec.Bidirectional)
97+
logger.Info("Creating NetBird Policy", "name", policyName, "description", nbPolicy.Spec.Description, "protocol", protocol, "sources", sourceGroupIDs, "destinations", destinationGroupIDs, "ports", ports, "bidirectional", nbPolicy.Spec.Bidirectional)
9898
policy, err := r.netbird.Policies.Create(ctx, api.PostApiPoliciesJSONRequestBody{
9999
Enabled: true,
100100
Name: policyName,
@@ -115,17 +115,17 @@ func (r *NBPolicyReconciler) createPolicy(ctx context.Context, req ctrl.Request,
115115
})
116116

117117
if err != nil {
118-
ctrl.Log.Error(errNetBirdAPI, "Error creating Policy", "namespace", req.Namespace, "name", req.Name, "err", err)
118+
logger.Error(errNetBirdAPI, "Error creating Policy", "err", err)
119119
nbPolicy.Status.Conditions = netbirdiov1.NBConditionFalse("APIError", fmt.Sprintf("Error creating policy: %v", err))
120120
return nil, err
121121
}
122122

123123
return policy.Id, nil
124124
}
125125

126-
func (r *NBPolicyReconciler) updatePolicy(ctx context.Context, req ctrl.Request, policyID *string, nbPolicy *netbirdiov1.NBPolicy, protocol string, sourceGroupIDs, destinationGroupIDs, ports []string) (*string, bool, error) {
126+
func (r *NBPolicyReconciler) updatePolicy(ctx context.Context, policyID *string, nbPolicy *netbirdiov1.NBPolicy, protocol string, sourceGroupIDs, destinationGroupIDs, ports []string, logger logr.Logger) (*string, bool, error) {
127127
policyName := fmt.Sprintf("%s %s", nbPolicy.Spec.Name, strings.ToUpper(protocol))
128-
ctrl.Log.Info("Updating NetBird Policy", "name", policyName, "description", nbPolicy.Spec.Description, "protocol", protocol, "sources", sourceGroupIDs, "destinations", destinationGroupIDs, "ports", ports, "bidirectional", nbPolicy.Spec.Bidirectional)
128+
logger.Info("Updating NetBird Policy", "name", policyName, "description", nbPolicy.Spec.Description, "protocol", protocol, "sources", sourceGroupIDs, "destinations", destinationGroupIDs, "ports", ports, "bidirectional", nbPolicy.Spec.Bidirectional)
129129
policy, err := r.netbird.Policies.Update(ctx, *policyID, api.PutApiPoliciesPolicyIdJSONRequestBody{
130130
Enabled: true,
131131
Name: policyName,
@@ -146,15 +146,15 @@ func (r *NBPolicyReconciler) updatePolicy(ctx context.Context, req ctrl.Request,
146146
})
147147

148148
if err != nil && !strings.Contains(err.Error(), "not found") {
149-
ctrl.Log.Error(errNetBirdAPI, "Error updating Policy", "namespace", req.Namespace, "name", req.Name, "err", err)
149+
logger.Error(errNetBirdAPI, "Error updating Policy", "err", err)
150150
nbPolicy.Status.Conditions = netbirdiov1.NBConditionFalse("APIError", fmt.Sprintf("Error updating policy: %v", err))
151151
return policyID, false, err
152152
}
153153

154154
requeue := false
155155

156156
if err != nil && strings.Contains(err.Error(), "not found") {
157-
ctrl.Log.Info("Policy deleted from NetBird API, recreating", "namespace", req.Namespace, "name", req.Name, "protocol", protocol)
157+
logger.Info("Policy deleted from NetBird API, recreating", "protocol", protocol)
158158
policyID = nil
159159
requeue = true
160160
nbPolicy.Status.Conditions = netbirdiov1.NBConditionFalse("Gone", "Policy deleted from NetBird API")
@@ -169,9 +169,8 @@ func (r *NBPolicyReconciler) updatePolicy(ctx context.Context, req ctrl.Request,
169169
// Reconcile is part of the main kubernetes reconciliation loop which aims to
170170
// move the current state of the cluster closer to the desired state.
171171
func (r *NBPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, err error) {
172-
_ = log.FromContext(ctx)
173-
174-
ctrl.Log.Info("NBPolicy: Reconciling", "namespace", req.Namespace, "name", req.Name)
172+
logger := ctrl.Log.WithName("NBPolicy").WithValues("namespace", req.Namespace, "name", req.Name)
173+
logger.Info("Reconciling NBPolicy")
175174

176175
var nbPolicy netbirdiov1.NBPolicy
177176
err = r.Client.Get(ctx, req.NamespacedName, &nbPolicy)
@@ -180,7 +179,7 @@ func (r *NBPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r
180179
err = nil
181180
}
182181
if err != nil {
183-
ctrl.Log.Error(errKubernetesAPI, "error getting NBPolicy", "err", err, "namespace", req.Namespace, "name", req.Name)
182+
logger.Error(errKubernetesAPI, "error getting NBPolicy", "err", err)
184183
}
185184
return ctrl.Result{RequeueAfter: defaultRequeueAfter}, err
186185
}
@@ -203,26 +202,26 @@ func (r *NBPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r
203202
if len(nbPolicy.Finalizers) == 0 {
204203
return ctrl.Result{}, nil
205204
}
206-
return ctrl.Result{}, r.handleDelete(ctx, req, nbPolicy)
205+
return ctrl.Result{}, r.handleDelete(ctx, nbPolicy, logger)
207206
}
208207

209-
resourceList, err := r.getResources(ctx, &nbPolicy)
208+
resourceList, err := r.getResources(ctx, &nbPolicy, logger)
210209
if err != nil {
211210
return ctrl.Result{}, err
212211
}
213212

214-
portMapping, destGroups, err := r.mapResources(ctx, req, &nbPolicy, resourceList)
213+
portMapping, destGroups, err := r.mapResources(ctx, &nbPolicy, resourceList, logger)
215214
if err != nil {
216215
return ctrl.Result{}, err
217216
}
218217

219-
sourceGroupIDs, err := r.groupNamesToIDs(ctx, req, nbPolicy.Spec.SourceGroups)
218+
sourceGroupIDs, err := r.groupNamesToIDs(ctx, nbPolicy.Spec.SourceGroups, logger)
220219
if err != nil {
221220
nbPolicy.Status.Conditions = netbirdiov1.NBConditionFalse("APIError", fmt.Sprintf("Error getting group IDs: %v", err))
222221
return ctrl.Result{}, err
223222
}
224223

225-
requeue, err := r.handlePolicies(ctx, req, &nbPolicy, sourceGroupIDs, destGroups, portMapping)
224+
requeue, err := r.handlePolicies(ctx, &nbPolicy, sourceGroupIDs, destGroups, portMapping, logger)
226225

227226
if requeue || err != nil {
228227
return ctrl.Result{Requeue: requeue}, err
@@ -233,7 +232,7 @@ func (r *NBPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r
233232
return ctrl.Result{}, nil
234233
}
235234

236-
func (r *NBPolicyReconciler) handlePolicies(ctx context.Context, req ctrl.Request, nbPolicy *netbirdiov1.NBPolicy, sourceGroups, destGroups []string, portMapping map[string][]int32) (bool, error) {
235+
func (r *NBPolicyReconciler) handlePolicies(ctx context.Context, nbPolicy *netbirdiov1.NBPolicy, sourceGroups, destGroups []string, portMapping map[string][]int32, logger logr.Logger) (bool, error) {
237236
requeue := false
238237

239238
for protocol, ports := range portMapping {
@@ -244,14 +243,14 @@ func (r *NBPolicyReconciler) handlePolicies(ctx context.Context, req ctrl.Reques
244243
case "udp":
245244
policyID = nbPolicy.Status.UDPPolicyID
246245
default:
247-
ctrl.Log.Error(errKubernetesAPI, "Unknown protocol", "namespace", req.Namespace, "name", req.Name, "protocol", protocol)
246+
logger.Error(errKubernetesAPI, "Unknown protocol", "protocol", protocol)
248247
nbPolicy.Status.Conditions = netbirdiov1.NBConditionFalse("ConfigError", fmt.Sprintf("Unknown protocol: %s", protocol))
249248
return requeue, errUnknownProtocol
250249
}
251250

252251
if len(nbPolicy.Spec.Protocols) > 0 && !util.Contains(nbPolicy.Spec.Protocols, protocol) {
253252
if policyID != nil {
254-
ctrl.Log.Info("Deleting protocol policy as NBPolicy has restricted protocols", "protocol", protocol)
253+
logger.Info("Deleting protocol policy as NBPolicy has restricted protocols", "protocol", protocol)
255254
err := r.netbird.Policies.Delete(ctx, *policyID)
256255
if err != nil && !strings.Contains(err.Error(), "not found") {
257256
nbPolicy.Status.Conditions = netbirdiov1.NBConditionFalse("APIError", fmt.Sprintf("Error deleting policy: %v", err))
@@ -260,20 +259,20 @@ func (r *NBPolicyReconciler) handlePolicies(ctx context.Context, req ctrl.Reques
260259
policyID = nil
261260

262261
} else {
263-
ctrl.Log.Info("Ignoring protocol as NBPolicy has restricted protocols", "protocol", protocol)
262+
logger.Info("Ignoring protocol as NBPolicy has restricted protocols", "protocol", protocol)
264263
}
265264
} else if len(ports) == 0 && policyID == nil {
266-
ctrl.Log.Info("0 ports found for protocol in policy", "namespace", req.Namespace, "name", req.Name, "protocol", protocol)
265+
logger.Info("0 ports found for protocol in policy", "protocol", protocol)
267266
continue
268267
} else if len(destGroups) == 0 && policyID == nil {
269-
ctrl.Log.Info("no destinations found for protocol in policy", "namespace", req.Namespace, "name", req.Name, "protocol", protocol)
268+
logger.Info("no destinations found for protocol in policy", "protocol", protocol)
270269
continue
271270
} else if len(sourceGroups) == 0 && policyID == nil {
272-
ctrl.Log.Info("no sources found for protocol in policy", "namespace", req.Namespace, "name", req.Name, "protocol", protocol)
271+
logger.Info("no sources found for protocol in policy", "protocol", protocol)
273272
continue
274273
} else if len(ports) == 0 || len(destGroups) == 0 || len(sourceGroups) == 0 {
275274
// Delete policy
276-
ctrl.Log.Info("Deleting policy", "namespace", req.Namespace, "name", req.Name, "protocol", protocol)
275+
logger.Info("Deleting policy", "protocol", protocol)
277276
err := r.netbird.Policies.Delete(ctx, *policyID)
278277
if err != nil && !strings.Contains(err.Error(), "not found") {
279278
nbPolicy.Status.Conditions = netbirdiov1.NBConditionFalse("APIError", fmt.Sprintf("Error deleting policy: %v", err))
@@ -291,9 +290,9 @@ func (r *NBPolicyReconciler) handlePolicies(ctx context.Context, req ctrl.Reques
291290

292291
var err error
293292
if policyID == nil {
294-
policyID, err = r.createPolicy(ctx, req, nbPolicy, protocol, sourceGroups, destGroups, stringPorts)
293+
policyID, err = r.createPolicy(ctx, nbPolicy, protocol, sourceGroups, destGroups, stringPorts, logger)
295294
} else {
296-
policyID, requeue, err = r.updatePolicy(ctx, req, policyID, nbPolicy, protocol, sourceGroups, destGroups, stringPorts)
295+
policyID, requeue, err = r.updatePolicy(ctx, policyID, nbPolicy, protocol, sourceGroups, destGroups, stringPorts, logger)
297296
}
298297
if err != nil {
299298
return requeue, err
@@ -306,7 +305,7 @@ func (r *NBPolicyReconciler) handlePolicies(ctx context.Context, req ctrl.Reques
306305
case "udp":
307306
nbPolicy.Status.UDPPolicyID = policyID
308307
default:
309-
ctrl.Log.Error(errKubernetesAPI, "Unknown protocol", "namespace", req.Namespace, "name", req.Name, "protocol", protocol)
308+
logger.Error(errKubernetesAPI, "Unknown protocol", "protocol", protocol)
310309
nbPolicy.Status.Conditions = netbirdiov1.NBConditionFalse("ConfigError", fmt.Sprintf("Unknown protocol: %s", protocol))
311310
return requeue, errUnknownProtocol
312311
}
@@ -315,7 +314,7 @@ func (r *NBPolicyReconciler) handlePolicies(ctx context.Context, req ctrl.Reques
315314
return requeue, nil
316315
}
317316

318-
func (r *NBPolicyReconciler) handleDelete(ctx context.Context, req ctrl.Request, nbPolicy netbirdiov1.NBPolicy) error {
317+
func (r *NBPolicyReconciler) handleDelete(ctx context.Context, nbPolicy netbirdiov1.NBPolicy, logger logr.Logger) error {
319318
if nbPolicy.Status.TCPPolicyID != nil {
320319
err := r.netbird.Policies.Delete(ctx, *nbPolicy.Status.TCPPolicyID)
321320
if err != nil {
@@ -334,17 +333,17 @@ func (r *NBPolicyReconciler) handleDelete(ctx context.Context, req ctrl.Request,
334333
nbPolicy.Finalizers = util.Without(nbPolicy.Finalizers, "netbird.io/cleanup")
335334
err := r.Client.Update(ctx, &nbPolicy)
336335
if err != nil {
337-
ctrl.Log.Error(errKubernetesAPI, "Error updating NBPolicy", "namespace", req.Namespace, "name", req.Name, "err", err)
336+
logger.Error(errKubernetesAPI, "Error updating NBPolicy", "err", err)
338337
return err
339338
}
340339
}
341340
return nil
342341
}
343342

344-
func (r *NBPolicyReconciler) groupNamesToIDs(ctx context.Context, req ctrl.Request, groupNames []string) ([]string, error) {
343+
func (r *NBPolicyReconciler) groupNamesToIDs(ctx context.Context, groupNames []string, logger logr.Logger) ([]string, error) {
345344
groups, err := r.netbird.Groups.List(ctx)
346345
if err != nil {
347-
ctrl.Log.Error(errNetBirdAPI, "Error listing Groups", "namespace", req.Namespace, "name", req.Name, "err", err)
346+
logger.Error(errNetBirdAPI, "Error listing Groups", "err", err)
348347
return nil, err
349348
}
350349

0 commit comments

Comments
 (0)