Skip to content

ipam/aws: enhance findSuitableSubnet logic#37229

Merged
gandro merged 1 commit intocilium:mainfrom
liyihuang:pr/liyihuang/enhance_find_subnet_logic
Mar 6, 2025
Merged

ipam/aws: enhance findSuitableSubnet logic#37229
gandro merged 1 commit intocilium:mainfrom
liyihuang:pr/liyihuang/enhance_find_subnet_logic

Conversation

@liyihuang
Copy link
Copy Markdown
Contributor

@liyihuang liyihuang commented Jan 23, 2025

  • Implement struct to get route table from ec2 api
  • Implement new logic and function to enhance the findSuitableSubnet logic
  • Updates the unit test and mock ec2 api
  • Update the doc

I know I need to rebease this one if #36922 get merged first.

Fixes: #32052

When creating a new ENI in AWS, trying the best to select a subnet with the same route table as the host's primary ENI to prevent unexpected routing behavior.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 23, 2025
@liyihuang
Copy link
Copy Markdown
Contributor Author

/test

1 similar comment
@liyihuang
Copy link
Copy Markdown
Contributor Author

/test

@liyihuang
Copy link
Copy Markdown
Contributor Author

/ci-ipsec-e2e

@liyihuang liyihuang marked this pull request as ready for review January 24, 2025 14:19
@liyihuang liyihuang requested review from a team as code owners January 24, 2025 14:19
@liyihuang liyihuang requested review from a user, doniacld and pippolo84 January 24, 2025 14:19
Copy link
Copy Markdown
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! ✔️

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I feel like the new behaviour should be documented in Documentation/network/concepts/ipam/eni.rst, wdyt?

@ghost ghost added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Jan 29, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 29, 2025
@liyihuang
Copy link
Copy Markdown
Contributor Author

I feel like the new behaviour should be documented in Documentation/network/concepts/ipam/eni.rst, wdyt?

you are absolutely correct. Updated it.

@liyihuang liyihuang force-pushed the pr/liyihuang/enhance_find_subnet_logic branch from 02f2055 to aeb3758 Compare January 30, 2025 17:23
@liyihuang liyihuang requested a review from a user January 30, 2025 17:23
@liyihuang liyihuang force-pushed the pr/liyihuang/enhance_find_subnet_logic branch 2 times, most recently from 6dd4971 to b3924a8 Compare January 30, 2025 18:11
@liyihuang
Copy link
Copy Markdown
Contributor Author

/test

@liyihuang
Copy link
Copy Markdown
Contributor Author

/ci-e2e-upgrade

@liyihuang
Copy link
Copy Markdown
Contributor Author

/ci-ipsec-e2e

@liyihuang
Copy link
Copy Markdown
Contributor Author

/ci-eks

1 similar comment
@liyihuang
Copy link
Copy Markdown
Contributor Author

/ci-eks

@liyihuang
Copy link
Copy Markdown
Contributor Author

/ci-e2e-upgrade

@liyihuang
Copy link
Copy Markdown
Contributor Author

/ci-eks

@liyihuang
Copy link
Copy Markdown
Contributor Author

/ci-ipsec-e2e

@liyihuang liyihuang force-pushed the pr/liyihuang/enhance_find_subnet_logic branch from e024e1e to 3bba137 Compare March 3, 2025 15:13
@liyihuang liyihuang requested a review from a team as a code owner March 3, 2025 15:13
@liyihuang liyihuang requested a review from asauber March 3, 2025 15:13
@liyihuang
Copy link
Copy Markdown
Contributor Author

/test

- Implement struct to get route table from ec2 api
- Implement new logic and function to enhance the findSuitableSubnet logic
- Updates the unit test and mock ec2 api
- Update the doc

Signed-off-by: Liyi Huang  <[email protected]>
@liyihuang liyihuang force-pushed the pr/liyihuang/enhance_find_subnet_logic branch from 3bba137 to 57aeb10 Compare March 3, 2025 16:32
@liyihuang
Copy link
Copy Markdown
Contributor Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 6, 2025
@gandro gandro added this pull request to the merge queue Mar 6, 2025
Merged via the queue into cilium:main with commit eec7036 Mar 6, 2025
67 checks passed
@liyihuang liyihuang deleted the pr/liyihuang/enhance_find_subnet_logic branch March 6, 2025 11:54
HadrienPatte added a commit that referenced this pull request Aug 25, 2025
github-merge-queue bot pushed a commit that referenced this pull request Aug 25, 2025
HadrienPatte added a commit that referenced this pull request Nov 28, 2025
Fixes #43004

This PR does two main logical changes:
* Add a safety check in `findSubnetInSameRouteTableWithNodeSubnet` to
  cover the case when a route table subnet is not present in the
  manager's subnets map.
* Update `syncInfrastructure` to discard subnets from route tables that
  are not tracked in the manager's subnet map.

I also added a new test case for each of those two scenarios.

Related PR: #37229

Signed-off-by: Hadrien Patte <[email protected]>
HadrienPatte added a commit that referenced this pull request Nov 28, 2025
Fixes #43004

This PR does two main logical changes:
* Add a safety check in `findSubnetInSameRouteTableWithNodeSubnet` to
  cover the case when a route table subnet is not present in the
  manager's subnets map.
* Update `syncInfrastructure` to discard subnets from route tables that
  are not tracked in the manager's subnet map.

I also added a new test case for each of those two scenarios.

Related PR: #37229

Signed-off-by: Hadrien Patte <[email protected]>
HadrienPatte added a commit to DataDog/cilium that referenced this pull request Dec 1, 2025
Fixes cilium#43004

Add a safety check in `findSubnetInSameRouteTableWithNodeSubnet` to
cover the case when a route table subnet is not present in the
manager's subnets map.

I also added a new test case for this scenario.

Related PR: cilium#37229

Signed-off-by: Hadrien Patte <[email protected]>
HadrienPatte added a commit that referenced this pull request Dec 1, 2025
Fixes #43004

Add a safety check in `findSubnetInSameRouteTableWithNodeSubnet` to
cover the case when a route table subnet is not present in the
manager's subnets map.

I also added a new test case for this scenario.

Related PR: #37229

Signed-off-by: Hadrien Patte <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Dec 2, 2025
Fixes #43004

Add a safety check in `findSubnetInSameRouteTableWithNodeSubnet` to
cover the case when a route table subnet is not present in the
manager's subnets map.

I also added a new test case for this scenario.

Related PR: #37229

Signed-off-by: Hadrien Patte <[email protected]>
tklauser pushed a commit that referenced this pull request Dec 3, 2025
[ upstream commit 216081a ]

Fixes #43004

Add a safety check in `findSubnetInSameRouteTableWithNodeSubnet` to
cover the case when a route table subnet is not present in the
manager's subnets map.

I also added a new test case for this scenario.

Related PR: #37229

Signed-off-by: Hadrien Patte <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
tklauser pushed a commit that referenced this pull request Dec 3, 2025
[ upstream commit 216081a ]

Fixes #43004

Add a safety check in `findSubnetInSameRouteTableWithNodeSubnet` to
cover the case when a route table subnet is not present in the
manager's subnets map.

I also added a new test case for this scenario.

Related PR: #37229

Signed-off-by: Hadrien Patte <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Dec 4, 2025
[ upstream commit 216081a ]

Fixes #43004

Add a safety check in `findSubnetInSameRouteTableWithNodeSubnet` to
cover the case when a route table subnet is not present in the
manager's subnets map.

I also added a new test case for this scenario.

Related PR: #37229

Signed-off-by: Hadrien Patte <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
nddq pushed a commit to nddq/cilium that referenced this pull request Feb 17, 2026
[ upstream commit 216081a ]

Fixes cilium#43004

Add a safety check in `findSubnetInSameRouteTableWithNodeSubnet` to
cover the case when a route table subnet is not present in the
manager's subnets map.

I also added a new test case for this scenario.

Related PR: cilium#37229

Signed-off-by: Hadrien Patte <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CFP: Add extra logic for cilium ENI mode for picking the right subnet

5 participants