Skip to content

Conversation

@thatmattlong
Copy link
Collaborator

@thatmattlong thatmattlong commented Sep 2, 2021

Reason for Change:

Let Pods query IMDS when CNS is doing the IPAM (when IPs come from secondary IPs of NC)

Requirements:

Notes:

const (
AzureDNS = "168.63.129.16"
AzureDNS = "168.63.129.16"
AzureIMDS = "169.254.169.254"
Copy link
Collaborator

Choose a reason for hiding this comment

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

idk what this is for, specifically, but should we do it for the whole link-local block?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is specifically to allow http traffic to the metadata service from pods. Since NCs are considered "secondary interfaces" and there is an explicit deny rule on the host to drop http traffic from secondary interfaces. So really we only want to snat traffic to this one IP against the host IP , since it gets dropped otherwise. For the rest of link-local there is no explicit deny rule so no need to snat.

// https://github.com/Azure/AgentBaker/pull/367/files
podTrafficAccept := fmt.Sprintf(" -m iprange ! --dst-range 168.63.129.16-168.63.129.16 -s %s ", ncSubnetPrefix.String())
snatPrimaryIPJump := fmt.Sprintf("%s --to %s", iptables.Snat, info.ncPrimaryIP)
snatHostIPJump := fmt.Sprintf("%s --to %s", iptables.Snat, info.hostPrimaryIP)
Copy link
Member

Choose a reason for hiding this comment

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

add a comment what the rule is?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@tamilmani1989 tamilmani1989 left a comment

Choose a reason for hiding this comment

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

lgtm

@rbtr rbtr merged commit 2e6a5f6 into Azure:master Sep 3, 2021
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.

3 participants