From 2af9005637687712e759fcdfc053e3983f5f1bfd Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Mon, 19 Aug 2024 18:22:42 -0700 Subject: [PATCH 01/10] change gw address to .2 --- cns/hnsclient/hnsclient_windows.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index cc24b7917a..0366d4f933 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -52,6 +52,8 @@ const ( // Name of the loopback adapter needed to create Host NC apipa network hostNCLoopbackAdapterName = "LoopbackAdapterHostNCConnectivity" + defaultHnsGwIPAddress = "169.254.128.2" + hnsLoopbackAdapterIPAddress = "169.254.128.1" // protocolTCP indicates the TCP protocol identifier in HCN protocolTCP = "6" @@ -297,7 +299,7 @@ func createHostNCApipaNetwork( if interfaceExists, _ := networkcontainers.InterfaceExists(hostNCLoopbackAdapterName); !interfaceExists { ipconfig := cns.IPConfiguration{ IPSubnet: cns.IPSubnet{ - IPAddress: localIPConfiguration.GatewayIPAddress, + IPAddress: hnsLoopbackAdapterIPAddress, PrefixLength: localIPConfiguration.IPSubnet.PrefixLength, }, GatewayIPAddress: localIPConfiguration.GatewayIPAddress, @@ -569,6 +571,9 @@ func CreateHostNCApipaEndpoint( return endpoint.Id, nil } + if localIPConfiguration.GatewayIPAddress == "169.254.128.1" { + localIPConfiguration.GatewayIPAddress = defaultHnsGwIPAddress + } if network, err = createHostNCApipaNetwork(localIPConfiguration); err != nil { logger.Errorf("[Azure CNS] Failed to create HostNCApipaNetwork. Error: %v", err) return "", err From 28711d50d8c904d48bbcd074e895b7b3d8fcddf1 Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Tue, 20 Aug 2024 14:32:34 -0700 Subject: [PATCH 02/10] refactor and add UTs --- cns/hnsclient/hnsclient_windows.go | 13 +++++++--- cns/hnsclient/hnsclient_windows_test.go | 34 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 cns/hnsclient/hnsclient_windows_test.go diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index 0366d4f933..569e7561d8 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -571,9 +571,7 @@ func CreateHostNCApipaEndpoint( return endpoint.Id, nil } - if localIPConfiguration.GatewayIPAddress == "169.254.128.1" { - localIPConfiguration.GatewayIPAddress = defaultHnsGwIPAddress - } + adhocAdjustIPConfig(&localIPConfiguration) if network, err = createHostNCApipaNetwork(localIPConfiguration); err != nil { logger.Errorf("[Azure CNS] Failed to create HostNCApipaNetwork. Error: %v", err) return "", err @@ -605,6 +603,15 @@ func CreateHostNCApipaEndpoint( return endpoint.Id, nil } +// adhocAdjustIPConfig applies adhoc change on gw IP address +func adhocAdjustIPConfig(localIPConfiguration *cns.IPConfiguration) { + // When gw address is 169.254.128.1, should use .2 instead. If gw address is not .1, that mean this value is + // configured from dnc, we should keep it + if localIPConfiguration.GatewayIPAddress == "169.254.128.1" { + localIPConfiguration.GatewayIPAddress = defaultHnsGwIPAddress + } +} + func getHostNCApipaEndpointName( networkContainerID string) string { return hostNCApipaEndpointNamePrefix + "-" + networkContainerID diff --git a/cns/hnsclient/hnsclient_windows_test.go b/cns/hnsclient/hnsclient_windows_test.go new file mode 100644 index 0000000000..b7a15e4c38 --- /dev/null +++ b/cns/hnsclient/hnsclient_windows_test.go @@ -0,0 +1,34 @@ +package hnsclient + +import ( + "testing" + + "github.com/Azure/azure-container-networking/cns" + "github.com/stretchr/testify/assert" +) + +func TestAdhocAdjustIPConfig(t *testing.T) { + tests := []struct { + name string + ipConfig cns.IPConfiguration + expected cns.IPConfiguration + }{ + { + name: "expect no change when gw address is not 169.254.128.1", + ipConfig: cns.IPConfiguration{GatewayIPAddress: "169.254.128.3"}, + expected: cns.IPConfiguration{GatewayIPAddress: "169.254.128.3"}, + }, + { + name: "expect default gw address is set when gw address is 169.254.128.1", + ipConfig: cns.IPConfiguration{GatewayIPAddress: "169.254.128.1"}, + expected: cns.IPConfiguration{GatewayIPAddress: "169.254.128.2"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + adhocAdjustIPConfig(&tt.ipConfig) + assert.Equal(t, tt.expected.GatewayIPAddress, tt.ipConfig.GatewayIPAddress) + }) + } +} From 49e4c3c6ac5472a98eee9194ebd718f8a581e6d8 Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Tue, 20 Aug 2024 16:10:32 -0700 Subject: [PATCH 03/10] fix lint --- cns/hnsclient/hnsclient_windows_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cns/hnsclient/hnsclient_windows_test.go b/cns/hnsclient/hnsclient_windows_test.go index b7a15e4c38..a5b260afdd 100644 --- a/cns/hnsclient/hnsclient_windows_test.go +++ b/cns/hnsclient/hnsclient_windows_test.go @@ -26,6 +26,7 @@ func TestAdhocAdjustIPConfig(t *testing.T) { } for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { adhocAdjustIPConfig(&tt.ipConfig) assert.Equal(t, tt.expected.GatewayIPAddress, tt.ipConfig.GatewayIPAddress) From b470f25f620e7887288473e862a0b7afc7489537 Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Fri, 23 Aug 2024 09:41:05 -0700 Subject: [PATCH 04/10] fix loopback adapter default gw address --- cns/hnsclient/hnsclient_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index 569e7561d8..0f91198b4f 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -302,7 +302,7 @@ func createHostNCApipaNetwork( IPAddress: hnsLoopbackAdapterIPAddress, PrefixLength: localIPConfiguration.IPSubnet.PrefixLength, }, - GatewayIPAddress: localIPConfiguration.GatewayIPAddress, + GatewayIPAddress: hnsLoopbackAdapterIPAddress, } logger.Printf("Print interfaces before creating loopback adapter") LogNetworkInterfaces() From db64cfa4827c76aa68bfca8afd71a7341e49ecea Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Wed, 2 Oct 2024 15:33:20 -0700 Subject: [PATCH 05/10] remote address should always be .1 --- cns/hnsclient/hnsclient_windows.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index 0f91198b4f..4d1fb508e3 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -54,6 +54,7 @@ const ( defaultHnsGwIPAddress = "169.254.128.2" hnsLoopbackAdapterIPAddress = "169.254.128.1" + apipaRemoteIPAddress = "169.254.128.1" // protocolTCP indicates the TCP protocol identifier in HCN protocolTCP = "6" @@ -508,7 +509,7 @@ func configureHostNCApipaEndpoint( endpointPolicies, err := configureAclSettingHostNCApipaEndpoint( protocolList, networkContainerApipaIP, - hostApipaIP, + apipaRemoteIPAddress, allowNCToHostCommunication, allowHostToNCCommunication, ncPolicies) From f4a3c9b40b5e593e28737953207be2e6b8f5e91f Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Wed, 16 Oct 2024 19:07:04 -0700 Subject: [PATCH 06/10] address comment --- cns/hnsclient/hnsclient_windows.go | 9 ++++++--- cns/hnsclient/hnsclient_windows_test.go | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index abc85433ae..ebf99397a3 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -576,7 +576,7 @@ func CreateHostNCApipaEndpoint( return endpoint.Id, nil } - adhocAdjustIPConfig(&localIPConfiguration) + adjustIPConfig(&localIPConfiguration) if network, err = createHostNCApipaNetwork(localIPConfiguration); err != nil { logger.Errorf("[Azure CNS] Failed to create HostNCApipaNetwork. Error: %v", err) return "", err @@ -608,8 +608,11 @@ func CreateHostNCApipaEndpoint( return endpoint.Id, nil } -// adhocAdjustIPConfig applies adhoc change on gw IP address -func adhocAdjustIPConfig(localIPConfiguration *cns.IPConfiguration) { +// adjustIPConfig applies change on gw IP address. +// currently cns using the same ip address "169.254.128.1" for both apipa gw and loopback adapter. This cause conflict +// issue when hns get restarted and not able to rehydrate the apipa endpoints. This func is to overwrite the address to 169.254.128.2 +// when the gateway address is 169.254.128.1 +func adjustIPConfig(localIPConfiguration *cns.IPConfiguration) { // When gw address is 169.254.128.1, should use .2 instead. If gw address is not .1, that mean this value is // configured from dnc, we should keep it if localIPConfiguration.GatewayIPAddress == "169.254.128.1" { diff --git a/cns/hnsclient/hnsclient_windows_test.go b/cns/hnsclient/hnsclient_windows_test.go index a5b260afdd..c0d4f70bd9 100644 --- a/cns/hnsclient/hnsclient_windows_test.go +++ b/cns/hnsclient/hnsclient_windows_test.go @@ -28,7 +28,7 @@ func TestAdhocAdjustIPConfig(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - adhocAdjustIPConfig(&tt.ipConfig) + adjustIPConfig(&tt.ipConfig) assert.Equal(t, tt.expected.GatewayIPAddress, tt.ipConfig.GatewayIPAddress) }) } From 5424d040fe7e3cf825fa24f943fcef0febf65bea Mon Sep 17 00:00:00 2001 From: ZetaoZhuang Date: Wed, 16 Oct 2024 19:24:38 -0700 Subject: [PATCH 07/10] adjust comment --- cns/hnsclient/hnsclient_windows.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index ebf99397a3..8451d3a90c 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -609,9 +609,8 @@ func CreateHostNCApipaEndpoint( } // adjustIPConfig applies change on gw IP address. -// currently cns using the same ip address "169.254.128.1" for both apipa gw and loopback adapter. This cause conflict -// issue when hns get restarted and not able to rehydrate the apipa endpoints. This func is to overwrite the address to 169.254.128.2 -// when the gateway address is 169.254.128.1 +// Currently, cns using the same ip address "169.254.128.1" for both apipa gw and loopback adapter. This cause conflict issue when hns get restarted and not able to rehydrate the apipa endpoints. +// This func is to overwrite the address to 169.254.128.2 when the gateway address is 169.254.128.1 func adjustIPConfig(localIPConfiguration *cns.IPConfiguration) { // When gw address is 169.254.128.1, should use .2 instead. If gw address is not .1, that mean this value is // configured from dnc, we should keep it From d4b2c5ddf7a05ed9b893445008f8a29be0266a04 Mon Sep 17 00:00:00 2001 From: ZetaoZhuang Date: Fri, 18 Oct 2024 11:27:26 -0700 Subject: [PATCH 08/10] ddress comment --- cns/hnsclient/hnsclient_windows.go | 11 ++++++----- cns/hnsclient/hnsclient_windows_test.go | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index 8451d3a90c..3fd2c19431 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -17,6 +17,7 @@ import ( "github.com/pkg/errors" ) +// TODO redesign hnsclient on windows const ( // Name of the external hns network ExtHnsNetworkName = "ext" @@ -53,9 +54,9 @@ const ( // Name of the loopback adapter needed to create Host NC apipa network hostNCLoopbackAdapterName = "LoopbackAdapterHostNCConnectivity" + // HNS rehydration issue requires this GW to be different than the loopback adapter ip, so we set it to .2 defaultHnsGwIPAddress = "169.254.128.2" hnsLoopbackAdapterIPAddress = "169.254.128.1" - apipaRemoteIPAddress = "169.254.128.1" // protocolTCP indicates the TCP protocol identifier in HCN protocolTCP = "6" @@ -513,7 +514,7 @@ func configureHostNCApipaEndpoint( endpointPolicies, err := configureAclSettingHostNCApipaEndpoint( protocolList, networkContainerApipaIP, - apipaRemoteIPAddress, + hnsLoopbackAdapterIPAddress, allowNCToHostCommunication, allowHostToNCCommunication, ncPolicies) @@ -576,7 +577,7 @@ func CreateHostNCApipaEndpoint( return endpoint.Id, nil } - adjustIPConfig(&localIPConfiguration) + updateGwForLocalIPConfiguration(&localIPConfiguration) if network, err = createHostNCApipaNetwork(localIPConfiguration); err != nil { logger.Errorf("[Azure CNS] Failed to create HostNCApipaNetwork. Error: %v", err) return "", err @@ -608,10 +609,10 @@ func CreateHostNCApipaEndpoint( return endpoint.Id, nil } -// adjustIPConfig applies change on gw IP address. +// updateGwForLocalIPConfiguration applies change on APIPA NW gw IP address. // Currently, cns using the same ip address "169.254.128.1" for both apipa gw and loopback adapter. This cause conflict issue when hns get restarted and not able to rehydrate the apipa endpoints. // This func is to overwrite the address to 169.254.128.2 when the gateway address is 169.254.128.1 -func adjustIPConfig(localIPConfiguration *cns.IPConfiguration) { +func updateGwForLocalIPConfiguration(localIPConfiguration *cns.IPConfiguration) { // When gw address is 169.254.128.1, should use .2 instead. If gw address is not .1, that mean this value is // configured from dnc, we should keep it if localIPConfiguration.GatewayIPAddress == "169.254.128.1" { diff --git a/cns/hnsclient/hnsclient_windows_test.go b/cns/hnsclient/hnsclient_windows_test.go index c0d4f70bd9..8b01d2f839 100644 --- a/cns/hnsclient/hnsclient_windows_test.go +++ b/cns/hnsclient/hnsclient_windows_test.go @@ -28,7 +28,7 @@ func TestAdhocAdjustIPConfig(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - adjustIPConfig(&tt.ipConfig) + updateGwForLocalIPConfiguration(&tt.ipConfig) assert.Equal(t, tt.expected.GatewayIPAddress, tt.ipConfig.GatewayIPAddress) }) } From a71947c5146bea1c5e114374f25807701d572e77 Mon Sep 17 00:00:00 2001 From: ZetaoZhuang Date: Fri, 18 Oct 2024 14:16:20 -0700 Subject: [PATCH 09/10] fix func comment --- cns/hnsclient/hnsclient_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index 3fd2c19431..cc33eb6b63 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -609,7 +609,7 @@ func CreateHostNCApipaEndpoint( return endpoint.Id, nil } -// updateGwForLocalIPConfiguration applies change on APIPA NW gw IP address. +// updateGwForLocalIPConfiguration applies change on gw IP address for apipa NW and endpoint. // Currently, cns using the same ip address "169.254.128.1" for both apipa gw and loopback adapter. This cause conflict issue when hns get restarted and not able to rehydrate the apipa endpoints. // This func is to overwrite the address to 169.254.128.2 when the gateway address is 169.254.128.1 func updateGwForLocalIPConfiguration(localIPConfiguration *cns.IPConfiguration) { From b5e220458970dc01bdb17efeef371ce89532bc34 Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Mon, 21 Oct 2024 12:15:19 -0700 Subject: [PATCH 10/10] fix loopback adapter gw --- cns/hnsclient/hnsclient_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index cc33eb6b63..58bf884b74 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -308,7 +308,7 @@ func createHostNCApipaNetwork( IPAddress: hnsLoopbackAdapterIPAddress, PrefixLength: localIPConfiguration.IPSubnet.PrefixLength, }, - GatewayIPAddress: hnsLoopbackAdapterIPAddress, + GatewayIPAddress: localIPConfiguration.GatewayIPAddress, } logger.Printf("Print interfaces before creating loopback adapter") LogNetworkInterfaces()