lint #520
lint #520
Conversation
| workloads, err := c.store.ListNodeWorkloads(ctx, nodename, nil) | ||
| if err != nil { | ||
| log.Errorf(ctx, "[setAllWorkloadsOnNodeDown] failed to list node workloads, node %v, err: %v", nodename, errors.WithStack(err)) | ||
| return |
There was a problem hiding this comment.
需要注意一下这里跟之前的行为是不一样的，之前ListNodeWorkloads报错是会直接return err，在这里并不会阻塞后续SetNode的各种行为。我觉得这里的似乎更合理一些，但是我不一定对（逃
There was a problem hiding this comment.
我倒是关心如果 set workloads down 里出错, 恐怕很难及时知道.
似乎应该把 live 上的 sentry 部署起来.
| @@ -57,6 +57,40 @@ func (c *Calcium) GetNode(ctx context.Context, nodename string) (*types.Node, er | |||
| return node, logger.Err(ctx, errors.WithStack(err)) | |||
| } | |||
|
|
|||
| func (c *Calcium) setAllWorkloadsOnNodeDown(ctx context.Context, nodename string) { | |||
There was a problem hiding this comment.
这里没有返回error，感觉没必要，因为不想因为这里的error就终止SetNode的其它操作，而且该打的错误日志在里面也都打过了。不过看起来不太好看
There was a problem hiding this comment.
你指的是 workload 的后续 mark down 操作么
There was a problem hiding this comment.
不是，是SetNode后面的修改metadata的其它操作
There was a problem hiding this comment.
你指的是 workload 的后续 mark down 操作么
node_test.go里改了个测试用例，就是ListNodeWorkloads报错不会影响node本身metadata的更新
| } | ||
| } | ||
| } | ||
|
|
||
| // SetNode set node available or not | ||
| func (c *Calcium) SetNode(ctx context.Context, opts *types.SetNodeOptions) (*types.Node, error) { // nolint | ||
| logger := log.WithField("Calcium", "SetNode").WithField("opts", opts) |
There was a problem hiding this comment.
借个楼，L102（原L68）这句litter.Dump要不要留着？看着挺实用的。
There was a problem hiding this comment.
好像 litter.Dump 是 pretty print? 要不要改成打印成单行的日志, 这样 grep 和查询都更好一点.
|
可以推送一下 live 的 sentry |
lint之前挂了，看了一下说是这里嵌套层数太多，所以做了一点微小的改动。