Skip to content

Only pull the aws resources from its own VPC#41783

Merged
pippolo84 merged 2 commits intocilium:mainfrom
liyihuang:pr/liyihuang/vpc_filter
Oct 1, 2025
Merged

Only pull the aws resources from its own VPC#41783
pippolo84 merged 2 commits intocilium:mainfrom
liyihuang:pr/liyihuang/vpc_filter

Conversation

@liyihuang
Copy link
Copy Markdown
Contributor

@liyihuang liyihuang commented Sep 19, 2025 •
edited
Loading

This PR will make aws operator to read the metadata info to get its own VPC ID and use it as filter before it pulls the aws vpc/subnet/route/security group so it can reduce the cpu/memory and other potential issues(like too many responds causing ec2 client timeout etc)

With this PR, we will see the that we only sync one VPC

time=2025-09-20T03:16:59.21773155Z level=info msg="Synchronized ENI information" module=operator.operator-controlplane.leader-lifecycle.legacy-cell subsys=eni numInstances=9 numVPCs=1 numSubnets=6 numRouteTables=3 numSecurityGroups=4

without this PR, in the same AWS account, we can see that we sync other resources from other VPCs

time=2025-09-20T03:21:32.90667992Z level=info msg="Synchronized ENI information" module=operator.operator-controlplane.leader-lifecycle.legacy-cell subsys=eni numInstances=9 numVPCs=6 numSubnets=29 numRouteTables=14 numSecurityGroups=23

Fixes: #41392

aws operator pulls its own VPC resources based on its own vpc id

@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 Sep 19, 2025
@liyihuang liyihuang changed the title Pr/liyihuang/vpc filter Only pull the aws resources from its own VPC Sep 19, 2025
@liyihuang liyihuang added integration/cloud Related to integration with cloud environments such as AKS, EKS, GKE, etc. area/eni Impacts ENI based IPAM. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Sep 19, 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 Sep 19, 2025
@liyihuang liyihuang added release-note/misc This PR makes changes that have no direct user impact. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Sep 19, 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 Sep 19, 2025
@liyihuang
Copy link
Copy Markdown
Contributor Author

liyihuang commented Sep 19, 2025

/test

@liyihuang liyihuang marked this pull request as ready for review September 20, 2025 03:25
@liyihuang liyihuang requested review from a team as code owners September 20, 2025 03:25
HadrienPatte
HadrienPatte reviewed Sep 21, 2025
View reviewed changes
@liyihuang
Copy link
Copy Markdown
Contributor Author

liyihuang commented Sep 21, 2025

@HadrienPatte thanks for the review. I will adjust those changes later today. I actually have a question for you since datadog is using the ENI GC where I didn't do the filter for that call. Do you think that will help you? I remember there was a PR from the datadog saying that you have to set the max result to 1000 for ENI if I'm not mistaken.

@HadrienPatte
Copy link
Copy Markdown
Member

HadrienPatte commented Sep 22, 2025

datadog is using the ENI GC where I didn't do the filter for that call. Do you think that will help you?

I don't think it's necessary to add VPC filtering to the ENI GC logic as it is already filtering on cluster name which should be more fine grained than VPC ID.

@liyihuang
Copy link
Copy Markdown
Contributor Author

liyihuang commented Sep 22, 2025 •
edited
Loading

datadog is using the ENI GC where I didn't do the filter for that call. Do you think that will help you?

I don't think it's necessary to add VPC filtering to the ENI GC logic as it is already filtering on cluster name which should be more fine grained than VPC ID.

yes, if using the default name. no, if using the user defined name. Maybe we should append the user define tag with default tags to resolve this particular issue

@liyihuang liyihuang force-pushed the pr/liyihuang/vpc_filter branch from 2dc31f6 to ff60513 Compare September 22, 2025 20:23
@liyihuang
Copy link
Copy Markdown
Contributor Author

liyihuang commented Sep 22, 2025

/test

@liyihuang liyihuang force-pushed the pr/liyihuang/vpc_filter branch 3 times, most recently from 7a3718f to 784d0c4 Compare September 23, 2025 03:59
bimmlerd
bimmlerd approved these changes Sep 23, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

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

Looks okay

@liyihuang liyihuang force-pushed the pr/liyihuang/vpc_filter branch from 784d0c4 to 1d5ce34 Compare September 23, 2025 13:17
pippolo84
pippolo84 approved these changes Sep 23, 2025
View reviewed changes
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.

Thanks! 💯

LGTM minus a non-blocking nit left inline.

@liyihuang liyihuang force-pushed the pr/liyihuang/vpc_filter branch from 1d5ce34 to 2824277 Compare September 23, 2025 18:32
@liyihuang
Copy link
Copy Markdown
Contributor Author

liyihuang commented Sep 23, 2025

/test

@liyihuang liyihuang force-pushed the pr/liyihuang/vpc_filter branch from 2824277 to 504375b Compare September 23, 2025 20:36
@liyihuang
Copy link
Copy Markdown
Contributor Author

liyihuang commented Sep 23, 2025

/test

@liyihuang
pkg/aws: filter aws resources through vpc id
dbe5911 
Make the aws operator to figure out its own vpcID to filter common AWS
resources to reduce the aws calls and operator's CPU and memory
pressure.

Signed-off-by: Liyi Huang <liyi.huang@isovalent.com>
@liyihuang
pkg/aws: refactor metadata package
96ab3ec 
Refactor metadata package and related var and add metadata_mock so we
can properly perform the test for aws pkg.

Signed-off-by: Liyi Huang <liyi.huang@isovalent.com>
@liyihuang liyihuang force-pushed the pr/liyihuang/vpc_filter branch from 504375b to 96ab3ec Compare September 24, 2025 02:31
@liyihuang
Copy link
Copy Markdown
Contributor Author

liyihuang commented Sep 24, 2025

/test

@liyihuang liyihuang mentioned this pull request Sep 24, 2025
Closed
@dwj300
Copy link
Copy Markdown
Contributor

dwj300 commented Sep 29, 2025

@liyihuang @joamaki , can we get this merged before the next patch release?

@liyihuang
Copy link
Copy Markdown
Contributor Author

liyihuang commented Sep 29, 2025

this is pending review from @hemanthmalla. If it get approves, I dont know if we can backport to 1.18 since we usually only backport the bug fix.

hemanthmalla
hemanthmalla approved these changes Sep 29, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@hemanthmalla hemanthmalla left a comment

Choose a reason for hiding this comment

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

Thank you.

@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 Sep 29, 2025
@dwj300
Copy link
Copy Markdown
Contributor

dwj300 commented Sep 30, 2025

@liyihuang We'd love to start testing this via the daily build, any chance we can get this merged?

@liyihuang
Copy link
Copy Markdown
Contributor Author

liyihuang commented Sep 30, 2025

I dont have the permission to merge PR to main.

I guess if you want to test now, you can test the PR image and chart.

You can find it here

https://github.com/cilium/cilium/actions/runs/17964550021/job/51094783064?pr=41783.

helm template -n kube-system oci://quay.io/cilium-charts-dev/cilium --version 1.19.0-dev-dev.297-96ab3ec6d0
helm install cilium -n kube-system oci://quay.io/cilium-charts-dev/cilium --version 1.19.0-dev-dev.297-96ab3ec6d0

@pippolo84 pippolo84 mentioned this pull request Oct 1, 2025
Merged
@pippolo84 pippolo84 added this pull request to the merge queue Oct 1, 2025
@pippolo84
Copy link
Copy Markdown
Member

pippolo84 commented Oct 1, 2025

Merging this since all the comments have been addressed and it is also blocking #41954

Merged via the queue into cilium:main with commit 8c91170 Oct 1, 2025
73 checks passed
@maintainer-s-little-helper maintainer-s-little-helper bot removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Oct 1, 2025
@gandro gandro mentioned this pull request Oct 8, 2025
Closed
@liyihuang liyihuang deleted the pr/liyihuang/vpc_filter branch November 12, 2025 18:33
@cilium-release-bot cilium-release-bot bot moved this to Released in cilium v1.19.0 Feb 3, 2026
@liyihuang liyihuang mentioned this pull request Feb 5, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pippolo84 pippolo84 pippolo84 approved these changes

@hemanthmalla hemanthmalla hemanthmalla approved these changes

@bimmlerd bimmlerd bimmlerd approved these changes

@HadrienPatte HadrienPatte Awaiting requested review from HadrienPatte

Assignees

No one assigned

Labels

area/eni Impacts ENI based IPAM. integration/cloud Related to integration with cloud environments such as AKS, EKS, GKE, etc. release-note/misc This PR makes changes that have no direct user impact.

Projects

No open projects
cilium v1.19.0
Status: Released

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

no proper filtering in aws/cloud enviroment for vpc/subnets etc...

7 participants

@liyihuang @HadrienPatte @dwj300 @pippolo84 @hemanthmalla @bimmlerd @msune