[do-not-review]Singularitywork #2416
Conversation
cns/wireserver/net.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| secondaryIP := "" |
There was a problem hiding this comment.
nit: var string secondaryIP instead of empty string init
There was a problem hiding this comment.
also Note: we need to pass macAddress in this function & return interface based on i.IsPrimary=false & i.MACAddress==mac
| if service.state.secondaryInterface == nil { | ||
| res, err := service.wscli.GetInterfaces(ctx) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "failed to get interfaces from IMDS") |
There was a problem hiding this comment.
nit: would be good to add comment on what is IMDS
| hostSubnet: response.PodIPInfo[i].HostPrimaryIPInfo.Subnet, | ||
| hostPrimaryIP: response.PodIPInfo[i].HostPrimaryIPInfo.PrimaryIP, | ||
| hostGateway: response.PodIPInfo[i].HostPrimaryIPInfo.Gateway, | ||
| hostSubnet: response.PodIPInfo[i].HostSecondaryIPInfo.Subnet, |
There was a problem hiding this comment.
todo: change the fields returned here as per NIC type (primary or secondary) as this is a common code path
cni/network/network.go
Outdated
|
|
||
| addSnatInterface(nwCfg, defaultCniResult) | ||
| //defaultCniResult := convertInterfaceInfoToCniResult(ipamAddResult.defaultInterfaceInfo, args.IfName) | ||
| secondaryCniResult := convertInterfaceInfoToCniResult(ipamAddResult.secondaryInterfacesInfo[0], args.IfName) |
There was a problem hiding this comment.
we need branching here to program either default interface or secondary interface - TBD on approach/config to use
| zap.String("network", networkID), | ||
| zap.String("subnet", nwInfo.Subnets[0].Prefix.String())) | ||
| // logger.Info("Found network with subnet", | ||
| // zap.String("network", networkID), |
There was a problem hiding this comment.
why have we commented this part? todo: uncomment
| @@ -666,7 +676,7 @@ func (plugin *NetPlugin) createNetworkInternal( | |||
|
|
|||
| // construct network info with ipv4/ipv6 subnets | |||
| func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo *network.NetworkInfo) error { | |||
| for _, ipConfig := range ipamAddResult.defaultInterfaceInfo.IPConfigs { | |||
| for _, ipConfig := range ipamAddResult.secondaryInterfacesInfo[0].IPConfigs { | |||
There was a problem hiding this comment.
add branching or call different function on L678 (new function for secondary nics)
cni/network/network.go
Outdated
| @@ -707,15 +717,24 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) | |||
| epInfo := network.EndpointInfo{} | |||
|
|
|||
| defaultInterfaceInfo := opt.ipamAddResult.defaultInterfaceInfo | |||
| epDNSInfo, err := getEndpointDNSSettings(opt.nwCfg, defaultInterfaceInfo.DNS, opt.k8sNamespace) | |||
| secondaryInterfaceInfo := opt.ipamAddResult.secondaryInterfacesInfo | |||
There was a problem hiding this comment.
need branching here too for primary vs secondary nics
| @@ -489,6 +491,7 @@ type IPConfigsRequest struct { | |||
| OrchestratorContext json.RawMessage `json:"orchestratorContext"` | |||
| Ifname string `json:"ifname"` // Used by delegated IPAM | |||
| SecondaryInterfacesExist bool `json:"secondaryInterfacesExist"` // will be set by SWIFT v2 validator func | |||
| ProgramSecondaryNICOnly bool `json:"programSecondaryNICOnly"` | |||
There was a problem hiding this comment.
todo: kmurudi: remove & use the previous field if possible to set
| }, err | ||
| } | ||
| } | ||
| // if service.Options[common.OptManageEndpointState] == true { |
There was a problem hiding this comment.
todo: we need to discuss branching here or take it up in my PR
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return nw.newEndpointImplHnsV2(cli, epInfo[0]) | ||
| return nw.newEndpointImplHnsV2(cli, epInfo[1]) |
There was a problem hiding this comment.
need to choose the right endpoint here based on interface
| @@ -206,6 +207,10 @@ func (nw *network) addIPv6NeighborEntryForGateway(epInfo *EndpointInfo, plc plat | |||
| func (nw *network) configureHcnEndpoint(epInfo *EndpointInfo) (*hcn.HostComputeEndpoint, error) { | |||
| infraEpName, _ := ConstructEndpointID(epInfo.ContainerID, epInfo.NetNsPath, epInfo.IfName) | |||
|
|
|||
| mac := epInfo.MacAddress.String() | |||
There was a problem hiding this comment.
will these new fields be needed for every request? (primary, non-swift2.0, secondary) if not need branching here too
| @@ -338,6 +339,12 @@ func (nm *networkManager) CreateEndpoint(cli apipaClient, networkID string, epIn | |||
| return err | |||
| } | |||
|
|
|||
| for _, n := range nw.Endpoints { | |||
| n.IfName = "eth0" | |||
| n.Id = n.Id + "eth0" | |||
There was a problem hiding this comment.
add constant variable for this string addition
| @@ -290,10 +290,14 @@ func (nm *networkManager) configureHcnNetwork(nwInfo *NetworkInfo, extIf *extern | |||
| hcnNetwork.Type = hcn.L2Bridge | |||
| case opModeTunnel: | |||
| hcnNetwork.Type = hcn.L2Tunnel | |||
| case opModeTransparent: | |||
| hcnNetwork.Type = hcn.Transparent | |||
| hcnNetwork.Flags = 1024 | |||
There was a problem hiding this comment.
todo: determine later if we need this flag constant or hns changes will support this
kmurudi
force-pushed
the
singularitywork
branch
from
87b27d6 to
bedd713
Compare
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
Pull request closed due to inactivity. |
Reason for Change:
Issue Fixed:
Requirements:
Notes: