Add a log-level flag to set the log verbosity level #380

NilanjanDaw · 2022-02-14T07:45:26Z

What this PR does / why we need it:
Enabled k8s complaint logging, and set default info log level at 0, and installer info log at 1. This also enables a v flag to explicitly set the verbosity level.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #265

Additional Information:
Sample logs when verbosity. is set at v 1:
./bin/byoh-hostagent-linux-amd64 -v=1

I0218 10:49:55.857830   17007 host_reconciler.go:55] controller/byohost "msg"="Reconcile request received" "name"="host1" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost"
I0218 10:49:55.865774   17007 host_reconciler.go:258] controller/byohost "msg"="Installing K8s" "name"="host1" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost"
I0218 10:49:55.865873   17007 installer.go:175]  "msg"="Current OS will be handled as"  "OS"="Ubuntu_20.04.1_x86-64"
I0218 10:49:55.868022   17007 bundle_downloader.go:69]  "msg"="Cache miss"  "path"="/var/lib/byoh/bundles/projects.registry.vmware.com.cluster_api_provider_bringyourownhost/v1.22.3-v0.1.0_alpha.2"
I0218 10:49:55.868145   17007 bundle_downloader.go:95]  "msg"="Downloading bundle"  "from"="projects.registry.vmware.com/cluster_api_provider_bringyourownhost/byoh-bundle-ubuntu_20.04.1_x86-64_k8s_v1.22.3:v0.1.0_alpha.2"
Pulling image 'projects.registry.vmware.com/cluster_api_provider_bringyourownhost/byoh-bundle-ubuntu_20.04.1_x86-64_k8s_v1.22.3@sha256:a55799ab41dd2fa980762b52fd9a0d702b8ef90a9e221b81b10309ab80133ab1'
Extracting layer 'sha256:b6881e4eb7c63056752526272d0d861a9e25e0571f26bfd3dcde26c7d75e1f1f' (1/1)
I0218 10:50:16.273861   17007 installer.go:245]  "msg"="Installing: SWAP"
I0218 10:50:16.273964   17007 installer.go:242]  "msg"="/usr/bin/bash -c swapoff -a \u0026\u0026 sed -ri '/\\sswap\\s/s/^#?/#/' /etc/fstab"
I0218 10:50:16.296747   17007 installer.go:245]  "msg"="Installing: FIREWALL"
I0218 10:50:16.296835   17007 installer.go:242]  "msg"="/usr/bin/bash -c ufw disable"
I0218 10:50:16.707648   17007 installer.go:244]  "msg"="WARN: uid is 0 but '/etc' is owned by 1001\nWARN: uid is 0 but '/usr' is owned by 1001\n"
I0218 10:50:16.707737   17007 installer.go:243]  "msg"="Firewall stopped and disabled on system startup\n"
I0218 10:50:16.707785   17007 installer.go:245]  "msg"="Installing: KERNEL MODULES"
I0218 10:50:16.707863   17007 installer.go:242]  "msg"="/usr/bin/bash -c modprobe overlay \u0026\u0026 modprobe br_netfilter"
I0218 10:50:16.718402   17007 installer.go:245]  "msg"="Installing: OS CONFIGURATION"
I0218 10:50:16.718480   17007 installer.go:242]  "msg"="/usr/bin/bash -c tar -C / -xvf '/var/lib/byoh/bundles/projects.registry.vmware.com.cluster_api_provider_bringyourownhost/v1.22.3-v0.1.0_alpha.2/conf.tar' \u0026\u0026 sysctl --system"
I0218 10:50:16.734479   17007 installer.go:244]  "msg"="sysctl: setting key \"net.ipv4.conf.all.promote_secondaries\": Invalid argument\n"

Sample logs when verbosity is set at v 0:
./bin/byoh-hostagent-linux-amd64 -v=0

I0218 10:50:47.311906   17533 host_registrar.go:37] Registering ByoHost
I0218 10:50:47.315843   17533 host_registrar.go:71] Add Network Info
I0218 10:50:48.971030   17533 deleg.go:130] controller-runtime/metrics "msg"="Metrics server is starting to listen"  "addr"=":8080"
I0218 10:50:49.051230   17533 deleg.go:130]  "msg"="Starting metrics server"  "path"="/metrics"
I0218 10:50:49.051374   17533 controller.go:178] controller/byohost "msg"="Starting EventSource" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost" "source"="kind source: *v1beta1.ByoHost"
I0218 10:50:49.051440   17533 controller.go:186] controller/byohost "msg"="Starting Controller" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost"
I0218 10:50:49.152773   17533 controller.go:220] controller/byohost "msg"="Starting workers" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost" "worker count"=1
I0218 10:50:49.152969   17533 host_reconciler.go:55] controller/byohost "msg"="Reconcile request received" "name"="host1" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost"
I0218 10:50:49.158968   17533 host_reconciler.go:258] controller/byohost "msg"="Installing K8s" "name"="host1" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost"
I0218 10:51:06.888301   17533 host_reconciler.go:182] controller/byohost "msg"="cleaning up directory /run/kubeadm/*" "name"="host1" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost"
I0218 10:51:06.888634   17533 host_reconciler.go:182] controller/byohost "msg"="cleaning up directory /etc/cni/net.d/*" "name"="host1" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost"
I0218 10:51:06.889059   17533 host_reconciler.go:249] controller/byohost "msg"="Bootstraping k8s Node" "name"="host1" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost"
I0218 10:51:24.833477   17533 host_reconciler.go:141] controller/byohost "msg"="k8s node successfully bootstrapped" "name"="host1" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="ByoHost"

codecov-commenter · 2022-02-14T07:52:09Z

Codecov Report

Merging #380 (f80281a) into main (d3cc0d7) will decrease coverage by 0.91%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #380      +/-   ##
==========================================
- Coverage   68.45%   67.54%   -0.92%     
==========================================
  Files          23       23              
  Lines        1693     1716      +23     
==========================================
  Hits         1159     1159              
- Misses        462      485      +23     
  Partials       72       72

Impacted Files	Coverage Δ
agent/main.go	`16.90% <0.00%> (-3.27%)`	⬇️

dharmjit · 2022-02-14T07:56:02Z

Thanks for this PR @NilanjanDaw, Is it possible to add some sample log output for both levels.

NilanjanDaw · 2022-02-14T08:10:30Z

Is it possible to add some sample log output for both levels.

Added sample log details to the PR under Additional Information

anusha94

lgtm

dharmjit · 2022-02-14T10:27:18Z

Is it possible to add some sample log output for both levels.

Added sample log details to the PR under Additional Information

Isn't it that with increasing log verbose levels we get more logs. source - https://github.com/go-logr/logr#how-do-i-choose-my-v-levels.

Also, Do you see installers detailed logs with increased log level verbosity?

pshail · 2022-02-14T11:33:47Z

@NilanjanDaw / @dharmjit - i recommend align to this https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md

NilanjanDaw · 2022-02-14T16:36:16Z

If we are to enable mulit-level verbosity then I think we need to modify our logging statements from logger.Info("this is log statement") to something like logger.V(<log_level>).Info("this is log statement") as suggested here https://github.com/kubernetes/klog/blob/v1.0.0/examples/klogr/main.go

@pshail @dharmjit @anusha94 Is that something we want to do?

pshail · 2022-02-15T07:37:28Z

as long as the set verbosity level is used across the instance of the run we should be fine, keeping different level at different part of the code pieces can get very confusing.
I would suggest to align the verbosity level, to begin with, as per the k8s sig recommendations and rest is fine as implemented in terms of defaulting to one level and giving log-level flag to override in a specific run, that should just work fine without the need to introduce addition call to .V(level) for each info level log i presume

agent/main.go

agent/help_flag_test.go

agent/main.go

Setting two log levels, a log level of 0 prints Info level logs and above, while log level of 1 and above prints error logs only

- Update the flag handling library from go-flag to pflag - Hide unnecessary commandline flags - Set default log level to 0 - set installer log level at 1

anusha94 · 2022-02-18T12:01:15Z

Make sure to squash the commits and messages before merge.

agent/main.go

dharmjit

lgtm

vmwclabot added the cla-not-required label Feb 14, 2022

NilanjanDaw self-assigned this Feb 14, 2022

NilanjanDaw requested review from dharmjit and anusha94 February 14, 2022 07:48

anusha94 previously approved these changes Feb 14, 2022

View reviewed changes

NilanjanDaw enabled auto-merge (squash) February 14, 2022 10:05

NilanjanDaw dismissed anusha94’s stale review via 41df339 February 16, 2022 06:08

NilanjanDaw requested review from pshail and anusha94 February 16, 2022 06:43

pshail reviewed Feb 16, 2022

View reviewed changes

agent/main.go Outdated Show resolved Hide resolved

pshail reviewed Feb 16, 2022

View reviewed changes

agent/main.go Outdated Show resolved Hide resolved

anusha94 reviewed Feb 16, 2022

View reviewed changes

agent/help_flag_test.go Outdated Show resolved Hide resolved

dharmjit reviewed Feb 16, 2022

View reviewed changes

agent/help_flag_test.go Outdated Show resolved Hide resolved

agent/main.go Outdated Show resolved Hide resolved

pshail previously approved these changes Feb 18, 2022

View reviewed changes

NilanjanDaw added 6 commits February 18, 2022 09:06

Add a log-level flag to set the log verbosity level

85d4249

Setting two log levels, a log level of 0 prints Info level logs and above, while log level of 1 and above prints error logs only

moving to k8s style verbosity management

e1c9849

enabling k8s compliant logging

a5740be

added error handling

fd96e6f

reducing size of main

36855bd

Update flag handling to pflag and set default log to 0

f80281a

- Update the flag handling library from go-flag to pflag - Hide unnecessary commandline flags - Set default log level to 0 - set installer log level at 1

NilanjanDaw dismissed pshail’s stale review via f80281a February 18, 2022 10:59

NilanjanDaw force-pushed the adjust-verbosity-installer branch from 9aeffca to f80281a Compare February 18, 2022 10:59

github-actions bot added the area/test-release label Feb 18, 2022

NilanjanDaw disabled auto-merge February 18, 2022 11:01

NilanjanDaw added 2 commits February 18, 2022 11:10

fix linting error

014eccd

aesthetic changes

b4b791b

anusha94 approved these changes Feb 18, 2022

View reviewed changes

NilanjanDaw requested review from dharmjit and pshail February 18, 2022 12:03

dharmjit reviewed Feb 21, 2022

View reviewed changes

agent/main.go Show resolved Hide resolved

NilanjanDaw requested a review from dharmjit February 21, 2022 04:48

dharmjit approved these changes Feb 21, 2022

View reviewed changes

NilanjanDaw merged commit fb51a6d into vmware-tanzu:main Feb 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a log-level flag to set the log verbosity level #380

Add a log-level flag to set the log verbosity level #380

NilanjanDaw commented Feb 14, 2022 •

edited

Loading

codecov-commenter commented Feb 14, 2022 •

edited

Loading

dharmjit commented Feb 14, 2022

NilanjanDaw commented Feb 14, 2022 •

edited

Loading

anusha94 left a comment

dharmjit commented Feb 14, 2022

pshail commented Feb 14, 2022

NilanjanDaw commented Feb 14, 2022 •

edited

Loading

pshail commented Feb 15, 2022 •

edited

Loading

anusha94 commented Feb 18, 2022

dharmjit left a comment

Add a log-level flag to set the log verbosity level #380

Add a log-level flag to set the log verbosity level #380

Conversation

NilanjanDaw commented Feb 14, 2022 • edited Loading

codecov-commenter commented Feb 14, 2022 • edited Loading

Codecov Report

dharmjit commented Feb 14, 2022

NilanjanDaw commented Feb 14, 2022 • edited Loading

anusha94 left a comment

Choose a reason for hiding this comment

dharmjit commented Feb 14, 2022

pshail commented Feb 14, 2022

NilanjanDaw commented Feb 14, 2022 • edited Loading

pshail commented Feb 15, 2022 • edited Loading

anusha94 commented Feb 18, 2022

dharmjit left a comment

Choose a reason for hiding this comment

NilanjanDaw commented Feb 14, 2022 •

edited

Loading

codecov-commenter commented Feb 14, 2022 •

edited

Loading

NilanjanDaw commented Feb 14, 2022 •

edited

Loading

NilanjanDaw commented Feb 14, 2022 •

edited

Loading

pshail commented Feb 15, 2022 •

edited

Loading