Create check to ensure PDB have MaxUnavailable and MinAvailable set effectively #507

fahlmant · 2023-03-07T12:58:47Z

Creates two checks for PDBs:

Recommends changes to PDBs that have MaxUnavailable set to 0
Recommends changes to PDBs that have MinAvailable set to the Replica value from related Deployments

This is a WIP. Tests still need to be added

pkg/templates/pdb/template.go

docs/generated/checks.md

pkg/templates/pdb/template.go

docs/generated/checks.md

pkg/builtinchecks/yamls/pdbs.yaml

pkg/templates/pdb/template.go

tremes · 2023-03-15T11:54:54Z

It's still not clear to me when and how this new check can be enabled. It seems it won't be enabled by default so will it be a custom check?

fahlmant · 2023-03-15T13:33:27Z

@tremes the PR is still a WIP

fahlmant · 2023-03-20T17:10:21Z

Tests currently are not working, adding a Deployment to the lintContext seems to not be working as the mocked object is showing that there's no Kind specified

pkg/lintcontext/mocks/pod.go

pkg/templates/pdbminavailable/template.go

tremes · 2023-04-04T07:39:30Z

pkg/templates/pdbminavailable/template.go

+					replicas, _ := extract.Replicas(dl)
+					if isPercent {
+						// Calulate the actual value of the MinAvailable with respect to the Replica count if a percentage is set
+						pdbMinAvailable = int(replicas) * (value / 100)


Won't it be always zero? I guess you need something like pdbMinAvailable = float64(replicas) * (float64(value) / float64(100))

In the end this still has to be an integer. It doesn't matter too much though because as long as it's less than 100%, it should be a valid config. For accuracy though, I think this will work:
pdbMinAvailable = int(math.Floor(float64(replicas) * (float64(value) / float64(100))))

This finds the exact value of minavailable based on a percentage, and then rounds down so cases like 1 replica with a 50% minavaiable (which calculates to 0.5) won't cause this remediation to fire. I'm not sure exactly how kubernetes handles this sort of case, so I'm erring on the side of permissive at first. It might be a good place for iteration

@tremes is right but if we change order (multiplication first) it will work without conversion and floor

pdbMinAvailable = (int(replicas) * value) / 100

…ffectively

e2etests/bats-tests.sh

docs/generated/checks.md

janisz · 2023-04-04T12:04:32Z

pkg/templates/pdbminavailable/template.go

+				if err != nil {
+					return []diagnostic.Diagnostic{
+						{
+							Message: fmt.Sprintf("pdb has invalid MinAvailable value: %v", err),


in other places PDB is capital.

Suggested change

Message: fmt.Sprintf("pdb has invalid MinAvailable value: %v", err),

Message: fmt.Sprintf("PDB has invalid MinAvailable value: %w", err),

also in other places we use errors.Wrap

pkg/templates/pdbminavailable/template.go

dhaus67

Just minor things, otherwise LGTM!

docs/generated/checks.md

tests/checks/pdb-min-available.yaml

janisz

Just small comments. I think we should use errors.Wrap to wrap errors.

pkg/templates/pdbminavailable/template.go

janisz · 2023-04-05T14:48:41Z

pkg/templates/pdbminavailable/template.go

+					replicas, _ := extract.Replicas(dl)
+					if isPercent {
+						// Calulate the actual value of the MinAvailable with respect to the Replica count if a percentage is set
+						pdbMinAvailable = int(replicas) * (value / 100)


@tremes is right but if we change order (multiplication first) it will work without conversion and floor

pdbMinAvailable = (int(replicas) * value) / 100

pkg/templates/pdbminavailable/template.go

janisz · 2023-04-05T14:51:38Z

tests/checks/pdb-min-available.yaml

+  replicas: 1
+  selector:
+    matchLabels:
+      name: cloud-ingress-operator


I think we should add a check for EOL

Can you clarify this a bit? I'm not sure what is being asked here

pkg/templates/pdbmaxunavailable/template.go

pkg/templates/pdbminavailable/template.go

pkg/builtinchecks/yamls/pdbs-max-unavailable.yaml

pkg/builtinchecks/yamls/pdbs-min-available.yaml

maorfr · 2023-09-11T08:35:09Z

https://issues.redhat.com/browse/DVO-13

…ffectively (stackrox#507)

fahlmant requested a review from janisz as a code owner March 7, 2023 12:58

fahlmant force-pushed the pdb-deployment-check branch from 3707060 to e3f0455 Compare March 7, 2023 15:46

janisz requested a review from dhaus67 March 7, 2023 18:20

tremes reviewed Mar 8, 2023

View reviewed changes

pkg/templates/pdb/template.go Outdated Show resolved Hide resolved

tremes reviewed Mar 8, 2023

View reviewed changes

docs/generated/checks.md Outdated Show resolved Hide resolved

tremes reviewed Mar 9, 2023

View reviewed changes

pkg/templates/pdb/template.go Outdated Show resolved Hide resolved

tremes reviewed Mar 9, 2023

View reviewed changes

pkg/templates/pdb/template.go Outdated Show resolved Hide resolved

tremes reviewed Mar 10, 2023

View reviewed changes

pkg/templates/pdb/template.go Outdated Show resolved Hide resolved

janisz mentioned this pull request Mar 13, 2023

feat: add PodDisruptionBudget kind to registered objectkinds #485

Closed

dhaus67 reviewed Mar 14, 2023

View reviewed changes

fahlmant changed the title ~~[WIP] Create check to ensure PDB have MaxUnavailable set effectively~~ [WIP] Create check to ensure PDB have MaxUnavailable and MinAvailable set effectively Apr 3, 2023

fahlmant force-pushed the pdb-deployment-check branch 2 times, most recently from 93fc9da to 6d6cc89 Compare April 3, 2023 21:04

fahlmant changed the title ~~[WIP] Create check to ensure PDB have MaxUnavailable and MinAvailable set effectively~~ Create check to ensure PDB have MaxUnavailable and MinAvailable set effectively Apr 3, 2023

fahlmant force-pushed the pdb-deployment-check branch 2 times, most recently from fd7cbfa to 0f2dfd2 Compare April 3, 2023 21:24

tremes reviewed Apr 4, 2023

View reviewed changes

pkg/lintcontext/mocks/pod.go Outdated Show resolved Hide resolved

tremes reviewed Apr 4, 2023

View reviewed changes

pkg/templates/pdbminavailable/template.go Outdated Show resolved Hide resolved

tremes reviewed Apr 4, 2023

View reviewed changes

Create check to ensure PDB have MaxUnavailable and MinAvailable set e…

73ceb30

…ffectively

fahlmant force-pushed the pdb-deployment-check branch from 0f2dfd2 to 73ceb30 Compare April 4, 2023 10:59

janisz requested a review from dhaus67 April 4, 2023 11:33

janisz requested changes Apr 4, 2023

View reviewed changes

Fixes based on feedback

00f573b

janisz self-requested a review April 5, 2023 14:35

dhaus67 approved these changes Apr 5, 2023

View reviewed changes

docs/generated/checks.md Outdated Show resolved Hide resolved

tests/checks/pdb-min-available.yaml Outdated Show resolved Hide resolved

janisz requested changes Apr 5, 2023

View reviewed changes

jholecek-rh reviewed Apr 5, 2023

View reviewed changes

pkg/templates/pdbmaxunavailable/template.go Outdated Show resolved Hide resolved

jholecek-rh reviewed Apr 5, 2023

View reviewed changes

pkg/templates/pdbmaxunavailable/template.go Outdated Show resolved Hide resolved

jholecek-rh reviewed Apr 5, 2023

View reviewed changes

pkg/templates/pdbminavailable/template.go Outdated Show resolved Hide resolved

jholecek-rh reviewed Apr 5, 2023

View reviewed changes

pkg/templates/pdbminavailable/template.go Outdated Show resolved Hide resolved

jholecek-rh reviewed Apr 5, 2023

View reviewed changes

pkg/templates/pdbminavailable/template.go Outdated Show resolved Hide resolved

jholecek-rh reviewed Apr 5, 2023

View reviewed changes

pkg/builtinchecks/yamls/pdbs-max-unavailable.yaml Outdated Show resolved Hide resolved

jholecek-rh reviewed Apr 5, 2023

View reviewed changes

pkg/builtinchecks/yamls/pdbs-min-available.yaml Outdated Show resolved Hide resolved

More fixes based on feedback

373117e

janisz self-requested a review April 7, 2023 15:13

janisz approved these changes Apr 7, 2023

View reviewed changes

janisz merged commit e94f8f3 into stackrox:main Apr 7, 2023
7 checks passed

abrad3 pushed a commit to abrad3/kube-linter that referenced this pull request Sep 28, 2023

Create check to ensure PDB have MaxUnavailable and MinAvailable set e…

6a9c2f9

…ffectively (stackrox#507)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create check to ensure PDB have MaxUnavailable and MinAvailable set effectively #507

Create check to ensure PDB have MaxUnavailable and MinAvailable set effectively #507

fahlmant commented Mar 7, 2023 •

edited

tremes commented Mar 15, 2023

fahlmant commented Mar 15, 2023

fahlmant commented Mar 20, 2023

tremes Apr 4, 2023

fahlmant Apr 4, 2023

janisz Apr 5, 2023

janisz Apr 4, 2023

janisz Apr 4, 2023

dhaus67 left a comment

janisz left a comment

janisz Apr 5, 2023

janisz Apr 5, 2023

fahlmant Apr 5, 2023

maorfr commented Sep 11, 2023

	Message: fmt.Sprintf("pdb has invalid MinAvailable value: %v", err),
	Message: fmt.Sprintf("PDB has invalid MinAvailable value: %w", err),

Create check to ensure PDB have MaxUnavailable and MinAvailable set effectively #507

Create check to ensure PDB have MaxUnavailable and MinAvailable set effectively #507

Conversation

fahlmant commented Mar 7, 2023 • edited

tremes commented Mar 15, 2023

fahlmant commented Mar 15, 2023

fahlmant commented Mar 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhaus67 left a comment

Choose a reason for hiding this comment

janisz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maorfr commented Sep 11, 2023

fahlmant commented Mar 7, 2023 •

edited