Skip to content

Commit

Permalink
Merge pull request #567 from jakobmoellerdev/OCPBUGS-28727-stream-std…
Browse files Browse the repository at this point in the history
…err-logging

OCPBUGS-28727: fix: allow stderr reporting via error and stdout reporting via logs when no parsing is run
  • Loading branch information
openshift-merge-bot[bot] authored Feb 7, 2024
2 parents 39f73d4 + 8075bba commit f4c0b7b
Show file tree
Hide file tree
Showing 25 changed files with 763 additions and 678 deletions.
49 changes: 2 additions & 47 deletions internal/controllers/lvmcluster/logpassthrough/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,18 @@ type ArgPassable interface {

// Options represents all pass-through options for logging verbosity
type Options struct {
CSISideCar *CSISideCarOptions
TopoLVMController *TopoLVMControllerOptions
TopoLVMNode *TopoLVMNodeOptions
VGManager *VGmanagerOptions
VGManager *VGmanagerOptions
}

// NewOptions creates a new option set and binds it's values against a given flagset.
func NewOptions() *Options {
opts := &Options{
CSISideCar: &CSISideCarOptions{},
TopoLVMController: &TopoLVMControllerOptions{},
TopoLVMNode: &TopoLVMNodeOptions{},
VGManager: &VGmanagerOptions{},
VGManager: &VGmanagerOptions{},
}
return opts
}

func (o *Options) BindFlags(fs *pflag.FlagSet) {
o.CSISideCar.BindFlags(fs)
o.TopoLVMController.BindFlags(fs)
o.TopoLVMNode.BindFlags(fs)
o.VGManager.BindFlags(fs)
}

Expand Down Expand Up @@ -86,42 +77,6 @@ func (o *KlogOptions) BindFlags(fs *pflag.FlagSet) {
fs.StringVar(&o.VModule, "vmodule", "", "comma-separated list of pattern=N settings for file-filtered logging")
}

type CSISideCarOptions struct {
KlogOptions
}

func (o *CSISideCarOptions) BindFlags(fs *pflag.FlagSet) {
bindFlagsWithPrefix(&o.KlogOptions, fs, "csi-sidecar")
}

type TopoLVMControllerOptions struct {
KlogOptions
ZapOptions
}

func (o *TopoLVMControllerOptions) AsArgs() []string {
return append(o.ZapOptions.AsArgs(), o.KlogOptions.AsArgs()...)
}

func (o *TopoLVMControllerOptions) BindFlags(fs *pflag.FlagSet) {
bindFlagsWithPrefix(&o.KlogOptions, fs, "topolvm-controller")
bindFlagsWithPrefix(&o.ZapOptions, fs, "topolvm-controller")
}

type TopoLVMNodeOptions struct {
KlogOptions
ZapOptions
}

func (o *TopoLVMNodeOptions) BindFlags(fs *pflag.FlagSet) {
bindFlagsWithPrefix(&o.KlogOptions, fs, "topolvm-node")
bindFlagsWithPrefix(&o.ZapOptions, fs, "topolvm-node")
}

func (o *TopoLVMNodeOptions) AsArgs() []string {
return append(o.ZapOptions.AsArgs(), o.KlogOptions.AsArgs()...)
}

type VGmanagerOptions struct {
KlogOptions
ZapOptions
Expand Down
27 changes: 0 additions & 27 deletions internal/controllers/lvmcluster/logpassthrough/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,38 +16,11 @@ func TestCSISideCarOptions_BindFlags(t *testing.T) {
{
"pass-all",
[]string{
"--csi-sidecar-v=1",
"--csi-sidecar-vmodule=bla=1",
"--topolvm-controller-v=2",
"--topolvm-controller-vmodule=bla=2",
"--topolvm-controller-zap-log-level=debug",
"--topolvm-node-v=3",
"--topolvm-node-vmodule=bla=3",
"--topolvm-node-zap-log-level=debug",
"--vgmanager-v=4",
"--vgmanager-vmodule=bla=4",
"--vgmanager-zap-log-level=debug",
},
func(a *assert.Assertions, o *Options) {
a.Equal(o.CSISideCar.VModule, "bla=1")
a.Equal(o.CSISideCar.V, "1")
a.Contains(o.CSISideCar.AsArgs(), "--v=1")
a.Contains(o.CSISideCar.AsArgs(), "--vmodule=bla=1")

a.Equal(o.TopoLVMController.VModule, "bla=2")
a.Equal(o.TopoLVMController.V, "2")
a.Equal(o.TopoLVMController.ZapLogLevel, "debug")
a.Contains(o.TopoLVMController.AsArgs(), "--v=2")
a.Contains(o.TopoLVMController.AsArgs(), "--vmodule=bla=2")
a.Contains(o.TopoLVMController.AsArgs(), "--zap-log-level=debug")

a.Equal(o.TopoLVMNode.VModule, "bla=3")
a.Equal(o.TopoLVMNode.V, "3")
a.Equal(o.TopoLVMNode.ZapLogLevel, "debug")
a.Contains(o.TopoLVMNode.AsArgs(), "--v=3")
a.Contains(o.TopoLVMNode.AsArgs(), "--vmodule=bla=3")
a.Contains(o.TopoLVMNode.AsArgs(), "--zap-log-level=debug")

a.Equal(o.VGManager.VModule, "bla=4")
a.Equal(o.VGManager.V, "4")
a.Equal(o.VGManager.ZapLogLevel, "debug")
Expand Down
42 changes: 24 additions & 18 deletions internal/controllers/vgmanager/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (r *Reconciler) getFinalizer() string {

func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := log.FromContext(ctx)
logger.V(2).Info("reconciling")
logger.V(1).Info("reconciling")

// Check if this LVMVolumeGroup needs to be processed on this node
volumeGroup := &lvmv1alpha1.LVMVolumeGroup{}
Expand Down Expand Up @@ -153,17 +153,19 @@ func (r *Reconciler) reconcile(
}
}

blockDevices, err := r.LSBLK.ListBlockDevices()
blockDevices, err := r.LSBLK.ListBlockDevices(ctx)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to list block devices: %w", err)
}

logger.V(1).Info("block devices", "blockDevices", blockDevices)

wiped, err := r.wipeDevicesIfNecessary(ctx, volumeGroup, nodeStatus, blockDevices)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to wipe devices: %w", err)
}
if wiped {
blockDevices, err = r.LSBLK.ListBlockDevices()
blockDevices, err = r.LSBLK.ListBlockDevices(ctx)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to list block devices after wiping devices: %w", err)
}
Expand All @@ -182,19 +184,23 @@ func (r *Reconciler) reconcile(
return ctrl.Result{}, err
}

pvs, err := r.LVM.ListPVs("")
logger.V(1).Info("new devices", "newDevices", newDevices)

pvs, err := r.LVM.ListPVs(ctx, "")
if err != nil {
return ctrl.Result{}, fmt.Errorf("physical volumes could not be fetched: %w", err)
}

bdi, err := r.LSBLK.BlockDeviceInfos(newDevices)
bdi, err := r.LSBLK.BlockDeviceInfos(ctx, newDevices)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to get block device infos: %w", err)
}

logger.V(1).Info("block device infos", "bdi", bdi)

devices := r.filterDevices(ctx, newDevices, pvs, bdi, r.Filters(volumeGroup))

vgs, err := r.LVM.ListVGs(true)
vgs, err := r.LVM.ListVGs(ctx, true)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to list volume groups: %w", err)
}
Expand All @@ -220,7 +226,7 @@ func (r *Reconciler) reconcile(
return ctrl.Result{}, err
}

logger.V(2).Info("no new available devices discovered, verifying existing setup")
logger.V(1).Info("no new available devices discovered, verifying existing setup")

// since the last reconciliation there could have been corruption on the LVs, so we need to verify them again
if err := r.validateLVs(ctx, volumeGroup); err != nil {
Expand Down Expand Up @@ -287,7 +293,7 @@ func (r *Reconciler) reconcile(
}

// refresh list of vgs to be used in status
vgs, err = r.LVM.ListVGs(true)
vgs, err = r.LVM.ListVGs(ctx, true)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to list volume groups: %w", err)
}
Expand Down Expand Up @@ -423,7 +429,7 @@ func (r *Reconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alpha1
}

// Check if volume group exists
vgs, err := r.LVM.ListVGs(true)
vgs, err := r.LVM.ListVGs(ctx, true)
if err != nil {
return fmt.Errorf("failed to list volume groups, %w", err)
}
Expand All @@ -444,13 +450,13 @@ func (r *Reconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alpha1
if volumeGroup.Spec.ThinPoolConfig != nil {
thinPoolName := volumeGroup.Spec.ThinPoolConfig.Name
logger := logger.WithValues("ThinPool", thinPoolName)
thinPoolExists, err := r.LVM.LVExists(thinPoolName, volumeGroup.Name)
thinPoolExists, err := r.LVM.LVExists(ctx, thinPoolName, volumeGroup.Name)
if err != nil {
return fmt.Errorf("failed to check existence of thin pool %q in volume group %q. %v", thinPoolName, volumeGroup.Name, err)
}

if thinPoolExists {
if err := r.LVM.DeleteLV(thinPoolName, volumeGroup.Name); err != nil {
if err := r.LVM.DeleteLV(ctx, thinPoolName, volumeGroup.Name); err != nil {
err := fmt.Errorf("failed to delete thin pool %s in volume group %s: %w", thinPoolName, volumeGroup.Name, err)
if _, err := r.setVolumeGroupFailedStatus(ctx, volumeGroup, vgs, FilteredBlockDevices{}, err); err != nil {
logger.Error(err, "failed to set status to failed")
Expand All @@ -463,7 +469,7 @@ func (r *Reconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alpha1
}
}

if err = r.LVM.DeleteVG(existingVG); err != nil {
if err = r.LVM.DeleteVG(ctx, existingVG); err != nil {
err := fmt.Errorf("failed to delete volume group %s: %w", volumeGroup.Name, err)
if _, err := r.setVolumeGroupFailedStatus(ctx, volumeGroup, vgs, FilteredBlockDevices{}, err); err != nil {
logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName())
Expand Down Expand Up @@ -515,7 +521,7 @@ func (r *Reconciler) validateLVs(ctx context.Context, volumeGroup *lvmv1alpha1.L
return nil
}

resp, err := r.LVM.ListLVs(volumeGroup.Name)
resp, err := r.LVM.ListLVs(ctx, volumeGroup.Name)
if err != nil {
return fmt.Errorf("could not get logical volumes found inside volume group, volume group content is degraded or corrupt: %w", err)
}
Expand Down Expand Up @@ -548,7 +554,7 @@ func (r *Reconciler) validateLVs(ctx context.Context, volumeGroup *lvmv1alpha1.L

if lvAttr.State != StateActive {
// If inactive, try activating it
err := r.LVM.ActivateLV(lv.Name, volumeGroup.Name)
err := r.LVM.ActivateLV(ctx, lv.Name, volumeGroup.Name)
if err != nil {
return fmt.Errorf("could not activate the inactive logical volume, maybe external repairs are necessary/already happening or there is another"+
"entity conflicting with vg-manager, cannot proceed until volume is activated again: lv_attr: %s", lvAttr)
Expand Down Expand Up @@ -578,7 +584,7 @@ func (r *Reconciler) addThinPoolToVG(ctx context.Context, vgName string, config
}
logger := log.FromContext(ctx).WithValues("VGName", vgName, "ThinPool", config.Name)

resp, err := r.LVM.ListLVs(vgName)
resp, err := r.LVM.ListLVs(ctx, vgName)
if err != nil {
return fmt.Errorf("failed to list logical volumes in the volume group %q. %v", vgName, err)
}
Expand All @@ -604,7 +610,7 @@ func (r *Reconciler) addThinPoolToVG(ctx context.Context, vgName string, config
}

logger.Info("creating lvm thinpool")
if err := r.LVM.CreateLV(config.Name, vgName, config.SizePercent); err != nil {
if err := r.LVM.CreateLV(ctx, config.Name, vgName, config.SizePercent); err != nil {
return fmt.Errorf("failed to create thinpool: %w", err)
}
logger.Info("successfully created thinpool")
Expand All @@ -627,7 +633,7 @@ func (r *Reconciler) extendThinPool(ctx context.Context, vgName string, lvSize s
return fmt.Errorf("failed to parse lvSize. %v", err)
}

vg, err := r.LVM.GetVG(vgName)
vg, err := r.LVM.GetVG(ctx, vgName)
if err != nil {
return fmt.Errorf("failed to get volume group. %q, %v", vgName, err)
}
Expand Down Expand Up @@ -657,7 +663,7 @@ func (r *Reconciler) extendThinPool(ctx context.Context, vgName string, lvSize s
}

logger.Info("extending lvm thinpool")
if err := r.LVM.ExtendLV(config.Name, vgName, config.SizePercent); err != nil {
if err := r.LVM.ExtendLV(ctx, config.Name, vgName, config.SizePercent); err != nil {
return fmt.Errorf("failed to extend thinpool: %w", err)
}
logger.Info("successfully extended thinpool")
Expand Down
Loading

0 comments on commit f4c0b7b

Please sign in to comment.