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

add policy route when use old active gateway node for centralized subnet #2722

Merged
merged 1 commit into from
May 3, 2023

Conversation

hongzhen-ma
Copy link
Collaborator

@hongzhen-ma hongzhen-ma commented Apr 26, 2023

What type of this PR

  • Bug fixes

Which issue(s) this PR fixes:

Fixes #(issue-number)

WHAT

🤖 Generated by Copilot at 905d29b

Improve policy route handling for centralized subnets with ecmp. Remove activateGateway field from subnet status and use node's tunnelIP as next hop.

🤖 Generated by Copilot at 905d29b

We're sailing on the network sea, with policy routes to guide us
We don't need activateGateway, we've got tunnelIP beside us
So haul away the subnet status, and make the ecmp steady
We'll reach the centralized subnets, with our code review ready

HOW

🤖 Generated by Copilot at 905d29b

  • Remove the update of subnet status with activate gateway field for ecmp subnets (link)
  • Use tunnel IP as next hop for policy route for centralized subnets (link)

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative.
  • There is a commented out code block that needs to be removed.
  • The logic for handling the ActivateGateway field when ecmp is enabled needs to be reviewed.

@github-actions
Copy link
Contributor

  • The commit message is missing. Please add a descriptive commit message.
  • There is a potential bug in the code. When subnet.Spec.EnableEcmp is true, the ActivateGateway field is deleted from subnet.Status. However, if subnet.Spec.EnableEcmp is changed back to false, the ActivateGateway field is not added back to subnet.Status, which could cause incorrect behavior. This should be fixed by adding the ActivateGateway field back to subnet.Status when subnet.Spec.EnableEcmp is changed back to false.
  • There are format errors in the code. Please run a linter or formatter to fix them.
  • There are no performance issues in the code.
  • There are ways to improve the code. Instead of using time.Sleep(1 * time.Minute) in the test, it would be better to use a WaitUntil function to wait for the policy route to be updated.

@hongzhen-ma hongzhen-ma merged commit 7eed834 into master May 3, 2023
@hongzhen-ma hongzhen-ma deleted the ecmp branch May 3, 2023 09:29
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.

2 participants