perf: only enqueue subnet when necessary

[oilbeater]

Only enqueue subnet when necessary to improve performance.

Also fix vpc reconcile static routes and policy may incorrectly delete rules created by u2o and egress-gateway. This can be auto recover before because vpc will enqueue all subnets when updated.

Fix #6235

Made with Cursor

[gemini-code-assist]

Summary of Changes

Hello @oilbeater, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces performance optimizations by refining how subnets are processed and enqueued within the controller. It ensures that resources are not wasted on subnets that are being deleted or are already in a ready state, thereby improving the efficiency of subnet and VPC reconciliation processes.

Highlights

• Subnet Finalizer Optimization: The handleSubnetFinalizer logic in handleAddOrUpdateSubnet has been reordered to execute earlier. This allows for an early exit if a subnet is marked for deletion, preventing unnecessary subsequent processing and improving efficiency.
• Conditional Subnet Enqueueing: In handleAddOrUpdateVpc, subnets are now only enqueued for update if their status is IsNotReady(). This prevents redundant enqueueing of already healthy subnets when a VPC configuration changes, accelerating updates and reducing controller load.

Changelog

• pkg/controller/subnet.go
  • Reordered the handleSubnetFinalizer call to occur earlier in the handleAddOrUpdateSubnet function, enabling an early return for deleted subnets.
  • Removed the duplicate handleSubnetFinalizer call from its previous position in handleAddOrUpdateSubnet.
• pkg/controller/vpc.go
  • Modified the handleAddOrUpdateVpc function to conditionally enqueue subnets for update only if their status is IsNotReady(), aiming to accelerate updates and prevent unnecessary processing.

Activity

• No specific activity (comments, reviews, etc.) has been provided for this pull request.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request aims to improve performance by reducing unnecessary subnet reconciliations. However, the modification in pkg/controller/vpc.go to only enqueue non-ready subnets when a VPC is updated introduces a security risk. Subnets in a "Ready" state will skip reconciliation, leading to stale network configurations, such as outdated Load Balancer references or routing rules, potentially bypassing intended security policies. This inconsistent state handling has been identified as a medium-severity vulnerability.

[gemini-code-assist]

The change to conditionally enqueue only non-ready subnets when a VPC is updated introduces a security vulnerability. VPC configuration changes (e.g., Load Balancer updates, BFD settings, or routing changes) require all associated subnets, including 'Ready' ones, to be re-reconciled. Skipping 'Ready' subnets can lead to stale security or network configurations, such as outdated Load Balancer references or incorrect routing rules, potentially bypassing intended network security policies or service exposure changes.

Remediation: To ensure configuration consistency and prevent security risks, remove the conditional check and always enqueue all subnets belonging to the VPC when it is updated.

Suggested change

	// Accelerate subnet update when vpc config is updated.
	// In case VPC not set namespaces, subnet will backoff and may take long time to back to ready.
	if subnet.Status.IsNotReady() {
	c.addOrUpdateSubnetQueue.Add(subnet.Name)
	}
	c.addOrUpdateSubnetQueue.Add(subnet.Name)

[coveralls]

Pull Request Test Coverage Report for Build 21808576888

Details

• 7 of 28 (25.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.03%) to 22.916%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/controller/vpc.go	7	28	25.0%

Files with Coverage Reduction	New Missed Lines	%
pkg/controller/vpc.go	1	15.69%

Totals
Change from base Build 21807583595:	0.03%
Covered Lines:	12453
Relevant Lines:	54342

---

💛 - Coveralls