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

Init container changes for getting all the primary attached interfaces #3218

Draft
wants to merge 1 commit into
base: multi-nic-support
Choose a base branch
from

Conversation

jaydeokar
Copy link
Contributor

@jaydeokar jaydeokar commented Feb 28, 2025

What type of PR is this?
feature

Which issue does this PR fix?:
N/A

What does this PR do / Why do we need it?:
This adds changes to Init container for CNI. When Multiple NICs are initialized, the init container will identify these interfaces to set kernel parameters.
We skip efa-only interfaces since these cannot be used for IP traffic

Testing done on this change:
Tested on single card instance and multi-card instance

time="2025-02-28T07:26:45Z" level=info msg="Copying CNI plugin binaries ..."
time="2025-02-28T07:26:45Z" level=info msg="Copied all CNI plugin binaries to /host/opt/cni/bin"
time="2025-02-28T07:26:45Z" level=info msg="found primary interfaces for the host [eth0]"
time="2025-02-28T07:26:45Z" level=info msg="Found primaryIF [eth0]"
time="2025-02-28T07:26:45Z" level=info msg="Updated net/ipv4/conf/eth0/rp_filter to 2\n"
time="2025-02-28T07:26:45Z" level=info msg="Updated net/ipv4/tcp_early_demux to 1\n"
time="2025-02-28T07:26:45Z" level=info msg="CNI init container done"

Will this PR introduce any new dependencies?:
No new dependencies.. We already made IMDS calls. But we do have a bit more IMDS calls than previous

Will this break upgrades or downgrades? Has updating a running cluster been tested?:
No, should not break upgrade or downgrade..

Does this change require updates to the CNI daemonset config files to work?:
No

Does this PR introduce any user-facing change?:
Not with this change

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jaydeokar jaydeokar requested a review from M00nF1sh February 28, 2025 07:49
@jaydeokar jaydeokar assigned jayanthvn and unassigned jayanthvn Feb 28, 2025
@jaydeokar jaydeokar requested a review from jayanthvn February 28, 2025 07:50
ec2Metadata := ec2metadata.NewFromConfig(awsconfig)
imds := awsutils.TypedIMDS{ec2Metadata}

macAddresses, err := imds.GetMACs(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this return all ENIs - i.e, even secondary ENIs -

// GetMACs returns the interface addresses attached to the instance.

Copy link
Contributor Author

@jaydeokar jaydeokar Mar 1, 2025

Choose a reason for hiding this comment

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

Yes.. But we are filtering the ENIs which are attached to device-number 0..

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