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

fix update snat rules not effect correctly #2554

Merged
merged 1 commit into from Mar 27, 2023
Merged

Conversation

qiutingjun
Copy link
Contributor

@qiutingjun qiutingjun commented Mar 26, 2023

What type of this PR

Examples of user facing changes:

  • Bug fixes

While update the internal CIDR of the snat rule, it will not really take effect in the vpc nat gateway pod, the rule k8s shows does not match the actual rule in gateway pod.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 26, 2023

  • The commit message should be more descriptive and informative.
  • There are no potential bugs or format errors in the code patch.
  • There is a performance issue in the handleUpdateIptablesSnatRule function where it deletes and creates SNAT rules even if there is no change in the internal CIDR. This can be optimized to avoid unnecessary operations.
  • The code can be improved by adding comments to explain the purpose of each function and variable.

@hongzhen-ma
Copy link
Collaborator

hongzhen-ma commented Mar 27, 2023

上边机器人 review意见第三条
There is a performance issue in the handleUpdateIptablesSnatRule function where it deletes and creates SNAT rules even if there is no change in the internal CIDR. This can be optimized to avoid unnecessary operations.

是否需要考虑下?
这个如果考虑修改的话,可以等合入之后,一起看下 dnat snat 和 fip 的处理。

@qiutingjun
Copy link
Contributor Author

qiutingjun commented Mar 27, 2023

上边机器人 review意见第三条 There is a performance issue in the handleUpdateIptablesSnatRule function where it deletes and creates SNAT rules even if there is no change in the internal CIDR. This can be optimized to avoid unnecessary operations.

是否需要考虑下?

The rule has been checked by function 'enqueueUpdateIptablesSnatRule'.If there is no changes it will not be passed here for processing.

@bobz965 bobz965 merged commit f955143 into kubeovn:master Mar 27, 2023
21 of 52 checks passed
@zhangzujian zhangzujian mentioned this pull request Mar 27, 2023
1 task
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.

None yet

3 participants