refactor and move the nmagentConfig code from cns to namgent package.

cns/configuration/configuration.go

@@ -3,12 +3,8 @@ package configuration
 
 import (
 	"encoding/json"
-	"net"
-	"net/url"
 	"os"
 	"path/filepath"
-	"strconv"
-	"strings"
 
 	"github.com/Azure/azure-container-networking/cns"
 	"github.com/Azure/azure-container-networking/cns/logger"
@@ -23,28 +19,28 @@ const (
 )
 
 type CNSConfig struct {
-	ChannelMode                 string
-	EnablePprof                 bool
-	EnableSubnetScarcity        bool
-	InitializeFromCNI           bool
-	ManagedSettings             ManagedSettings
-	MetricsBindAddress          string
-	SyncHostNCTimeoutMs         int
-	SyncHostNCVersionIntervalMs int
-	TLSCertificatePath          string
-	TLSEndpoint                 string
-	TLSPort                     string
-	TLSSubjectName              string
-	TelemetrySettings           TelemetrySettings
-	UseHTTPS                    bool
-	WireserverIP                string
-	KeyVaultSettings            KeyVaultSettings
-	MSISettings                 MSISettings
-	ProgramSNATIPTables         bool
-	ManageEndpointState         bool
-	CNIConflistScenario         string
-	EnableCNIConflistGeneration bool
-	CNIConflistFilepath         string
+	ChannelMode                          string
+	EnablePprof                          bool
+	EnableSubnetScarcity                 bool
+	InitializeFromCNI                    bool
+	ManagedSettings                      ManagedSettings
+	MetricsBindAddress                   string
+	SyncHostNCTimeoutMs                  int
+	SyncHostNCVersionIntervalMs          int
+	TLSCertificatePath                   string
+	TLSEndpoint                          string
+	TLSPort                              string
+	TLSSubjectName                       string
+	TelemetrySettings                    TelemetrySettings
+	UseHTTPS                             bool
+	WireserverIP                         string
+	KeyVaultSettings                     KeyVaultSettings
+	MSISettings                          MSISettings
+	ProgramSNATIPTables                  bool
+	ManageEndpointState                  bool
+	CNIConflistScenario                  string
+	EnableCNIConflistGeneration          bool
+	CNIConflistFilepath                  string
 	PopulateHomeAzCacheRetryIntervalSecs int
 }
 
@@ -110,53 +106,6 @@ func getConfigFilePath(cmdLineConfigPath string) (string, error) {
 	return configpath, nil
 }
 
-type NMAgentConfig struct {
-	Host   string
-	Port   uint16
-	UseTLS bool
-}
-
-func (c *CNSConfig) NMAgentConfig() (NMAgentConfig, error) {
-	host := "168.63.129.16" // wireserver's IP
-
-	if c.WireserverIP != "" {
-		host = c.WireserverIP
-	}
-
-	if strings.Contains(host, "http") {
-		parts, err := url.Parse(host)
-		if err != nil {
-			return NMAgentConfig{}, errors.Wrap(err, "parsing WireserverIP as URL")
-		}
-		host = parts.Host
-	}
-
-	// create an NMAgent Client based on provided configuration
-	if strings.Contains(host, ":") {
-		host, prt, err := net.SplitHostPort(host) //nolint:govet // it's fine to shadow err here
-		if err != nil {
-			return NMAgentConfig{}, errors.Wrap(err, "splitting wireserver IP into host port")
-		}
-
-		port, err := strconv.ParseUint(prt, 10, 16) //nolint:gomnd // obvious from ParseUint docs
-		if err != nil {
-			return NMAgentConfig{}, errors.Wrap(err, "parsing wireserver port value as uint16")
-		}
-
-		return NMAgentConfig{
-			Host: host,
-			Port: uint16(port),
-		}, nil
-	}
-
-	return NMAgentConfig{
-		Host: host,
-
-		// nolint:gomnd // there's no benefit to constantizing a well-known port
-		Port: 80,
-	}, nil
-}
-
 // ReadConfig returns a CNS config from file or an error.
 func ReadConfig(cmdLineConfigPath string) (*CNSConfig, error) {
 	configpath, err := getConfigFilePath(cmdLineConfigPath)

cns/configuration/configuration_test.go

@@ -6,7 +6,6 @@ import (
 	"testing"
 
 	"github.com/Azure/azure-container-networking/common"
-	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -262,99 +261,3 @@ func TestSetCNSConfigDefaults(t *testing.T) {
 		})
 	}
 }
-
-func TestNMAgentConfig(t *testing.T) {
-	tests := []struct {
-		name      string
-		conf      CNSConfig
-		exp       NMAgentConfig
-		shouldErr bool
-	}{
-		{
-			"empty",
-			CNSConfig{
-				WireserverIP: "",
-			},
-			NMAgentConfig{
-				Host: "168.63.129.16",
-				Port: 80,
-			},
-			false,
-		},
-		{
-			"ip",
-			CNSConfig{
-				WireserverIP: "127.0.0.1",
-			},
-			NMAgentConfig{
-				Host: "127.0.0.1",
-				Port: 80,
-			},
-			false,
-		},
-		{
-			"ipport",
-			CNSConfig{
-				WireserverIP: "127.0.0.1:8080",
-			},
-			NMAgentConfig{
-				Host: "127.0.0.1",
-				Port: 8080,
-			},
-			false,
-		},
-		{
-			"scheme",
-			CNSConfig{
-				WireserverIP: "http://127.0.0.1:8080",
-			},
-			NMAgentConfig{
-				Host: "127.0.0.1",
-				Port: 8080,
-			},
-			false,
-		},
-		{
-			"invalid URL",
-			CNSConfig{
-				WireserverIP: "a string containing \"http\" with an invalid character: \x7F",
-			},
-			NMAgentConfig{},
-			true,
-		},
-		{
-			"invalid host:port",
-			CNSConfig{
-				WireserverIP: "way:too:many:colons",
-			},
-			NMAgentConfig{},
-			true,
-		},
-		{
-			"too big for a uint16 port",
-			CNSConfig{
-				WireserverIP: "127.0.0.1:4815162342",
-			},
-			NMAgentConfig{},
-			true,
-		},
-	}
-
-	for _, test := range tests {
-		test := test
-		t.Run(test.name, func(t *testing.T) {
-			got, err := test.conf.NMAgentConfig()
-			if err != nil && !test.shouldErr {
-				t.Fatal("unexpected error fetching nmagent config: err:", err)
-			}
-
-			if err == nil && test.shouldErr {
-				t.Fatal("expected error fetching nmagent config but received none")
-			}
-
-			if !cmp.Equal(got, test.exp) {
-				t.Error("received config differs from expectation: diff:", cmp.Diff(got, test.exp))
-			}
-		})
-	}
-}

cns/service/main.go

@@ -510,13 +510,13 @@ func main() {
 	z, _ := zap.NewProduction()
 	go healthserver.Start(z, cnsconfig.MetricsBindAddress)
 
-	nmaConfig, err := cnsconfig.NMAgentConfig()
+	nmaConfig, err := nmagent.NewConfig(cnsconfig.WireserverIP)
 	if err != nil {
-		logger.Errorf("[Azure CNS] Failed to produce NMAgent config from supplied configuration: %v", err)
+		logger.Errorf("[Azure CNS] Failed to produce NMAgent config from the supplied wireserver ip: %v", err)
 		return
 	}
 
-	nmaClient, err := nmagent.NewClient(nmagent.Config(nmaConfig))
+	nmaClient, err := nmagent.NewClient(nmaConfig)
 	if err != nil {
 		logger.Errorf("[Azure CNS] Failed to start nmagent client due to error: %v", err)
 		return

nmagent/config.go

@@ -1,6 +1,14 @@
 package nmagent
 
-import "github.com/Azure/azure-container-networking/nmagent/internal"
+import (
+	"net"
+	"net/url"
+	"strconv"
+	"strings"
+
+	"github.com/Azure/azure-container-networking/nmagent/internal"
+	"github.com/pkg/errors"
+)
 
 // Config is a configuration for an NMAgent Client.
 type Config struct {
@@ -34,3 +42,44 @@ func (c Config) Validate() error {
 	}
 	return err
 }
+
+// NewConfig returns a nmagent client config using the provided wireserverIP string
+func NewConfig(wireserverIP string) (Config, error) {
+	host := "168.63.129.16" // wireserver's IP
+
+	if wireserverIP != "" {
+		host = wireserverIP
+	}
+
+	if strings.Contains(host, "http") {
+		parts, err := url.Parse(host)
+		if err != nil {
+			return Config{}, errors.Wrap(err, "parsing WireserverIP as URL")
+		}
+		host = parts.Host
+	}
+
+	if strings.Contains(host, ":") {
+		host, prt, err := net.SplitHostPort(host) //nolint:govet // it's fine to shadow ergit r here
+		if err != nil {
+			return Config{}, errors.Wrap(err, "splitting wireserver IP into host port")
+		}
+
+		port, err := strconv.ParseUint(prt, 10, 16) //nolint:gomnd // obvious from ParseUint docs
+		if err != nil {
+			return Config{}, errors.Wrap(err, "parsing wireserver port value as uint16")
+		}
+
+		return Config{
+			Host: host,
+			Port: uint16(port),
+		}, nil
+	}
+
+	return Config{
+		Host: host,
+
+		// nolint:gomnd // there's no benefit to constantizing a well-known port
+		Port: 80,
+	}, nil
+}

nmagent/config_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/Azure/azure-container-networking/nmagent"
+	"github.com/google/go-cmp/cmp"
 )
 
 func TestConfig(t *testing.T) {
@@ -57,3 +58,85 @@ func TestConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestNMAgentConfig(t *testing.T) {
+	tests := []struct {
+		name         string
+		wireserverIP string
+		exp          nmagent.Config
+		shouldErr    bool
+	}{
+		{
+			"empty",
+			"",
+			nmagent.Config{
+				Host: "168.63.129.16",
+				Port: 80,
+			},
+			false,
+		},
+		{
+			"ip",
+			"127.0.0.1",
+			nmagent.Config{
+				Host: "127.0.0.1",
+				Port: 80,
+			},
+			false,
+		},
+		{
+			"ipport",
+			"127.0.0.1:8080",
+			nmagent.Config{
+				Host: "127.0.0.1",
+				Port: 8080,
+			},
+			false,
+		},
+		{
+			"scheme",
+			"http://127.0.0.1:8080",
+			nmagent.Config{
+				Host: "127.0.0.1",
+				Port: 8080,
+			},
+			false,
+		},
+		{
+			"invalid URL",
+			"a string containing \"http\" with an invalid character: \x7F",
+			nmagent.Config{},
+			true,
+		},
+		{
+			"invalid host:port",
+			"way:too:many:colons",
+			nmagent.Config{},
+			true,
+		},
+		{
+			"too big for a uint16 port",
+			"127.0.0.1:4815162342",
+			nmagent.Config{},
+			true,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			got, err := nmagent.NewConfig(test.wireserverIP)
+			if err != nil && !test.shouldErr {
+				t.Fatal("unexpected error fetching nmagent config: err:", err)
+			}
+
+			if err == nil && test.shouldErr {
+				t.Fatal("expected error fetching nmagent config but received none")
+			}
+
+			if !cmp.Equal(got, test.exp) {
+				t.Error("received config differs from expectation: diff:", cmp.Diff(got, test.exp))
+			}
+		})
+	}
+}