From 53682557aaef928353aead235f95b216aa91b91f Mon Sep 17 00:00:00 2001 From: Matheus Moraes Date: Mon, 27 Mar 2023 11:00:02 -0300 Subject: [PATCH 1/9] feat: check metrics server in cluster sanitizer --- internal/sanitize/cluster.go | 16 +++++++++ internal/sanitize/cluster_test.go | 57 +++++++++++++++++++++---------- internal/scrub/cluster.go | 4 +++ 3 files changed, 59 insertions(+), 18 deletions(-) diff --git a/internal/sanitize/cluster.go b/internal/sanitize/cluster.go index 1ce709dd..051a790d 100644 --- a/internal/sanitize/cluster.go +++ b/internal/sanitize/cluster.go @@ -23,6 +23,7 @@ type ( // ClusterLister list available Clusters on a cluster. ClusterLister interface { ListVersion() (string, string) + HasMetrics() bool } ) @@ -36,6 +37,21 @@ func NewCluster(co *issues.Collector, lister ClusterLister) *Cluster { // Sanitize cleanse the resource. func (c *Cluster) Sanitize(ctx context.Context) error { + if err := c.checkVersion(ctx); err != nil { + return err + } + c.checkMetricsServer(ctx) + return nil +} + +func (c *Cluster) checkMetricsServer(ctx context.Context) { + ctx = internal.WithFQN(ctx, "Metrics") + if !c.HasMetrics() { + c.AddCode(ctx, 402) + } +} + +func (c *Cluster) checkVersion(ctx context.Context) error { major, minor := c.ListVersion() m, err := strconv.Atoi(major) diff --git a/internal/sanitize/cluster_test.go b/internal/sanitize/cluster_test.go index caaebebf..17e9c028 100644 --- a/internal/sanitize/cluster_test.go +++ b/internal/sanitize/cluster_test.go @@ -4,37 +4,53 @@ import ( "context" "testing" + "github.com/stretchr/testify/assert" + "github.com/derailed/popeye/internal" "github.com/derailed/popeye/internal/client" "github.com/derailed/popeye/internal/issues" "github.com/derailed/popeye/pkg/config" - "github.com/stretchr/testify/assert" ) func TestClusterSanitize(t *testing.T) { uu := map[string]struct { major, minor string - e issues.Issues + metrics bool + e issues.Outcome }{ "good": { major: "1", minor: "15", - e: issues.Issues{ - { - GVR: "clusters", - Group: issues.Root, - Message: "[POP-406] K8s version OK", - Level: config.OkLevel, + metrics: true, + e: map[string]issues.Issues{ + "Version": { + { + GVR: "clusters", + Group: issues.Root, + Message: "[POP-406] K8s version OK", + Level: config.OkLevel, + }, }, }, }, "guizard": { major: "1", minor: "11", - e: issues.Issues{ - { - GVR: "clusters", - Group: issues.Root, - Message: "[POP-405] Is this a jurassic cluster? Might want to upgrade K8s a bit", - Level: config.WarnLevel, + metrics: false, + e: map[string]issues.Issues{ + "Version": { + { + GVR: "clusters", + Group: issues.Root, + Message: "[POP-405] Is this a jurassic cluster? Might want to upgrade K8s a bit", + Level: config.WarnLevel, + }, + }, + "Metrics": { + { + GVR: "clusters", + Group: issues.Root, + Message: "[POP-402] No metrics-server detected", + Level: config.InfoLevel, + }, }, }, }, @@ -44,10 +60,10 @@ func TestClusterSanitize(t *testing.T) { for k := range uu { u := uu[k] t.Run(k, func(t *testing.T) { - cl := NewCluster(issues.NewCollector(loadCodes(t), makeConfig(t)), newCluster(u.major, u.minor)) + cl := NewCluster(issues.NewCollector(loadCodes(t), makeConfig(t)), newCluster(u.major, u.minor, u.metrics)) assert.Nil(t, cl.Sanitize(ctx)) - assert.Equal(t, u.e, cl.Outcome()["Version"]) + assert.Equal(t, u.e, cl.Outcome()) }) } } @@ -69,12 +85,17 @@ func makeContext(gvr, section string) context.Context { type cluster struct { major, minor string + metrics bool } -func newCluster(major, minor string) cluster { - return cluster{major: major, minor: minor} +func newCluster(major, minor string, metrics bool) cluster { + return cluster{major: major, minor: minor, metrics: metrics} } func (c cluster) ListVersion() (string, string) { return c.major, c.minor } + +func (c cluster) HasMetrics() bool { + return c.metrics +} diff --git a/internal/scrub/cluster.go b/internal/scrub/cluster.go index f440f9f3..0d665df3 100644 --- a/internal/scrub/cluster.go +++ b/internal/scrub/cluster.go @@ -40,3 +40,7 @@ func NewCluster(ctx context.Context, c *Cache, codes *issues.Codes) Sanitizer { func (d *Cluster) Sanitize(ctx context.Context) error { return sanitize.NewCluster(d.Collector, d).Sanitize(ctx) } + +func (d *Cluster) HasMetrics() bool { + return d.client.HasMetrics() +} From 2282212cb93123178457b09f92e75b71eb1b2fe9 Mon Sep 17 00:00:00 2001 From: Matheus Moraes Date: Mon, 27 Mar 2023 11:00:46 -0300 Subject: [PATCH 2/9] feat: update POP-402 message --- docs/codes.md | 2 +- internal/issues/assets/codes.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/codes.md b/docs/codes.md index c3474606..32cf3940 100644 --- a/docs/codes.md +++ b/docs/codes.md @@ -57,7 +57,7 @@ | ---------- | ----------------------------------------------------------- | -------- | ---------------- | | 400 | Used? Unable to locate resource reference | 1 | | | 401 | Key "%s" used? Unable to locate key reference | 1 | | -| 402 | No metric-server detected %v | 1 | | +| 402 | No metrics-server detected | 1 | | | 403 | Deprecated %s API group "%s". Use "%s" instead | 2 | | | 404 | Deprecation check failed. %v | 1 | | | 405 | Is this a jurassic cluster? Might want to upgrade K8s a bit | 2 | | diff --git a/internal/issues/assets/codes.yml b/internal/issues/assets/codes.yml index f42e6a99..371c60c3 100644 --- a/internal/issues/assets/codes.yml +++ b/internal/issues/assets/codes.yml @@ -106,7 +106,7 @@ codes: message: Key "%s" used? Unable to locate key reference severity: 1 402: - message: No metric-server detected %v + message: No metrics-server detected severity: 1 403: message: Deprecated %s API group "%s". Use "%s" instead From 82add41a125ee61b3901d7fc5668a0207943d900 Mon Sep 17 00:00:00 2001 From: Matheus Moraes Date: Mon, 27 Mar 2023 16:41:49 -0300 Subject: [PATCH 3/9] fix: call AddErr() when clusterrolebindings list fails --- internal/scrub/cr.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/scrub/cr.go b/internal/scrub/cr.go index 076a13dd..a9d87862 100644 --- a/internal/scrub/cr.go +++ b/internal/scrub/cr.go @@ -23,29 +23,29 @@ type ClusterRole struct { // NewClusterRole return a new ClusterRole scruber. func NewClusterRole(ctx context.Context, c *Cache, codes *issues.Codes) Sanitizer { - crb := ClusterRole{ + cr := ClusterRole{ client: c.factory.Client(), Config: c.config, Collector: issues.NewCollector(codes, c.config), } var err error - crb.ClusterRole, err = c.clusterroles() + cr.ClusterRole, err = c.clusterroles() if err != nil { - crb.AddErr(ctx, err) + cr.AddErr(ctx, err) } - crb.ClusterRoleBinding, err = c.clusterrolebindings() + cr.ClusterRoleBinding, err = c.clusterrolebindings() if err != nil { - crb.AddCode(ctx, 402, err) + cr.AddErr(ctx, err) } - crb.RoleBinding, err = c.rolebindings() + cr.RoleBinding, err = c.rolebindings() if err != nil { - crb.AddErr(ctx, err) + cr.AddErr(ctx, err) } - return &crb + return &cr } // Sanitize all available ClusterRoles. From f412d8923e3631a2a8347ed389f16217e0fa9120 Mon Sep 17 00:00:00 2001 From: Matheus Moraes Date: Mon, 27 Mar 2023 16:42:05 -0300 Subject: [PATCH 4/9] fix: call AddErr() when clusterroles list fails --- internal/scrub/crb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/scrub/crb.go b/internal/scrub/crb.go index f4747e13..913966d6 100644 --- a/internal/scrub/crb.go +++ b/internal/scrub/crb.go @@ -37,7 +37,7 @@ func NewClusterRoleBinding(ctx context.Context, c *Cache, codes *issues.Codes) S crb.ClusterRole, err = c.clusterroles() if err != nil { - crb.AddCode(ctx, 402, err) + crb.AddErr(ctx, err) } crb.Role, err = c.roles() From 8c27a53e4917b0cc4bbe1a949733666c20ad67de Mon Sep 17 00:00:00 2001 From: Matheus Moraes Date: Mon, 27 Mar 2023 16:42:29 -0300 Subject: [PATCH 5/9] fix: call AddErr() when clusterroles list fails --- internal/scrub/rb.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/scrub/rb.go b/internal/scrub/rb.go index f8c76dbc..49821096 100644 --- a/internal/scrub/rb.go +++ b/internal/scrub/rb.go @@ -23,29 +23,29 @@ type RoleBinding struct { // NewRoleBinding return a new RoleBinding scruber. func NewRoleBinding(ctx context.Context, c *Cache, codes *issues.Codes) Sanitizer { - crb := RoleBinding{ + rb := RoleBinding{ client: c.factory.Client(), Config: c.config, Collector: issues.NewCollector(codes, c.config), } var err error - crb.RoleBinding, err = c.rolebindings() + rb.RoleBinding, err = c.rolebindings() if err != nil { - crb.AddErr(ctx, err) + rb.AddErr(ctx, err) } - crb.ClusterRole, err = c.clusterroles() + rb.ClusterRole, err = c.clusterroles() if err != nil { - crb.AddCode(ctx, 402, err) + rb.AddErr(ctx, err) } - crb.Role, err = c.roles() + rb.Role, err = c.roles() if err != nil { - crb.AddErr(ctx, err) + rb.AddErr(ctx, err) } - return &crb + return &rb } // Sanitize all available RoleBindings. From 3d37484b84fa30b70a206ef081853a306f7835d3 Mon Sep 17 00:00:00 2001 From: Matheus Moraes Date: Mon, 27 Mar 2023 16:42:41 -0300 Subject: [PATCH 6/9] fix: call AddErr() when clusterrolebindings list fails --- internal/scrub/ro.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/scrub/ro.go b/internal/scrub/ro.go index e801020c..6c616771 100644 --- a/internal/scrub/ro.go +++ b/internal/scrub/ro.go @@ -23,29 +23,29 @@ type Role struct { // NewRole return a new Role scruber. func NewRole(ctx context.Context, c *Cache, codes *issues.Codes) Sanitizer { - crb := Role{ + ro := Role{ client: c.factory.Client(), Config: c.config, Collector: issues.NewCollector(codes, c.config), } var err error - crb.Role, err = c.roles() + ro.Role, err = c.roles() if err != nil { - crb.AddErr(ctx, err) + ro.AddErr(ctx, err) } - crb.ClusterRoleBinding, err = c.clusterrolebindings() + ro.ClusterRoleBinding, err = c.clusterrolebindings() if err != nil { - crb.AddCode(ctx, 402, err) + ro.AddErr(ctx, err) } - crb.RoleBinding, err = c.rolebindings() + ro.RoleBinding, err = c.rolebindings() if err != nil { - crb.AddErr(ctx, err) + ro.AddErr(ctx, err) } - return &crb + return &ro } // Sanitize all available Roles. From b52cfb8dadf537d174721d9a34150d16bff83cae Mon Sep 17 00:00:00 2001 From: Matheus Moraes Date: Mon, 27 Mar 2023 16:42:50 -0300 Subject: [PATCH 7/9] fix: call AddErr() when namespaces list fails --- internal/scrub/np.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/scrub/np.go b/internal/scrub/np.go index 7e015fac..ddedc55f 100644 --- a/internal/scrub/np.go +++ b/internal/scrub/np.go @@ -37,7 +37,7 @@ func NewNetworkPolicy(ctx context.Context, c *Cache, codes *issues.Codes) Saniti n.Namespace, err = c.namespaces() if err != nil { - n.AddCode(ctx, 402, err) + n.AddErr(ctx, err) } n.Pod, err = c.pods() From 26acd53a7c953bcbbbb5a302f32415e40b334472 Mon Sep 17 00:00:00 2001 From: Matheus Moraes Date: Mon, 27 Mar 2023 16:42:58 -0300 Subject: [PATCH 8/9] fix: call AddErr() when nodes list fails --- internal/scrub/hpa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/scrub/hpa.go b/internal/scrub/hpa.go index 33c55b24..52b3dc91 100644 --- a/internal/scrub/hpa.go +++ b/internal/scrub/hpa.go @@ -67,7 +67,7 @@ func NewHorizontalPodAutoscaler(ctx context.Context, c *Cache, codes *issues.Cod h.Node, err = c.nodes() if err != nil { - h.AddCode(ctx, 402, err) + h.AddErr(ctx, err) } h.NodesMetrics, _ = c.nodesMx() From b66644dd1b52d9965d138d597307c8982aec8aba Mon Sep 17 00:00:00 2001 From: Matheus Moraes Date: Tue, 11 Apr 2023 12:38:46 -0300 Subject: [PATCH 9/9] fix: check metrics before version in cluster sanitizer --- internal/sanitize/cluster.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/sanitize/cluster.go b/internal/sanitize/cluster.go index 051a790d..1d6f8177 100644 --- a/internal/sanitize/cluster.go +++ b/internal/sanitize/cluster.go @@ -37,10 +37,10 @@ func NewCluster(co *issues.Collector, lister ClusterLister) *Cluster { // Sanitize cleanse the resource. func (c *Cluster) Sanitize(ctx context.Context) error { + c.checkMetricsServer(ctx) if err := c.checkVersion(ctx); err != nil { return err } - c.checkMetricsServer(ctx) return nil }