fix(metallb): remove conflicting IPAddressPools when metallb admission webhook rejects ktf's pool

[pmalek]

Fixes #832

[codecov-commenter]

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (05a2a38) 59.87% compared to head (be750a2) 59.80%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #834      +/-   ##
==========================================
- Coverage   59.87%   59.80%   -0.08%     
==========================================
  Files          48       48              
  Lines        3983     3993      +10     
==========================================
+ Hits         2385     2388       +3     
- Misses       1297     1304       +7     
  Partials      301      301              

Flag	Coverage Δ
integration-test	59.80% <11.11%> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
pkg/clusters/addons/metallb/metallb.go	66.12% <11.11%> (-2.82%)	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rainest]

Should we not just have an option to disable pool creation? Deleting whatever's there without knowing what it is seems a bit much.

[pmalek]

Should we not just have an option to disable pool creation? Deleting whatever's there without knowing what it is seems a bit much.

I see your point OTOH if with a configuration option you wouldn't be able to back off from a failed e.g. test run using ktf. With such a mechanism it is able "to recover".

I'm open to suggestions on this one.

[rainest]

There aren't any situations where we expect other IPAddressPools to appear without the user expecting them to be there, right? In the case that raised this, it's because the user both enabled the metallb addon and provided their own IPAddressPool. An option to disable addon pool creation enables that bring-your-own pool approach.

KTF can recover in the already exists case since it can be reasonably sure that's its IPAddressPool from a previous run. It can't make the same assumption if some other pool simply conflicts. If you don't want that other pool, you shouldn't create it in the first place:

#834

[pmalek]

#835 introduced disabling IPAddressPool. This will have to be included in e.g. KIC through e.g. an env flag when running tests.