Skip to content

Commit

Permalink
chore: setup ctx logging
Browse files Browse the repository at this point in the history
  • Loading branch information
jakobmoellerdev committed Aug 25, 2023
1 parent 3361576 commit a232997
Show file tree
Hide file tree
Showing 17 changed files with 180 additions and 187 deletions.
44 changes: 17 additions & 27 deletions controllers/lvm_volumegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (
"fmt"

lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"
k8serror "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
cutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/log"
)

const (
Expand All @@ -40,7 +40,7 @@ func (c lvmVG) getName() string {
}

func (c lvmVG) ensureCreated(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error {
unitLogger := r.Log.WithValues("topolvmNode", c.getName())
logger := log.FromContext(ctx).WithValues("topolvmNode", c.getName())

lvmVolumeGroups := lvmVolumeGroups(r.Namespace, lvmCluster.Spec.Storage.DeviceClasses)

Expand All @@ -66,47 +66,37 @@ func (c lvmVG) ensureCreated(r *LVMClusterReconciler, ctx context.Context, lvmCl
return fmt.Errorf("%s failed to reconcile: %w", c.getName(), err)
}

unitLogger.Info("LVMVolumeGroup applied to cluster", "operation", result, "name", volumeGroup.Name)
logger.Info("LVMVolumeGroup applied to cluster", "operation", result, "name", volumeGroup.Name)
}
return nil
}

func (c lvmVG) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error {
logger := log.FromContext(ctx).WithValues("topolvmNode", c.getName())
vgcrs := lvmVolumeGroups(r.Namespace, lvmCluster.Spec.Storage.DeviceClasses)
allVGsDeleted := true

for _, vgcr := range vgcrs {
currentLvmVg := &lvmv1alpha1.LVMVolumeGroup{}

err := r.Client.Get(ctx, types.NamespacedName{Name: vgcr.Name, Namespace: vgcr.Namespace}, currentLvmVg)
if err != nil {
// already deleted in previous reconcile
if k8serror.IsNotFound(err) {
r.Log.Info("LVMVolumeGroup already deleted", "name", vgcr.Name)
continue
}
r.Log.Error(err, "failed to retrieve LVMVolumeGroup", "name", vgcr.Name)
return err
for _, volumeGroup := range vgcrs {
if err := r.Client.Get(ctx, client.ObjectKeyFromObject(volumeGroup), volumeGroup); err != nil {
return client.IgnoreNotFound(err)
}

// if not deleted, initiate deletion
if currentLvmVg.GetDeletionTimestamp().IsZero() {
err = r.Client.Delete(ctx, currentLvmVg)
if err != nil {
r.Log.Error(err, "failed to delete LVMVolumeGroup", "name", currentLvmVg.Name)
return err
if volumeGroup.GetDeletionTimestamp().IsZero() {
if err := r.Client.Delete(ctx, volumeGroup); err != nil {
return fmt.Errorf("failed to delete LVMVolumeGroup %s: %w", volumeGroup.GetName(), err)
}
r.Log.Info("initiated LVMVolumeGroup deletion", "name", currentLvmVg.Name)
logger.Info("initiated LVMVolumeGroup deletion", "name", volumeGroup.Name)
allVGsDeleted = false
} else {
// Has the VG been cleaned up on all hosts?
exists := doesVGExistOnHosts(currentLvmVg.Name, lvmCluster)
exists := doesVGExistOnHosts(volumeGroup.Name, lvmCluster)
if !exists {
// Remove finalizer
cutil.RemoveFinalizer(currentLvmVg, lvmvgFinalizer)
err = r.Client.Update(ctx, currentLvmVg)
if err != nil {
return err
if update := cutil.RemoveFinalizer(volumeGroup, lvmvgFinalizer); update {
if err := r.Client.Update(ctx, volumeGroup); err != nil {
return fmt.Errorf("failed to remove finalizer from LVMVolumeGroup")
}
}
} else {
allVGsDeleted = false
Expand Down
26 changes: 14 additions & 12 deletions controllers/lvmcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"os"
"time"

"github.com/go-logr/logr"
configv1 "github.com/openshift/api/config/v1"
secv1client "github.com/openshift/client-go/security/clientset/versioned/typed/security/v1"
lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"
Expand Down Expand Up @@ -76,7 +75,6 @@ const (
type LVMClusterReconciler struct {
client.Client
Scheme *runtime.Scheme
Log logr.Logger
ClusterType ClusterType
SecurityClient secv1client.SecurityV1Interface
Namespace string
Expand Down Expand Up @@ -108,8 +106,8 @@ type LVMClusterReconciler struct {
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.10.0/pkg/reconcile
func (r *LVMClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
r.Log = log.Log.WithName(ControllerName).WithValues("Request.Name", req.Name, "Request.Namespace", req.Namespace)
r.Log.Info("reconciling")
logger := log.FromContext(ctx)
logger.Info("reconciling")

// Checks that only a single LVMCluster instance exists
lvmClusterList := &lvmv1alpha1.LVMClusterList{}
Expand Down Expand Up @@ -151,6 +149,7 @@ func (r *LVMClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)

// errors returned by this will be updated in the reconcileSucceeded condition of the LVMCluster
func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alpha1.LVMCluster) (ctrl.Result, error) {
logger := log.FromContext(ctx)

// The resource was deleted
if !instance.DeletionTimestamp.IsZero() {
Expand All @@ -166,7 +165,7 @@ func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alp
fmt.Errorf("found PVCs provisioned by topolvm, waiting for their deletion: %w", err)
}

r.Log.Info("processing LVMCluster deletion")
logger.Info("processing LVMCluster deletion")
if err := r.processDelete(ctx, instance); err != nil {
// check every 10 seconds if there are still PVCs present or the LogicalVolumes are removed
return ctrl.Result{Requeue: true}, fmt.Errorf("failed to process LVMCluster deletion")
Expand All @@ -178,7 +177,7 @@ func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alp
if err := r.Client.Update(ctx, instance); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to update LvmCluster with finalizer: %w", err)
}
r.Log.Info("successfully added finalizer")
logger.Info("successfully added finalizer")
}

resources := []resourceManager{
Expand Down Expand Up @@ -215,12 +214,13 @@ func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alp
resourceSyncElapsedTime, internal.NewMultiError(errs))
}

r.Log.Info("successfully reconciled LVMCluster", "resourceSyncElapsedTime", resourceSyncElapsedTime)
logger.Info("successfully reconciled LVMCluster", "resourceSyncElapsedTime", resourceSyncElapsedTime)

return ctrl.Result{}, nil
}

func (r *LVMClusterReconciler) updateLVMClusterStatus(ctx context.Context, instance *lvmv1alpha1.LVMCluster) error {
logger := log.FromContext(ctx)

vgNodeMap := make(map[string][]lvmv1alpha1.NodeStatus)

Expand Down Expand Up @@ -262,7 +262,7 @@ func (r *LVMClusterReconciler) updateLVMClusterStatus(ctx context.Context, insta
instance.Status.State = lvmv1alpha1.LVMStatusProgressing
instance.Status.Ready = false

r.Log.Info("calculating readiness of LVMCluster", "expectedVGCount", expectedVGCount, "readyVGCount", readyVGCount)
logger.Info("calculating readiness of LVMCluster", "expectedVGCount", expectedVGCount, "readyVGCount", readyVGCount)

if isFailed {
instance.Status.State = lvmv1alpha1.LVMStatusFailed
Expand All @@ -288,11 +288,12 @@ func (r *LVMClusterReconciler) updateLVMClusterStatus(ctx context.Context, insta
if err = r.Client.Status().Update(ctx, instance); err != nil {
return fmt.Errorf("failed to update LVMCluster status: %w", err)
}
r.Log.Info("successfully updated the LVMCluster status")
logger.Info("successfully updated the LVMCluster status")
return nil
}

func (r *LVMClusterReconciler) getExpectedVGCount(ctx context.Context, instance *lvmv1alpha1.LVMCluster) (int, error) {
logger := log.FromContext(ctx)
var vgCount int

nodeList := &corev1.NodeList{}
Expand All @@ -305,7 +306,7 @@ func (r *LVMClusterReconciler) getExpectedVGCount(ctx context.Context, instance
ignoreDueToNoSchedule := false
for _, taint := range nodeList.Items[i].Spec.Taints {
if taint.Effect == corev1.TaintEffectNoSchedule {
r.Log.V(1).Info("even though node selector matches, NoSchedule forces ignore of the Node",
logger.V(1).Info("even though node selector matches, NoSchedule forces ignore of the Node",
"node", nodeList.Items[i].GetName())
ignoreDueToNoSchedule = true
break
Expand Down Expand Up @@ -337,6 +338,7 @@ func (r *LVMClusterReconciler) getExpectedVGCount(ctx context.Context, instance
// checkIfOpenshift checks to see if the operator is running on an OCP cluster.
// It does this by querying for the "privileged" SCC which exists on all OCP clusters.
func (r *LVMClusterReconciler) checkIfOpenshift(ctx context.Context) error {
logger := log.FromContext(ctx)
if r.ClusterType == "" {
// cluster type has not been determined yet
// Check if the privileged SCC exists on the cluster (this is one of the default SCCs)
Expand All @@ -345,12 +347,12 @@ func (r *LVMClusterReconciler) checkIfOpenshift(ctx context.Context) error {
if errors.IsNotFound(err) {
// Not an Openshift cluster
r.ClusterType = ClusterTypeOther
r.Log.Info("openshiftSCC not found, setting cluster type to other")
logger.Info("openshiftSCC not found, setting cluster type to other")
} else {
return fmt.Errorf("failed to get SCC %s", openshiftSCCPrivilegedName)
}
} else {
r.Log.Info("openshiftSCC found, setting cluster type to openshift")
logger.Info("openshiftSCC found, setting cluster type to openshift")
r.ClusterType = ClusterTypeOCP
}
}
Expand Down
7 changes: 4 additions & 3 deletions controllers/lvmcluster_controller_watches.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"

lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"
"sigs.k8s.io/controller-runtime/pkg/log"

appsv1 "k8s.io/api/apps/v1"
storagev1 "k8s.io/api/storage/v1"
Expand Down Expand Up @@ -49,14 +50,14 @@ func (r *LVMClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
}

func (r *LVMClusterReconciler) getLVMClusterObjsForReconcile(ctx context.Context, obj client.Object) []reconcile.Request {

foundLVMClusterList := &lvmv1alpha1.LVMClusterList{}
listOps := &client.ListOptions{
Namespace: obj.GetNamespace(),
}

err := r.Client.List(ctx, foundLVMClusterList, listOps)
if err != nil {
r.Log.Error(err, "getLVMClusterObjsForReconcile: Failed to get LVMCluster objs")
if err := r.Client.List(ctx, foundLVMClusterList, listOps); err != nil {
log.FromContext(ctx).Error(err, "getLVMClusterObjsForReconcile: Failed to get LVMCluster objs")
return []reconcile.Request{}
}

Expand Down
22 changes: 11 additions & 11 deletions controllers/scc.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/log"
)

const (
Expand All @@ -40,18 +41,18 @@ func (c openshiftSccs) getName() string {
return sccName
}

func (c openshiftSccs) ensureCreated(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error {
unitLogger := r.Log.WithValues("resourceManager", c.getName())
func (c openshiftSccs) ensureCreated(r *LVMClusterReconciler, ctx context.Context, _ *lvmv1alpha1.LVMCluster) error {
logger := log.FromContext(ctx).WithValues("resourceManager", c.getName())
if !IsOpenshift(r) {
unitLogger.Info("not creating SCCs as this is not an Openshift cluster")
logger.Info("not creating SCCs as this is not an Openshift cluster")
return nil
}
sccs := getAllSCCs(r.Namespace)
for _, scc := range sccs {
_, err := r.SecurityClient.SecurityContextConstraints().Get(ctx, scc.Name, metav1.GetOptions{})
if err == nil {
// Don't update the SCC
unitLogger.Info("SecurityContextConstraint exists, skipping creation", "name", scc.Name)
logger.Info("SecurityContextConstraint exists, skipping creation", "name", scc.Name)
continue
}

Expand All @@ -63,24 +64,23 @@ func (c openshiftSccs) ensureCreated(r *LVMClusterReconciler, ctx context.Contex
return fmt.Errorf("%s failed to reconcile: %w", c.getName(), err)
}

unitLogger.Info("SecurityContextConstraint created", "name", scc.Name)
logger.Info("SecurityContextConstraint created", "name", scc.Name)
}

return nil
}

func (c openshiftSccs) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error {
func (c openshiftSccs) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, _ *lvmv1alpha1.LVMCluster) error {
logger := log.FromContext(ctx).WithValues("resourceManager", c.getName())
if IsOpenshift(r) {
var err error
sccs := getAllSCCs(r.Namespace)
for _, scc := range sccs {
err = r.SecurityClient.SecurityContextConstraints().Delete(ctx, scc.Name, metav1.DeleteOptions{})
if err != nil {
if err = r.SecurityClient.SecurityContextConstraints().Delete(ctx, scc.Name, metav1.DeleteOptions{}); err != nil {
if !errors.IsNotFound(err) {
r.Log.Error(err, "failed to delete SecurityContextConstraint", "SecurityContextConstraint", scc.Name)
return err
return fmt.Errorf("failed to delete SecurityContextConstraint %s: %w", scc.GetName(), err)
}
r.Log.Info("SecurityContextConstraint is already deleted", "SecurityContextConstraint", scc.Name)
logger.Info("SecurityContextConstraint is already deleted", "SecurityContextConstraint", scc.Name)
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ const (
testNodeName = "test-node"
)

var _ = BeforeSuite(func() {
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
var _ = BeforeSuite(func(ctx context.Context) {
logger := zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))
logf.SetLogger(logger)

ctx, cancel = context.WithCancel(context.TODO())
ctx, cancel = context.WithCancel(ctx)

By("bootstrapping test environment")
testEnv = &envtest.Environment{
Expand Down Expand Up @@ -114,7 +115,6 @@ var _ = BeforeSuite(func() {
Scheme: k8sManager.GetScheme(),
SecurityClient: secv1client.NewForConfigOrDie(k8sManager.GetConfig()),
Namespace: testLvmClusterNamespace,
Log: ctrl.Log.WithName("controllers").WithName("LvmCluster"),
ImageName: testImageName,
}).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())
Expand Down
14 changes: 8 additions & 6 deletions controllers/topolvm_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
cutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/log"
)

const (
Expand All @@ -51,7 +52,7 @@ func (c topolvmController) getName() string {
//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;update;delete;get;list;watch

func (c topolvmController) ensureCreated(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error {
unitLogger := r.Log.WithValues("resourceManager", c.getName())
logger := log.FromContext(ctx).WithValues("resourceManager", c.getName())

// get the desired state of topolvm controller deployment
desiredDeployment := getControllerDeployment(lvmCluster, r.Namespace, r.ImageName, c.topoLVMLeaderElectionPassthrough)
Expand All @@ -73,23 +74,24 @@ func (c topolvmController) ensureCreated(r *LVMClusterReconciler, ctx context.Co
if err != nil {
return fmt.Errorf("could not create/update csi controller: %w", err)
}
unitLogger.Info("Deployment applied to cluster", "operation", result, "name", desiredDeployment.Name)
logger.Info("Deployment applied to cluster", "operation", result, "name", desiredDeployment.Name)

if err := verifyDeploymentReadiness(existingDeployment); err != nil {
return fmt.Errorf("csi controller is not ready: %w", err)
}
unitLogger.Info("Deployment is ready", "name", desiredDeployment.Name)
logger.Info("Deployment is ready", "name", desiredDeployment.Name)

return nil
}

func (c topolvmController) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error {
func (c topolvmController) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, _ *lvmv1alpha1.LVMCluster) error {
logger := log.FromContext(ctx).WithValues("resourceManager", c.getName())
existingDeployment := &appsv1.Deployment{}

err := r.Client.Get(ctx, types.NamespacedName{Name: TopolvmControllerDeploymentName, Namespace: r.Namespace}, existingDeployment)
// already deleted in previous reconcile
if k8serror.IsNotFound(err) {
r.Log.Info("csi controller deleted", "TopolvmController", existingDeployment.Name)
logger.Info("csi controller deleted", "TopolvmController", existingDeployment.Name)
return nil
}

Expand All @@ -107,7 +109,7 @@ func (c topolvmController) ensureDeleted(r *LVMClusterReconciler, ctx context.Co
return fmt.Errorf("failed to delete topolvm controller deployment %s: %w", existingDeployment.GetName(), err)
}

r.Log.Info("initiated topolvm controller deployment deletion", "TopolvmController", existingDeployment.Name)
logger.Info("initiated topolvm controller deployment deletion", "TopolvmController", existingDeployment.Name)
return nil
}

Expand Down
Loading

0 comments on commit a232997

Please sign in to comment.