-
Notifications
You must be signed in to change notification settings - Fork 260
Multitenancy Support using vlanid #156
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
Conversation
…working into multitenancy
…working into multitenancy
…working into multitenancy
…working into multitenancy # Conflicts: # cns/dnccontract.go
…iner-networking into multitenancy # Conflicts: # cns/dnccontract.go # cns/restserver/restserver.go
Added getnetworkcontainerbyorchestrator context api and relevant test cases
…working into multitenancy # Conflicts: # cns/dnccontract.go # cns/restserver/restserver.go
…iner-networking into multitenancy # Conflicts: # client/cnsclient/cnsclient.go # cni/network/network.go
…working into multitenancy # Conflicts: # client/cnsclient/cnsclient.go # cni/network/network.go # cns/service/main.go
cni/network/network.go
Outdated
| if ipAddr.To4() != nil { | ||
| resultIpconfig.Version = "4" | ||
| resultIpconfig.Address = net.IPNet{IP: ipAddr, Mask: net.CIDRMask(int(ipconfig.IPSubnet.PrefixLength), 32)} | ||
| } else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed
cni/network/network.go
Outdated
| gwIP := net.ParseIP(route.GatewayIPAddress) | ||
| result.Routes = append(result.Routes, &types.Route{Dst: *routeIPnet, GW: gwIP}) | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can return route here instead of having else.
cni/network/network.go
Outdated
| } else { | ||
| gwIP := net.ParseIP(networkConfig.IPConfiguration.GatewayIPAddress) | ||
| dstIP := net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: resultIpconfig.Address.Mask} | ||
| result.Routes = append(result.Routes, &types.Route{Dst: dstIP, GW: gwIP}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if result.Routes==nil?
cni/network/network.go
Outdated
| return convertToCniResult(networkConfig), networkConfig.MultiTenancyInfo.ID, *subnetPrefix, nil | ||
| } | ||
|
|
||
| func getPodNameWithoutSuffix(podName string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
func getPodNameWithoutSuffix(podName string) string {
nameSplit := strings.Split(podName, "-")
if len(nameSplit) < 2 {
return podName
}
return strings.Join(nameSplit[:2], "-")
}…working into ovsabstract # Conflicts: # cni/network/network.go # cns/service/main.go # network/endpoint.go # network/endpoint_linux.go
| return nil, err | ||
| } | ||
|
|
||
| defer res.Body.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if we need to close in case we get err!=nil
| @@ -0,0 +1,85 @@ | |||
| package cnsclient | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move it under cns package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cni/azure-linux.conflist
Outdated
| "type":"azure-vnet", | ||
| "mode":"bridge", | ||
| "bridge":"azure0", | ||
| "multiTenancy":true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a new conflist for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cni/azure-linux.conflist
Outdated
| "type":"azure-vnet", | ||
| "mode":"bridge", | ||
| "bridge":"azure0", | ||
| "multiTenancy":true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add enablesnat: true
cni/network/network.go
Outdated
| // Plugin name. | ||
| name = "azure-vnet" | ||
| name = "azure-vnet" | ||
| namespaceKey = "K8S_POD_NAMESPACE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove it as it is not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| } | ||
|
|
||
| func (service *httpRestService) getNetworkContainerResponse(req cns.GetNetworkContainerRequest) cns.GetNetworkContainerResponse { | ||
| var containerID string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log at beginning and end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is internal function
cns/restserver/restserver.go
Outdated
| } | ||
|
|
||
| func (service *httpRestService) getNetworkContainerByOrchestratorContext(w http.ResponseWriter, r *http.Request) { | ||
| log.Printf("[Azure CNS] getNetworkContainer") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cns/service/main.go
Outdated
| { | ||
| Name: acn.OptStopAzureVnet, | ||
| Shorthand: acn.OptStopAzureVnetAlias, | ||
| Description: "Start Azure-CNM if flag is true", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix description or rename flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
common/utils.go
Outdated
| return err | ||
| } | ||
|
|
||
| func ExecuteShellCommand(command string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
platform specific
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
network/api.go
Outdated
|
|
||
| OptVethName = "vethname" | ||
| OptVethName = "vethname" | ||
| OptEnableSnatOnHost = "enableSnatOnHost" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
…s set to true without reboot
cni/network/network.go
Outdated
| } | ||
|
|
||
| func getContainerNetworkConfiguration(namespace string, podName string) (*cniTypesCurr.Result, *cns.GetNetworkContainerResponse, net.IPNet, error) { | ||
| cnsClient, err := cnsclient.NewCnsClient("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets leave it upto the caller to give CNS address.. ideally, it should come from cni config (when CNI is used)
cni/network/network.go
Outdated
| podInfo := cns.KubernetesPodInfo{PodName: podName, PodNamespace: namespace} | ||
| orchestratorContext, err := json.Marshal(podInfo) | ||
| if err != nil { | ||
| log.Printf("Marshalling azure container instance info failed with %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Marshalling azure container instance info failed with %v" --> "Marshalling cns.KubernetesPodInfo failed with %v"
cni/network/network.go
Outdated
|
|
||
| networkConfig, err := cnsClient.GetNetworkConfiguration(orchestratorContext) | ||
| if err != nil { | ||
| log.Printf("GetNetworkConfiguration failed with %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets print %v orchestrator context to help debug
cni/network/network.go
Outdated
| return podName | ||
| } | ||
|
|
||
| log.Printf("Final namesplit %v", nameSplit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Pod name after splitting based on - : %v"
cni/network/network.go
Outdated
| Name: args.IfName, | ||
| } | ||
|
|
||
| snatIface := &cniTypesCurr.Interface{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move it out to a linux specific method
network/ovs_networkclient_linux.go
Outdated
| } | ||
|
|
||
| if client.enableSnatOnHost { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove empty line
network/ovs_networkclient_linux.go
Outdated
|
|
||
| _, err := net.InterfaceByName(internetBridgeName) | ||
| if err == nil { | ||
| log.Printf("Internet Bridge already exists") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace internet with SNAT everywhere...
| } | ||
|
|
||
| func (client *OVSNetworkClient) AddBridgeRules(extIf *externalInterface) error { | ||
| //primary := extIf.IPAddresses[0].IP.String() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove dead code
network/ovs_networkclient_linux.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func (client *OVSNetworkClient) AddBridgeRules(extIf *externalInterface) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AddL2Rules
ovsctl/ovsctl.go
Outdated
| ) | ||
|
|
||
| const ( | ||
| macAddress = "12:34:56:78:9a:bc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultMacForArpResponse
| epInfo.Data[network.VlanIDKey] = cnsNwConfig.MultiTenancyInfo.ID | ||
| epInfo.Data[network.LocalIPKey] = cnsNwConfig.LocalIPConfiguration.IPSubnet.IPAddress + "/" + strconv.Itoa(int(cnsNwConfig.LocalIPConfiguration.IPSubnet.PrefixLength)) | ||
| epInfo.Data[network.InternetBridgeIPKey] = cnsNwConfig.LocalIPConfiguration.GatewayIPAddress + "/" + strconv.Itoa(int(cnsNwConfig.LocalIPConfiguration.IPSubnet.PrefixLength)) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove space
cns/NetworkContainerContract.go
Outdated
| PrimaryInterfaceIdentifier string // Primary CA. | ||
| AuthorizationToken string | ||
| OrchestratorInfo OrchestratorInfo | ||
| LocalIPConfiguration IPConfiguration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this? Is this from your dnc change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i changed this both in vendor and in my branch
cns/cnsclient/cnsclient.go
Outdated
|
|
||
| defer res.Body.Close() | ||
|
|
||
| if res.StatusCode != 200 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http.StatusOK instead of 200
cns/restserver/restserver.go
Outdated
| getNetworkContainerResponse = cns.GetNetworkContainerResponse{ | ||
| IPConfiguration: savedReq.IPConfiguration, | ||
| Routes: savedReq.Routes, | ||
| CnetAddressSpace: savedReq.CnetAddressSpace, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix spacing
cns/service/main.go
Outdated
|
|
||
| // Create the key value store. | ||
| pluginConfig.Store, err = store.NewJsonFileStore(platform.CNMRuntimePath + pluginName + ".json") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove space
network/ovs_networkclient_linux.go
Outdated
| } | ||
|
|
||
| func createInternetBridge(internetBridgeIP string, mainInterface string) error { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove space
network/ovs_networkclient_linux.go
Outdated
|
|
||
| ip, addr, _ := net.ParseCIDR(internetBridgeIP) | ||
|
|
||
| log.Printf("Assigning %v on internet bridge", internetBridgeIP) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move ParseCIDR below log.Printf
network/ovs_networkclient_linux.go
Outdated
| log.Printf("[net] Failed to delete bridge %v, err:%v.", internetBridgeName, err) | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return err here?
network/ovs_networkclient_linux.go
Outdated
| mac := extIf.MacAddress.String() | ||
| macHex := strings.Replace(mac, ":", "", -1) | ||
|
|
||
| /*if err := ovsctl.AddVMIpAcceptRule(client.bridgeName, primary, mac); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove commented section if not necessary
ovsctl/ovsctl.go
Outdated
|
|
||
| ofport = strings.Trim(ofport, "\n") | ||
|
|
||
| return ofport, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return strings.Trim(ofport, "\n"), nil
| return result | ||
| } | ||
|
|
||
| func getPodNameWithoutSuffix(podName string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the use case of this feature?
What this PR does / why we need it:
This PR adds multitenancy support in both cnm and cni. The user has the option to pass vlanid. If vlanid is specified, then OVS network will be setup and the packets coming out of container will be tagged with corresponding vlan id. In case of CNM, the vlanid can be specified as option in create network request. CNI will get vlanid after querying CNS for podname and namespace.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Release note: