Skip to content
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

make sure gw pod exist before eip creation #4924

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

zbb88888
Copy link
Collaborator

Pull Request

What type of this PR

Examples of user facing changes:

  • Bug fixes

#4922

Which issue(s) this PR fixes

Fixes #(issue-number)

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Jan 14, 2025
@coveralls
Copy link

coveralls commented Jan 14, 2025

Pull Request Test Coverage Report for Build 12800143555

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.002%) to 22.331%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/controller/vpc_nat_gw_eip.go 0 4 0.0%
Totals Coverage Status
Change from base Build 12799483227: -0.002%
Covered Lines: 10376
Relevant Lines: 46464

💛 - Coveralls

@cruickshankpg
Copy link

thanks for this. I think this will prevent the leak when the EIP is misconfigured. There's still the risk that the nat gateway is broken in some way and the exec in pod fails which may cause the IP to leak but that's more of an edge case

@zbb88888
Copy link
Collaborator Author

thanks for this. I think this will prevent the leak when the EIP is misconfigured. There's still the risk that the nat gateway is broken in some way and the exec in pod fails which may cause the IP to leak but that's more of an edge case

yeah, if the pod crashes, and can not run up later, the EIP is leaked. how about talking about theses cases in detail later, we will try to help fix them too.

@zbb88888 zbb88888 force-pushed the create-eip-after-pod-exist branch from f7968a0 to e2b51ef Compare January 15, 2025 02:58
@zbb88888
Copy link
Collaborator Author

@oilbeater 这个ut降低跟这个pr应该也没关系

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 15, 2025
@zbb88888 zbb88888 force-pushed the create-eip-after-pod-exist branch from e2b51ef to df1ea7e Compare January 16, 2025 01:19
@zbb88888 zbb88888 merged commit ccff9b0 into kubeovn:master Jan 16, 2025
5 of 6 checks passed
@zbb88888 zbb88888 deleted the create-eip-after-pod-exist branch January 16, 2025 01:19
zbb88888 added a commit that referenced this pull request Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants