Skip to content

Conversation

@erfrimod
Copy link
Contributor

@erfrimod erfrimod commented Oct 4, 2018

Azure CNI doesn’t program any port mappings. Its not supported in Azure CNI.
Earlier it worked because the port mapping was programmed through NAT network which was added by accident.

WINCNI does support it and here is the code that does it...

@saiyan86
Copy link
Contributor

saiyan86 commented Oct 4, 2018

@erfrimod Thanks for contributing! Will there be anything else except for PortMappings in RuntimeConfig?

@erfrimod
Copy link
Contributor Author

erfrimod commented Oct 5, 2018

@saiyan86 I just updated with further changes so that the RuntimeConfig portmappings are converted into Endpoing policy objects and sent to HNS.

@madhanrm
Copy link
Contributor

madhanrm commented Oct 5, 2018

@madhanrm
Copy link
Contributor

madhanrm commented Oct 9, 2018

@saiyan86 Looks like Azure CNI for Linux doesn't handle PortMappings support, and we are just returning nil. Can you take care of implementing this for Linux, when required.

@feiskyer This is the place we would impement DNS Capabilities that you are exposing in Kubelet

@madhanrm
Copy link
Contributor

madhanrm commented Oct 9, 2018

/lgtm

@sharmasushant
Copy link
Contributor

@saiyan86 Looks like Azure CNI for Linux doesn't handle PortMappings support, and we are just returning nil. Can you take care of implementing this for Linux, when required.

@madhanrm portmap plugin handles this for linux. please take a look at conflist file. Let me know if I misunderstood your comment.

@dineshgovindasamy
Copy link

/lgtm

@sharmasushant sharmasushant changed the title Adding PortMapping support to Azure cni. Adding PortMapping support to Azure cni for Windows. Oct 10, 2018
Copy link
Contributor

@saiyan86 saiyan86 left a comment

Choose a reason for hiding this comment

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

/lgtm

@saiyan86
Copy link
Contributor

@erfrimod Could you please resolve the conflicts so we can merge this PR? Thanks!

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.

5 participants