fixed pdbMinAvailableGreaterThanHPAMinReplicas and added validation for pdbMinAvailableEqualToHPAMinReplicas.

[YuMuuu]

This PR fixes #1071

Checklist

• [x] I have signed the CLA
• [x] I have updated/added any relevant documentation

Description

What's the goal of this PR?

What changes did you make?

What alternative solution should we consider, if any?

• I couldn't connect to circle ci for some reason so I ran test local only/

[CLAassistant]

All committers have signed the CLA.

[vitorvezani]

Hi @YuMuuu, thanks for your contribution!

After reviewing the changes and discussing internally, we believe this update introduces a breaking change. To avoid impacting current Polaris clients, we suggest the following approach:

• Retain the current check, but modify its logic (and docs) to check only for >, instead
• Introduce a complementary check that specifically handles the = case.

Both changes we plan to release in a new minor version. This approach ensures compatibility with existing clients while addressing the functionality you're aiming for.

I understand this may require additional changes to your PR. Please let me know if you have the bandwidth to make these adjustments, or if you'd prefer someone else to take it from here.

Thanks again for your efforts!

[YuMuuu]

@vitorvezani Thanks for your reply. I agree with your idea. I’ll keep the name pdbMinAvailableGreaterThanHPAMinReplicas, fix logic, and create other complement validation. I’d like to fix the PR as you mentioned, but it might take time because I’m not a golang expert.

[YuMuuu]

I made a logic fix to pdbMinAvailableGreaterThanHPAMinReplicas and added validation for pdbMinAvailableEqualToHPAMinReplicas.

The names of the validations and their specifications have been aligned; however, it was found that pdbMinAvailableGreaterThanHPAMinReplicas and pdbMinAvailableEqualToHPAMinReplicas exhibit conflicting behavior.

Specifically, the following cases are now allowed under pdbMinAvailableEqualToHPAMinReplicas:

test/checks/pdbMinAvailableEqualToHPAMinReplicas/success-gt-percent.yaml

• minAvailable: 15
• minReplicas: 10

test/checks/pdbMinAvailableEqualToHPAMinReplicas/success-gt-scalar.yaml

• minAvailable: 10
• minReplicas: 5

These configurations are clearly incorrect. While applying both pdbMinAvailableGreaterThanHPAMinReplicas and pdbMinAvailableEqualToHPAMinReplicas together resolves the issue, using only pdbMinAvailableEqualToHPAMinReplicas could allow such incorrect configurations. Is this behavior acceptable?

[vitorvezani]

I believe this approach is acceptable, as it avoids triggering two policies for the same underlying issue. With both checks enabled by default, we can ensure all cases are covered.

Let me know if you have any thoughts or concerns, @sudermanjr

[YuMuuu]

@vitorvezani @sudermanjr Releasing it in the following order might be better I guess:

1. change the logic of pdbMinAvailableGreaterThanHPAMinReplicas and create new pdbMinAvailableGreaterThanOrEsualToHPAMinReplicas
2. deprecate the pdbMinAvailableGreaterThanHPAMinReplicas
3. delete pdbMinAvailableGreaterThanHPAMinReplicas <- next major version

What do you think?

[vitorvezani]

That's a valid approach as well. However, I see a potential issue during the first release: whereas if pdbMinAvailable > HPAMinReplicas, it would trigger both checks simultaneously. I also think that a breaking change for this is a bit too much, having 2 similar checks that are mutually-exclusive sounds reasonable (although not perfect).

I'll discuss both options with Andy when he's back from his time off and keep you updated.

[YuMuuu]

Is Andy on a long vacation?

[YuMuuu]

No....(;_;) Could you please re-open this PR?. @vitorvezani

[YuMuuu]

@vitorvezani @sudermanjr What do you think this idea ? https://github.com/FairwindsOps/polaris/pull/1073#issuecomment-2394879611

I'm fine with either merging this PR as it is or implementing the suggested idea.

[YuMuuu]

@vitorvezani @sudermanjr I aplogize for bothering you while you are busy, but I want you to merge this PR. could you review it? 🙏

[YuMuuu]

@vitorvezani @sudermanjr could you review it? 🙏