From 8fb227a06236c4b1bb54c7dedf8867305ef477cc Mon Sep 17 00:00:00 2001 From: neaggarwMS <31906480+neaggarwMS@users.noreply.github.com> Date: Tue, 18 Aug 2020 23:18:31 -0700 Subject: [PATCH 1/4] Create ImdsClientInterface --- cns/dockerclient/dockerclient.go | 6 +++--- cns/imdsclient/api.go | 7 +++++++ cns/restserver/api_test.go | 3 ++- cns/restserver/ipam_test.go | 3 ++- cns/restserver/restserver.go | 6 +++--- cns/service/main.go | 3 ++- 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/cns/dockerclient/dockerclient.go b/cns/dockerclient/dockerclient.go index cd67ed0c17..ac4ab06a1d 100644 --- a/cns/dockerclient/dockerclient.go +++ b/cns/dockerclient/dockerclient.go @@ -25,19 +25,19 @@ const ( // DockerClient specifies a client to connect to docker. type DockerClient struct { connectionURL string - imdsClient *imdsclient.ImdsClient + imdsClient imdsclient.ImdsClientInterface } // NewDockerClient create a new docker client. func NewDockerClient(url string) (*DockerClient, error) { return &DockerClient{ connectionURL: url, - imdsClient: &imdsclient.ImdsClient{}, + imdsClient: new(imdsclient.ImdsClient), }, nil } // NewDefaultDockerClient create a new docker client. -func NewDefaultDockerClient(imdsClient *imdsclient.ImdsClient) (*DockerClient, error) { +func NewDefaultDockerClient(imdsClient imdsclient.ImdsClientInterface) (*DockerClient, error) { return &DockerClient{ connectionURL: defaultDockerConnectionURL, imdsClient: imdsClient, diff --git a/cns/imdsclient/api.go b/cns/imdsclient/api.go index 3c8bebd3ca..0af765f07c 100644 --- a/cns/imdsclient/api.go +++ b/cns/imdsclient/api.go @@ -68,3 +68,10 @@ type ContainerVersion struct { NetworkContainerID string ProgrammedVersion string } + +// An ImdsInterface performs CRUD operations on IP reservations +type ImdsClientInterface interface { + GetNetworkContainerInfoFromHost(networkContainerID string, primaryAddress string, authToken string, apiVersion string) (*ContainerVersion, error) + GetPrimaryInterfaceInfoFromHost() (*InterfaceInfo, error) + GetPrimaryInterfaceInfoFromMemory() (*InterfaceInfo, error) +} diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 6bb9f86b8d..3ed6fdc008 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -17,6 +17,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" + "github.com/Azure/azure-container-networking/cns/imdsclient" "github.com/Azure/azure-container-networking/cns/logger" acncommon "github.com/Azure/azure-container-networking/common" ) @@ -656,7 +657,7 @@ func startService() { var err error // Create the service. config := common.ServiceConfig{} - service, err = NewHTTPRestService(&config) + service, err = NewHTTPRestService(&config, new(imdsclient.ImdsClient)) if err != nil { fmt.Printf("Failed to create CNS object %v\n", err) os.Exit(1) diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 6e4cc4326f..48702023d2 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" + "github.com/Azure/azure-container-networking/cns/imdsclient" ) var ( @@ -40,7 +41,7 @@ var ( func getTestService() *HTTPRestService { var config common.ServiceConfig - httpsvc, _ := NewHTTPRestService(&config) + httpsvc, _ := NewHTTPRestService(&config, new(imdsclient.ImdsClient)) svc = httpsvc.(*HTTPRestService) setOrchestratorTypeInternal(cns.KubernetesCRD) diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 062750e902..dfc9dd634c 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -37,7 +37,7 @@ var ( type HTTPRestService struct { *cns.Service dockerClient *dockerclient.DockerClient - imdsClient *imdsclient.ImdsClient + imdsClient imdsclient.ImdsClientInterface ipamClient *ipamclient.IpamClient networkContainer *networkcontainers.NetworkContainers PodIPIDByOrchestratorContext map[string]string // OrchestratorContext is key and value is Pod IP uuid. @@ -102,13 +102,13 @@ type HTTPService interface { } // NewHTTPRestService creates a new HTTP Service object. -func NewHTTPRestService(config *common.ServiceConfig) (HTTPService, error) { +func NewHTTPRestService(config *common.ServiceConfig, imdsClientInterface imdsclient.ImdsClientInterface) (HTTPService, error) { service, err := cns.NewService(config.Name, config.Version, config.ChannelMode, config.Store) if err != nil { return nil, err } - imdsClient := &imdsclient.ImdsClient{} + imdsClient := imdsClientInterface routingTable := &routes.RoutingTable{} nc := &networkcontainers.NetworkContainers{} dc, err := dockerclient.NewDefaultDockerClient(imdsClient) diff --git a/cns/service/main.go b/cns/service/main.go index ba6edda866..e4b3c4dfcf 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -23,6 +23,7 @@ import ( "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/hnsclient" + "github.com/Azure/azure-container-networking/cns/imdsclient" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/requestcontroller" "github.com/Azure/azure-container-networking/cns/requestcontroller/kubecontroller" @@ -367,7 +368,7 @@ func main() { } // Create CNS object. - httpRestService, err := restserver.NewHTTPRestService(&config) + httpRestService, err := restserver.NewHTTPRestService(&config, new(imdsclient.ImdsClient)) if err != nil { logger.Errorf("Failed to create CNS object, err:%v.\n", err) return From eaf2ffc21783d9b2712e89875ea518731cdb05a5 Mon Sep 17 00:00:00 2001 From: neaggarwMS <31906480+neaggarwMS@users.noreply.github.com> Date: Wed, 19 Aug 2020 23:53:46 -0700 Subject: [PATCH 2/4] Pass HostPrimaryInterface details to CNS --- Makefile | 1 + cns/NetworkContainerContract.go | 3 +- cns/cnsclient/cnsclient_test.go | 3 +- cns/mock/imdsclientmock.go | 49 +++++++++++++++++++++++++++++++++ cns/restserver/api_test.go | 4 +-- cns/restserver/ipam_test.go | 12 ++++++-- cns/restserver/restserver.go | 2 +- cns/restserver/util.go | 8 +++++- 8 files changed, 74 insertions(+), 8 deletions(-) create mode 100644 cns/mock/imdsclientmock.go diff --git a/Makefile b/Makefile index 61c94cd7c3..dc78470d43 100644 --- a/Makefile +++ b/Makefile @@ -51,6 +51,7 @@ CNSFILES = \ $(wildcard cns/networkcontainers/*.go) \ $(wildcard cns/requestcontroller/*.go) \ $(wildcard cns/requestcontroller/kubecontroller/*.go) \ + $(wildcard cns/mock/*.go) \ $(COREFILES) \ $(CNMFILES) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 8b8b12fe32..bbb7e91508 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -207,7 +207,8 @@ type PodIpInfo struct { // DeleteNetworkContainerRequest specifies the details about the request to delete a specifc network container. type HostIPInfo struct { - IPConfig IPSubnet + PrimaryIP string + Subnet string } // DeleteNetworkContainerRequest specifies the details about the request to delete a specifc network container. diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index df58661ebc..acdbba613d 100644 --- a/cns/cnsclient/cnsclient_test.go +++ b/cns/cnsclient/cnsclient_test.go @@ -15,6 +15,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/cns/mock" "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/log" "github.com/google/uuid" @@ -117,7 +118,7 @@ func TestMain(m *testing.M) { logger.InitLogger(logName, 0, 0, tmpLogDir+"/") config := common.ServiceConfig{} - httpRestService, err := restserver.NewHTTPRestService(&config) + httpRestService, err := restserver.NewHTTPRestService(&config, mock.NewMockImdsClient()) svc = httpRestService.(*restserver.HTTPRestService) svc.Name = "cns-test-server" if err != nil { diff --git a/cns/mock/imdsclientmock.go b/cns/mock/imdsclientmock.go new file mode 100644 index 0000000000..de16b31491 --- /dev/null +++ b/cns/mock/imdsclientmock.go @@ -0,0 +1,49 @@ +// Copyright 2017 Microsoft. All rights reserved. +// MIT License + +package mock + +import ( + "github.com/Azure/azure-container-networking/cns/imdsclient" + "github.com/Azure/azure-container-networking/cns/logger" +) + +var ( + HostPrimaryIpTest = "10.0.0.4" + HostSubnetTest = "10.0.0.0/24" +) + +// ImdsClient can be used to connect to VM Host agent in Azure. +type ImdsClientTest struct { +} + +func NewMockImdsClient() *ImdsClientTest { + return &ImdsClientTest{} +} + +// GetNetworkContainerInfoFromHost- Mock implementation to return Container version info. +func (imdsClient *ImdsClientTest) GetNetworkContainerInfoFromHost(networkContainerID string, primaryAddress string, authToken string, apiVersion string) (*imdsclient.ContainerVersion, error) { + + ret := &imdsclient.ContainerVersion{} + + return ret, nil +} + +// GetPrimaryInterfaceInfoFromHost - Mock implementation to return Host interface info +func (imdsClient *ImdsClientTest) GetPrimaryInterfaceInfoFromHost() (*imdsclient.InterfaceInfo, error) { + logger.Printf("[Azure CNS] GetPrimaryInterfaceInfoFromHost") + + interfaceInfo := &imdsclient.InterfaceInfo{ + Subnet: HostSubnetTest, + PrimaryIP: HostPrimaryIpTest, + } + + return interfaceInfo, nil +} + +// GetPrimaryInterfaceInfoFromMemory - Mock implementation to return host interface info +func (imdsClient *ImdsClientTest) GetPrimaryInterfaceInfoFromMemory() (*imdsclient.InterfaceInfo, error) { + logger.Printf("[Azure CNS] GetPrimaryInterfaceInfoFromMemory") + + return imdsClient.GetPrimaryInterfaceInfoFromHost() +} diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 3ed6fdc008..96f91cd015 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -17,8 +17,8 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" - "github.com/Azure/azure-container-networking/cns/imdsclient" "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/cns/mock" acncommon "github.com/Azure/azure-container-networking/common" ) @@ -657,7 +657,7 @@ func startService() { var err error // Create the service. config := common.ServiceConfig{} - service, err = NewHTTPRestService(&config, new(imdsclient.ImdsClient)) + service, err = NewHTTPRestService(&config, mock.NewMockImdsClient()) if err != nil { fmt.Printf("Failed to create CNS object %v\n", err) os.Exit(1) diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 48702023d2..1e573cd528 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -11,7 +11,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" - "github.com/Azure/azure-container-networking/cns/imdsclient" + "github.com/Azure/azure-container-networking/cns/mock" ) var ( @@ -41,7 +41,7 @@ var ( func getTestService() *HTTPRestService { var config common.ServiceConfig - httpsvc, _ := NewHTTPRestService(&config, new(imdsclient.ImdsClient)) + httpsvc, _ := NewHTTPRestService(&config, mock.NewMockImdsClient()) svc = httpsvc.(*HTTPRestService) setOrchestratorTypeInternal(cns.KubernetesCRD) @@ -99,6 +99,14 @@ func requestIpAddressAndGetState(t *testing.T, req cns.GetIPConfigRequest) (ipCo t.Fatalf("Pod IP Prefix length is not added as expected ipConfig %+v, expected: %+v", PodIpInfo.PodIPConfig, subnetPrfixLength) } + if reflect.DeepEqual(PodIpInfo.HostPrimaryIPInfo.PrimaryIP, mock.HostPrimaryIpTest) != true { + t.Fatalf("Host PrimaryIP is not added as expected ipConfig %+v, expected primaryIP: %+v", PodIpInfo.HostPrimaryIPInfo, mock.HostPrimaryIpTest) + } + + if reflect.DeepEqual(PodIpInfo.HostPrimaryIPInfo.Subnet, mock.HostSubnetTest) != true { + t.Fatalf("Host Subnet is not added as expected ipConfig %+v, expected Host subnet: %+v", PodIpInfo.HostPrimaryIPInfo, mock.HostSubnetTest) + } + // retrieve podinfo from orchestrator context if err := json.Unmarshal(req.OrchestratorContext, &podInfo); err != nil { return ipState, err diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index dfc9dd634c..065960ac11 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -111,7 +111,7 @@ func NewHTTPRestService(config *common.ServiceConfig, imdsClientInterface imdscl imdsClient := imdsClientInterface routingTable := &routes.RoutingTable{} nc := &networkcontainers.NetworkContainers{} - dc, err := dockerclient.NewDefaultDockerClient(imdsClient) + dc, err := dockerclient.NewDefaultDockerClient(imdsClientInterface) if err != nil { return nil, err diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 806763e81b..566cad221a 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -661,7 +661,13 @@ func (service *HTTPRestService) populateIpConfigInfoUntransacted(ipConfigStatus podIpInfo.NetworkContainerPrimaryIPConfig = primaryIpConfiguration - // TODO Add Host Primary ipinfo + hostInterfaceInfo, err := service.imdsClient.GetPrimaryInterfaceInfoFromMemory() + if err != nil { + return fmt.Errorf("") + } + + podIpInfo.HostPrimaryIPInfo.PrimaryIP = hostInterfaceInfo.PrimaryIP + podIpInfo.HostPrimaryIPInfo.Subnet = hostInterfaceInfo.Subnet return nil } From 3f40541a6e0a1ce86a615a8e709206ad79b19136 Mon Sep 17 00:00:00 2001 From: neaggarwMS <31906480+neaggarwMS@users.noreply.github.com> Date: Fri, 21 Aug 2020 11:11:11 -0700 Subject: [PATCH 3/4] Incorporate feedback --- Makefile | 2 +- cns/cnsclient/cnsclient_test.go | 4 ++-- .../imdsclientmock.go => fakes/imdsclientfake.go} | 4 ++-- cns/restserver/api_test.go | 4 ++-- cns/restserver/ipam_test.go | 12 ++++++------ 5 files changed, 13 insertions(+), 13 deletions(-) rename cns/{mock/imdsclientmock.go => fakes/imdsclientfake.go} (96%) diff --git a/Makefile b/Makefile index dc78470d43..0d94b03661 100644 --- a/Makefile +++ b/Makefile @@ -51,7 +51,7 @@ CNSFILES = \ $(wildcard cns/networkcontainers/*.go) \ $(wildcard cns/requestcontroller/*.go) \ $(wildcard cns/requestcontroller/kubecontroller/*.go) \ - $(wildcard cns/mock/*.go) \ + $(wildcard cns/fakes/*.go) \ $(COREFILES) \ $(CNMFILES) diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index acdbba613d..fb49b0d9cd 100644 --- a/cns/cnsclient/cnsclient_test.go +++ b/cns/cnsclient/cnsclient_test.go @@ -14,8 +14,8 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" + "github.com/Azure/azure-container-networking/cns/fakes" "github.com/Azure/azure-container-networking/cns/logger" - "github.com/Azure/azure-container-networking/cns/mock" "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/log" "github.com/google/uuid" @@ -118,7 +118,7 @@ func TestMain(m *testing.M) { logger.InitLogger(logName, 0, 0, tmpLogDir+"/") config := common.ServiceConfig{} - httpRestService, err := restserver.NewHTTPRestService(&config, mock.NewMockImdsClient()) + httpRestService, err := restserver.NewHTTPRestService(&config, fakes.NewFakeImdsClient()) svc = httpRestService.(*restserver.HTTPRestService) svc.Name = "cns-test-server" if err != nil { diff --git a/cns/mock/imdsclientmock.go b/cns/fakes/imdsclientfake.go similarity index 96% rename from cns/mock/imdsclientmock.go rename to cns/fakes/imdsclientfake.go index de16b31491..e2e9413af8 100644 --- a/cns/mock/imdsclientmock.go +++ b/cns/fakes/imdsclientfake.go @@ -1,7 +1,7 @@ // Copyright 2017 Microsoft. All rights reserved. // MIT License -package mock +package fakes import ( "github.com/Azure/azure-container-networking/cns/imdsclient" @@ -17,7 +17,7 @@ var ( type ImdsClientTest struct { } -func NewMockImdsClient() *ImdsClientTest { +func NewFakeImdsClient() *ImdsClientTest { return &ImdsClientTest{} } diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 96f91cd015..8ab654ad7f 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -17,8 +17,8 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" + "github.com/Azure/azure-container-networking/cns/fakes" "github.com/Azure/azure-container-networking/cns/logger" - "github.com/Azure/azure-container-networking/cns/mock" acncommon "github.com/Azure/azure-container-networking/common" ) @@ -657,7 +657,7 @@ func startService() { var err error // Create the service. config := common.ServiceConfig{} - service, err = NewHTTPRestService(&config, mock.NewMockImdsClient()) + service, err = NewHTTPRestService(&config, fakes.NewFakeImdsClient()) if err != nil { fmt.Printf("Failed to create CNS object %v\n", err) os.Exit(1) diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 1e573cd528..0f39dcdd64 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -11,7 +11,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" - "github.com/Azure/azure-container-networking/cns/mock" + "github.com/Azure/azure-container-networking/cns/fakes" ) var ( @@ -41,7 +41,7 @@ var ( func getTestService() *HTTPRestService { var config common.ServiceConfig - httpsvc, _ := NewHTTPRestService(&config, mock.NewMockImdsClient()) + httpsvc, _ := NewHTTPRestService(&config, fakes.NewFakeImdsClient()) svc = httpsvc.(*HTTPRestService) setOrchestratorTypeInternal(cns.KubernetesCRD) @@ -99,12 +99,12 @@ func requestIpAddressAndGetState(t *testing.T, req cns.GetIPConfigRequest) (ipCo t.Fatalf("Pod IP Prefix length is not added as expected ipConfig %+v, expected: %+v", PodIpInfo.PodIPConfig, subnetPrfixLength) } - if reflect.DeepEqual(PodIpInfo.HostPrimaryIPInfo.PrimaryIP, mock.HostPrimaryIpTest) != true { - t.Fatalf("Host PrimaryIP is not added as expected ipConfig %+v, expected primaryIP: %+v", PodIpInfo.HostPrimaryIPInfo, mock.HostPrimaryIpTest) + if reflect.DeepEqual(PodIpInfo.HostPrimaryIPInfo.PrimaryIP, fakes.HostPrimaryIpTest) != true { + t.Fatalf("Host PrimaryIP is not added as expected ipConfig %+v, expected primaryIP: %+v", PodIpInfo.HostPrimaryIPInfo, fakes.HostPrimaryIpTest) } - if reflect.DeepEqual(PodIpInfo.HostPrimaryIPInfo.Subnet, mock.HostSubnetTest) != true { - t.Fatalf("Host Subnet is not added as expected ipConfig %+v, expected Host subnet: %+v", PodIpInfo.HostPrimaryIPInfo, mock.HostSubnetTest) + if reflect.DeepEqual(PodIpInfo.HostPrimaryIPInfo.Subnet, fakes.HostSubnetTest) != true { + t.Fatalf("Host Subnet is not added as expected ipConfig %+v, expected Host subnet: %+v", PodIpInfo.HostPrimaryIPInfo, fakes.HostSubnetTest) } // retrieve podinfo from orchestrator context From c0cf40a921b0a926f4b7b714d88e472db209b2e8 Mon Sep 17 00:00:00 2001 From: neaggarwMS <31906480+neaggarwMS@users.noreply.github.com> Date: Fri, 21 Aug 2020 11:26:07 -0700 Subject: [PATCH 4/4] add error message --- cns/restserver/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 566cad221a..0fed060471 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -663,7 +663,7 @@ func (service *HTTPRestService) populateIpConfigInfoUntransacted(ipConfigStatus hostInterfaceInfo, err := service.imdsClient.GetPrimaryInterfaceInfoFromMemory() if err != nil { - return fmt.Errorf("") + return fmt.Errorf("Failed to get the HostInterfaceInfo %s", err) } podIpInfo.HostPrimaryIPInfo.PrimaryIP = hostInterfaceInfo.PrimaryIP