fix ipam default log file

Open Icarus9913 opened this issue 2 years ago • 4 comments

Signed-off-by: Icarus9913 [email protected]

Aug 09 '22 09:08 Icarus9913

Codecov Report

Merging #548 (450e856) into main (09011b3) will increase coverage by 0.55%. The diff coverage is 92.30%.

@@            Coverage Diff             @@
##             main     #548      +/-   ##
==========================================
+ Coverage   80.03%   80.58%   +0.55%     
==========================================
  Files           7        7              
  Lines         586      577       -9     
==========================================
- Hits          469      465       -4     
+ Misses         96       93       -3     
+ Partials       21       19       -2

Flag	Coverage Δ
unittests	`80.58% <92.30%> (+0.55%)`	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cmd/spiderpool/cmd/cni_types.go	`100.00% <ø> (ø)`
pkg/logutils/log.go	`71.85% <83.33%> (+0.62%)`	:arrow_up:
cmd/spiderpool/cmd/command_add.go	`70.32% <100.00%> (ø)`
cmd/spiderpool/cmd/command_delete.go	`75.64% <100.00%> (ø)`
cmd/spiderpool/cmd/utils.go	`100.00% <100.00%> (+25.00%)`	:arrow_up:

Aug 14 '22 06:08 codecov[bot]

对于日志文件的处理：

使用lumberjack配置，其Write接口会去根据指定日志文件路径，递归创建文件夹，再创建日志文件。若创建失败，则抛err给上层。但是，zap源码调用Write接口采用fmt.Fprintf函数，且忽略error，因此就会出现，创建路径失败也无任何提示，且日志也不会输出到stderr。因此，需要提前将日志文件路径递归创建成功 https://github.com/natefinch/lumberjack/blob/47ffae23317c5951a2a6267a069cf676edf53eb6/lumberjack.go#L208-L212 https://github.com/uber-go/zap/blob/4b03bc56483d64b4714462e513099701e62b43d3/zapcore/entry.go#L235-L260
建议使用zap封装的Open提前先去创建日志文件，创建失败则error向上抛，交由我们自己判断。 https://github.com/uber-go/zap/blob/4b03bc56483d64b4714462e513099701e62b43d3/sink.go#L139
参考calico和cilium源码，发现对于日志文件未能成功创建，都选择忽略。很疑惑。 https://github.com/projectcalico/calico/blob/e2e120cab7b6bd353aad75f58a04dce7ffe357e4/cni-plugin/internal/pkg/utils/utils.go#L740-L753 https://github.com/cilium/cilium/blob/4bd2478db37a6859c57372dcc97ac43922d26e90/plugins/cilium-cni/main.go#L345-L350 https://github.com/cilium/cilium/blob/4bd2478db37a6859c57372dcc97ac43922d26e90/pkg/logging/hooks/file_rotation.go#L71-L95

总结

日志文件能否创建成功在"/var/log/spidernet/spiderpool.log"取决于调用spiderpool二进制用户的权限。参考calico和cilium，直接忽略，并未panic，且日志输出与stdout/stderr。我们目前代码，会去提前递归创建日志文件路径，失败即panic。

Aug 14 '22 07:08 Icarus9913

CI中，会先在机器上运行unit test因权限不够panic

Aug 14 '22 07:08 Icarus9913

Kudos, SonarCloud Quality Gate passed!