Skip to content

Commit

Permalink
Use slog in controllers (#92)
Browse files Browse the repository at this point in the history 
Currently we use the `slog` logging library in the servers and the
`logr` library in the controllers. To make things a bit more consistent
this patch changes the controllers to use `slog`.

Signed-off-by: Juan Hernandez <[email protected]>
  • Loading branch information
@jhernand
jhernand authored Apr 18, 2024
1 parent 7e1c102 commit 029ce20
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 55 deletions.
2 changes: 1 addition & 1 deletion internal/cmd/operator/start_controller_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func (c *ControllerManagerCommand) run(cmd *cobra.Command, argv []string) error

if err = (&controllers.Reconciler{
Client: mgr.GetClient(),
Logger: ctrl.Log.WithName("controller").WithName("ORAN-O2IMS"),
Logger: slog.With("controller", "ORAN-O2IMS"),
}).SetupWithManager(mgr); err != nil {
logger.Error(
"Unable to create controller",
Expand Down
106 changes: 83 additions & 23 deletions internal/controllers/orano2ims_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controllers
import (
"context"
"fmt"
"log/slog"
"time"

"k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -28,7 +29,6 @@ import (
"k8s.io/client-go/util/retry"
k8sptr "k8s.io/utils/ptr"

"github.com/go-logr/logr"
oranv1alpha1 "github.com/openshift-kni/oran-o2ims/api/v1alpha1"
"github.com/openshift-kni/oran-o2ims/internal/controllers/utils"

Expand Down Expand Up @@ -61,14 +61,14 @@ import (
// Reconciler reconciles a ORANO2IMS object
type Reconciler struct {
client.Client
Logger logr.Logger
Logger *slog.Logger
}

// reconcilerTask contains the information and logic needed to perform a single reconciliation
// task. This reduces the need to pass things like the current state of the object as function
// parameters.
type reconcilerTask struct {
logger logr.Logger
logger *slog.Logger
client client.Client
object *oranv1alpha1.ORANO2IMS
}
Expand All @@ -91,7 +91,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (resul
err = nil
return ctrl.Result{RequeueAfter: 5 * time.Minute}, err
}
r.Logger.Error(err, "Unable to fetch ORANO2IMS")
r.Logger.Error(
"Unable to fetch ORANO2IMS",
slog.String("error", err.Error()),
)
}

// Create and run the task:
Expand All @@ -112,15 +115,21 @@ func (t *reconcilerTask) run(ctx context.Context) (nextReconcile ctrl.Result, er
if t.object.Spec.MetadataServer || t.object.Spec.DeploymentManagerServer || t.object.Spec.ResourceServer || t.object.Spec.AlarmSubscriptionServer {
err = t.createIngress(ctx)
if err != nil {
t.logger.Error(err, "Failed to deploy Ingress.")
t.logger.Error(
"Failed to deploy Ingress.",
slog.String("error", err.Error()),
)
return
}
}

// Create the client service account.
err = t.createServiceAccount(ctx, utils.ORANO2IMSClientSAName)
if err != nil {
t.logger.Error(err, "Failed to create client service account")
t.logger.Error(
"Failed to create client service account",
slog.String("error", err.Error()),
)
return
}

Expand All @@ -129,21 +138,30 @@ func (t *reconcilerTask) run(ctx context.Context) (nextReconcile ctrl.Result, er
// Create the needed ServiceAccount.
err = t.createServiceAccount(ctx, utils.ORANO2IMSResourceServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy ServiceAccount for the Resource server.")
t.logger.Error(
"Failed to deploy ServiceAccount for the Resource server.",
slog.String("error", err.Error()),
)
return
}

// Create the Service needed for the Resource server.
err = t.createService(ctx, utils.ORANO2IMSResourceServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy Service for Resource server.")
t.logger.Error(
"Failed to deploy Service for Resource server.",
slog.String("error", err.Error()),
)
return
}

// Create the resource-server deployment.
err = t.deployServer(ctx, utils.ORANO2IMSResourceServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy the Resource server.")
t.logger.Error(
"Failed to deploy the Resource server.",
slog.String("error", err.Error()),
)
return
}
}
Expand All @@ -153,21 +171,30 @@ func (t *reconcilerTask) run(ctx context.Context) (nextReconcile ctrl.Result, er
// Create the needed ServiceAccount.
err = t.createServiceAccount(ctx, utils.ORANO2IMSMetadataServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy ServiceAccount for Metadata server.")
t.logger.Error(
"Failed to deploy ServiceAccount for Metadata server.",
slog.String("error", err.Error()),
)
return
}

// Create the Service needed for the Metadata server.
err = t.createService(ctx, utils.ORANO2IMSMetadataServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy Service for Metadata server.")
t.logger.Error(
"Failed to deploy Service for Metadata server.",
slog.String("error", err.Error()),
)
return
}

// Create the metadata-server deployment.
err = t.deployServer(ctx, utils.ORANO2IMSMetadataServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy the Metadata server.")
t.logger.Error(
"Failed to deploy the Metadata server.",
slog.String("error", err.Error()),
)
return
}
}
Expand All @@ -177,38 +204,56 @@ func (t *reconcilerTask) run(ctx context.Context) (nextReconcile ctrl.Result, er
// Create the service account, role and binding:
err = t.createServiceAccount(ctx, utils.ORANO2IMSDeploymentManagerServerName)
if err != nil {
t.logger.Error(err, "Failed to create deployment manager service account")
t.logger.Error(
"Failed to create deployment manager service account",
slog.String("error", err.Error()),
)
return
}
err = t.createDeploymentManagerClusterRole(ctx)
if err != nil {
t.logger.Error(err, "Failed to create deployment manager cluster role")
t.logger.Error(
"Failed to create deployment manager cluster role",
slog.String("error", err.Error()),
)
return
}
err = t.createDeploymentManagerClusterRoleBinding(ctx)
if err != nil {
t.logger.Error(err, "Failed to create deployment manager cluster role binding")
t.logger.Error(
"Failed to create deployment manager cluster role binding",
slog.String("error", err.Error()),
)
return
}

// Create authz ConfigMap.
err = t.createConfigMap(ctx, utils.ORANO2IMSConfigMapName)
if err != nil {
t.logger.Error(err, "Failed to deploy ConfigMap for Deployment Manager server.")
t.logger.Error(
"Failed to deploy ConfigMap for Deployment Manager server.",
slog.String("error", err.Error()),
)
return
}

// Create the Service needed for the Deployment Manager server.
err = t.createService(ctx, utils.ORANO2IMSDeploymentManagerServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy Service for Deployment Manager server.")
t.logger.Error(
"Failed to deploy Service for Deployment Manager server.",
slog.String("error", err.Error()),
)
return
}

// Create the deployment-manager-server deployment.
err = t.deployServer(ctx, utils.ORANO2IMSDeploymentManagerServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy the Deployment Manager server.")
t.logger.Error(
"Failed to deploy the Deployment Manager server.",
slog.String("error", err.Error()),
)
return
}
}
Expand All @@ -217,35 +262,50 @@ func (t *reconcilerTask) run(ctx context.Context) (nextReconcile ctrl.Result, er
// Create authz ConfigMap.
err = t.createConfigMap(ctx, utils.ORANO2IMSConfigMapName)
if err != nil {
t.logger.Error(err, "Failed to deploy ConfigMap for alarm subscription server.")
t.logger.Error(
"Failed to deploy ConfigMap for alarm subscription server.",
slog.String("error", err.Error()),
)
return
}

// Create the needed ServiceAccount.
err = t.createServiceAccount(ctx, utils.ORANO2IMSAlarmSubscriptionServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy ServiceAccount for Alarm Subscription server.")
t.logger.Error(
"Failed to deploy ServiceAccount for Alarm Subscription server.",
slog.String("error", err.Error()),
)
return
}

// Create the Service needed for the alarm subscription server.
err = t.createService(ctx, utils.ORANO2IMSAlarmSubscriptionServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy Service for Alarm Subscription server.")
t.logger.Error(
"Failed to deploy Service for Alarm Subscription server.",
slog.String("error", err.Error()),
)
return
}

// Create the alarm subscription-server deployment.
err = t.deployServer(ctx, utils.ORANO2IMSAlarmSubscriptionServerName)
if err != nil {
t.logger.Error(err, "Failed to deploy the alarm subscription server.")
t.logger.Error(
"Failed to deploy the alarm subscription server.",
slog.String("error", err.Error()),
)
return
}
}

err = t.updateORANO2ISMStatus(ctx)
if err != nil {
t.logger.Error(err, fmt.Sprintf("Failed to update status for ORANO2IMS %s.", t.object.Name))
t.logger.Error(
"Failed to update status for ORANO2IMS",
slog.String("name", t.object.Name),
)
nextReconcile = ctrl.Result{RequeueAfter: 30 * time.Second}
}
return
Expand Down
33 changes: 2 additions & 31 deletions internal/controllers/orano2ims_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ package controllers
import (
"context"
"fmt"
"testing"
"time"

"github.com/go-logr/logr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

Expand All @@ -31,12 +29,9 @@ import (
networkingv1 "k8s.io/api/networking/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

oranv1alpha1 "github.com/openshift-kni/oran-o2ims/api/v1alpha1"
Expand All @@ -47,34 +42,10 @@ import (
// These tests use Ginkgo (BDD-style Go testing framework). Refer to
// http://onsi.github.io/ginkgo/ to learn more about Ginkgo.

var suitescheme = scheme.Scheme

func getFakeClientFromObjects(objs ...client.Object) (client.WithWatch, error) {
return fake.NewClientBuilder().WithScheme(suitescheme).WithObjects(objs...).WithStatusSubresource(&oranv1alpha1.ORANO2IMS{}).Build(), nil
}

func TestORANO2IMSReconciler(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Controller Suite")
return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).WithStatusSubresource(&oranv1alpha1.ORANO2IMS{}).Build(), nil
}

var _ = BeforeSuite(func() {
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))

By("Bootstrapping test environment")

suitescheme.AddKnownTypes(oranv1alpha1.GroupVersion, &oranv1alpha1.ORANO2IMS{})
suitescheme.AddKnownTypes(oranv1alpha1.GroupVersion, &oranv1alpha1.ORANO2IMSList{})
suitescheme.AddKnownTypes(networkingv1.SchemeGroupVersion, &networkingv1.Ingress{})
suitescheme.AddKnownTypes(networkingv1.SchemeGroupVersion, &networkingv1.IngressList{})
suitescheme.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.ServiceAccount{})
suitescheme.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.ServiceAccountList{})
suitescheme.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.Service{})
suitescheme.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.ServiceList{})
suitescheme.AddKnownTypes(appsv1.SchemeGroupVersion, &appsv1.Deployment{})
suitescheme.AddKnownTypes(appsv1.SchemeGroupVersion, &appsv1.DeploymentList{})
})

var _ = DescribeTable(
"Reconciler",
func(objs []client.Object, request reconcile.Request, validate func(result ctrl.Result, reconciler Reconciler)) {
Expand All @@ -95,7 +66,7 @@ var _ = DescribeTable(
// Initialize the O-RAN O2IMS reconciler.
r := &Reconciler{
Client: fakeClient,
Logger: logr.Discard(),
Logger: logger,
}

// Reconcile.
Expand Down
Loading

0 comments on commit 029ce20

Please sign in to comment.