Skip to content

Add webhook for ipaddressallocation #1134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TaoZou1
Copy link
Contributor

@TaoZou1 TaoZou1 commented Jul 2, 2025

Search all the service under the same namespace. If the ip has been used in the service, operator will reject the deletion of ipaddressallocation

Test Done:
case 1:

  1. create an ipaddressallocation with ip 192.168.0.50
  2. update svc with loadBalancerIP: 192.168.0.50
  3. wait for svc ready
  4. delete the ipaddressallocation
  5. check if return error like: Error from server (Forbidden): admission webhook
    "ipaddressallocation.validating.crd.nsx.vmware.com" denied the request: cannot delete IPAddressAllocation ip2: IP 192.168.0.50 is still in use by Service milk-svc

case 2:

  1. create an ipaddressallocation with ip size 4, 192.168.0.52/30
  2. update svc with loadBalancerIP: 192.168.0.55
  3. wait for svc ready
  4. delete the ipaddressallocation
  5. check if return error like: Error from server (Forbidden): admission webhook
    "ipaddressallocation.validating.crd.nsx.vmware.com" denied the request: cannot delete IPAddressAllocation ip1: IP 192.168.0.55 is still in use by Service milk-svc

@TaoZou1 TaoZou1 requested review from zhengxiexie and yanjunz97 July 2, 2025 08:45
@codecov-commenter
Copy link

codecov-commenter commented Jul 2, 2025

Codecov Report

Attention: Patch coverage is 82.35294% with 6 lines in your changes missing coverage. Please review.

Project coverage is 73.03%. Comparing base (57dd60d) to head (75cb195).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
...ipaddressallocation/ipaddressallocation_webhook.go 82.35% 4 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1134      +/-   ##
==========================================
- Coverage   74.11%   73.03%   -1.08%     
==========================================
  Files         137      137              
  Lines       21572    21702     +130     
==========================================
- Hits        15988    15851     -137     
- Misses       4576     4853     +277     
+ Partials     1008      998      -10     
Flag Coverage Δ
unit-tests 73.03% <82.35%> (-1.08%) ⬇️
Files with missing lines Coverage Δ
...ipaddressallocation/ipaddressallocation_webhook.go 82.45% <82.35%> (-0.16%) ⬇️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

yanjunz97
yanjunz97 previously approved these changes Jul 3, 2025
Copy link
Contributor

@yanjunz97 yanjunz97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@TaoZou1
Copy link
Contributor Author

TaoZou1 commented Jul 7, 2025

/e2e

1 similar comment
@TaoZou1
Copy link
Contributor Author

TaoZou1 commented Jul 8, 2025

/e2e

@TaoZou1
Copy link
Contributor Author

TaoZou1 commented Jul 10, 2025

/e2e

@TaoZou1 TaoZou1 force-pushed the ipcheck branch 2 times, most recently from 46b7405 to 861d543 Compare July 11, 2025 07:42
zhengxiexie
zhengxiexie previously approved these changes Jul 14, 2025
Copy link
Contributor

@zhengxiexie zhengxiexie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Search all the service under the same namespace. If the ip has been used
in the service, operator will reject the deletion of ipaddressallocation

Test Done:
1. create an ipaddressallocation with ip 192.168.0.50
2. update svc with loadBalancerIP: 192.168.0.50
3. wait for svc ready
4. delete the ipaddressallocation
5. check if return error like:
Error from server (Forbidden): admission webhook
"ipaddressallocation.validating.crd.nsx.vmware.com" denied the request: cannot
delete IPAddressAllocation ip2: IP 192.168.0.50 is still in use by Service
milk-svc
@TaoZou1
Copy link
Contributor Author

TaoZou1 commented Jul 17, 2025

/e2e

1 similar comment
@TaoZou1
Copy link
Contributor Author

TaoZou1 commented Jul 17, 2025

/e2e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants