From 5d0922536977e8bcbec8f37a72b708d9218764c1 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Thu, 27 Jun 2024 11:53:22 -0300 Subject: [PATCH 01/21] fix typo --- pkg/config/checks/missingPodDisruptionBudget.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/checks/missingPodDisruptionBudget.yaml b/pkg/config/checks/missingPodDisruptionBudget.yaml index 9741544c8..e99a80dcc 100644 --- a/pkg/config/checks/missingPodDisruptionBudget.yaml +++ b/pkg/config/checks/missingPodDisruptionBudget.yaml @@ -14,7 +14,7 @@ schema: properties: template: type: object - properites: + properties: metadata: type: object properties: From 655b247455e964845388305f640e3d83806a7946 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Thu, 27 Jun 2024 11:53:30 -0300 Subject: [PATCH 02/21] fix failure message --- pkg/config/checks/hpaMinAvailability.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/checks/hpaMinAvailability.yaml b/pkg/config/checks/hpaMinAvailability.yaml index 5bc00104e..b57eecc9a 100644 --- a/pkg/config/checks/hpaMinAvailability.yaml +++ b/pkg/config/checks/hpaMinAvailability.yaml @@ -1,5 +1,5 @@ successMessage: HPA has a valid min replica configuration -failureMessage: HPA maxReplicas should be greater than minReplicas +failureMessage: HPA minReplicas should be 2 or more category: Reliability target: autoscaling/HorizontalPodAutoscaler schema: From ede7f2ae82d0e27620618b6148f7367e9a7665e5 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Thu, 27 Jun 2024 11:56:46 -0300 Subject: [PATCH 03/21] fix changelog --- docs/changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.md b/docs/changelog.md index cf4a8fac7..a828380f6 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -6,6 +6,10 @@ meta: --- +## 9.1.1 +* Fix `hpaMinAvailability` failure message +* Fix `missingPodDisruptionBudget` typo + ## 9.1.0 * Add HPA `minAvailable` and HPA `maxAvailable` checks * Fix typo for PDB `minAvailable` From 11f6f8e3d30080dcdd603fe362add87849cad358 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Thu, 27 Jun 2024 14:11:19 -0300 Subject: [PATCH 04/21] fix missingPodDisruptionBudget validation --- .../checks/missingPodDisruptionBudget.yaml | 12 ++++++++-- .../failure.empty-labels.yaml | 22 +++++++++++++++++++ .../failure.no-labels.yaml | 21 ++++++++++++++++++ .../failure.no-metadata.yaml | 20 +++++++++++++++++ 4 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 test/checks/missingPodDisruptionBudget/failure.empty-labels.yaml create mode 100644 test/checks/missingPodDisruptionBudget/failure.no-labels.yaml create mode 100644 test/checks/missingPodDisruptionBudget/failure.no-metadata.yaml diff --git a/pkg/config/checks/missingPodDisruptionBudget.yaml b/pkg/config/checks/missingPodDisruptionBudget.yaml index e99a80dcc..1ff42ad85 100644 --- a/pkg/config/checks/missingPodDisruptionBudget.yaml +++ b/pkg/config/checks/missingPodDisruptionBudget.yaml @@ -4,9 +4,9 @@ category: Reliability target: Controller controllers: include: - - Deployment + - Deployment schema: - '$schema': http://json-schema.org/draft-07/schema + "$schema": http://json-schema.org/draft-07/schema# type: object properties: spec: @@ -21,6 +21,14 @@ schema: labels: type: object minProperties: 1 + required: + - labels + required: + - metadata + required: + - template + required: + - spec additionalSchemaStrings: policy/PodDisruptionBudget: | type: object diff --git a/test/checks/missingPodDisruptionBudget/failure.empty-labels.yaml b/test/checks/missingPodDisruptionBudget/failure.empty-labels.yaml new file mode 100644 index 000000000..848ac002c --- /dev/null +++ b/test/checks/missingPodDisruptionBudget/failure.empty-labels.yaml @@ -0,0 +1,22 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: {} # missing labels + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 2 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper diff --git a/test/checks/missingPodDisruptionBudget/failure.no-labels.yaml b/test/checks/missingPodDisruptionBudget/failure.no-labels.yaml new file mode 100644 index 000000000..c65d6924f --- /dev/null +++ b/test/checks/missingPodDisruptionBudget/failure.no-labels.yaml @@ -0,0 +1,21 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: {} # missing labels + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 2 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper diff --git a/test/checks/missingPodDisruptionBudget/failure.no-metadata.yaml b/test/checks/missingPodDisruptionBudget/failure.no-metadata.yaml new file mode 100644 index 000000000..2234eac5a --- /dev/null +++ b/test/checks/missingPodDisruptionBudget/failure.no-metadata.yaml @@ -0,0 +1,20 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 2 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper From 2e305a3628703c0a5a52026582f2c482ec039101 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Thu, 27 Jun 2024 17:56:27 -0300 Subject: [PATCH 05/21] add tests for pdbMinAvailableLessThenHPAMaxReplicas --- .../checks/missingPodDisruptionBudget.yaml | 12 ++---- .../failure.yaml | 42 +++++++++++++++++++ .../success.equals.yaml | 42 +++++++++++++++++++ .../success.lt.yaml | 42 +++++++++++++++++++ .../success.no-hpa.yaml | 24 +++++++++++ .../success.no-match.yaml | 42 +++++++++++++++++++ .../success.no-pdb.yaml | 32 ++++++++++++++ 7 files changed, 228 insertions(+), 8 deletions(-) create mode 100644 test/checks/pdbMinAvailableLessThenHPAMaxReplicas/failure.yaml create mode 100644 test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.equals.yaml create mode 100644 test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.lt.yaml create mode 100644 test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-hpa.yaml create mode 100644 test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-match.yaml create mode 100644 test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-pdb.yaml diff --git a/pkg/config/checks/missingPodDisruptionBudget.yaml b/pkg/config/checks/missingPodDisruptionBudget.yaml index 1ff42ad85..434299be4 100644 --- a/pkg/config/checks/missingPodDisruptionBudget.yaml +++ b/pkg/config/checks/missingPodDisruptionBudget.yaml @@ -8,27 +8,23 @@ controllers: schema: "$schema": http://json-schema.org/draft-07/schema# type: object + required: [spec] properties: spec: type: object + required: [template] properties: template: type: object + required: [metadata] properties: metadata: type: object + required: [labels] properties: labels: type: object minProperties: 1 - required: - - labels - required: - - metadata - required: - - template - required: - - spec additionalSchemaStrings: policy/PodDisruptionBudget: | type: object diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/failure.yaml b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/failure.yaml new file mode 100644 index 000000000..26912d9e0 --- /dev/null +++ b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/failure.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 10 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: zookeeper + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.equals.yaml b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.equals.yaml new file mode 100644 index 000000000..a3ae87687 --- /dev/null +++ b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.equals.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 5 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: zookeeper + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.lt.yaml b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.lt.yaml new file mode 100644 index 000000000..874dca5b1 --- /dev/null +++ b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.lt.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 2 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: zookeeper + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-hpa.yaml b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-hpa.yaml new file mode 100644 index 000000000..9e9f0d5ce --- /dev/null +++ b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-hpa.yaml @@ -0,0 +1,24 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 5 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-match.yaml b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-match.yaml new file mode 100644 index 000000000..5993d4ab5 --- /dev/null +++ b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-match.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 5 + selector: + matchLabels: + app.kubernetes.io/name: no-match +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: no-match + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-pdb.yaml b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-pdb.yaml new file mode 100644 index 000000000..818153354 --- /dev/null +++ b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-pdb.yaml @@ -0,0 +1,32 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: zookeeper + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 From d50c665fd79df35f5213c813c165f9a5f6697610 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Thu, 27 Jun 2024 17:57:37 -0300 Subject: [PATCH 06/21] add simple success test --- .../success.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.yaml diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.yaml b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.yaml new file mode 100644 index 000000000..034257aad --- /dev/null +++ b/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.yaml @@ -0,0 +1,14 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper \ No newline at end of file From 584e0bdde7435ec3ea97092cc0e7a4866088b597 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Fri, 28 Jun 2024 11:47:40 -0300 Subject: [PATCH 07/21] fix typo --- .../failure.yaml | 0 .../success.equals.yaml | 0 .../success.lt.yaml | 0 .../success.no-hpa.yaml | 0 .../success.no-match.yaml | 0 .../success.no-pdb.yaml | 0 .../success.yaml | 0 7 files changed, 0 insertions(+), 0 deletions(-) rename test/checks/{pdbMinAvailableLessThenHPAMaxReplicas => pdbMinAvailableLessThanHPAMaxReplicas}/failure.yaml (100%) rename test/checks/{pdbMinAvailableLessThenHPAMaxReplicas => pdbMinAvailableLessThanHPAMaxReplicas}/success.equals.yaml (100%) rename test/checks/{pdbMinAvailableLessThenHPAMaxReplicas => pdbMinAvailableLessThanHPAMaxReplicas}/success.lt.yaml (100%) rename test/checks/{pdbMinAvailableLessThenHPAMaxReplicas => pdbMinAvailableLessThanHPAMaxReplicas}/success.no-hpa.yaml (100%) rename test/checks/{pdbMinAvailableLessThenHPAMaxReplicas => pdbMinAvailableLessThanHPAMaxReplicas}/success.no-match.yaml (100%) rename test/checks/{pdbMinAvailableLessThenHPAMaxReplicas => pdbMinAvailableLessThanHPAMaxReplicas}/success.no-pdb.yaml (100%) rename test/checks/{pdbMinAvailableLessThenHPAMaxReplicas => pdbMinAvailableLessThanHPAMaxReplicas}/success.yaml (100%) diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/failure.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThenHPAMaxReplicas/failure.yaml rename to test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure.yaml diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.equals.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.equals.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.equals.yaml rename to test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.equals.yaml diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.lt.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.lt.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.lt.yaml rename to test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.lt.yaml diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-hpa.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-hpa.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-hpa.yaml rename to test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-hpa.yaml diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-match.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-match.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-match.yaml rename to test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-match.yaml diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-pdb.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-pdb.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.no-pdb.yaml rename to test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-pdb.yaml diff --git a/test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThenHPAMaxReplicas/success.yaml rename to test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.yaml From 151177027524a7f7aab2b587c552c35120b8aeaf Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Fri, 28 Jun 2024 13:05:00 -0300 Subject: [PATCH 08/21] lowercasing warnings --- pkg/kube/resource.go | 10 +++++----- pkg/kube/resources.go | 6 +++--- pkg/validator/schema.go | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/kube/resource.go b/pkg/kube/resource.go index 95d4eb29c..55c47951e 100644 --- a/pkg/kube/resource.go +++ b/pkg/kube/resource.go @@ -181,7 +181,7 @@ func resolveControllerFromPod(ctx context.Context, podResource kubeAPICoreV1.Pod err = cacheAllObjectsOfKind(ctx, firstOwner.APIVersion, firstOwner.Kind, dynamicClient, restMapper, objectCache) } if err != nil { - logrus.Warnf("Error caching objects of Kind %s %v", firstOwner.Kind, err) + logrus.Warnf("error caching objects of Kind %s %v", firstOwner.Kind, err) break } abstractObject, ok = objectCache[key] @@ -193,7 +193,7 @@ func resolveControllerFromPod(ctx context.Context, podResource kubeAPICoreV1.Pod objMeta, err := meta.Accessor(&abstractObject) if err != nil { - logrus.Warnf("Error retrieving parent metadata %s of API %s and Kind %s because of error: %v ", firstOwner.Name, firstOwner.APIVersion, firstOwner.Kind, err) + logrus.Warnf("error retrieving parent metadata %s of API %s and Kind %s because of error: %v ", firstOwner.Name, firstOwner.APIVersion, firstOwner.Kind, err) return GenericResource{}, err } podSpec := GetPodSpec(abstractObject.Object) @@ -221,7 +221,7 @@ func cacheSingleObject(ctx context.Context, apiVersion, kind, namespace, name st logrus.Debugf("Caching a single %s", kind) object, err := getObject(ctx, namespace, kind, apiVersion, name, dynamicClient, restMapper) if err != nil { - logrus.Warnf("Error retrieving object %s/%s/%s/%s because of error: %v", kind, apiVersion, namespace, name, err) + logrus.Warnf("error retrieving object %s/%s/%s/%s because of error: %v", kind, apiVersion, namespace, name, err) return err } key := fmt.Sprintf("%s/%s/%s", object.GetKind(), object.GetNamespace(), object.GetName()) @@ -235,13 +235,13 @@ func cacheAllObjectsOfKind(ctx context.Context, apiVersion, kind string, dynamic fqKind := schema.FromAPIVersionAndKind(apiVersion, kind) mapping, err := restMapper.RESTMapping(fqKind.GroupKind(), fqKind.Version) if err != nil { - logrus.Warnf("Error retrieving mapping of API %s and Kind %s because of error: %v", apiVersion, kind, err) + logrus.Warnf("error retrieving mapping of API %s and Kind %s because of error: %v", apiVersion, kind, err) return err } objects, err := dynamicClient.Resource(mapping.Resource).Namespace("").List(ctx, kubeAPIMetaV1.ListOptions{}) if err != nil { - logrus.Warnf("Error retrieving parent object API %s and Kind %s because of error: %v", mapping.Resource.Version, mapping.Resource.Resource, err) + logrus.Warnf("error retrieving parent object API %s and Kind %s because of error: %v", mapping.Resource.Version, mapping.Resource.Resource, err) return err } for idx, object := range objects.Items { diff --git a/pkg/kube/resources.go b/pkg/kube/resources.go index 3ae7de083..5fe419606 100644 --- a/pkg/kube/resources.go +++ b/pkg/kube/resources.go @@ -206,7 +206,7 @@ func CreateResourceProviderFromPath(directory string) (*ResourceProvider, error) } err = resources.addResourcesFromYaml(string(contents)) if err != nil { - logrus.Warnf("Skipping %s: cannot add resource from YAML: %v", path, err) + logrus.Warnf("skipping %s: cannot add resource from YAML: %v", path, err) } return nil } @@ -340,7 +340,7 @@ func CreateResourceProviderFromAPI(ctx context.Context, kube kubernetes.Interfac groupKind := parseGroupKind(maybeTransformKindIntoGroupKind(string(kind))) mapping, err := restMapper.RESTMapping(groupKind) if err != nil { - logrus.Warnf("Error retrieving mapping of Kind %s because of error: %v", kind, err) + logrus.Warnf("error retrieving mapping of Kind %s because of error: %v", kind, err) return nil, err } if c.Namespace != "" && mapping.Scope.Name() != meta.RESTScopeNameNamespace { @@ -351,7 +351,7 @@ func CreateResourceProviderFromAPI(ctx context.Context, kube kubernetes.Interfac logrus.Info("Loading " + kind) objects, err := dynamic.Resource(mapping.Resource).Namespace(c.Namespace).List(ctx, metav1.ListOptions{}) if err != nil { - logrus.Warnf("Error retrieving parent object API %s and Kind %s because of error: %v", mapping.Resource.Version, mapping.Resource.Resource, err) + logrus.Warnf("error retrieving parent object API %s and Kind %s because of error: %v", mapping.Resource.Version, mapping.Resource.Resource, err) return nil, err } for _, obj := range objects.Items { diff --git a/pkg/validator/schema.go b/pkg/validator/schema.go index 79f19bfe3..c35fefa93 100644 --- a/pkg/validator/schema.go +++ b/pkg/validator/schema.go @@ -380,7 +380,7 @@ func applySchemaCheck(conf *config.Configuration, checkID string, test schemaTes break } if test.ResourceProvider == nil { - logrus.Warnf("No ResourceProvider available, check %s will not work in this context (e.g. admission control)", checkID) + logrus.Warnf("no ResourceProvider available, check %s will not work in this context (e.g. admission control)", checkID) break } resources := test.ResourceProvider.Resources[groupkind] From 777abaffda528680b742a3994117ed175d42a210 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Mon, 1 Jul 2024 11:40:51 -0300 Subject: [PATCH 09/21] WIP implement pdbMinAvailableLessThanHPAMaxReplicas --- docs/checks/reliability.md | 1 + pkg/config/checks.go | 1 + ...pdbMinAvailableLessThanHPAMaxReplicas.yaml | 7 + pkg/config/default.yaml | 1 + pkg/config/examples/config-full.yaml | 1 + pkg/validator/custom.go | 19 +++ pkg/validator/pdbhpavalidator.go | 150 ++++++++++++++++++ pkg/validator/schema.go | 2 + .../failure-percent.yaml | 43 +++++ .../{failure.yaml => failure-scalar.yaml} | 0 .../success.hpa-no-match.yaml | 42 +++++ .../success.pdb-no-match.yaml | 42 +++++ .../success.percent-no-replica.yaml | 42 +++++ test/mutation_test.go | 2 + test/schema_test.go | 15 +- 15 files changed, 362 insertions(+), 6 deletions(-) create mode 100644 pkg/config/checks/pdbMinAvailableLessThanHPAMaxReplicas.yaml create mode 100644 pkg/validator/custom.go create mode 100644 pkg/validator/pdbhpavalidator.go create mode 100644 test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-percent.yaml rename test/checks/pdbMinAvailableLessThanHPAMaxReplicas/{failure.yaml => failure-scalar.yaml} (100%) create mode 100644 test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.hpa-no-match.yaml create mode 100644 test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.pdb-no-match.yaml create mode 100644 test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.percent-no-replica.yaml diff --git a/docs/checks/reliability.md b/docs/checks/reliability.md index b551e1f69..74c613076 100644 --- a/docs/checks/reliability.md +++ b/docs/checks/reliability.md @@ -21,6 +21,7 @@ key | default | description `topologySpreadConstraint` | `warning` | Fails when there is no topology spread constraint on the pod `hpaMaxAvailability` | `warning` | Fails when `maxAvailable` lesser or equal than `minAvailable` (if defined) for a HorizontalPodAutoscaler `hpaMinAvailability` | `warning` | Fails when `minAvailable` (if defined) lesser or equal to one for a HorizontalPodAutoscaler +`pdbMinAvailableLessThanHPAMaxReplicas` | `warning` | Fails when PDB `minAvailable` is greater than HPA `maxReplicas` ## Background diff --git a/pkg/config/checks.go b/pkg/config/checks.go index 7d1cb334d..032b493bf 100644 --- a/pkg/config/checks.go +++ b/pkg/config/checks.go @@ -69,6 +69,7 @@ var ( "rolebindingClusterAdminRole", "hpaMaxAvailability", "hpaMinAvailability", + "pdbMinAvailableLessThanHPAMaxReplicas", } // BuiltInChecks contains the checks that come pre-installed w/ Polaris diff --git a/pkg/config/checks/pdbMinAvailableLessThanHPAMaxReplicas.yaml b/pkg/config/checks/pdbMinAvailableLessThanHPAMaxReplicas.yaml new file mode 100644 index 000000000..13f66b076 --- /dev/null +++ b/pkg/config/checks/pdbMinAvailableLessThanHPAMaxReplicas.yaml @@ -0,0 +1,7 @@ +successMessage: PDB and HPA are correctly configured +failureMessage: PDB minAvailable is less than HPA max replicas +category: Reliability +target: Controller +controllers: + include: + - Deployment diff --git a/pkg/config/default.yaml b/pkg/config/default.yaml index 8796fac90..f9ee7cf4f 100644 --- a/pkg/config/default.yaml +++ b/pkg/config/default.yaml @@ -12,6 +12,7 @@ checks: topologySpreadConstraint: warning hpaMaxAvailability: warning hpaMinAvailability: warning + pdbMinAvailableLessThanHPAMaxReplicas: warning # efficiency cpuRequestsMissing: warning diff --git a/pkg/config/examples/config-full.yaml b/pkg/config/examples/config-full.yaml index 2f9598641..576c9824e 100644 --- a/pkg/config/examples/config-full.yaml +++ b/pkg/config/examples/config-full.yaml @@ -12,6 +12,7 @@ checks: metadataAndInstanceMismatched: warning hpaMaxAvailability: warning hpaMinAvailability: warning + pdbMinAvailableLessThanHPAMaxReplicas: warning # efficiency cpuRequestsMissing: warning diff --git a/pkg/validator/custom.go b/pkg/validator/custom.go new file mode 100644 index 000000000..417a5183a --- /dev/null +++ b/pkg/validator/custom.go @@ -0,0 +1,19 @@ +package validator + +import ( + "sync" + + "github.com/qri-io/jsonschema" +) + +type validatorFunction func(test schemaTestCase) (bool, []jsonschema.ValError, error) + +var validatorMapper = map[string]validatorFunction{} +var lock = &sync.Mutex{} + +func registerCustomChecks(name string, check validatorFunction) { + lock.Lock() + defer lock.Unlock() + + validatorMapper[name] = check +} diff --git a/pkg/validator/pdbhpavalidator.go b/pkg/validator/pdbhpavalidator.go new file mode 100644 index 000000000..a9ebdfb77 --- /dev/null +++ b/pkg/validator/pdbhpavalidator.go @@ -0,0 +1,150 @@ +package validator + +import ( + "fmt" + "strconv" + "strings" + + "github.com/fairwindsops/polaris/pkg/kube" + "github.com/qri-io/jsonschema" + "github.com/sirupsen/logrus" + appsv1 "k8s.io/api/apps/v1" + autoscalingv1 "k8s.io/api/autoscaling/v1" + policyv1 "k8s.io/api/policy/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func init() { + registerCustomChecks("pdbMinAvailableLessThanHPAMaxReplicas", pdbMinAvailableLessThanHPAMaxReplicas) +} + +func pdbMinAvailableLessThanHPAMaxReplicas(test schemaTestCase) (bool, []jsonschema.ValError, error) { + if test.ResourceProvider == nil { + logrus.Debug("ResourceProvider is nil") + return true, nil, nil + } + + deployment := &appsv1.Deployment{} + err := runtime.DefaultUnstructuredConverter.FromUnstructured(test.Resource.Resource.Object, deployment) + if err != nil { + logrus.Warnf("error converting unstructured to Deployment: %v", err) + return true, nil, nil + } + + attachedPDB, err := hasPDBAttached(*deployment, test.ResourceProvider.Resources["policy/PodDisruptionBudget"]) + if err != nil { + logrus.Warnf("error getting PodDisruptionBudget: %v", err) + return true, nil, nil + } + + attachedHPA, err := hasHPAAttached(*deployment, test.ResourceProvider.Resources["autoscaling/HorizontalPodAutoscaler"]) + if err != nil { + logrus.Warnf("error getting HorizontalPodAutoscaler: %v", err) + return true, nil, nil + } + + if attachedPDB != nil && attachedHPA != nil { + logrus.Debugf("both PDB and HPA are attached to deployment %s", deployment.Name) + + pdbMinAvailable, isPercent, err := getIntOrPercentValueSafely(attachedPDB.Spec.MinAvailable) + if err != nil { + logrus.Warnf("error getting getIntOrPercentValueSafely: %v", err) + return true, nil, nil + } + + if isPercent { + // if the value is a percentage, we need to calculate the actual value + if deployment.Spec.Replicas == nil { + logrus.Debug("deployment.Spec.Replicas is nil") + return true, nil, nil + } + + pdbMinAvailable, err = intstr.GetScaledValueFromIntOrPercent(attachedPDB.Spec.MinAvailable, int(*deployment.Spec.Replicas), true) + if err != nil { + logrus.Warnf("error getting minAvailable value from PodDisruptionBudget: %v", err) + return true, nil, nil + } + } + + if pdbMinAvailable > int(attachedHPA.Spec.MaxReplicas) { + return false, []jsonschema.ValError{ + { + PropertyPath: "spec.minAvailable", + InvalidValue: pdbMinAvailable, + Message: fmt.Sprintf("The minAvailable value in the PodDisruptionBudget(%s) is %d, which is less than the maxReplicas value in the HorizontalPodAutoscaler(%s) (%d)", attachedPDB.Name, pdbMinAvailable, attachedHPA.Name, attachedHPA.Spec.MaxReplicas), + }, + }, nil + } + } + + return true, nil, nil +} + +func hasPDBAttached(deployment appsv1.Deployment, pdbs []kube.GenericResource) (*policyv1.PodDisruptionBudget, error) { + for _, generic := range pdbs { + pdb := &policyv1.PodDisruptionBudget{} + err := runtime.DefaultUnstructuredConverter.FromUnstructured(generic.Resource.Object, pdb) + if err != nil { + return nil, fmt.Errorf("error converting unstructured to PodDisruptionBudget: %v", err) + } + + if pdb.Spec.Selector == nil { + logrus.Debug("pdb.Spec.Selector is nil") + continue + } + + if matchesPDBForDeployment(deployment.Spec.Template.Labels, pdb.Spec.Selector.MatchLabels) { + return pdb, nil + } + } + return nil, nil +} + +// matchesPDBForDeployment checks if the labels of the deployment match the labels of the PDB +func matchesPDBForDeployment(deploymentLabels, pdbLabels map[string]string) bool { + for key, value := range pdbLabels { + if deploymentLabels[key] == value { + return true + } + } + return false +} + +func hasHPAAttached(deployment appsv1.Deployment, hpas []kube.GenericResource) (*autoscalingv1.HorizontalPodAutoscaler, error) { + for _, generic := range hpas { + hpa := &autoscalingv1.HorizontalPodAutoscaler{} + err := runtime.DefaultUnstructuredConverter.FromUnstructured(generic.Resource.Object, hpa) + if err != nil { + return nil, fmt.Errorf("error converting unstructured to HorizontalPodAutoscaler: %v", err) + } + + if hpa.Spec.ScaleTargetRef.Kind == "Deployment" && hpa.Spec.ScaleTargetRef.Name == deployment.Name { + return hpa, nil + } + } + return nil, nil +} + +// getIntOrPercentValueSafely is a safer version of getIntOrPercentValue based on private function intstr.getIntOrPercentValueSafely +func getIntOrPercentValueSafely(intOrStr *intstr.IntOrString) (int, bool, error) { + switch intOrStr.Type { + case intstr.Int: + return intOrStr.IntValue(), false, nil + case intstr.String: + isPercent := false + s := intOrStr.StrVal + if strings.HasSuffix(s, "%") { + isPercent = true + s = strings.TrimSuffix(intOrStr.StrVal, "%") + } else { + return 0, false, fmt.Errorf("invalid type: string is not a percentage") + } + v, err := strconv.Atoi(s) + if err != nil { + return 0, false, fmt.Errorf("invalid value %q: %v", intOrStr.StrVal, err) + } + return int(v), isPercent, nil + } + return 0, false, fmt.Errorf("invalid type: neither int nor percentage") +} diff --git a/pkg/validator/schema.go b/pkg/validator/schema.go index c35fefa93..73c96d649 100644 --- a/pkg/validator/schema.go +++ b/pkg/validator/schema.go @@ -369,6 +369,8 @@ func applySchemaCheck(conf *config.Configuration, checkID string, test schemaTes passes, issues, err = check.CheckContainer(test.Container) } else if check.Validator.SchemaURI != "" { passes, issues, err = check.CheckObject(test.Resource.Resource.Object) + } else if validatorMapper[checkID] != nil { + passes, issues, err = validatorMapper[checkID](test) } else { passes, issues, err = true, []jsonschema.ValError{}, nil } diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-percent.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-percent.yaml new file mode 100644 index 000000000..3deb27149 --- /dev/null +++ b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-percent.yaml @@ -0,0 +1,43 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + replicas: 10 + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 60% # 0.6 * 10 = 6 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: zookeeper + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-scalar.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure.yaml rename to test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-scalar.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.hpa-no-match.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.hpa-no-match.yaml new file mode 100644 index 000000000..d1e5c3e84 --- /dev/null +++ b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.hpa-no-match.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 5 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: no-match + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.pdb-no-match.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.pdb-no-match.yaml new file mode 100644 index 000000000..2267abf5c --- /dev/null +++ b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.pdb-no-match.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 5 + selector: + matchLabels: + app.kubernetes.io/name: no-match +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: zookeeper + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.percent-no-replica.yaml b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.percent-no-replica.yaml new file mode 100644 index 000000000..8e389d60a --- /dev/null +++ b/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.percent-no-replica.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 50% + selector: + matchLabels: + app.kubernetes.io/name: zookeeper +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: zookeeper + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 diff --git a/test/mutation_test.go b/test/mutation_test.go index bfc74eb63..b0814b967 100644 --- a/test/mutation_test.go +++ b/test/mutation_test.go @@ -34,6 +34,8 @@ func TestMutations(t *testing.T) { c, err := config.Parse([]byte(configYaml)) assert.NoError(t, err) assert.Len(t, c.Mutations, 0) + + _, mutatedYamlContentMap, mutationTestCasesMap := initTestCases() for mutationStr := range mutationTestCasesMap { if len(mutationTestCasesMap[mutationStr]) == 0 { panic("No test cases found for " + mutationStr) diff --git a/test/schema_test.go b/test/schema_test.go index ca09d6d92..0cad1931f 100644 --- a/test/schema_test.go +++ b/test/schema_test.go @@ -29,8 +29,6 @@ import ( "github.com/fairwindsops/polaris/pkg/validator" ) -var testCases = []testCase{} - type testCase struct { check string filename string @@ -40,10 +38,7 @@ type testCase struct { manifest string } -var mutatedYamlContentMap = map[string]string{} -var mutationTestCasesMap = map[string][]testCase{} - -func init() { +func initTestCases() ([]testCase, map[string]string, map[string][]testCase) { checkToTest := os.Getenv("POLARIS_CHECK_TEST") // if set, only run tests for this check _, baseDir, _, _ := runtime.Caller(0) baseDir = filepath.Dir(baseDir) + "/checks" @@ -51,6 +46,12 @@ func init() { if err != nil { panic(err) } + if checkToTest != "" { + fmt.Printf("POLARIS_CHECK_TEST is set... Running tests for '%s' only\n", checkToTest) + } + var testCases = []testCase{} + var mutatedYamlContentMap = map[string]string{} + var mutationTestCasesMap = map[string][]testCase{} for _, dir := range dirs { check := dir.Name() if checkToTest != "" && checkToTest != check { @@ -116,9 +117,11 @@ func init() { } } } + return testCases, mutatedYamlContentMap, mutationTestCasesMap } func TestChecks(t *testing.T) { + testCases, _, _ := initTestCases() for _, tc := range testCases { results, err := validator.ApplyAllSchemaChecksToResourceProvider(&tc.config, tc.resources) if err != nil { From 709c28cf40ad4f6250f75c0f87d62c8b2591630e Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Mon, 1 Jul 2024 18:29:57 -0300 Subject: [PATCH 10/21] change check name --- docs/checks/reliability.md | 2 +- pkg/config/checks.go | 2 +- ...cas.yaml => pdbMinAvailableGreaterThanHPAMaxReplicas.yaml} | 0 pkg/config/default.yaml | 2 +- pkg/config/examples/config-full.yaml | 2 +- pkg/validator/pdbhpavalidator.go | 4 ++-- 6 files changed, 6 insertions(+), 6 deletions(-) rename pkg/config/checks/{pdbMinAvailableLessThanHPAMaxReplicas.yaml => pdbMinAvailableGreaterThanHPAMaxReplicas.yaml} (100%) diff --git a/docs/checks/reliability.md b/docs/checks/reliability.md index 74c613076..4d7eac527 100644 --- a/docs/checks/reliability.md +++ b/docs/checks/reliability.md @@ -21,7 +21,7 @@ key | default | description `topologySpreadConstraint` | `warning` | Fails when there is no topology spread constraint on the pod `hpaMaxAvailability` | `warning` | Fails when `maxAvailable` lesser or equal than `minAvailable` (if defined) for a HorizontalPodAutoscaler `hpaMinAvailability` | `warning` | Fails when `minAvailable` (if defined) lesser or equal to one for a HorizontalPodAutoscaler -`pdbMinAvailableLessThanHPAMaxReplicas` | `warning` | Fails when PDB `minAvailable` is greater than HPA `maxReplicas` +`pdbMinAvailableGreaterThanHPAMaxReplicas` | `warning` | Fails when PDB `minAvailable` is greater than HPA `maxReplicas` ## Background diff --git a/pkg/config/checks.go b/pkg/config/checks.go index 032b493bf..e6560fe88 100644 --- a/pkg/config/checks.go +++ b/pkg/config/checks.go @@ -69,7 +69,7 @@ var ( "rolebindingClusterAdminRole", "hpaMaxAvailability", "hpaMinAvailability", - "pdbMinAvailableLessThanHPAMaxReplicas", + "pdbMinAvailableGreaterThanHPAMaxReplicas", } // BuiltInChecks contains the checks that come pre-installed w/ Polaris diff --git a/pkg/config/checks/pdbMinAvailableLessThanHPAMaxReplicas.yaml b/pkg/config/checks/pdbMinAvailableGreaterThanHPAMaxReplicas.yaml similarity index 100% rename from pkg/config/checks/pdbMinAvailableLessThanHPAMaxReplicas.yaml rename to pkg/config/checks/pdbMinAvailableGreaterThanHPAMaxReplicas.yaml diff --git a/pkg/config/default.yaml b/pkg/config/default.yaml index f9ee7cf4f..8db740fdd 100644 --- a/pkg/config/default.yaml +++ b/pkg/config/default.yaml @@ -12,7 +12,7 @@ checks: topologySpreadConstraint: warning hpaMaxAvailability: warning hpaMinAvailability: warning - pdbMinAvailableLessThanHPAMaxReplicas: warning + pdbMinAvailableGreaterThanHPAMaxReplicas: warning # efficiency cpuRequestsMissing: warning diff --git a/pkg/config/examples/config-full.yaml b/pkg/config/examples/config-full.yaml index 576c9824e..274c9f8a7 100644 --- a/pkg/config/examples/config-full.yaml +++ b/pkg/config/examples/config-full.yaml @@ -12,7 +12,7 @@ checks: metadataAndInstanceMismatched: warning hpaMaxAvailability: warning hpaMinAvailability: warning - pdbMinAvailableLessThanHPAMaxReplicas: warning + pdbMinAvailableGreaterThanHPAMaxReplicas: warning # efficiency cpuRequestsMissing: warning diff --git a/pkg/validator/pdbhpavalidator.go b/pkg/validator/pdbhpavalidator.go index a9ebdfb77..88283cafe 100644 --- a/pkg/validator/pdbhpavalidator.go +++ b/pkg/validator/pdbhpavalidator.go @@ -16,10 +16,10 @@ import ( ) func init() { - registerCustomChecks("pdbMinAvailableLessThanHPAMaxReplicas", pdbMinAvailableLessThanHPAMaxReplicas) + registerCustomChecks("pdbMinAvailableGreaterThanHPAMaxReplicas", pdbMinAvailableGreaterThanHPAMaxReplicas) } -func pdbMinAvailableLessThanHPAMaxReplicas(test schemaTestCase) (bool, []jsonschema.ValError, error) { +func pdbMinAvailableGreaterThanHPAMaxReplicas(test schemaTestCase) (bool, []jsonschema.ValError, error) { if test.ResourceProvider == nil { logrus.Debug("ResourceProvider is nil") return true, nil, nil From 2d63188f4b11b20841c89365cb52b4e484ebf15c Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Tue, 2 Jul 2024 14:11:04 -0300 Subject: [PATCH 11/21] rename testes --- .../failure-percent.yaml | 0 .../failure-scalar.yaml | 0 .../success.equals.yaml | 0 .../success.hpa-no-match.yaml | 0 .../success.lt.yaml | 0 .../success.no-hpa.yaml | 0 .../success.no-match.yaml | 0 .../success.no-pdb.yaml | 0 .../success.pdb-no-match.yaml | 0 .../success.percent-no-replica.yaml | 0 .../success.yaml | 0 11 files changed, 0 insertions(+), 0 deletions(-) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/failure-percent.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/failure-scalar.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/success.equals.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/success.hpa-no-match.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/success.lt.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/success.no-hpa.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/success.no-match.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/success.no-pdb.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/success.pdb-no-match.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/success.percent-no-replica.yaml (100%) rename test/checks/{pdbMinAvailableLessThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMaxReplicas}/success.yaml (100%) diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-percent.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/failure-percent.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-percent.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/failure-percent.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-scalar.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/failure-scalar.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/failure-scalar.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/failure-scalar.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.equals.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.equals.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.equals.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.equals.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.hpa-no-match.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.hpa-no-match.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.hpa-no-match.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.hpa-no-match.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.lt.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.lt.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.lt.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.lt.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-hpa.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-hpa.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-hpa.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-hpa.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-match.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-match.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-match.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-match.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-pdb.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-pdb.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.no-pdb.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-pdb.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.pdb-no-match.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.pdb-no-match.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.pdb-no-match.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.pdb-no-match.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.percent-no-replica.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.percent-no-replica.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.percent-no-replica.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.percent-no-replica.yaml diff --git a/test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.yaml similarity index 100% rename from test/checks/pdbMinAvailableLessThanHPAMaxReplicas/success.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.yaml From 8a35b14c656f764182940fd4ebdb5e342686d323 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Tue, 2 Jul 2024 14:24:51 -0300 Subject: [PATCH 12/21] fix check message --- pkg/validator/pdbhpavalidator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/validator/pdbhpavalidator.go b/pkg/validator/pdbhpavalidator.go index 88283cafe..42d8fbedb 100644 --- a/pkg/validator/pdbhpavalidator.go +++ b/pkg/validator/pdbhpavalidator.go @@ -72,7 +72,7 @@ func pdbMinAvailableGreaterThanHPAMaxReplicas(test schemaTestCase) (bool, []json { PropertyPath: "spec.minAvailable", InvalidValue: pdbMinAvailable, - Message: fmt.Sprintf("The minAvailable value in the PodDisruptionBudget(%s) is %d, which is less than the maxReplicas value in the HorizontalPodAutoscaler(%s) (%d)", attachedPDB.Name, pdbMinAvailable, attachedHPA.Name, attachedHPA.Spec.MaxReplicas), + Message: fmt.Sprintf("The minAvailable value in the PodDisruptionBudget(%s) is %d, which is greater than the maxReplicas value in the HorizontalPodAutoscaler(%s) (%d)", attachedPDB.Name, pdbMinAvailable, attachedHPA.Name, attachedHPA.Spec.MaxReplicas), }, }, nil } From 67337ddc5c3ff883d23f1c8564d737bd83e5af41 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Tue, 2 Jul 2024 15:05:38 -0300 Subject: [PATCH 13/21] change check name --- pkg/config/checks.go | 2 +- ...s.yaml => pdbMinAvailableGreaterThanHPAMinReplicas.yaml} | 2 +- pkg/validator/pdbhpavalidator.go | 6 +++--- .../failure-percent.yaml | 0 .../failure-scalar.yaml | 0 .../success.equals.yaml | 0 .../success.hpa-no-match.yaml | 0 .../success.lt.yaml | 0 .../success.no-hpa.yaml | 0 .../success.no-match.yaml | 0 .../success.no-pdb.yaml | 0 .../success.pdb-no-match.yaml | 0 .../success.percent-no-replica.yaml | 0 .../success.yaml | 0 14 files changed, 5 insertions(+), 5 deletions(-) rename pkg/config/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas.yaml => pdbMinAvailableGreaterThanHPAMinReplicas.yaml} (67%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/failure-percent.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/failure-scalar.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/success.equals.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/success.hpa-no-match.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/success.lt.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/success.no-hpa.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/success.no-match.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/success.no-pdb.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/success.pdb-no-match.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/success.percent-no-replica.yaml (100%) rename test/checks/{pdbMinAvailableGreaterThanHPAMaxReplicas => pdbMinAvailableGreaterThanHPAMinReplicas}/success.yaml (100%) diff --git a/pkg/config/checks.go b/pkg/config/checks.go index e6560fe88..9707d8457 100644 --- a/pkg/config/checks.go +++ b/pkg/config/checks.go @@ -69,7 +69,7 @@ var ( "rolebindingClusterAdminRole", "hpaMaxAvailability", "hpaMinAvailability", - "pdbMinAvailableGreaterThanHPAMaxReplicas", + "pdbMinAvailableGreaterThanHPAMinReplicas", } // BuiltInChecks contains the checks that come pre-installed w/ Polaris diff --git a/pkg/config/checks/pdbMinAvailableGreaterThanHPAMaxReplicas.yaml b/pkg/config/checks/pdbMinAvailableGreaterThanHPAMinReplicas.yaml similarity index 67% rename from pkg/config/checks/pdbMinAvailableGreaterThanHPAMaxReplicas.yaml rename to pkg/config/checks/pdbMinAvailableGreaterThanHPAMinReplicas.yaml index 13f66b076..cff63c7c1 100644 --- a/pkg/config/checks/pdbMinAvailableGreaterThanHPAMaxReplicas.yaml +++ b/pkg/config/checks/pdbMinAvailableGreaterThanHPAMinReplicas.yaml @@ -1,5 +1,5 @@ successMessage: PDB and HPA are correctly configured -failureMessage: PDB minAvailable is less than HPA max replicas +failureMessage: PDB minAvailable is greater than HPA minReplicas category: Reliability target: Controller controllers: diff --git a/pkg/validator/pdbhpavalidator.go b/pkg/validator/pdbhpavalidator.go index 42d8fbedb..f947b3956 100644 --- a/pkg/validator/pdbhpavalidator.go +++ b/pkg/validator/pdbhpavalidator.go @@ -16,10 +16,10 @@ import ( ) func init() { - registerCustomChecks("pdbMinAvailableGreaterThanHPAMaxReplicas", pdbMinAvailableGreaterThanHPAMaxReplicas) + registerCustomChecks("pdbMinAvailableGreaterThanHPAMinReplicas", pdbMinAvailableGreaterThanHPAMinReplicas) } -func pdbMinAvailableGreaterThanHPAMaxReplicas(test schemaTestCase) (bool, []jsonschema.ValError, error) { +func pdbMinAvailableGreaterThanHPAMinReplicas(test schemaTestCase) (bool, []jsonschema.ValError, error) { if test.ResourceProvider == nil { logrus.Debug("ResourceProvider is nil") return true, nil, nil @@ -67,7 +67,7 @@ func pdbMinAvailableGreaterThanHPAMaxReplicas(test schemaTestCase) (bool, []json } } - if pdbMinAvailable > int(attachedHPA.Spec.MaxReplicas) { + if attachedHPA.Spec.MinReplicas != nil && pdbMinAvailable > int(*attachedHPA.Spec.MinReplicas) { return false, []jsonschema.ValError{ { PropertyPath: "spec.minAvailable", diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/failure-percent.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-percent.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/failure-percent.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-percent.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/failure-scalar.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-scalar.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/failure-scalar.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-scalar.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.equals.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.equals.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.hpa-no-match.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.hpa-no-match.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.hpa-no-match.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.hpa-no-match.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.lt.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.lt.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.lt.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.lt.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-hpa.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-hpa.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-hpa.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-hpa.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-match.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-match.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-match.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-match.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-pdb.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-pdb.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.no-pdb.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-pdb.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.pdb-no-match.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.pdb-no-match.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.pdb-no-match.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.pdb-no-match.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.percent-no-replica.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.percent-no-replica.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.percent-no-replica.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.percent-no-replica.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMaxReplicas/success.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.yaml From e47fe55c0ed8c2dfd25654935dd8b331ac17a9d7 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Tue, 2 Jul 2024 15:06:12 -0300 Subject: [PATCH 14/21] minor fixes --- pkg/validator/pdbhpavalidator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/validator/pdbhpavalidator.go b/pkg/validator/pdbhpavalidator.go index f947b3956..d775279cd 100644 --- a/pkg/validator/pdbhpavalidator.go +++ b/pkg/validator/pdbhpavalidator.go @@ -72,7 +72,7 @@ func pdbMinAvailableGreaterThanHPAMinReplicas(test schemaTestCase) (bool, []json { PropertyPath: "spec.minAvailable", InvalidValue: pdbMinAvailable, - Message: fmt.Sprintf("The minAvailable value in the PodDisruptionBudget(%s) is %d, which is greater than the maxReplicas value in the HorizontalPodAutoscaler(%s) (%d)", attachedPDB.Name, pdbMinAvailable, attachedHPA.Name, attachedHPA.Spec.MaxReplicas), + Message: fmt.Sprintf("The minAvailable value in the PodDisruptionBudget(%s) is %d, which is greater than the minReplicas value in the HorizontalPodAutoscaler(%s) (%d)", attachedPDB.Name, pdbMinAvailable, attachedHPA.Name, *attachedHPA.Spec.MinReplicas), }, }, nil } From c26bfaba8715390e2a4b60e8af37f8f89ac33c2f Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Tue, 2 Jul 2024 15:24:47 -0300 Subject: [PATCH 15/21] improving tests --- .../failure-percent.yaml | 1 + .../failure-scalar.yaml | 1 + .../success.equals.yaml | 1 + .../success.hpa-no-match.yaml | 3 +- .../success.lt.yaml | 3 +- .../success.no-match.yaml | 3 +- .../success.no-min-replicas.yaml | 42 +++++++++++++++++++ .../success.no-pdb.yaml | 3 +- .../success.pdb-no-match.yaml | 3 +- .../success.percent-no-replica.yaml | 3 +- 10 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-min-replicas.yaml diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-percent.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-percent.yaml index 3deb27149..e8036816f 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-percent.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-percent.yaml @@ -33,6 +33,7 @@ spec: apiVersion: apps/v1 kind: Deployment name: zookeeper + minReplicas: 5 maxReplicas: 5 metrics: - type: Resource diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-scalar.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-scalar.yaml index 26912d9e0..97d33c081 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-scalar.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-scalar.yaml @@ -32,6 +32,7 @@ spec: apiVersion: apps/v1 kind: Deployment name: zookeeper + minReplicas: 5 maxReplicas: 5 metrics: - type: Resource diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml index a3ae87687..44b7e63e0 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml @@ -32,6 +32,7 @@ spec: apiVersion: apps/v1 kind: Deployment name: zookeeper + minReplicas: 5 maxReplicas: 5 metrics: - type: Resource diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.hpa-no-match.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.hpa-no-match.yaml index d1e5c3e84..c5ccc03af 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.hpa-no-match.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.hpa-no-match.yaml @@ -32,7 +32,8 @@ spec: apiVersion: apps/v1 kind: Deployment name: no-match - maxReplicas: 5 + minReplicas: 5 + maxReplicas: 7 metrics: - type: Resource resource: diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.lt.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.lt.yaml index 874dca5b1..4292c2acd 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.lt.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.lt.yaml @@ -32,7 +32,8 @@ spec: apiVersion: apps/v1 kind: Deployment name: zookeeper - maxReplicas: 5 + minReplicas: 5 + maxReplicas: 7 metrics: - type: Resource resource: diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-match.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-match.yaml index 5993d4ab5..1cd362598 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-match.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-match.yaml @@ -32,7 +32,8 @@ spec: apiVersion: apps/v1 kind: Deployment name: no-match - maxReplicas: 5 + minReplicas: 5 + maxReplicas: 7 metrics: - type: Resource resource: diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-min-replicas.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-min-replicas.yaml new file mode 100644 index 000000000..874dca5b1 --- /dev/null +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-min-replicas.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 2 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: zookeeper + maxReplicas: 5 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-pdb.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-pdb.yaml index 818153354..cc9d8fbbf 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-pdb.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.no-pdb.yaml @@ -22,7 +22,8 @@ spec: apiVersion: apps/v1 kind: Deployment name: zookeeper - maxReplicas: 5 + minReplicas: 5 + maxReplicas: 7 metrics: - type: Resource resource: diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.pdb-no-match.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.pdb-no-match.yaml index 2267abf5c..b9c1a4cce 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.pdb-no-match.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.pdb-no-match.yaml @@ -32,7 +32,8 @@ spec: apiVersion: apps/v1 kind: Deployment name: zookeeper - maxReplicas: 5 + minReplicas: 5 + maxReplicas: 7 metrics: - type: Resource resource: diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.percent-no-replica.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.percent-no-replica.yaml index 8e389d60a..e1fa8b1e3 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.percent-no-replica.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.percent-no-replica.yaml @@ -32,7 +32,8 @@ spec: apiVersion: apps/v1 kind: Deployment name: zookeeper - maxReplicas: 5 + minReplicas: 5 + maxReplicas: 7 metrics: - type: Resource resource: From ed358b1adb4df803088d8521b80cc9fe59ac0724 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Tue, 2 Jul 2024 15:26:17 -0300 Subject: [PATCH 16/21] improve tests --- .../{failure-percent.yaml => failure-gt-percent.yaml} | 2 +- .../{failure-scalar.yaml => failure-gt-scalar.yaml} | 2 +- .../success.equals.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/{failure-percent.yaml => failure-gt-percent.yaml} (97%) rename test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/{failure-scalar.yaml => failure-gt-scalar.yaml} (97%) diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-percent.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-percent.yaml similarity index 97% rename from test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-percent.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-percent.yaml index e8036816f..f31ca7ba2 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-percent.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-percent.yaml @@ -34,7 +34,7 @@ spec: kind: Deployment name: zookeeper minReplicas: 5 - maxReplicas: 5 + maxReplicas: 7 metrics: - type: Resource resource: diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-scalar.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-scalar.yaml similarity index 97% rename from test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-scalar.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-scalar.yaml index 97d33c081..bea1a930a 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-scalar.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-scalar.yaml @@ -33,7 +33,7 @@ spec: kind: Deployment name: zookeeper minReplicas: 5 - maxReplicas: 5 + maxReplicas: 7 metrics: - type: Resource resource: diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml index 44b7e63e0..53d9817e7 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml @@ -33,7 +33,7 @@ spec: kind: Deployment name: zookeeper minReplicas: 5 - maxReplicas: 5 + maxReplicas: 7 metrics: - type: Resource resource: From 780bc7fe38874071e82cc26af3ecddc9c2415926 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Tue, 2 Jul 2024 15:42:22 -0300 Subject: [PATCH 17/21] fix check name --- docs/checks/reliability.md | 2 +- pkg/config/default.yaml | 2 +- pkg/config/examples/config-full.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/checks/reliability.md b/docs/checks/reliability.md index 4d7eac527..416fdc7e4 100644 --- a/docs/checks/reliability.md +++ b/docs/checks/reliability.md @@ -21,7 +21,7 @@ key | default | description `topologySpreadConstraint` | `warning` | Fails when there is no topology spread constraint on the pod `hpaMaxAvailability` | `warning` | Fails when `maxAvailable` lesser or equal than `minAvailable` (if defined) for a HorizontalPodAutoscaler `hpaMinAvailability` | `warning` | Fails when `minAvailable` (if defined) lesser or equal to one for a HorizontalPodAutoscaler -`pdbMinAvailableGreaterThanHPAMaxReplicas` | `warning` | Fails when PDB `minAvailable` is greater than HPA `maxReplicas` +`pdbMinAvailableGreaterThanHPAMinReplicas` | `warning` | Fails when PDB `minAvailable` is greater than HPA `maxReplicas` ## Background diff --git a/pkg/config/default.yaml b/pkg/config/default.yaml index 8db740fdd..9fc5266ea 100644 --- a/pkg/config/default.yaml +++ b/pkg/config/default.yaml @@ -12,7 +12,7 @@ checks: topologySpreadConstraint: warning hpaMaxAvailability: warning hpaMinAvailability: warning - pdbMinAvailableGreaterThanHPAMaxReplicas: warning + pdbMinAvailableGreaterThanHPAMinReplicas: warning # efficiency cpuRequestsMissing: warning diff --git a/pkg/config/examples/config-full.yaml b/pkg/config/examples/config-full.yaml index 274c9f8a7..aa5247c5a 100644 --- a/pkg/config/examples/config-full.yaml +++ b/pkg/config/examples/config-full.yaml @@ -12,7 +12,7 @@ checks: metadataAndInstanceMismatched: warning hpaMaxAvailability: warning hpaMinAvailability: warning - pdbMinAvailableGreaterThanHPAMaxReplicas: warning + pdbMinAvailableGreaterThanHPAMinReplicas: warning # efficiency cpuRequestsMissing: warning From a53a2368c981450784009dbd6e22d13d3e3503aa Mon Sep 17 00:00:00 2001 From: Vitor Rodrigo Vezani Date: Wed, 3 Jul 2024 15:00:48 -0300 Subject: [PATCH 18/21] Update docs/checks/reliability.md Co-authored-by: Andy Suderman --- docs/checks/reliability.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/checks/reliability.md b/docs/checks/reliability.md index 416fdc7e4..8452fea90 100644 --- a/docs/checks/reliability.md +++ b/docs/checks/reliability.md @@ -21,7 +21,7 @@ key | default | description `topologySpreadConstraint` | `warning` | Fails when there is no topology spread constraint on the pod `hpaMaxAvailability` | `warning` | Fails when `maxAvailable` lesser or equal than `minAvailable` (if defined) for a HorizontalPodAutoscaler `hpaMinAvailability` | `warning` | Fails when `minAvailable` (if defined) lesser or equal to one for a HorizontalPodAutoscaler -`pdbMinAvailableGreaterThanHPAMinReplicas` | `warning` | Fails when PDB `minAvailable` is greater than HPA `maxReplicas` +`pdbMinAvailableGreaterThanHPAMinReplicas` | `warning` | Fails when PDB `minAvailable` is greater than HPA `minReplicas` ## Background From 29d8f74eb5dc32b1f3e7b81fc8909485c05e4c27 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Mon, 8 Jul 2024 11:06:38 -0300 Subject: [PATCH 19/21] fix/add tests --- ...dbhpavalidator.go => pdb_hpa_validator.go} | 0 .../failure-gt-percent.yaml | 6 +-- .../success-lt-percent.yaml | 44 +++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) rename pkg/validator/{pdbhpavalidator.go => pdb_hpa_validator.go} (100%) create mode 100644 test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success-lt-percent.yaml diff --git a/pkg/validator/pdbhpavalidator.go b/pkg/validator/pdb_hpa_validator.go similarity index 100% rename from pkg/validator/pdbhpavalidator.go rename to pkg/validator/pdb_hpa_validator.go diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-percent.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-percent.yaml index f31ca7ba2..257d60753 100644 --- a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-percent.yaml +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure-gt-percent.yaml @@ -19,7 +19,7 @@ kind: PodDisruptionBudget metadata: name: zookeeper-pdb spec: - minAvailable: 60% # 0.6 * 10 = 6 + minAvailable: 150% # 1.5 * 10 = 15 selector: matchLabels: app.kubernetes.io/name: zookeeper @@ -33,8 +33,8 @@ spec: apiVersion: apps/v1 kind: Deployment name: zookeeper - minReplicas: 5 - maxReplicas: 7 + minReplicas: 10 + maxReplicas: 15 metrics: - type: Resource resource: diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success-lt-percent.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success-lt-percent.yaml new file mode 100644 index 000000000..02bb809e3 --- /dev/null +++ b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success-lt-percent.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: zookeeper +spec: + replicas: 10 + template: + metadata: + labels: + app.kubernetes.io/name: zookeeper + foo: bar + spec: + containers: + - name: zookeeper + image: zookeeper +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: zookeeper-pdb +spec: + minAvailable: 50% # 0.5 * 10 = 5 + selector: + matchLabels: + app.kubernetes.io/name: zookeeper +--- +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + name: zookeeper-hpa +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: zookeeper + minReplicas: 10 + maxReplicas: 15 + metrics: + - type: Resource + resource: + name: cpu + target: + type: Utilization + averageUtilization: 50 From 57316789f918326c270806c68723d3fd2ae0fdff Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Mon, 8 Jul 2024 13:55:23 -0300 Subject: [PATCH 20/21] fixes from PR --- pkg/validator/pdb_hpa_validator.go | 8 ++++---- .../{success.equals.yaml => failure.equals.yaml} | 0 2 files changed, 4 insertions(+), 4 deletions(-) rename test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/{success.equals.yaml => failure.equals.yaml} (100%) diff --git a/pkg/validator/pdb_hpa_validator.go b/pkg/validator/pdb_hpa_validator.go index d775279cd..916ee499e 100644 --- a/pkg/validator/pdb_hpa_validator.go +++ b/pkg/validator/pdb_hpa_validator.go @@ -55,19 +55,19 @@ func pdbMinAvailableGreaterThanHPAMinReplicas(test schemaTestCase) (bool, []json if isPercent { // if the value is a percentage, we need to calculate the actual value - if deployment.Spec.Replicas == nil { - logrus.Debug("deployment.Spec.Replicas is nil") + if attachedHPA.Spec.MinReplicas == nil { + logrus.Debug("attachedHPA.Spec.MinReplicas is nil") return true, nil, nil } - pdbMinAvailable, err = intstr.GetScaledValueFromIntOrPercent(attachedPDB.Spec.MinAvailable, int(*deployment.Spec.Replicas), true) + pdbMinAvailable, err = intstr.GetScaledValueFromIntOrPercent(attachedPDB.Spec.MinAvailable, int(*attachedHPA.Spec.MinReplicas), true) if err != nil { logrus.Warnf("error getting minAvailable value from PodDisruptionBudget: %v", err) return true, nil, nil } } - if attachedHPA.Spec.MinReplicas != nil && pdbMinAvailable > int(*attachedHPA.Spec.MinReplicas) { + if attachedHPA.Spec.MinReplicas != nil && pdbMinAvailable >= int(*attachedHPA.Spec.MinReplicas) { return false, []jsonschema.ValError{ { PropertyPath: "spec.minAvailable", diff --git a/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml b/test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure.equals.yaml similarity index 100% rename from test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/success.equals.yaml rename to test/checks/pdbMinAvailableGreaterThanHPAMinReplicas/failure.equals.yaml From ffb3af7a535a158c0b740cecbec5a98ccde0c5b5 Mon Sep 17 00:00:00 2001 From: Vitor Vezani Date: Mon, 8 Jul 2024 13:58:00 -0300 Subject: [PATCH 21/21] fix error message --- pkg/validator/pdb_hpa_validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/validator/pdb_hpa_validator.go b/pkg/validator/pdb_hpa_validator.go index 916ee499e..e613df000 100644 --- a/pkg/validator/pdb_hpa_validator.go +++ b/pkg/validator/pdb_hpa_validator.go @@ -72,7 +72,7 @@ func pdbMinAvailableGreaterThanHPAMinReplicas(test schemaTestCase) (bool, []json { PropertyPath: "spec.minAvailable", InvalidValue: pdbMinAvailable, - Message: fmt.Sprintf("The minAvailable value in the PodDisruptionBudget(%s) is %d, which is greater than the minReplicas value in the HorizontalPodAutoscaler(%s) (%d)", attachedPDB.Name, pdbMinAvailable, attachedHPA.Name, *attachedHPA.Spec.MinReplicas), + Message: fmt.Sprintf("The minAvailable value in the PodDisruptionBudget(%s) is %d, which is greater or equal than the minReplicas value in the HorizontalPodAutoscaler(%s) (%d)", attachedPDB.Name, pdbMinAvailable, attachedHPA.Name, *attachedHPA.Spec.MinReplicas), }, }, nil }