From 7642f0ea22a541e314cf360593adec733a8da44e Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Wed, 31 May 2023 13:42:43 -0400 Subject: [PATCH 01/10] cosmic v6 port mapping policy fix --- cni/network/network.go | 2 ++ cni/network/network_windows.go | 33 ++++++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index e7d47a384b..e67b52b63c 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -764,6 +764,8 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) NATInfo: opt.natInfo, } + opt.nwCfg.IPV6Mode = opt.nwInfo.IPV6Mode + epPolicies := getPoliciesFromRuntimeCfg(opt.nwCfg) epInfo.Policies = append(epInfo.Policies, epPolicies...) diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index d315e3728f..9de6d88c8f 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -242,6 +242,7 @@ func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig) []policy.Policy { log.Printf("[net] RuntimeConfigs: %+v", nwCfg.RuntimeConfig) var policies []policy.Policy var protocol uint32 + for _, mapping := range nwCfg.RuntimeConfig.PortMappings { cfgProto := strings.ToUpper(strings.TrimSpace(mapping.Protocol)) @@ -267,13 +268,39 @@ func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig) []policy.Policy { Settings: rawPolicy, }) - policy := policy.Policy{ + policyv4 := policy.Policy{ Type: policy.EndpointPolicy, Data: hnsv2Policy, } - log.Printf("[net] Creating port mapping policy: %+v", policy) - policies = append(policies, policy) + log.Printf("[net] Creating port mapping policy: %+v", policyv4) + policies = append(policies, policyv4) + + // add port mapping policy for v6 if it's dualstack overlay mode + if nwCfg.IPV6Mode == string(util.DualStackOverlay) { + // To support hostport policy mapping for ipv6 in dualstack overlay mode + // uint32 NatFlagsIPv6 = 2 + rawPolicyv6, _ := json.Marshal(&hnsv2.PortMappingPolicySetting{ + ExternalPort: uint16(mapping.HostPort), + InternalPort: uint16(mapping.ContainerPort), + VIP: mapping.HostIp, + Protocol: protocol, + Flags: hnsv2.NatFlagsIPv6, + }) + + hnsv2Policyv6, _ := json.Marshal(&hnsv2.EndpointPolicy{ + Type: hnsv2.PortMapping, + Settings: rawPolicyv6, + }) + + policyv6 := policy.Policy{ + Type: policy.EndpointPolicy, + Data: hnsv2Policyv6, + } + + log.Printf("[net] Creating port mapping policy v6: %+v", policyv6) + policies = append(policies, policyv6) + } } return policies From ae9211e292c82e339a177234dcdcfcf4c8ab533a Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 2 Jun 2023 07:12:57 -0400 Subject: [PATCH 02/10] add fixes --- cni/network/network.go | 14 ++++++-------- cni/network/network_linux.go | 2 +- cni/network/network_windows.go | 4 ++-- network/network.go | 1 + network/network_windows.go | 5 ++--- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index e67b52b63c..82da26d439 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -40,6 +40,7 @@ const ( // Supported IP version. Currently support only IPv4 ipamV6 = "azure-vnet-ipamv6" defaultRequestTimeout = 15 * time.Second + dualStackOverlay = "dualStackOverlay" ) // CNI Operation Types @@ -637,14 +638,12 @@ func (plugin *NetPlugin) createNetworkInternal( NetNs: ipamAddConfig.args.Netns, Options: ipamAddConfig.options, DisableHairpinOnHostInterface: ipamAddConfig.nwCfg.DisableHairpinOnHostInterface, - IPV6Mode: ipamAddConfig.nwCfg.IPV6Mode, + IPV6Mode: ipamAddConfig.nwCfg.IPV6Mode, //TODO: check if this can be removed. IPAMType: ipamAddConfig.nwCfg.IPAM.Type, ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, + IsIPv6Enabled: ipamAddResult.ipv6Result != nil, } - // set IPv6Mode to dualStackOverlay mode - nwInfo.IPV6Mode = ipamAddConfig.nwCfg.IPAM.Mode - if err = addSubnetToNetworkInfo(ipamAddResult, &nwInfo); err != nil { log.Printf("[cni-net] Failed to add subnets to networkInfo due to %+v", err) return nwInfo, err @@ -764,9 +763,8 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) NATInfo: opt.natInfo, } - opt.nwCfg.IPV6Mode = opt.nwInfo.IPV6Mode - - epPolicies := getPoliciesFromRuntimeCfg(opt.nwCfg) + isIPv6Enabled := opt.resultV6 != nil + epPolicies := getPoliciesFromRuntimeCfg(opt.nwCfg, isIPv6Enabled) epInfo.Policies = append(epInfo.Policies, epPolicies...) // Populate addresses. @@ -776,7 +774,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) if opt.resultV6 != nil { // inject ipv6 routes to Linux pod - epInfo.IPV6Mode = string(util.IpamMode(opt.nwCfg.IPAM.Mode)) + epInfo.IPV6Mode = dualStackOverlay for _, ipconfig := range opt.resultV6.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } diff --git a/cni/network/network_linux.go b/cni/network/network_linux.go index 54789bcf79..985961d874 100644 --- a/cni/network/network_linux.go +++ b/cni/network/network_linux.go @@ -124,7 +124,7 @@ func getEndpointPolicies(PolicyArgs) ([]policy.Policy, error) { // getPoliciesFromRuntimeCfg returns network policies from network config. // getPoliciesFromRuntimeCfg is a dummy function for Linux platform. -func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig) []policy.Policy { +func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig, isIPv6Enabled bool) []policy.Policy { return nil } diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index 9de6d88c8f..f820e52e35 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -238,7 +238,7 @@ func getEndpointDNSSettings(nwCfg *cni.NetworkConfig, result *cniTypesCurr.Resul } // getPoliciesFromRuntimeCfg returns network policies from network config. -func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig) []policy.Policy { +func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig, isIPv6Enabled bool) []policy.Policy { log.Printf("[net] RuntimeConfigs: %+v", nwCfg.RuntimeConfig) var policies []policy.Policy var protocol uint32 @@ -277,7 +277,7 @@ func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig) []policy.Policy { policies = append(policies, policyv4) // add port mapping policy for v6 if it's dualstack overlay mode - if nwCfg.IPV6Mode == string(util.DualStackOverlay) { + if isIPv6Enabled { // To support hostport policy mapping for ipv6 in dualstack overlay mode // uint32 NatFlagsIPv6 = 2 rawPolicyv6, _ := json.Marshal(&hnsv2.PortMappingPolicySetting{ diff --git a/network/network.go b/network/network.go index f09725b027..c34d743383 100644 --- a/network/network.go +++ b/network/network.go @@ -74,6 +74,7 @@ type NetworkInfo struct { IPV6Mode string IPAMType string ServiceCidrs string + IsIPv6Enabled bool } // SubnetInfo contains subnet information for a container network. diff --git a/network/network_windows.go b/network/network_windows.go index b6a86ac88f..ebc0129a2f 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -12,7 +12,6 @@ import ( "strings" "time" - "github.com/Azure/azure-container-networking/cni/util" "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/network/policy" @@ -404,8 +403,8 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern if err != nil { // if network not found, create the HNS network. if errors.As(err, &hcn.NetworkNotFoundError{}) { - // in dualStackOverlay mode, add net routes to windows node - if nwInfo.IPV6Mode == string(util.DualStackOverlay) { + // add net routes to windows node in dualStackOverlay mode + if nwInfo.IsIPv6Enabled { if err := nm.addNewNetRules(nwInfo); err != nil { // nolint log.Printf("[net] Failed to add net rules due to %+v", err) return nil, err From a2f521c3cbae82a2a208a62bb594b6c0c2279aee Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 2 Jun 2023 14:57:45 -0400 Subject: [PATCH 03/10] add TODO --- cni/network/network.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/network.go b/cni/network/network.go index 82da26d439..e0b61874df 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -774,7 +774,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) if opt.resultV6 != nil { // inject ipv6 routes to Linux pod - epInfo.IPV6Mode = dualStackOverlay + epInfo.IPV6Mode = dualStackOverlay // TODO: can this IPV6Mode be deprecated and can we add IsIPv6Enabled flag for generic working for _, ipconfig := range opt.resultV6.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } From d369222ef696d79b50d263b419125c8d26ab63c9 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 2 Jun 2023 16:03:35 -0400 Subject: [PATCH 04/10] fix comments --- cni/network/network.go | 5 ++--- cni/network/network_windows.go | 2 +- network/network_windows.go | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index e0b61874df..4e2746f3b0 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -40,7 +40,6 @@ const ( // Supported IP version. Currently support only IPv4 ipamV6 = "azure-vnet-ipamv6" defaultRequestTimeout = 15 * time.Second - dualStackOverlay = "dualStackOverlay" ) // CNI Operation Types @@ -638,7 +637,7 @@ func (plugin *NetPlugin) createNetworkInternal( NetNs: ipamAddConfig.args.Netns, Options: ipamAddConfig.options, DisableHairpinOnHostInterface: ipamAddConfig.nwCfg.DisableHairpinOnHostInterface, - IPV6Mode: ipamAddConfig.nwCfg.IPV6Mode, //TODO: check if this can be removed. + IPV6Mode: ipamAddConfig.nwCfg.IPV6Mode, //TODO: check if IPV6Mode field can be deprecated IPAMType: ipamAddConfig.nwCfg.IPAM.Type, ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, IsIPv6Enabled: ipamAddResult.ipv6Result != nil, @@ -774,7 +773,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) if opt.resultV6 != nil { // inject ipv6 routes to Linux pod - epInfo.IPV6Mode = dualStackOverlay // TODO: can this IPV6Mode be deprecated and can we add IsIPv6Enabled flag for generic working + epInfo.IPV6Mode = string(util.IpamMode(opt.nwCfg.IPAM.Mode)) // TODO: check IPV6Mode field can be deprecated and can we add IsIPv6Enabled flag for generic working for _, ipconfig := range opt.resultV6.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index f820e52e35..abe4aefb91 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -276,7 +276,7 @@ func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig, isIPv6Enabled bool) []p log.Printf("[net] Creating port mapping policy: %+v", policyv4) policies = append(policies, policyv4) - // add port mapping policy for v6 if it's dualstack overlay mode + // add port mapping policy for v6 if we have IPV6 enabled if isIPv6Enabled { // To support hostport policy mapping for ipv6 in dualstack overlay mode // uint32 NatFlagsIPv6 = 2 diff --git a/network/network_windows.go b/network/network_windows.go index ebc0129a2f..858db47a6e 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -403,7 +403,7 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern if err != nil { // if network not found, create the HNS network. if errors.As(err, &hcn.NetworkNotFoundError{}) { - // add net routes to windows node in dualStackOverlay mode + // add net routes to windows node if we have IPv6 enabled if nwInfo.IsIPv6Enabled { if err := nm.addNewNetRules(nwInfo); err != nil { // nolint log.Printf("[net] Failed to add net rules due to %+v", err) From dea5a524720c9c2bef6a3d5f298242c736316ca0 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 2 Jun 2023 16:07:22 -0400 Subject: [PATCH 05/10] fix linter issue --- cni/network/network.go | 2 +- cni/network/network_linux.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index 4e2746f3b0..3f7e02054a 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -637,7 +637,7 @@ func (plugin *NetPlugin) createNetworkInternal( NetNs: ipamAddConfig.args.Netns, Options: ipamAddConfig.options, DisableHairpinOnHostInterface: ipamAddConfig.nwCfg.DisableHairpinOnHostInterface, - IPV6Mode: ipamAddConfig.nwCfg.IPV6Mode, //TODO: check if IPV6Mode field can be deprecated + IPV6Mode: ipamAddConfig.nwCfg.IPV6Mode, // TODO: check if IPV6Mode field can be deprecated IPAMType: ipamAddConfig.nwCfg.IPAM.Type, ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, IsIPv6Enabled: ipamAddResult.ipv6Result != nil, diff --git a/cni/network/network_linux.go b/cni/network/network_linux.go index 985961d874..81934bcbd6 100644 --- a/cni/network/network_linux.go +++ b/cni/network/network_linux.go @@ -124,7 +124,7 @@ func getEndpointPolicies(PolicyArgs) ([]policy.Policy, error) { // getPoliciesFromRuntimeCfg returns network policies from network config. // getPoliciesFromRuntimeCfg is a dummy function for Linux platform. -func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig, isIPv6Enabled bool) []policy.Policy { +func getPoliciesFromRuntimeCfg(_ *cni.NetworkConfig, isIPv6Enabled bool) []policy.Policy { return nil } From a39cf70f7d55b85934fd8cc8e1564969835e9470 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 2 Jun 2023 16:10:10 -0400 Subject: [PATCH 06/10] linter issue fix --- cni/network/network_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/network_linux.go b/cni/network/network_linux.go index 81934bcbd6..86cda418c1 100644 --- a/cni/network/network_linux.go +++ b/cni/network/network_linux.go @@ -124,7 +124,7 @@ func getEndpointPolicies(PolicyArgs) ([]policy.Policy, error) { // getPoliciesFromRuntimeCfg returns network policies from network config. // getPoliciesFromRuntimeCfg is a dummy function for Linux platform. -func getPoliciesFromRuntimeCfg(_ *cni.NetworkConfig, isIPv6Enabled bool) []policy.Policy { +func getPoliciesFromRuntimeCfg(_ *cni.NetworkConfig, _ bool) []policy.Policy { return nil } From b5136e834158d7b35d57f473081fcdedd263fa90 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 2 Jun 2023 16:32:37 -0400 Subject: [PATCH 07/10] fix an UT --- cni/network/network_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/network_windows_test.go b/cni/network/network_windows_test.go index 56c84f2e9f..3e498f1da7 100644 --- a/cni/network/network_windows_test.go +++ b/cni/network/network_windows_test.go @@ -253,7 +253,7 @@ func TestSetPoliciesFromNwCfg(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - policies := getPoliciesFromRuntimeCfg(&tt.nwCfg) + policies := getPoliciesFromRuntimeCfg(&tt.nwCfg, false) require.Condition(t, assert.Comparison(func() bool { return len(policies) > 0 && policies[0].Type == policy.EndpointPolicy })) From c1ed85ebdeba109bd0016a0f3f2c0db53d5e4165 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 2 Jun 2023 18:02:27 -0400 Subject: [PATCH 08/10] fix linter issue --- cns/client/client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cns/client/client.go b/cns/client/client.go index d3fe3d9a94..16b85710e2 100644 --- a/cns/client/client.go +++ b/cns/client/client.go @@ -400,7 +400,6 @@ func (c *Client) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) } req.Header.Set(headerContentType, contentTypeJSON) res, err := c.client.Do(req) - if err != nil { return nil, errors.Wrap(err, "http request failed") } From a7c204d2711306a2b2dbb416e5bc751d8d4dfabc Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 2 Jun 2023 18:11:41 -0400 Subject: [PATCH 09/10] linter issue fix --- cni/network/network_windows.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index abe4aefb91..31a8b3e385 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -280,7 +280,7 @@ func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig, isIPv6Enabled bool) []p if isIPv6Enabled { // To support hostport policy mapping for ipv6 in dualstack overlay mode // uint32 NatFlagsIPv6 = 2 - rawPolicyv6, _ := json.Marshal(&hnsv2.PortMappingPolicySetting{ + rawPolicyv6, _ := json.Marshal(&hnsv2.PortMappingPolicySetting{ // nolint ExternalPort: uint16(mapping.HostPort), InternalPort: uint16(mapping.ContainerPort), VIP: mapping.HostIp, @@ -288,7 +288,7 @@ func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig, isIPv6Enabled bool) []p Flags: hnsv2.NatFlagsIPv6, }) - hnsv2Policyv6, _ := json.Marshal(&hnsv2.EndpointPolicy{ + hnsv2Policyv6, _ := json.Marshal(&hnsv2.EndpointPolicy{ // nolint Type: hnsv2.PortMapping, Settings: rawPolicyv6, }) From 467be11c2f45b44efe3d30d24d21668457a8ee0d Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Wed, 7 Jun 2023 15:20:05 -0400 Subject: [PATCH 10/10] fix an UT flag --- cni/network/network_windows_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cni/network/network_windows_test.go b/cni/network/network_windows_test.go index 3e498f1da7..bf9af5d619 100644 --- a/cni/network/network_windows_test.go +++ b/cni/network/network_windows_test.go @@ -253,7 +253,8 @@ func TestSetPoliciesFromNwCfg(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - policies := getPoliciesFromRuntimeCfg(&tt.nwCfg, false) + isIPv6Enabled := false + policies := getPoliciesFromRuntimeCfg(&tt.nwCfg, isIPv6Enabled) require.Condition(t, assert.Comparison(func() bool { return len(policies) > 0 && policies[0].Type == policy.EndpointPolicy }))