From 659cfdcc2495d876c624a6a63976e4f9e4f81dfd Mon Sep 17 00:00:00 2001 From: YaoZengzeng Date: Fri, 24 Aug 2018 17:21:50 +0800 Subject: [PATCH] bugfix: make the output of some list operations optional Signed-off-by: YaoZengzeng --- cri/v1alpha1/cri.go | 4 +- cri/v1alpha1/cri_wrapper.go | 86 ++++++++++++++++++++---------------- cri/v1alpha2/cri.go | 6 +-- cri/v1alpha2/cri_wrapper.go | 88 +++++++++++++++++++++---------------- 4 files changed, 104 insertions(+), 80 deletions(-) diff --git a/cri/v1alpha1/cri.go b/cri/v1alpha1/cri.go index 71c105320b..0dc3aec66a 100644 --- a/cri/v1alpha1/cri.go +++ b/cri/v1alpha1/cri.go @@ -162,7 +162,7 @@ func NewCriManager(config *config.Config, ctrMgr mgr.ContainerMgr, imgMgr mgr.Im ) snapshotsSyncer.Start() - return NewCriWrapper(c), nil + return NewCriWrapper(c, config.Debug), nil } // StreamServerStart starts the stream server of CRI. @@ -621,7 +621,7 @@ func (c *CriManager) ListContainers(ctx context.Context, r *runtime.ListContaine for _, c := range containerList { container, err := toCriContainer(c) if err != nil { - // TODO: log an error message? + logrus.Warnf("failed to translate container %v to cri container in ListContainers: %v", c.ID, err) continue } containers = append(containers, container) diff --git a/cri/v1alpha1/cri_wrapper.go b/cri/v1alpha1/cri_wrapper.go index 0305e9d1ee..38038617e1 100644 --- a/cri/v1alpha1/cri_wrapper.go +++ b/cri/v1alpha1/cri_wrapper.go @@ -9,11 +9,15 @@ import ( // CriWrapper wraps CriManager and logs each operation for debugging convenice. type CriWrapper struct { *CriManager + + // Only when debug is true will we output the log of some + // list operations which is usually verbose and useless. + debug bool } // NewCriWrapper creates a brand new CriWrapper. -func NewCriWrapper(c *CriManager) *CriWrapper { - return &CriWrapper{CriManager: c} +func NewCriWrapper(c *CriManager, debug bool) *CriWrapper { + return &CriWrapper{CriManager: c, debug: debug} } // StreamServerStart starts the stream server of CRI. @@ -100,15 +104,17 @@ func (c *CriWrapper) PodSandboxStatus(ctx context.Context, r *runtime.PodSandbox // ListPodSandbox returns a list of Sandbox. func (c *CriWrapper) ListPodSandbox(ctx context.Context, r *runtime.ListPodSandboxRequest) (res *runtime.ListPodSandboxResponse, err error) { - logrus.Infof("ListPodSandbox with filter %+v", r.GetFilter()) - defer func() { - if err != nil { - logrus.Errorf("failed to ListPodSandbox: %v", err) - } else { - // NOTE: maybe log detailed sandbox items with higher log level. - logrus.Infof("success to ListPodSandbox: %+v", res.Items) - } - }() + if c.debug { + logrus.Infof("ListPodSandbox with filter %+v", r.GetFilter()) + defer func() { + if err != nil { + logrus.Errorf("failed to ListPodSandbox: %v", err) + } else { + // NOTE: maybe log detailed sandbox items with higher log level. + logrus.Infof("success to ListPodSandbox: %+v", res.Items) + } + }() + } return c.CriManager.ListPodSandbox(ctx, r) } @@ -170,15 +176,17 @@ func (c *CriWrapper) RemoveContainer(ctx context.Context, r *runtime.RemoveConta // ListContainers lists all containers matching the filter. func (c *CriWrapper) ListContainers(ctx context.Context, r *runtime.ListContainersRequest) (res *runtime.ListContainersResponse, err error) { - logrus.Infof("ListContainers with filter %+v", r.GetFilter()) - defer func() { - if err != nil { - logrus.Errorf("failed to list containers with filter %+v: %v", r.GetFilter(), err) - } else { - // NOTE: maybe log detailed container items with higher log level. - logrus.Infof("success to list containers with filter: %+v", r.GetFilter()) - } - }() + if c.debug { + logrus.Infof("ListContainers with filter %+v", r.GetFilter()) + defer func() { + if err != nil { + logrus.Errorf("failed to list containers with filter %+v: %v", r.GetFilter(), err) + } else { + // NOTE: maybe log detailed container items with higher log level. + logrus.Infof("success to list containers with filter: %+v", r.GetFilter()) + } + }() + } return c.CriManager.ListContainers(ctx, r) } @@ -211,14 +219,16 @@ func (c *CriWrapper) ContainerStats(ctx context.Context, r *runtime.ContainerSta // ListContainerStats returns stats of all running containers. func (c *CriWrapper) ListContainerStats(ctx context.Context, r *runtime.ListContainerStatsRequest) (res *runtime.ListContainerStatsResponse, err error) { - logrus.Infof("ListContainerStats with filter %+v", r.GetFilter()) - defer func() { - if err != nil { - logrus.Errorf("failed to get ListContainerStats: %v", err) - } else { - logrus.Infof("success to get ListContainerStats: %+v", res.GetStats()) - } - }() + if c.debug { + logrus.Infof("ListContainerStats with filter %+v", r.GetFilter()) + defer func() { + if err != nil { + logrus.Errorf("failed to get ListContainerStats: %v", err) + } else { + logrus.Infof("success to get ListContainerStats: %+v", res.GetStats()) + } + }() + } return c.CriManager.ListContainerStats(ctx, r) } @@ -317,15 +327,17 @@ func (c *CriWrapper) Status(ctx context.Context, r *runtime.StatusRequest) (res // ListImages lists existing images. func (c *CriWrapper) ListImages(ctx context.Context, r *runtime.ListImagesRequest) (res *runtime.ListImagesResponse, err error) { - logrus.Infof("ListImages with filter %+v", r.GetFilter()) - defer func() { - if err != nil { - logrus.Errorf("failed to list images with filter %+v: %v", r.GetFilter(), err) - } else { - // NOTE: maybe log detailed image items with higher log level. - logrus.Infof("success to list images with filter: %+v", r.GetFilter()) - } - }() + if c.debug { + logrus.Infof("ListImages with filter %+v", r.GetFilter()) + defer func() { + if err != nil { + logrus.Errorf("failed to list images with filter %+v: %v", r.GetFilter(), err) + } else { + // NOTE: maybe log detailed image items with higher log level. + logrus.Infof("success to list images with filter: %+v", r.GetFilter()) + } + }() + } return c.CriManager.ListImages(ctx, r) } diff --git a/cri/v1alpha2/cri.go b/cri/v1alpha2/cri.go index baf27b9e1a..a55e85f6bb 100644 --- a/cri/v1alpha2/cri.go +++ b/cri/v1alpha2/cri.go @@ -165,7 +165,7 @@ func NewCriManager(config *config.Config, ctrMgr mgr.ContainerMgr, imgMgr mgr.Im ) snapshotsSyncer.Start() - return NewCriWrapper(c), nil + return NewCriWrapper(c, config.Debug), nil } // StreamServerStart starts the stream server of CRI. @@ -636,7 +636,7 @@ func (c *CriManager) ListContainers(ctx context.Context, r *runtime.ListContaine for _, c := range containerList { container, err := toCriContainer(c) if err != nil { - // TODO: log an error message? + logrus.Warnf("failed to translate container %v to cri container in ListContainers: %v", c.ID, err) continue } containers = append(containers, container) @@ -814,7 +814,7 @@ func (c *CriManager) ListContainerStats(ctx context.Context, r *runtime.ListCont for _, container := range containers { cs, err := c.getContainerMetrics(ctx, container) if err != nil { - logrus.Errorf("failed to decode metrics of container %q: %v", container.ID, err) + logrus.Warnf("failed to decode metrics of container %q: %v", container.ID, err) continue } diff --git a/cri/v1alpha2/cri_wrapper.go b/cri/v1alpha2/cri_wrapper.go index 2ba949aded..b68fe4e376 100644 --- a/cri/v1alpha2/cri_wrapper.go +++ b/cri/v1alpha2/cri_wrapper.go @@ -10,11 +10,15 @@ import ( // CriWrapper wraps CriManager and logs each operation for debugging convenice. type CriWrapper struct { *CriManager + + // Only when debug is true will we output the log of some + // list operations which is usually verbose and useless. + debug bool } // NewCriWrapper creates a brand new CriWrapper. -func NewCriWrapper(c *CriManager) *CriWrapper { - return &CriWrapper{CriManager: c} +func NewCriWrapper(c *CriManager, debug bool) *CriWrapper { + return &CriWrapper{CriManager: c, debug: debug} } // StreamServerStart starts the stream server of CRI. @@ -101,15 +105,17 @@ func (c *CriWrapper) PodSandboxStatus(ctx context.Context, r *runtime.PodSandbox // ListPodSandbox returns a list of Sandbox. func (c *CriWrapper) ListPodSandbox(ctx context.Context, r *runtime.ListPodSandboxRequest) (res *runtime.ListPodSandboxResponse, err error) { - logrus.Infof("ListPodSandbox with filter %+v", r.GetFilter()) - defer func() { - if err != nil { - logrus.Errorf("failed to ListPodSandbox: %v", err) - } else { - // NOTE: maybe log detailed sandbox items with higher log level. - logrus.Infof("success to ListPodSandbox: %+v", res.Items) - } - }() + if c.debug { + logrus.Infof("ListPodSandbox with filter %+v", r.GetFilter()) + defer func() { + if err != nil { + logrus.Errorf("failed to ListPodSandbox: %v", err) + } else { + // NOTE: maybe log detailed sandbox items with higher log level. + logrus.Infof("success to ListPodSandbox: %+v", res.Items) + } + }() + } return c.CriManager.ListPodSandbox(ctx, r) } @@ -171,15 +177,17 @@ func (c *CriWrapper) RemoveContainer(ctx context.Context, r *runtime.RemoveConta // ListContainers lists all containers matching the filter. func (c *CriWrapper) ListContainers(ctx context.Context, r *runtime.ListContainersRequest) (res *runtime.ListContainersResponse, err error) { - logrus.Infof("ListContainers with filter %+v", r.GetFilter()) - defer func() { - if err != nil { - logrus.Errorf("failed to list containers with filter %+v: %v", r.GetFilter(), err) - } else { - // NOTE: maybe log detailed container items with higher log level. - logrus.Infof("success to list containers with filter: %+v", r.GetFilter()) - } - }() + if c.debug { + logrus.Infof("ListContainers with filter %+v", r.GetFilter()) + defer func() { + if err != nil { + logrus.Errorf("failed to list containers with filter %+v: %v", r.GetFilter(), err) + } else { + // NOTE: maybe log detailed container items with higher log level. + logrus.Infof("success to list containers with filter: %+v", r.GetFilter()) + } + }() + } return c.CriManager.ListContainers(ctx, r) } @@ -188,7 +196,7 @@ func (c *CriWrapper) ContainerStatus(ctx context.Context, r *runtime.ContainerSt logrus.Infof("ContainerStatus for %q", r.GetContainerId()) defer func() { if err != nil { - logrus.Errorf("failed to get ContainerStatus: %q, %v", r.GetContainerId(), err) + logrus.Warnf("failed to get ContainerStatus: %q, %v", r.GetContainerId(), err) } else { logrus.Infof("success to get ContainerStatus: %q, %+v", r.GetContainerId(), res.GetStatus()) } @@ -212,14 +220,16 @@ func (c *CriWrapper) ContainerStats(ctx context.Context, r *runtime.ContainerSta // ListContainerStats returns stats of all running containers. func (c *CriWrapper) ListContainerStats(ctx context.Context, r *runtime.ListContainerStatsRequest) (res *runtime.ListContainerStatsResponse, err error) { - logrus.Infof("ListContainerStats with filter %+v", r.GetFilter()) - defer func() { - if err != nil { - logrus.Errorf("failed to get ListContainerStats: %v", err) - } else { - logrus.Infof("success to get ListContainerStats: %+v", res.GetStats()) - } - }() + if c.debug { + logrus.Infof("ListContainerStats with filter %+v", r.GetFilter()) + defer func() { + if err != nil { + logrus.Errorf("failed to get ListContainerStats: %v", err) + } else { + logrus.Infof("success to get ListContainerStats: %+v", res.GetStats()) + } + }() + } return c.CriManager.ListContainerStats(ctx, r) } @@ -335,15 +345,17 @@ func (c *CriWrapper) Status(ctx context.Context, r *runtime.StatusRequest) (res // ListImages lists existing images. func (c *CriWrapper) ListImages(ctx context.Context, r *runtime.ListImagesRequest) (res *runtime.ListImagesResponse, err error) { - logrus.Infof("ListImages with filter %+v", r.GetFilter()) - defer func() { - if err != nil { - logrus.Errorf("failed to list images with filter %+v: %v", r.GetFilter(), err) - } else { - // NOTE: maybe log detailed image items with higher log level. - logrus.Infof("success to list images with filter: %+v", r.GetFilter()) - } - }() + if c.debug { + logrus.Infof("ListImages with filter %+v", r.GetFilter()) + defer func() { + if err != nil { + logrus.Errorf("failed to list images with filter %+v: %v", r.GetFilter(), err) + } else { + // NOTE: maybe log detailed image items with higher log level. + logrus.Infof("success to list images with filter: %+v", r.GetFilter()) + } + }() + } return c.CriManager.ListImages(ctx, r) }