From 8f344b5aec8bc092ab79950967b74b7f50e1e149 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Sat, 12 Jun 2021 20:49:27 -0500 Subject: [PATCH 1/6] feat: add flow to initialize cns state from cni Signed-off-by: Evan Baker --- cni/api/api.go | 12 +- cni/client/client.go | 25 ++-- cni/network/invoker_cns.go | 9 +- cni/network/multitenancy.go | 8 +- cni/network/network.go | 12 +- cni/network/network_test.go | 16 +-- cns/NetworkContainerContract.go | 133 ++++++++++++++++-- cns/cnireconciler/initialize.go | 36 +++++ cns/cnireconciler/version.go | 27 ++++ cns/cnsclient/apiclient.go | 2 +- cns/cnsclient/cnsclient_test.go | 6 +- cns/cnsclient/httpapi/client.go | 2 +- cns/configuration/cns_config.json | 23 +-- cns/configuration/configuration.go | 15 +- .../mockclients/cnsclient.go | 2 +- cns/networkcontainers/networkcontainers.go | 8 +- .../networkcontainers_linux.go | 8 +- cns/restserver/api_test.go | 17 +-- cns/restserver/internalapi.go | 12 +- cns/restserver/internalapi_test.go | 40 ++---- cns/restserver/ipam.go | 70 ++++----- cns/restserver/ipam_test.go | 34 ++--- cns/restserver/util.go | 32 ++--- cns/service/main.go | 34 ++++- .../kubecontroller/crdreconciler.go | 2 +- .../kubecontroller/crdrequestcontroller.go | 90 +++++++----- .../crdrequestcontroller_test.go | 11 +- .../kubecontroller/crdtranslator.go | 4 +- .../kubecontroller/crdtranslator_test.go | 12 +- 29 files changed, 440 insertions(+), 262 deletions(-) create mode 100644 cns/cnireconciler/initialize.go create mode 100644 cns/cnireconciler/version.go diff --git a/cni/api/api.go b/cni/api/api.go index 0d062331f0..63d4d07b21 100644 --- a/cni/api/api.go +++ b/cni/api/api.go @@ -9,15 +9,11 @@ import ( ) type PodNetworkInterfaceInfo struct { - PodName string - PodNamespace string + PodName string + PodNamespace string PodEndpointId string - ContainerID string - IPAddresses []net.IPNet -} - -type CNIState interface { - PrintResult() error + ContainerID string + IPAddresses []net.IPNet } type AzureCNIState struct { diff --git a/cni/client/client.go b/cni/client/client.go index 1020325abb..745f8dde48 100644 --- a/cni/client/client.go +++ b/cni/client/client.go @@ -14,21 +14,25 @@ import ( ) const ( - azureVnetBinName = "./azure-vnet" - azureVnetBinDirectory = "/opt/cni/bin" + azureVnetExecutable = "/opt/cni/bin/azure-vnet" ) -type CNIClient interface { - GetEndpointState() (api.CNIState, error) +type Client interface { + GetEndpointState() (*api.AzureCNIState, error) } -type AzureCNIClient struct { +var _ (Client) = (*client)(nil) + +type client struct { exec utilexec.Interface } -func (c *AzureCNIClient) GetEndpointState() (*api.AzureCNIState, error) { - cmd := c.exec.Command(azureVnetBinName) - cmd.SetDir(azureVnetBinDirectory) +func New(exec utilexec.Interface) *client { + return &client{exec: exec} +} + +func (c *client) GetEndpointState() (*api.AzureCNIState, error) { + cmd := c.exec.Command(azureVnetExecutable) envs := os.Environ() cmdenv := fmt.Sprintf("%s=%s", cni.Cmd, cni.CmdGetEndpointsState) @@ -49,9 +53,8 @@ func (c *AzureCNIClient) GetEndpointState() (*api.AzureCNIState, error) { return state, nil } -func (c *AzureCNIClient) GetVersion() (*semver.Version, error) { - cmd := c.exec.Command(azureVnetBinName, "-v") - cmd.SetDir(azureVnetBinDirectory) +func (c *client) GetVersion() (*semver.Version, error) { + cmd := c.exec.Command(azureVnetExecutable, "-v") output, err := cmd.CombinedOutput() if err != nil { diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 2c593de6a4..66568e426f 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -51,14 +51,15 @@ func NewCNSInvoker(podName, namespace string) (*CNSIPAMInvoker, error) { //Add uses the requestipconfig API in cns, and returns ipv4 and a nil ipv6 as CNS doesn't support IPv6 yet func (invoker *CNSIPAMInvoker) Add(nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, hostSubnetPrefix *net.IPNet, options map[string]interface{}) (*cniTypesCurr.Result, *cniTypesCurr.Result, error) { + endpointId := GetEndpointID(args) + // Parse Pod arguments. - podInfo := cns.KubernetesPodInfo{PodName: invoker.podName, PodNamespace: invoker.podNamespace} + podInfo := cns.NewPodInfo(args.ContainerID, endpointId, invoker.podName, invoker.podNamespace) orchestratorContext, err := json.Marshal(podInfo) if err != nil { return nil, nil, err } - endpointId := GetEndpointID(args) ipconfig := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: endpointId, @@ -179,14 +180,14 @@ func setHostOptions(nwCfg *cni.NetworkConfig, hostSubnetPrefix *net.IPNet, ncSub func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, options map[string]interface{}) error { // Parse Pod arguments. - podInfo := cns.KubernetesPodInfo{PodName: invoker.podName, PodNamespace: invoker.podNamespace} + endpointId := GetEndpointID(args) + podInfo := cns.NewPodInfo(args.ContainerID, endpointId, invoker.podName, invoker.podNamespace) orchestratorContext, err := json.Marshal(podInfo) if err != nil { return err } - endpointId := GetEndpointID(args) req := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: endpointId, diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 4472f63e63..db4c9bb47b 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -64,18 +64,14 @@ func getContainerNetworkConfiguration( return getContainerNetworkConfigurationInternal(nwCfg.CNSUrl, podNamespace, podNameWithoutSuffix, ifName) } -func getContainerNetworkConfigurationInternal( - address string, - namespace string, - podName string, - ifName string) (*cniTypesCurr.Result, *cns.GetNetworkContainerResponse, net.IPNet, error) { +func getContainerNetworkConfigurationInternal(address string, namespace string, podName string, ifName string) (*cniTypesCurr.Result, *cns.GetNetworkContainerResponse, net.IPNet, error) { cnsClient, err := cnsclient.GetCnsClient() if err != nil { log.Printf("Failed to get CNS client. Error: %v", err) return nil, nil, net.IPNet{}, err } - podInfo := cns.KubernetesPodInfo{PodName: podName, PodNamespace: namespace} + podInfo := cns.NewPodInfo("", "", podName, namespace) orchestratorContext, err := json.Marshal(podInfo) if err != nil { log.Printf("Marshalling KubernetesPodInfo failed with %v", err) diff --git a/cni/network/network.go b/cni/network/network.go index 6456142f35..0b18f3d663 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -155,7 +155,7 @@ func (plugin *netPlugin) Start(config *common.PluginConfig) error { return nil } -func (plugin *netPlugin) GetAllEndpointState(networkid string) (api.CNIState, error) { +func (plugin *netPlugin) GetAllEndpointState(networkid string) (*api.AzureCNIState, error) { st := api.AzureCNIState{ ContainerInterfaces: make(map[string]api.PodNetworkInterfaceInfo), } @@ -168,11 +168,11 @@ func (plugin *netPlugin) GetAllEndpointState(networkid string) (api.CNIState, er for _, ep := range eps { id := ep.Id info := api.PodNetworkInterfaceInfo{ - PodName: ep.PODName, - PodNamespace: ep.PODNameSpace, + PodName: ep.PODName, + PodNamespace: ep.PODNameSpace, PodEndpointId: ep.Id, - ContainerID: ep.ContainerID, - IPAddresses: ep.IPAddresses, + ContainerID: ep.ContainerID, + IPAddresses: ep.IPAddresses, } st.ContainerInterfaces[id] = info @@ -1097,7 +1097,7 @@ func (plugin *netPlugin) Update(args *cniSkel.CmdArgs) error { } // create struct with info for target POD - podInfo := cns.KubernetesPodInfo{PodName: k8sPodName, PodNamespace: k8sNamespace} + podInfo := cns.NewPodInfo("", "", k8sPodName, k8sNamespace) if orchestratorContext, err = json.Marshal(podInfo); err != nil { log.Printf("Marshalling KubernetesPodInfo failed with %v", err) return plugin.Errorf(err.Error()) diff --git a/cni/network/network_test.go b/cni/network/network_test.go index 308b125101..8e09233baf 100644 --- a/cni/network/network_test.go +++ b/cni/network/network_test.go @@ -113,17 +113,17 @@ func TestGetAllEndpointState(t *testing.T) { ContainerInterfaces: map[string]api.PodNetworkInterfaceInfo{ ep1.Id: { PodEndpointId: ep1.Id, - PodName: ep1.PODName, - PodNamespace: ep1.PODNameSpace, - ContainerID: ep1.ContainerID, - IPAddresses: ep1.IPAddresses, + PodName: ep1.PODName, + PodNamespace: ep1.PODNameSpace, + ContainerID: ep1.ContainerID, + IPAddresses: ep1.IPAddresses, }, ep2.Id: { PodEndpointId: ep2.Id, - PodName: ep2.PODName, - PodNamespace: ep2.PODNameSpace, - ContainerID: ep2.ContainerID, - IPAddresses: ep2.IPAddresses, + PodName: ep2.PODName, + PodNamespace: ep2.PODNameSpace, + ContainerID: ep2.ContainerID, + IPAddresses: ep2.IPAddresses, }, }, } diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 2089dbc72e..dee0a4c532 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -107,16 +107,133 @@ type ConfigureContainerNetworkingRequest struct { NetworkContainerid string } -// KubernetesPodInfo is an OrchestratorContext that holds PodName and PodNamespace. -type KubernetesPodInfo struct { - PodName string - PodNamespace string +// PodInfoByIPProvider to be implemented by initializers which provide a map +// of PodInfos by IP. +type PodInfoByIPProvider interface { + PodInfoByIP() map[string]PodInfo } -// GetOrchestratorContext will return the orchestratorcontext as a string -// TODO - should use a hashed name or can this be PODUid? -func (podinfo *KubernetesPodInfo) GetOrchestratorContextKey() string { - return podinfo.PodName + ":" + podinfo.PodNamespace +var _ PodInfoByIPProvider = (PodInfoByIPProviderFunc)(nil) + +// PodInfoByIPProviderFunc functional type which implements PodInfoByIPProvider. +// Allows one-off functional implementations of the PodInfoByIPProvider +// interface when a custom type definition is not necessary. +type PodInfoByIPProviderFunc func() map[string]PodInfo + +// PodInfoByIP implements PodInfoByIPProvider on PodInfByIPProviderFunc. +func (f PodInfoByIPProviderFunc) PodInfoByIP() map[string]PodInfo { + return f() +} + +var GlobalPodInfoScheme podInfoScheme + +// podInfoScheme indicates which schema should be used when generating +// the map key in the Key() function on a podInfo object. +type podInfoScheme int + +const ( + KubernetesPodInfoScheme podInfoScheme = iota + InterfaceIDPodInfoScheme +) + +// PodInfo represents the object that we are providing network for. +type PodInfo interface { + // InfraContainerID the CRI infra container for the pod namespace. + InfraContainerID() string + // InterfaceID a short hash of the infra container and the primary network + // interface of the pod net ns. + InterfaceID() string + // Key is a unique string representation of the PodInfo. + Key() string + // Name is the orchestrator pod name. + Name() string + // Namespace is the orchestrator pod namespace. + Namespace() string + // String is a string rep of PodInfo. + String() string +} + +var _ PodInfo = (*podInfo)(nil) + +// podInfo implements PodInfo for multiple schemas of Key +type podInfo struct { + PodInfraContainerID string + PodInterfaceID string + PodName string + PodNamespace string + Version podInfoScheme +} + +func (p *podInfo) InfraContainerID() string { + return p.PodInfraContainerID +} + +func (p *podInfo) InterfaceID() string { + return p.PodInterfaceID +} + +// Key is a unique string representation of the PodInfo. +// If the PodInfo.Version == kubernetes, the Key is composed of the +// orchestrator pod name and namespace. if the Version is interfaceID, key is +// composed of the CNI interfaceID, which is generated from the CRI infra +// container ID and the pod net ns primary interface name. +func (p *podInfo) Key() string { + if p.Version == InterfaceIDPodInfoScheme { + return p.PodInterfaceID + } + return p.PodName + ":" + p.PodNamespace +} + +func (p *podInfo) Name() string { + return p.PodName +} + +func (p *podInfo) Namespace() string { + return p.PodNamespace +} + +// String is a string rep of PodInfo. +// String calls Key(). +func (p *podInfo) String() string { + return p.Key() +} + +// NewPodInfo returns an implementation of PodInfo that returns the passed +// configuration for their namesake functions. +func NewPodInfo(infraContainerID, interfaceID, name, namespace string) PodInfo { + return &podInfo{ + PodInfraContainerID: infraContainerID, + PodInterfaceID: interfaceID, + PodName: name, + PodNamespace: namespace, + Version: GlobalPodInfoScheme, + } +} + +// NewPodInfoFromIPConfigRequest builds and returns an implementation of +// PodInfo from the provided IPConfigRequest. +func NewPodInfoFromIPConfigRequest(req IPConfigRequest) (PodInfo, error) { + p, err := UnmarshalPodInfo(req.OrchestratorContext) + if err != nil { + return nil, err + } + if GlobalPodInfoScheme == InterfaceIDPodInfoScheme && req.InfraContainerID == "" { + return nil, fmt.Errorf("need interfaceID for pod info but request was empty") + } + p.(*podInfo).PodInfraContainerID = req.InfraContainerID + p.(*podInfo).PodInterfaceID = req.PodInterfaceID + return p, nil +} + +// UnmarshalPodInfo wraps json.Unmarshal to return an implementation of +// PodInfo. +func UnmarshalPodInfo(b []byte) (PodInfo, error) { + p := &podInfo{} + if err := json.Unmarshal(b, p); err != nil { + return nil, err + } + p.Version = GlobalPodInfoScheme + return p, nil } // MultiTenancyInfo contains encap type and id. diff --git a/cns/cnireconciler/initialize.go b/cns/cnireconciler/initialize.go new file mode 100644 index 0000000000..f4cf73f933 --- /dev/null +++ b/cns/cnireconciler/initialize.go @@ -0,0 +1,36 @@ +package cnireconciler + +import ( + "github.com/Azure/azure-container-networking/cni/api" + "github.com/Azure/azure-container-networking/cni/client" + "github.com/Azure/azure-container-networking/cns" + "k8s.io/utils/exec" +) + +// NewCNIPodInfoProvider returns an implementation of cns.PodInfoByIPProvider +// that execs out to the CNI and uses the response to build the PodInfo map. +func NewCNIPodInfoProvider() (cns.PodInfoByIPProvider, error) { + cli := client.New(exec.New()) + state, err := cli.GetEndpointState() + if err != nil { + return nil, err + } + return cns.PodInfoByIPProviderFunc(func() map[string]cns.PodInfo { + return cniStateToPodInfoByIP(state) + }), nil +} + +// cniStateToPodInfoByIP converts an AzureCNIState dumped from a CNI exec +// into a PodInfo map, using the first endpoint IP as the key in the map. +func cniStateToPodInfoByIP(state *api.AzureCNIState) map[string]cns.PodInfo { + podInfoByIP := map[string]cns.PodInfo{} + for _, endpoint := range state.ContainerInterfaces { + podInfoByIP[endpoint.IPAddresses[0].IP.String()] = cns.NewPodInfo( + endpoint.ContainerID, + endpoint.PodEndpointId, + endpoint.PodName, + endpoint.PodNamespace, + ) + } + return podInfoByIP +} diff --git a/cns/cnireconciler/version.go b/cns/cnireconciler/version.go new file mode 100644 index 0000000000..c24c1a06e4 --- /dev/null +++ b/cns/cnireconciler/version.go @@ -0,0 +1,27 @@ +package cnireconciler + +import ( + "github.com/Azure/azure-container-networking/cni/client" + semver "github.com/hashicorp/go-version" + "k8s.io/utils/exec" +) + +const cniDumpStateVer = "1.4.2" + +// IsDumpStateVer checks if the CNI executable is a version that +// has the dump state command required to initialize CNS from CNI +// state and returns the result of that test or an error. Will always +// return false when there is an error. +func IsDumpStateVer() (bool, error) { + needVer, err := semver.NewVersion(cniDumpStateVer) + if err != nil { + return false, err + } + cnicli := client.New(exec.New()) + if ver, err := cnicli.GetVersion(); err != nil { + return false, err + } else if ver.LessThan(needVer) { + return false, nil + } + return true, nil +} diff --git a/cns/cnsclient/apiclient.go b/cns/cnsclient/apiclient.go index d39742907c..7e4a07fa37 100644 --- a/cns/cnsclient/apiclient.go +++ b/cns/cnsclient/apiclient.go @@ -7,7 +7,7 @@ import ( // APIClient interface to update cns state type APIClient interface { - ReconcileNCState(nc *cns.CreateNetworkContainerRequest, pods map[string]cns.KubernetesPodInfo, scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) error + ReconcileNCState(nc *cns.CreateNetworkContainerRequest, pods map[string]cns.PodInfo, scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) error CreateOrUpdateNC(nc cns.CreateNetworkContainerRequest) error UpdateIPAMPoolMonitor(scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) error GetNC(nc cns.GetNetworkContainerRequest) (cns.GetNetworkContainerResponse, error) diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index 98c9035f78..51fda866f6 100644 --- a/cns/cnsclient/cnsclient_test.go +++ b/cns/cnsclient/cnsclient_test.go @@ -225,7 +225,7 @@ func TestCNSClientRequestAndRelease(t *testing.T) { addTestStateToRestServer(t, secondaryIps) - podInfo := cns.KubernetesPodInfo{PodName: podName, PodNamespace: podNamespace} + podInfo := cns.NewPodInfo("", "", podName, podNamespace) orchestratorContext, err := json.Marshal(podInfo) if err != nil { t.Fatal(err) @@ -299,7 +299,7 @@ func TestCNSClientPodContextApi(t *testing.T) { addTestStateToRestServer(t, secondaryIps) - podInfo := cns.KubernetesPodInfo{PodName: podName, PodNamespace: podNamespace} + podInfo := cns.NewPodInfo("", "", podName, podNamespace) orchestratorContext, err := json.Marshal(podInfo) if err != nil { t.Fatal(err) @@ -339,7 +339,7 @@ func TestCNSClientDebugAPI(t *testing.T) { addTestStateToRestServer(t, secondaryIps) - podInfo := cns.KubernetesPodInfo{PodName: podName, PodNamespace: podNamespace} + podInfo := cns.NewPodInfo("", "", podName, podNamespace) orchestratorContext, err := json.Marshal(podInfo) if err != nil { t.Fatal(err) diff --git a/cns/cnsclient/httpapi/client.go b/cns/cnsclient/httpapi/client.go index 7307e17638..df07495c8c 100644 --- a/cns/cnsclient/httpapi/client.go +++ b/cns/cnsclient/httpapi/client.go @@ -36,7 +36,7 @@ func (client *Client) UpdateIPAMPoolMonitor(scalar nnc.Scaler, spec nnc.NodeNetw } // ReconcileNCState initializes cns state -func (client *Client) ReconcileNCState(ncRequest *cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.KubernetesPodInfo, scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) error { +func (client *Client) ReconcileNCState(ncRequest *cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo, scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) error { returnCode := client.RestService.ReconcileNCState(ncRequest, podInfoByIP, scalar, spec) if returnCode != 0 { diff --git a/cns/configuration/cns_config.json b/cns/configuration/cns_config.json index bd7e0dc1f1..560d00c7c8 100644 --- a/cns/configuration/cns_config.json +++ b/cns/configuration/cns_config.json @@ -1,23 +1,24 @@ { "TelemetrySettings": { - "TelemetryBatchSizeBytes": 16384, - "TelemetryBatchIntervalInSecs": 15, - "RefreshIntervalInSecs": 15, + "DebugMode": false, "DisableAll": false, "HeartBeatIntervalInMins": 30, - "DebugMode": false, - "SnapshotIntervalInMins": 60 + "RefreshIntervalInSecs": 15, + "SnapshotIntervalInMins": 60, + "TelemetryBatchIntervalInSecs": 15, + "TelemetryBatchSizeBytes": 16384 }, "ManagedSettings": { - "PrivateEndpoint": "", "InfrastructureNetworkID": "", "NodeID": "", - "NodeSyncIntervalInSeconds": 30 + "NodeSyncIntervalInSeconds": 30, + "PrivateEndpoint": "" }, "ChannelMode": "Direct", - "UseHTTPS" : false, - "TLSSubjectName" : "", - "TLSCertificatePath" : "", - "TLSPort" : "10091", + "InitializeFromCNI": false, + "TLSCertificatePath": "", + "TLSPort": "10091", + "TLSSubjectName": "", + "UseHTTPS": false, "WireserverIP": "168.63.129.16" } diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index 40111ae1f2..ecb3cf2846 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -18,17 +18,18 @@ const ( ) type CNSConfig struct { - TelemetrySettings TelemetrySettings - ManagedSettings ManagedSettings ChannelMode string - UseHTTPS bool - TLSSubjectName string + InitializeFromCNI bool + ManagedSettings ManagedSettings + SyncHostNCTimeoutMs time.Duration + SyncHostNCVersionIntervalMs time.Duration TLSCertificatePath string - TLSPort string TLSEndpoint string + TLSPort string + TLSSubjectName string + TelemetrySettings TelemetrySettings + UseHTTPS bool WireserverIP string - SyncHostNCVersionIntervalMs time.Duration - SyncHostNCTimeoutMs time.Duration } type TelemetrySettings struct { diff --git a/cns/multitenantcontroller/mockclients/cnsclient.go b/cns/multitenantcontroller/mockclients/cnsclient.go index 6ab1a390c8..7f8b73d997 100644 --- a/cns/multitenantcontroller/mockclients/cnsclient.go +++ b/cns/multitenantcontroller/mockclients/cnsclient.go @@ -35,7 +35,7 @@ func (m *MockAPIClient) EXPECT() *MockAPIClientMockRecorder { } // ReconcileNCState mocks base method -func (m *MockAPIClient) ReconcileNCState(nc *cns.CreateNetworkContainerRequest, pods map[string]cns.KubernetesPodInfo, scalar v1alpha.Scaler, spec v1alpha.NodeNetworkConfigSpec) error { +func (m *MockAPIClient) ReconcileNCState(nc *cns.CreateNetworkContainerRequest, pods map[string]cns.PodInfo, scalar v1alpha.Scaler, spec v1alpha.NodeNetworkConfigSpec) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ReconcileNCState", nc, pods, scalar, spec) ret0, _ := ret[0].(error) diff --git a/cns/networkcontainers/networkcontainers.go b/cns/networkcontainers/networkcontainers.go index 1f26f54ee3..8b5d5be803 100644 --- a/cns/networkcontainers/networkcontainers.go +++ b/cns/networkcontainers/networkcontainers.go @@ -206,17 +206,17 @@ func execPlugin(rt *libcni.RuntimeConf, netconf []byte, operation, path string) } // Attach - attaches network container to network. -func (cn *NetworkContainers) Attach(podInfo cns.KubernetesPodInfo, dockerContainerid string, netPluginConfig *NetPluginConfiguration) error { +func (cn *NetworkContainers) Attach(podInfo cns.PodInfo, dockerContainerid string, netPluginConfig *NetPluginConfiguration) error { logger.Printf("[Azure CNS] NetworkContainers.Attach called") - err := configureNetworkContainerNetworking(cniAdd, podInfo.PodName, podInfo.PodNamespace, dockerContainerid, netPluginConfig) + err := configureNetworkContainerNetworking(cniAdd, podInfo.Name(), podInfo.Namespace(), dockerContainerid, netPluginConfig) logger.Printf("[Azure CNS] NetworkContainers.Attach finished") return err } // Detach - detaches network container from network. -func (cn *NetworkContainers) Detach(podInfo cns.KubernetesPodInfo, dockerContainerid string, netPluginConfig *NetPluginConfiguration) error { +func (cn *NetworkContainers) Detach(podInfo cns.PodInfo, dockerContainerid string, netPluginConfig *NetPluginConfiguration) error { logger.Printf("[Azure CNS] NetworkContainers.Detach called") - err := configureNetworkContainerNetworking(cniDelete, podInfo.PodName, podInfo.PodNamespace, dockerContainerid, netPluginConfig) + err := configureNetworkContainerNetworking(cniDelete, podInfo.Name(), podInfo.Namespace(), dockerContainerid, netPluginConfig) logger.Printf("[Azure CNS] NetworkContainers.Detach finished") return err } diff --git a/cns/networkcontainers/networkcontainers_linux.go b/cns/networkcontainers/networkcontainers_linux.go index e2dd5abadb..e53f423bc2 100644 --- a/cns/networkcontainers/networkcontainers_linux.go +++ b/cns/networkcontainers/networkcontainers_linux.go @@ -4,7 +4,6 @@ package networkcontainers import ( - "encoding/json" "errors" "fmt" "os" @@ -45,8 +44,7 @@ func updateInterface(createNetworkContainerRequest cns.CreateNetworkContainerReq } } - var podInfo cns.KubernetesPodInfo - err := json.Unmarshal(createNetworkContainerRequest.OrchestratorContext, &podInfo) + podInfo, err := cns.UnmarshalPodInfo(createNetworkContainerRequest.OrchestratorContext) if err != nil { logger.Printf("[Azure CNS] Unmarshalling %s failed with error %v", createNetworkContainerRequest.NetworkContainerType, err) return err @@ -59,8 +57,8 @@ func updateInterface(createNetworkContainerRequest cns.CreateNetworkContainerReq NetNS: "", // Not needed for CNI update operation IfName: createNetworkContainerRequest.NetworkContainerid, Args: [][2]string{ - {k8sPodNamespaceStr, podInfo.PodNamespace}, - {k8sPodNameStr, podInfo.PodName}, + {k8sPodNamespaceStr, podInfo.Namespace()}, + {k8sPodNameStr, podInfo.Name()}, }, } diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 49cc8e5fb4..d9168fccb7 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -8,7 +8,6 @@ import ( "encoding/json" "encoding/xml" "fmt" - "github.com/Azure/azure-container-networking/store" "net/http" "net/http/httptest" "net/url" @@ -16,6 +15,8 @@ import ( "strings" "testing" + "github.com/Azure/azure-container-networking/store" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/fakes" @@ -706,7 +707,7 @@ func createOrUpdateNetworkContainerWithParams(t *testing.T, params createOrUpdat ipSubnet.IPAddress = params.ncIP ipSubnet.PrefixLength = 24 ipConfig.IPSubnet = ipSubnet - podInfo := cns.KubernetesPodInfo{PodName: "testpod", PodNamespace: "testpodnamespace"} + podInfo := cns.NewPodInfo("", "", "testpod", "testpodnamespace") context, _ := json.Marshal(podInfo) info := &cns.CreateNetworkContainerRequest{ @@ -774,9 +775,7 @@ func deleteNetworkContainerWithParams(t *testing.T, params createOrUpdateNetwork func getNetworkContainerByContext(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetNetworkContainerResponse - podInfo := cns.KubernetesPodInfo{ - PodName: params.podName, - PodNamespace: params.podNamespace} + podInfo := cns.NewPodInfo("", "", params.podName, params.podNamespace) podInfoBytes, err := json.Marshal(podInfo) getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} @@ -803,9 +802,7 @@ func getNetworkContainerByContext(t *testing.T, params createOrUpdateNetworkCont func getNonExistNetworkContainerByContext(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetNetworkContainerResponse - podInfo := cns.KubernetesPodInfo{ - PodName: params.podName, - PodNamespace: params.podNamespace} + podInfo := cns.NewPodInfo("", "", params.podName, params.podNamespace) podInfoBytes, err := json.Marshal(podInfo) getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} @@ -832,9 +829,7 @@ func getNonExistNetworkContainerByContext(t *testing.T, params createOrUpdateNet func getNetworkContainerByContextExpectedError(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetNetworkContainerResponse - podInfo := cns.KubernetesPodInfo{ - PodName: params.podName, - PodNamespace: params.podNamespace} + podInfo := cns.NewPodInfo("", "", params.podName, params.podNamespace) podInfoBytes, err := json.Marshal(podInfo) getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index d4c8a86430..bf80fff407 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -207,7 +207,8 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo } // This API will be called by CNS RequestController on CRD update. -func (service *HTTPRestService) ReconcileNCState(ncRequest *cns.CreateNetworkContainerRequest, podInfoByIp map[string]cns.KubernetesPodInfo, scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) int { +func (service *HTTPRestService) ReconcileNCState(ncRequest *cns.CreateNetworkContainerRequest, podInfoByIp map[string]cns.PodInfo, scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) int { + logger.Printf("Reconciling NC state with podInfo %+v", podInfoByIp) // check if ncRequest is null, then return as there is no CRD state yet if ncRequest == nil { logger.Printf("CNS starting with no NC state, podInfoMap count %d", len(podInfoByIp)) @@ -229,12 +230,11 @@ func (service *HTTPRestService) ReconcileNCState(ncRequest *cns.CreateNetworkCon if podInfo, exists := podInfoByIp[secIpConfig.IPAddress]; exists { logger.Printf("SecondaryIP %+v is allocated to Pod. %+v, ncId: %s", secIpConfig, podInfo, ncRequest.NetworkContainerid) - kubernetesPodInfo := cns.KubernetesPodInfo{ - PodName: podInfo.PodName, - PodNamespace: podInfo.PodNamespace, + jsonContext, err := json.Marshal(podInfo) + if err != nil { + logger.Errorf("Failed to marshal PodInfo, error: %v", err) + return UnexpectedError } - jsonContext, _ := json.Marshal(kubernetesPodInfo) - ipconfigRequest := cns.IPConfigRequest{ DesiredIPAddress: secIpConfig.IPAddress, OrchestratorContext: jsonContext, diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 42af4f64f5..9b5adbdfa6 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -5,7 +5,6 @@ package restserver import ( "context" - "encoding/json" "fmt" "os" "reflect" @@ -209,7 +208,7 @@ func TestReconcileNCWithEmptyState(t *testing.T) { setOrchestratorTypeInternal(cns.KubernetesCRD) expectedNcCount := len(svc.state.ContainerStatus) - expectedAllocatedPods := make(map[string]cns.KubernetesPodInfo) + expectedAllocatedPods := make(map[string]cns.PodInfo) returnCode := svc.ReconcileNCState(nil, expectedAllocatedPods, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) if returnCode != Success { t.Errorf("Unexpected failure on reconcile with no state %d", returnCode) @@ -235,16 +234,10 @@ func TestReconcileNCWithExistingState(t *testing.T) { } req := generateNetworkContainerRequest(secondaryIPConfigs, "reconcileNc1", "-1") - expectedAllocatedPods := make(map[string]cns.KubernetesPodInfo) - expectedAllocatedPods["10.0.0.6"] = cns.KubernetesPodInfo{ - PodName: "reconcilePod1", - PodNamespace: "PodNS1", - } + expectedAllocatedPods := make(map[string]cns.PodInfo) + expectedAllocatedPods["10.0.0.6"] = cns.NewPodInfo("", "", "reconcilePod1", "PodNS1") - expectedAllocatedPods["10.0.0.7"] = cns.KubernetesPodInfo{ - PodName: "reconcilePod2", - PodNamespace: "PodNS1", - } + expectedAllocatedPods["10.0.0.7"] = cns.NewPodInfo("", "", "reconcilePod2", "PodNS1") expectedNcCount := len(svc.state.ContainerStatus) returnCode := svc.ReconcileNCState(&req, expectedAllocatedPods, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) @@ -272,17 +265,11 @@ func TestReconcileNCWithSystemPods(t *testing.T) { } req := generateNetworkContainerRequest(secondaryIPConfigs, uuid.New().String(), "-1") - expectedAllocatedPods := make(map[string]cns.KubernetesPodInfo) - expectedAllocatedPods["10.0.0.6"] = cns.KubernetesPodInfo{ - PodName: "customerpod1", - PodNamespace: "PodNS1", - } + expectedAllocatedPods := make(map[string]cns.PodInfo) + expectedAllocatedPods["10.0.0.6"] = cns.NewPodInfo("", "", "customerpod1", "PodNS1") // Allocate non-vnet IP for system pod - expectedAllocatedPods["192.168.0.1"] = cns.KubernetesPodInfo{ - PodName: "systempod", - PodNamespace: "kube-system", - } + expectedAllocatedPods["192.168.0.1"] = cns.NewPodInfo("", "", "systempod", "kube-system") expectedNcCount := len(svc.state.ContainerStatus) returnCode := svc.ReconcileNCState(&req, expectedAllocatedPods, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) @@ -420,12 +407,12 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) // Validate IP state if ipStatus.OrchestratorContext != nil { - var podInfo cns.KubernetesPodInfo - if err := json.Unmarshal(ipStatus.OrchestratorContext, &podInfo); err != nil { + podInfo, err := cns.UnmarshalPodInfo(ipStatus.OrchestratorContext) + if err != nil { t.Fatalf("Failed to add IPConfig to state: %+v with error: %v", ipStatus, err) } - if _, exists := svc.PodIPIDByOrchestratorContext[podInfo.GetOrchestratorContextKey()]; exists { + if _, exists := svc.PodIPIDByOrchestratorContext[podInfo.Key()]; exists { if ipStatus.State != cns.Allocated { t.Fatalf("IPId: %s State is not Allocated, ipStatus: %+v", ipid, ipStatus) } @@ -477,7 +464,7 @@ func generateNetworkContainerRequest(secondaryIps map[string]cns.SecondaryIPConf return req } -func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkContainerRequest, expectedNcCount int, expectedAllocatedPods map[string]cns.KubernetesPodInfo) { +func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkContainerRequest, expectedNcCount int, expectedAllocatedPods map[string]cns.PodInfo) { if ncRequest == nil { // check svc ContainerStatus will be empty if len(svc.state.ContainerStatus) != expectedNcCount { @@ -492,7 +479,7 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon } for ipaddress, podInfo := range expectedAllocatedPods { - ipId := svc.PodIPIDByOrchestratorContext[podInfo.GetOrchestratorContextKey()] + ipId := svc.PodIPIDByOrchestratorContext[podInfo.Key()] ipConfigstate := svc.PodIPConfigState[ipId] if ipConfigstate.State != cns.Allocated { @@ -505,8 +492,7 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon } // Valdate pod context - var expectedPodInfo cns.KubernetesPodInfo - json.Unmarshal(ipConfigstate.OrchestratorContext, &expectedPodInfo) + expectedPodInfo, _ := cns.UnmarshalPodInfo(ipConfigstate.OrchestratorContext) if reflect.DeepEqual(expectedPodInfo, podInfo) != true { t.Fatalf("OrchestrationContext: is not same, expected: %+v, actual %+v", expectedPodInfo, podInfo) } diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 756da2c0e2..03346130b2 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -55,46 +55,33 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r } func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r *http.Request) { - var ( - req cns.IPConfigRequest - statusCode int - returnMessage string - err error - ) - - statusCode = UnexpectedError - operationName := "releaseIPConfigHandler" + req := cns.IPConfigRequest{} + resp := cns.Response{} + var err error defer func() { - resp := cns.Response{} - - if err != nil { - resp.ReturnCode = statusCode - resp.Message = returnMessage - } - err = service.Listener.Encode(w, &resp) logger.ResponseEx(service.Name, req, resp, resp.ReturnCode, ReturnCodeToString(resp.ReturnCode), err) }() err = service.Listener.Decode(w, r, &req) - logger.Request(service.Name+operationName, req, err) + logger.Request(service.Name+"releaseIPConfigHandler", req, err) if err != nil { - returnMessage = err.Error() - logger.Errorf("releaseIPConfigHandler decode failed becase %v, release IP config info %s", - returnMessage, req) + resp.ReturnCode = UnexpectedError + resp.Message = err.Error() + logger.Errorf("releaseIPConfigHandler decode failed becase %v, release IP config info %s", resp.Message, req) return } - podInfo, statusCode, returnMessage := service.validateIpConfigRequest(req) + var podInfo cns.PodInfo + podInfo, resp.ReturnCode, resp.Message = service.validateIpConfigRequest(req) if err = service.releaseIPConfig(podInfo); err != nil { - statusCode = NotFound - returnMessage = err.Error() - logger.Errorf("releaseIPConfigHandler releaseIPConfig failed because %v, release IP config info %s", returnMessage, req) + resp.ReturnCode = NotFound + resp.Message = err.Error() + logger.Errorf("releaseIPConfigHandler releaseIPConfig failed because %v, release IP config info %s", resp.Message, req) return } - return } // MarkIPAsPendingRelease will set the IPs which are in PendingProgramming or Available to PendingRelease state @@ -363,37 +350,37 @@ func filterIPConfigMap(toBeAdded map[string]cns.IPConfigurationStatus, f func(cn } //SetIPConfigAsAllocated takes a lock of the service, and sets the ipconfig in the CNS state as allocated, does not take a lock -func (service *HTTPRestService) setIPConfigAsAllocated(ipconfig cns.IPConfigurationStatus, podInfo cns.KubernetesPodInfo, marshalledOrchestratorContext json.RawMessage) (cns.IPConfigurationStatus, error) { +func (service *HTTPRestService) setIPConfigAsAllocated(ipconfig cns.IPConfigurationStatus, podInfo cns.PodInfo, marshalledOrchestratorContext json.RawMessage) (cns.IPConfigurationStatus, error) { ipconfig, err := service.updateIPConfigState(ipconfig.ID, cns.Allocated, marshalledOrchestratorContext) if err != nil { return cns.IPConfigurationStatus{}, err } - service.PodIPIDByOrchestratorContext[podInfo.GetOrchestratorContextKey()] = ipconfig.ID + service.PodIPIDByOrchestratorContext[podInfo.Key()] = ipconfig.ID return ipconfig, nil } //SetIPConfigAsAllocated and sets the ipconfig in the CNS state as allocated, does not take a lock -func (service *HTTPRestService) setIPConfigAsAvailable(ipconfig cns.IPConfigurationStatus, podInfo cns.KubernetesPodInfo) (cns.IPConfigurationStatus, error) { +func (service *HTTPRestService) setIPConfigAsAvailable(ipconfig cns.IPConfigurationStatus, podInfo cns.PodInfo) (cns.IPConfigurationStatus, error) { ipconfig, err := service.updateIPConfigState(ipconfig.ID, cns.Available, nil) if err != nil { return cns.IPConfigurationStatus{}, err } - delete(service.PodIPIDByOrchestratorContext, podInfo.GetOrchestratorContextKey()) + delete(service.PodIPIDByOrchestratorContext, podInfo.Key()) logger.Printf("[setIPConfigAsAvailable] Deleted outdated pod info %s from PodIPIDByOrchestratorContext since IP %s with ID %s will be released and set as Available", - podInfo.GetOrchestratorContextKey(), ipconfig.IPAddress, ipconfig.ID) + podInfo.Key(), ipconfig.IPAddress, ipconfig.ID) return ipconfig, nil } ////SetIPConfigAsAllocated takes a lock of the service, and sets the ipconfig in the CNS stateas Available // Todo - CNI should also pass the IPAddress which needs to be released to validate if that is the right IP allcoated // in the first place. -func (service *HTTPRestService) releaseIPConfig(podInfo cns.KubernetesPodInfo) error { +func (service *HTTPRestService) releaseIPConfig(podInfo cns.PodInfo) error { service.Lock() defer service.Unlock() - ipID := service.PodIPIDByOrchestratorContext[podInfo.GetOrchestratorContextKey()] + ipID := service.PodIPIDByOrchestratorContext[podInfo.Key()] if ipID != "" { if ipconfig, isExist := service.PodIPConfigState[ipID]; isExist { logger.Printf("[releaseIPConfig] Releasing IP %+v for pod %+v", ipconfig.IPAddress, podInfo) @@ -436,7 +423,7 @@ func (service *HTTPRestService) MarkExistingIPsAsPending(pendingIPIDs []string) return nil } -func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.KubernetesPodInfo) (cns.PodIpInfo, bool, error) { +func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.PodInfo) (cns.PodIpInfo, bool, error) { var ( podIpInfo cns.PodIpInfo isExist bool @@ -445,7 +432,7 @@ func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.KubernetesPodInf service.RLock() defer service.RUnlock() - ipID := service.PodIPIDByOrchestratorContext[podInfo.GetOrchestratorContextKey()] + ipID := service.PodIPIDByOrchestratorContext[podInfo.Key()] if ipID != "" { if ipState, isExist := service.PodIPConfigState[ipID]; isExist { err := service.populateIpConfigInfoUntransacted(ipState, &podIpInfo) @@ -459,7 +446,7 @@ func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.KubernetesPodInf return podIpInfo, isExist, nil } -func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.KubernetesPodInfo, desiredIPAddress string, orchestratorContext json.RawMessage) (cns.PodIpInfo, error) { +func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.PodInfo, desiredIPAddress string, orchestratorContext json.RawMessage) (cns.PodIpInfo, error) { var podIpInfo cns.PodIpInfo service.Lock() defer service.Unlock() @@ -474,8 +461,7 @@ func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.KubernetesPo logger.Printf("[AllocateDesiredIPConfig]: IP Config [%+v] is already allocated to this Pod [%+v]", ipConfig, podInfo) found = true } else { - var pInfo cns.KubernetesPodInfo - err := json.Unmarshal(ipConfig.OrchestratorContext, &pInfo) + pInfo, err := cns.UnmarshalPodInfo(ipConfig.OrchestratorContext) if err != nil { return podIpInfo, fmt.Errorf("[AllocateDesiredIPConfig] Failed to unmarshal IPState [%+v] OrchestratorContext, err: %v", ipConfig, err) } @@ -502,7 +488,7 @@ func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.KubernetesPo return podIpInfo, fmt.Errorf("Requested IP not found in pool") } -func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.KubernetesPodInfo, orchestratorContext json.RawMessage) (cns.PodIpInfo, error) { +func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.PodInfo, orchestratorContext json.RawMessage) (cns.PodIpInfo, error) { var podIpInfo cns.PodIpInfo service.Lock() @@ -530,15 +516,17 @@ func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.Kuberne // If IPConfig is already allocated for pod, it returns that else it returns one of the available ipconfigs. func requestIPConfigHelper(service *HTTPRestService, req cns.IPConfigRequest) (cns.PodIpInfo, error) { var ( - podInfo cns.KubernetesPodInfo podIpInfo cns.PodIpInfo isExist bool - err error ) // check if ipconfig already allocated for this pod and return if exists or error // if error, ipstate is nil, if exists, ipstate is not nil and error is nil - json.Unmarshal(req.OrchestratorContext, &podInfo) + podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) + if err != nil { + return podIpInfo, err + } + if podIpInfo, isExist, err = service.GetExistingIPConfig(podInfo); err != nil || isExist { return podIpInfo, err } diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 5ab176c60c..a93d361ad7 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -20,24 +20,15 @@ var ( testIP1 = "10.0.0.1" testPod1GUID = "898fb8f1-f93e-4c96-9c31-6b89098949a3" - testPod1Info = cns.KubernetesPodInfo{ - PodName: "testpod1", - PodNamespace: "testpod1namespace", - } + testPod1Info = cns.NewPodInfo("", "", "testpod1", "testpod1namespace") testIP2 = "10.0.0.2" testPod2GUID = "b21e1ee1-fb7e-4e6d-8c68-22ee5049944e" - testPod2Info = cns.KubernetesPodInfo{ - PodName: "testpod2", - PodNamespace: "testpod2namespace", - } + testPod2Info = cns.NewPodInfo("", "", "testpod2", "testpod2namespace") testIP3 = "10.0.0.3" testPod3GUID = "718e04ac-5a13-4dce-84b3-040accaa9b41" - testPod3Info = cns.KubernetesPodInfo{ - PodName: "testpod3", - PodNamespace: "testpod3namespace", - } + testPod3Info = cns.NewPodInfo("", "", "testpod3", "testpod3namespace") testIP4 = "10.0.0.4" testPod4GUID = "718e04ac-5a13-4dce-84b3-040accaa9b42" @@ -73,7 +64,6 @@ func NewPodState(ipaddress string, prefixLength uint8, id, ncid, state string, n func requestIpAddressAndGetState(t *testing.T, req cns.IPConfigRequest) (cns.IPConfigurationStatus, error) { var ( - podInfo cns.KubernetesPodInfo ipState cns.IPConfigurationStatus PodIpInfo cns.PodIpInfo err error @@ -114,17 +104,18 @@ func requestIpAddressAndGetState(t *testing.T, req cns.IPConfigRequest) (cns.IPC } // retrieve podinfo from orchestrator context - if err := json.Unmarshal(req.OrchestratorContext, &podInfo); err != nil { + podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) + if err != nil { return ipState, err } - ipId := svc.PodIPIDByOrchestratorContext[podInfo.GetOrchestratorContextKey()] + ipId := svc.PodIPIDByOrchestratorContext[podInfo.Key()] ipState = svc.PodIPConfigState[ipId] return ipState, err } -func NewPodStateWithOrchestratorContext(ipaddress, id, ncid, state string, prefixLength uint8, ncVersion int, orchestratorContext cns.KubernetesPodInfo) (cns.IPConfigurationStatus, error) { +func NewPodStateWithOrchestratorContext(ipaddress, id, ncid, state string, prefixLength uint8, ncVersion int, orchestratorContext cns.PodInfo) (cns.IPConfigurationStatus, error) { ipconfig := newSecondaryIPConfig(ipaddress, ncVersion) b, err := json.Marshal(orchestratorContext) return cns.IPConfigurationStatus{ @@ -155,13 +146,12 @@ func UpdatePodIpConfigState(t *testing.T, svc *HTTPRestService, ipconfigs map[st // update ipconfigs to expected state for ipId, ipconfig := range ipconfigs { if ipconfig.State == cns.Allocated { - var podInfo cns.KubernetesPodInfo - - if err := json.Unmarshal(ipconfig.OrchestratorContext, &podInfo); err != nil { + podInfo, err := cns.UnmarshalPodInfo(ipconfig.OrchestratorContext) + if err != nil { return fmt.Errorf("Failed to add IPConfig to state: %+v with error: %v", ipconfig, err) } - svc.PodIPIDByOrchestratorContext[podInfo.GetOrchestratorContextKey()] = ipId + svc.PodIPIDByOrchestratorContext[podInfo.Key()] = ipId svc.PodIPConfigState[ipId] = ipconfig } } @@ -200,7 +190,7 @@ func TestIPAMGetNextAvailableIPConfig(t *testing.T) { svc := getTestService() // Add already allocated pod ip to state - svc.PodIPIDByOrchestratorContext[testPod1Info.GetOrchestratorContextKey()] = testPod1GUID + svc.PodIPIDByOrchestratorContext[testPod1Info.Key()] = testPod1GUID state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available, 0) @@ -676,7 +666,7 @@ func TestIPAMMarkExistingIPConfigAsPending(t *testing.T) { svc := getTestService() // Add already allocated pod ip to state - svc.PodIPIDByOrchestratorContext[testPod1Info.GetOrchestratorContextKey()] = testPod1GUID + svc.PodIPIDByOrchestratorContext[testPod1Info.Key()] = testPod1GUID state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available, 0) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index aa4bd557e7..7a757a7936 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -165,8 +165,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw case cns.AzureFirstParty: fallthrough case cns.WebApps: // todo: Is WebApps an OrchastratorType or ContainerType? - var podInfo cns.KubernetesPodInfo - err := json.Unmarshal(req.OrchestratorContext, &podInfo) + podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) if err != nil { errBuf := fmt.Sprintf("Unmarshalling %s failed with error %v", req.NetworkContainerType, err) return UnexpectedError, errBuf @@ -178,7 +177,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw service.state.ContainerIDByOrchestratorContext = make(map[string]string) } - service.state.ContainerIDByOrchestratorContext[podInfo.PodName+podInfo.PodNamespace] = req.NetworkContainerid + service.state.ContainerIDByOrchestratorContext[podInfo.Key()] = req.NetworkContainerid break case cns.KubernetesCRD: @@ -226,9 +225,9 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN if exists { // pod ip exists, validate if state is not allocated, else fail if ipConfigStatus.State == cns.Allocated { - var expectedPodInfo cns.KubernetesPodInfo + var expectedPodInfo cns.PodInfo if len(ipConfigStatus.OrchestratorContext) != 0 { - json.Unmarshal(ipConfigStatus.OrchestratorContext, &expectedPodInfo) + expectedPodInfo, _ = cns.UnmarshalPodInfo(ipConfigStatus.OrchestratorContext) } errMsg := fmt.Sprintf("Failed to delete an Allocated IP %v, PodInfo %+v", ipConfigStatus, expectedPodInfo) return InconsistentIPConfigState, errMsg @@ -350,8 +349,7 @@ func (service *HTTPRestService) getNetworkContainerResponse(req cns.GetNetworkCo case cns.DBforPostgreSQL: fallthrough case cns.AzureFirstParty: - var podInfo cns.KubernetesPodInfo - err := json.Unmarshal(req.OrchestratorContext, &podInfo) + podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) if err != nil { getNetworkContainerResponse.Response.ReturnCode = UnexpectedError getNetworkContainerResponse.Response.Message = fmt.Sprintf("Unmarshalling orchestrator context failed with error %v", err) @@ -360,8 +358,7 @@ func (service *HTTPRestService) getNetworkContainerResponse(req cns.GetNetworkCo logger.Printf("pod info %+v", podInfo) - context := podInfo.PodName + podInfo.PodNamespace - containerID, exists = service.state.ContainerIDByOrchestratorContext[context] + containerID, exists = service.state.ContainerIDByOrchestratorContext[podInfo.Key()] if exists { // If the goal state is available with CNS, check if the NC is pending VFP programming @@ -403,7 +400,7 @@ func (service *HTTPRestService) getNetworkContainerResponse(req cns.GetNetworkCo return getNetworkContainerResponse } - containerID = service.state.ContainerIDByOrchestratorContext[context] + containerID = service.state.ContainerIDByOrchestratorContext[podInfo.Key()] } logger.Printf("containerid %v", containerID) @@ -540,8 +537,7 @@ func (service *HTTPRestService) attachOrDetachHelper(req cns.ConfigureContainerN returnMessage := "" switch service.state.OrchestratorType { case cns.Batch: - var podInfo cns.KubernetesPodInfo - err := json.Unmarshal(existing.CreateNetworkContainerRequest.OrchestratorContext, &podInfo) + podInfo, err := cns.UnmarshalPodInfo(existing.CreateNetworkContainerRequest.OrchestratorContext) if err != nil { returnCode = UnexpectedError returnMessage = fmt.Sprintf("Unmarshalling orchestrator context failed with error %+v", err) @@ -672,22 +668,20 @@ func (service *HTTPRestService) SendNCSnapShotPeriodically(ctx context.Context, } } -func (service *HTTPRestService) validateIpConfigRequest(ipConfigRequest cns.IPConfigRequest) (cns.KubernetesPodInfo, int, string) { - var podInfo cns.KubernetesPodInfo - +func (service *HTTPRestService) validateIpConfigRequest(ipConfigRequest cns.IPConfigRequest) (cns.PodInfo, int, string) { if service.state.OrchestratorType != cns.KubernetesCRD { - return podInfo, UnsupportedOrchestratorType, fmt.Sprintf("ReleaseIPConfig API supported only for kubernetes orchestrator") + return nil, UnsupportedOrchestratorType, "ReleaseIPConfig API supported only for kubernetes orchestrator" } if ipConfigRequest.OrchestratorContext == nil { - return podInfo, EmptyOrchestratorContext, fmt.Sprintf("OrchastratorContext is not set in the req: %+v", ipConfigRequest) + return nil, EmptyOrchestratorContext, fmt.Sprintf("OrchastratorContext is not set in the req: %+v", ipConfigRequest) } // retrieve podinfo from orchestrator context - if err := json.Unmarshal(ipConfigRequest.OrchestratorContext, &podInfo); err != nil { + podInfo, err := cns.NewPodInfoFromIPConfigRequest(ipConfigRequest) + if err != nil { return podInfo, UnsupportedOrchestratorContext, err.Error() } - return podInfo, Success, "" } diff --git a/cns/service/main.go b/cns/service/main.go index ce001f8006..5339996c17 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -21,6 +21,7 @@ import ( "github.com/Azure/azure-container-networking/cnm/ipam" "github.com/Azure/azure-container-networking/cnm/network" "github.com/Azure/azure-container-networking/cns" + cni "github.com/Azure/azure-container-networking/cns/cnireconciler" "github.com/Azure/azure-container-networking/cns/cnsclient" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/configuration" @@ -373,6 +374,7 @@ func main() { logDirectory := acn.GetArg(acn.OptLogLocation).(string) ipamQueryUrl := acn.GetArg(acn.OptIpamQueryUrl).(string) ipamQueryInterval := acn.GetArg(acn.OptIpamQueryInterval).(int) + startCNM := acn.GetArg(acn.OptStartAzureCNM).(bool) vers := acn.GetArg(acn.OptVersion).(bool) createDefaultExtNetworkType := acn.GetArg(acn.OptCreateDefaultExtNetworkType).(string) @@ -426,6 +428,31 @@ func main() { configuration.SetCNSConfigDefaults(&cnsconfig) logger.Printf("[Azure CNS] Read config :%+v", cnsconfig) + // We might be configured to reinitialize state from the CNI instead of the apiserver. + // If so, we should check that the the CNI is new enough to support the state commands, + // otherwise we fall back to the existing behavior. + if cnsconfig.InitializeFromCNI { + isGoodVer, err := cni.IsDumpStateVer() + if err != nil { + logger.Errorf("error checking CNI ver: %v", err) + } + + // override the prior config flag with the result of the ver check. + cnsconfig.InitializeFromCNI = isGoodVer + + if cnsconfig.InitializeFromCNI { + // Set the PodInfoVersion by initialization type, so that the + // PodInfo maps use the correct key schema + cns.GlobalPodInfoScheme = cns.InterfaceIDPodInfoScheme + } + } + if cnsconfig.InitializeFromCNI { + logger.Printf("Initializing from CNI") + } else { + logger.Printf("Initializing from Kubernetes") + } + logger.Printf("Set GlobalPodInfoScheme %v", cns.GlobalPodInfoScheme) + if cnsconfig.WireserverIP != "" { nmagentclient.WireserverIP = cnsconfig.WireserverIP } @@ -773,7 +800,12 @@ func InitializeCRDState(httpRestService cns.HTTPService, cnsconfig configuration httpRestServiceImplementation.SetNodeOrchestrator(&orchestrator) // Get crd implementation of request controller - requestController, err = kubecontroller.New(httpRestServiceImplementation, kubeConfig) + requestController, err = kubecontroller.New( + kubecontroller.Config{ + InitializeFromCNI: cnsconfig.InitializeFromCNI, + KubeConfig: kubeConfig, + Service: httpRestServiceImplementation, + }) if err != nil { logger.Errorf("[Azure CNS] Failed to make crd request controller :%v", err) return err diff --git a/cns/singletenantcontroller/kubecontroller/crdreconciler.go b/cns/singletenantcontroller/kubecontroller/crdreconciler.go index 0b9318814e..47b6f5ce3a 100644 --- a/cns/singletenantcontroller/kubecontroller/crdreconciler.go +++ b/cns/singletenantcontroller/kubecontroller/crdreconciler.go @@ -60,7 +60,7 @@ func (r *CrdReconciler) Reconcile(request reconcile.Request) (reconcile.Result, len(networkContainer.IPAssignments)) // Otherwise, create NC request and hand it off to CNS - ncRequest, err = CRDStatusToNCRequest(nodeNetConfig.Status) + ncRequest, err = crdStatusToNCRequest(nodeNetConfig.Status) if err != nil { logger.Errorf("[cns-rc] Error translating crd status to nc request %v", err) //requeue diff --git a/cns/singletenantcontroller/kubecontroller/crdrequestcontroller.go b/cns/singletenantcontroller/kubecontroller/crdrequestcontroller.go index 960c4c4003..5b4bc38cd0 100644 --- a/cns/singletenantcontroller/kubecontroller/crdrequestcontroller.go +++ b/cns/singletenantcontroller/kubecontroller/crdrequestcontroller.go @@ -8,6 +8,7 @@ import ( "sync" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/cnireconciler" "github.com/Azure/azure-container-networking/cns/cnsclient" "github.com/Azure/azure-container-networking/cns/cnsclient/httpapi" "github.com/Azure/azure-container-networking/cns/logger" @@ -33,12 +34,21 @@ const ( prometheusAddress = "0" //0 means disabled ) +// Config has crdRequestController options +type Config struct { + // InitializeFromCNI whether or not to initialize CNS state from k8s/CRDs + InitializeFromCNI bool + KubeConfig *rest.Config + Service *restserver.HTTPRestService +} + var _ singletenantcontroller.RequestController = (*requestController)(nil) // requestController // - watches CRD status changes // - updates CRD spec type requestController struct { + cfg Config mgr manager.Manager //Manager starts the reconcile loop which watches for crd status changes KubeClient KubeClient //KubeClient is a cached client which interacts with API server directAPIClient DirectAPIClient //Direct client to interact with API server @@ -64,8 +74,8 @@ func GetKubeConfig() (*rest.Config, error) { return k8sconfig, nil } -// New given a reference to CNS's HTTPRestService state, returns a crdRequestController struct -func New(restService *restserver.HTTPRestService, kubeconfig *rest.Config) (*requestController, error) { +//NewCrdRequestController given a reference to CNS's HTTPRestService state, returns a crdRequestController struct +func New(cfg Config) (*requestController, error) { //Check that logger package has been intialized if logger.Log == nil { @@ -90,13 +100,13 @@ func New(restService *restserver.HTTPRestService, kubeconfig *rest.Config) (*req } // Create a direct client to the API server which we use to list pods when initializing cns state before reconcile loop - directAPIClient, err := NewAPIDirectClient(kubeconfig) + directAPIClient, err := NewAPIDirectClient(cfg.KubeConfig) if err != nil { return nil, fmt.Errorf("Error creating direct API Client: %v", err) } // Create a direct client to the API server configured to get nodenetconfigs to get nnc for same reason above - directCRDClient, err := NewCRDDirectClient(kubeconfig, &nnc.GroupVersion) + directCRDClient, err := NewCRDDirectClient(cfg.KubeConfig, &nnc.GroupVersion) if err != nil { return nil, fmt.Errorf("Error creating direct CRD client: %v", err) } @@ -104,7 +114,7 @@ func New(restService *restserver.HTTPRestService, kubeconfig *rest.Config) (*req // Create manager for CrdRequestController // MetricsBindAddress is the tcp address that the controller should bind to // for serving prometheus metrics, set to "0" to disable - mgr, err := ctrl.NewManager(kubeconfig, ctrl.Options{ + mgr, err := ctrl.NewManager(cfg.KubeConfig, ctrl.Options{ Scheme: scheme, MetricsBindAddress: prometheusAddress, Namespace: k8sNamespace, @@ -116,7 +126,7 @@ func New(restService *restserver.HTTPRestService, kubeconfig *rest.Config) (*req //Create httpClient httpClient := &httpapi.Client{ - RestService: restService, + RestService: cfg.Service, } //Create reconciler @@ -134,6 +144,7 @@ func New(restService *restserver.HTTPRestService, kubeconfig *rest.Config) (*req // Create the requestController rc := requestController{ + cfg: cfg, mgr: mgr, KubeClient: mgr.GetClient(), directAPIClient: directAPIClient, @@ -150,8 +161,8 @@ func New(restService *restserver.HTTPRestService, kubeconfig *rest.Config) (*req func (rc *requestController) Init(ctx context.Context) error { logger.Printf("InitRequestController") - defer rc.lock.Unlock() rc.lock.Lock() + defer rc.lock.Unlock() if err := rc.initCNS(ctx); err != nil { logger.Errorf("[cns-rc] Error initializing cns state: %v", err) @@ -191,25 +202,16 @@ func (rc *requestController) Start(ctx context.Context) error { // return if RequestController is started func (rc *requestController) IsStarted() bool { - defer rc.lock.Unlock() rc.lock.Lock() + defer rc.lock.Unlock() return rc.Started } // InitCNS initializes cns by passing pods and a createnetworkcontainerrequest func (rc *requestController) initCNS(ctx context.Context) error { - var ( - pods *corev1.PodList - pod corev1.Pod - podInfo cns.KubernetesPodInfo - nodeNetConfig *nnc.NodeNetworkConfig - podInfoByIP map[string]cns.KubernetesPodInfo - ncRequest cns.CreateNetworkContainerRequest - err error - ) - // Get nodeNetConfig using direct client - if nodeNetConfig, err = rc.getNodeNetConfigDirect(ctx, rc.nodeName, k8sNamespace); err != nil { + nodeNetConfig, err := rc.getNodeNetConfigDirect(ctx, rc.nodeName, k8sNamespace) + if err != nil { // If the CRD is not defined, exit if rc.isNotDefined(err) { logger.Errorf("CRD is not defined on cluster: %v", err) @@ -237,35 +239,47 @@ func (rc *requestController) initCNS(ctx context.Context) error { } // Convert to CreateNetworkContainerRequest - if ncRequest, err = CRDStatusToNCRequest(nodeNetConfig.Status); err != nil { + ncRequest, err := crdStatusToNCRequest(nodeNetConfig.Status) + if err != nil { logger.Errorf("Error when converting nodeNetConfig status into CreateNetworkContainerRequest: %v", err) return err } - // Get all pods using direct client - if pods, err = rc.getAllPods(ctx, rc.nodeName); err != nil { - logger.Errorf("Error when getting all pods when initializing cns: %v", err) - return err - } + var podInfoByIPProvider cns.PodInfoByIPProvider - // Convert pod list to map of pod ip -> kubernetes pod info - if len(pods.Items) != 0 { - podInfoByIP = make(map[string]cns.KubernetesPodInfo) - for _, pod = range pods.Items { - //Only add pods that aren't on the host network - if !pod.Spec.HostNetwork { - podInfo = cns.KubernetesPodInfo{ - PodName: pod.Name, - PodNamespace: pod.Namespace, - } - podInfoByIP[pod.Status.PodIP] = podInfo - } + if rc.cfg.InitializeFromCNI { + // rebuild CNS state from CNI + logger.Printf("initializing CNS from CNI") + podInfoByIPProvider, err = cnireconciler.NewCNIPodInfoProvider() + if err != nil { + return err } + } else { + logger.Printf("initializing CNS from apiserver") + // Get all pods using direct client + pods, err := rc.getAllPods(ctx, rc.nodeName) + if err != nil { + logger.Errorf("error when getting all pods when initializing cns: %v", err) + return err + } + podInfoByIPProvider = cns.PodInfoByIPProviderFunc(func() map[string]cns.PodInfo { + return rc.kubePodsToPodInfoByIP(pods.Items) + }) } // Call cnsclient init cns passing those two things - return rc.CNSClient.ReconcileNCState(&ncRequest, podInfoByIP, nodeNetConfig.Status.Scaler, nodeNetConfig.Spec) + return rc.CNSClient.ReconcileNCState(&ncRequest, podInfoByIPProvider.PodInfoByIP(), nodeNetConfig.Status.Scaler, nodeNetConfig.Spec) +} +// kubePodsToPodInfoByIP maps kubernetes pods to cns.PodInfos by IP +func (rc *requestController) kubePodsToPodInfoByIP(pods []corev1.Pod) map[string]cns.PodInfo { + podInfoByIP := map[string]cns.PodInfo{} + for _, pod := range pods { + if !pod.Spec.HostNetwork { + podInfoByIP[pod.Status.PodIP] = cns.NewPodInfo("", "", pod.Name, pod.Namespace) + } + } + return podInfoByIP } // UpdateCRDSpec updates the CRD spec diff --git a/cns/singletenantcontroller/kubecontroller/crdrequestcontroller_test.go b/cns/singletenantcontroller/kubecontroller/crdrequestcontroller_test.go index e698bb5dcd..affd2dba6d 100644 --- a/cns/singletenantcontroller/kubecontroller/crdrequestcontroller_test.go +++ b/cns/singletenantcontroller/kubecontroller/crdrequestcontroller_test.go @@ -93,7 +93,7 @@ func (mc MockKubeClient) Update(ctx context.Context, obj runtime.Object, opts .. type MockCNSClient struct { MockCNSUpdated bool MockCNSInitialized bool - Pods map[string]cns.KubernetesPodInfo + Pods map[string]cns.PodInfo NCRequest *cns.CreateNetworkContainerRequest } @@ -115,7 +115,7 @@ func (mi *MockCNSClient) GetNC(nc cns.GetNetworkContainerRequest) (cns.GetNetwor return cns.GetNetworkContainerResponse{NetworkContainerID: nc.NetworkContainerid}, nil } -func (mi *MockCNSClient) ReconcileNCState(ncRequest *cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.KubernetesPodInfo, scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) error { +func (mi *MockCNSClient) ReconcileNCState(ncRequest *cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo, scalar nnc.Scaler, spec nnc.NodeNetworkConfigSpec) error { mi.MockCNSInitialized = true mi.Pods = podInfoByIP mi.NCRequest = ncRequest @@ -178,7 +178,7 @@ func (mc *MockDirectAPIClient) ListPods(ctx context.Context, namespace, node str } func TestNewCrdRequestController(t *testing.T) { //Test making request controller without logger initialized, should fail - _, err := New(nil, nil) + _, err := New(Config{}) if err == nil { t.Fatalf("Expected error when making NewCrdRequestController without initializing logger, got nil error") } else if !strings.Contains(err.Error(), "logger") { @@ -198,7 +198,7 @@ func TestNewCrdRequestController(t *testing.T) { } }() - _, err = New(nil, nil) + _, err = New(Config{}) if err == nil { t.Fatalf("Expected error when making NewCrdRequestController without setting " + nodeNameEnvVar + " env var, got nil error") } else if !strings.Contains(err.Error(), nodeNameEnvVar) { @@ -660,12 +660,15 @@ func TestInitRequestController(t *testing.T) { } mockCNSClient := &MockCNSClient{} rc := &requestController{ + cfg: Config{}, directAPIClient: mockAPIDirectClient, directCRDClient: mockCRDDirectClient, CNSClient: mockCNSClient, nodeName: existingNNCName, } + logger.InitLogger("Azure CNS RequestController", 0, 0, "") + if err := rc.initCNS(context.Background()); err != nil { t.Fatalf("Expected no failure to init cns when given mock clients") } diff --git a/cns/singletenantcontroller/kubecontroller/crdtranslator.go b/cns/singletenantcontroller/kubecontroller/crdtranslator.go index 04f35fbc3f..f0aceb1dc0 100644 --- a/cns/singletenantcontroller/kubecontroller/crdtranslator.go +++ b/cns/singletenantcontroller/kubecontroller/crdtranslator.go @@ -10,8 +10,8 @@ import ( nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" ) -// CRDStatusToNCRequest translates a crd status to createnetworkcontainer request -func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetworkContainerRequest, error) { +// crdStatusToNCRequest translates a crd status to createnetworkcontainer request +func crdStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetworkContainerRequest, error) { var ( ncRequest cns.CreateNetworkContainerRequest nc nnc.NetworkContainer diff --git a/cns/singletenantcontroller/kubecontroller/crdtranslator_test.go b/cns/singletenantcontroller/kubecontroller/crdtranslator_test.go index 7fcb6fbffc..5fb7711b6f 100644 --- a/cns/singletenantcontroller/kubecontroller/crdtranslator_test.go +++ b/cns/singletenantcontroller/kubecontroller/crdtranslator_test.go @@ -43,7 +43,7 @@ func TestStatusToNCRequestMalformedPrimaryIP(t *testing.T) { } // Test with malformed primary ip - _, err = CRDStatusToNCRequest(status) + _, err = crdStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with malformed ip to fail.") @@ -73,7 +73,7 @@ func TestStatusToNCRequestMalformedIPAssignment(t *testing.T) { } // Test with malformed ip assignment - _, err = CRDStatusToNCRequest(status) + _, err = crdStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with malformed ip assignment to fail.") @@ -103,7 +103,7 @@ func TestStatusToNCRequestPrimaryIPInCIDR(t *testing.T) { } // Test with primary ip not in CIDR form - _, err = CRDStatusToNCRequest(status) + _, err = crdStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with primary ip not CIDR, to fail.") @@ -133,7 +133,7 @@ func TestStatusToNCRequestIPAssignmentNotCIDR(t *testing.T) { } // Test with ip assignment not in CIDR form - _, err = CRDStatusToNCRequest(status) + _, err = crdStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with ip assignment not CIDR, to fail.") @@ -163,7 +163,7 @@ func TestStatusToNCRequestWithIncorrectSubnetAddressSpace(t *testing.T) { } // Test with ip assignment not in CIDR form - _, err = CRDStatusToNCRequest(status) + _, err = crdStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with ip assignment not CIDR, to fail.") @@ -200,7 +200,7 @@ func TestStatusToNCRequestSuccess(t *testing.T) { } // Test with ips formed correctly as CIDRs - ncRequest, err = CRDStatusToNCRequest(status) + ncRequest, err = crdStatusToNCRequest(status) if err != nil { t.Fatalf("Expected translation of CRD status to succeed, got error :%v", err) From d92604681fe066e9fd3bc95ed45b9e89fe359c24 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Mon, 21 Jun 2021 08:40:00 -0700 Subject: [PATCH 2/6] address review comments --- cni/network/invoker_cns.go | 19 ++++---- cni/network/multitenancy.go | 5 ++- cni/network/network.go | 5 ++- cns/NetworkContainerContract.go | 41 ++++++++++------- cns/NetworkContainerContract_test.go | 44 +++++++++++++++++++ .../{initialize.go => podinfoprovider.go} | 0 cns/restserver/util.go | 7 ++- .../kubecontroller/crdreconciler.go | 2 +- .../kubecontroller/crdrequestcontroller.go | 2 +- .../kubecontroller/crdtranslator.go | 4 +- .../kubecontroller/crdtranslator_test.go | 12 ++--- 11 files changed, 101 insertions(+), 40 deletions(-) create mode 100644 cns/NetworkContainerContract_test.go rename cns/cnireconciler/{initialize.go => podinfoprovider.go} (100%) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 66568e426f..c95fbe7a58 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -50,11 +50,11 @@ func NewCNSInvoker(podName, namespace string) (*CNSIPAMInvoker, error) { //Add uses the requestipconfig API in cns, and returns ipv4 and a nil ipv6 as CNS doesn't support IPv6 yet func (invoker *CNSIPAMInvoker) Add(nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, hostSubnetPrefix *net.IPNet, options map[string]interface{}) (*cniTypesCurr.Result, *cniTypesCurr.Result, error) { - - endpointId := GetEndpointID(args) - // Parse Pod arguments. - podInfo := cns.NewPodInfo(args.ContainerID, endpointId, invoker.podName, invoker.podNamespace) + podInfo := cns.KubernetesPodInfo{ + PodName: invoker.podName, + PodNamespace: invoker.podNamespace, + } orchestratorContext, err := json.Marshal(podInfo) if err != nil { return nil, nil, err @@ -62,7 +62,7 @@ func (invoker *CNSIPAMInvoker) Add(nwCfg *cni.NetworkConfig, args *cniSkel.CmdAr ipconfig := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, - PodInterfaceID: endpointId, + PodInterfaceID: GetEndpointID(args), InfraContainerID: args.ContainerID, } @@ -178,10 +178,11 @@ func setHostOptions(nwCfg *cni.NetworkConfig, hostSubnetPrefix *net.IPNet, ncSub // Delete calls into the releaseipconfiguration API in CNS func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, options map[string]interface{}) error { - // Parse Pod arguments. - endpointId := GetEndpointID(args) - podInfo := cns.NewPodInfo(args.ContainerID, endpointId, invoker.podName, invoker.podNamespace) + podInfo := cns.KubernetesPodInfo{ + PodName: invoker.podName, + PodNamespace: invoker.podNamespace, + } orchestratorContext, err := json.Marshal(podInfo) if err != nil { @@ -190,7 +191,7 @@ func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConf req := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, - PodInterfaceID: endpointId, + PodInterfaceID: GetEndpointID(args), InfraContainerID: args.ContainerID, } diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index db4c9bb47b..5561633e74 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -71,7 +71,10 @@ func getContainerNetworkConfigurationInternal(address string, namespace string, return nil, nil, net.IPNet{}, err } - podInfo := cns.NewPodInfo("", "", podName, namespace) + podInfo := cns.KubernetesPodInfo{ + PodName: podName, + PodNamespace: namespace, + } orchestratorContext, err := json.Marshal(podInfo) if err != nil { log.Printf("Marshalling KubernetesPodInfo failed with %v", err) diff --git a/cni/network/network.go b/cni/network/network.go index 0b18f3d663..241deafaac 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -1097,7 +1097,10 @@ func (plugin *netPlugin) Update(args *cniSkel.CmdArgs) error { } // create struct with info for target POD - podInfo := cns.NewPodInfo("", "", k8sPodName, k8sNamespace) + podInfo := cns.KubernetesPodInfo{ + PodName: k8sPodName, + PodNamespace: k8sNamespace, + } if orchestratorContext, err = json.Marshal(podInfo); err != nil { log.Printf("Marshalling KubernetesPodInfo failed with %v", err) return plugin.Errorf(err.Error()) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index dee0a4c532..e318181dc1 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -153,14 +153,23 @@ type PodInfo interface { String() string } +type KubernetesPodInfo struct { + PodName string + PodNamespace string +} + +type KubernetesPodInfo struct { + PodName string + PodNamespace string +} + var _ PodInfo = (*podInfo)(nil) // podInfo implements PodInfo for multiple schemas of Key type podInfo struct { + KubernetesPodInfo PodInfraContainerID string PodInterfaceID string - PodName string - PodNamespace string Version podInfoScheme } @@ -202,14 +211,27 @@ func (p *podInfo) String() string { // configuration for their namesake functions. func NewPodInfo(infraContainerID, interfaceID, name, namespace string) PodInfo { return &podInfo{ + KubernetesPodInfo: KubernetesPodInfo{ + PodName: name, + PodNamespace: namespace, + }, PodInfraContainerID: infraContainerID, PodInterfaceID: interfaceID, - PodName: name, - PodNamespace: namespace, Version: GlobalPodInfoScheme, } } +// UnmarshalPodInfo wraps json.Unmarshal to return an implementation of +// PodInfo. +func UnmarshalPodInfo(b []byte) (PodInfo, error) { + p := &podInfo{} + if err := json.Unmarshal(b, p); err != nil { + return nil, err + } + p.Version = GlobalPodInfoScheme + return p, nil +} + // NewPodInfoFromIPConfigRequest builds and returns an implementation of // PodInfo from the provided IPConfigRequest. func NewPodInfoFromIPConfigRequest(req IPConfigRequest) (PodInfo, error) { @@ -225,17 +247,6 @@ func NewPodInfoFromIPConfigRequest(req IPConfigRequest) (PodInfo, error) { return p, nil } -// UnmarshalPodInfo wraps json.Unmarshal to return an implementation of -// PodInfo. -func UnmarshalPodInfo(b []byte) (PodInfo, error) { - p := &podInfo{} - if err := json.Unmarshal(b, p); err != nil { - return nil, err - } - p.Version = GlobalPodInfoScheme - return p, nil -} - // MultiTenancyInfo contains encap type and id. type MultiTenancyInfo struct { EncapType string diff --git a/cns/NetworkContainerContract_test.go b/cns/NetworkContainerContract_test.go new file mode 100644 index 0000000000..7e96a7310b --- /dev/null +++ b/cns/NetworkContainerContract_test.go @@ -0,0 +1,44 @@ +package cns + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestUnmarshalPodInfo(t *testing.T) { + marshalledKubernetesPodInfo, _ := json.Marshal(KubernetesPodInfo{PodName: "pod", PodNamespace: "namespace"}) + tests := []struct { + name string + b []byte + want *podInfo + }{ + { + name: "orchestrator context", + b: []byte(`{"PodName":"pod","PodNamespace":"namespace"}`), + want: &podInfo{ + KubernetesPodInfo: KubernetesPodInfo{ + PodName: "pod", + PodNamespace: "namespace", + }, + }, + }, + { + name: "marshalled orchestrator context", + b: marshalledKubernetesPodInfo, + want: &podInfo{ + KubernetesPodInfo: KubernetesPodInfo{ + PodName: "pod", + PodNamespace: "namespace", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, _ := UnmarshalPodInfo(tt.b) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/cns/cnireconciler/initialize.go b/cns/cnireconciler/podinfoprovider.go similarity index 100% rename from cns/cnireconciler/initialize.go rename to cns/cnireconciler/podinfoprovider.go diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 7a757a7936..93b44af5b1 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -177,8 +177,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw service.state.ContainerIDByOrchestratorContext = make(map[string]string) } - service.state.ContainerIDByOrchestratorContext[podInfo.Key()] = req.NetworkContainerid - break + service.state.ContainerIDByOrchestratorContext[podInfo.Name()+podInfo.Namespace()] = req.NetworkContainerid case cns.KubernetesCRD: // Validate and Update the SecondaryIpConfig state @@ -358,7 +357,7 @@ func (service *HTTPRestService) getNetworkContainerResponse(req cns.GetNetworkCo logger.Printf("pod info %+v", podInfo) - containerID, exists = service.state.ContainerIDByOrchestratorContext[podInfo.Key()] + containerID, exists = service.state.ContainerIDByOrchestratorContext[podInfo.Name()+podInfo.Namespace()] if exists { // If the goal state is available with CNS, check if the NC is pending VFP programming @@ -677,7 +676,7 @@ func (service *HTTPRestService) validateIpConfigRequest(ipConfigRequest cns.IPCo return nil, EmptyOrchestratorContext, fmt.Sprintf("OrchastratorContext is not set in the req: %+v", ipConfigRequest) } - // retrieve podinfo from orchestrator context + // retrieve podinfo from orchestrator context podInfo, err := cns.NewPodInfoFromIPConfigRequest(ipConfigRequest) if err != nil { return podInfo, UnsupportedOrchestratorContext, err.Error() diff --git a/cns/singletenantcontroller/kubecontroller/crdreconciler.go b/cns/singletenantcontroller/kubecontroller/crdreconciler.go index 47b6f5ce3a..0b9318814e 100644 --- a/cns/singletenantcontroller/kubecontroller/crdreconciler.go +++ b/cns/singletenantcontroller/kubecontroller/crdreconciler.go @@ -60,7 +60,7 @@ func (r *CrdReconciler) Reconcile(request reconcile.Request) (reconcile.Result, len(networkContainer.IPAssignments)) // Otherwise, create NC request and hand it off to CNS - ncRequest, err = crdStatusToNCRequest(nodeNetConfig.Status) + ncRequest, err = CRDStatusToNCRequest(nodeNetConfig.Status) if err != nil { logger.Errorf("[cns-rc] Error translating crd status to nc request %v", err) //requeue diff --git a/cns/singletenantcontroller/kubecontroller/crdrequestcontroller.go b/cns/singletenantcontroller/kubecontroller/crdrequestcontroller.go index 5b4bc38cd0..e6c72a4893 100644 --- a/cns/singletenantcontroller/kubecontroller/crdrequestcontroller.go +++ b/cns/singletenantcontroller/kubecontroller/crdrequestcontroller.go @@ -239,7 +239,7 @@ func (rc *requestController) initCNS(ctx context.Context) error { } // Convert to CreateNetworkContainerRequest - ncRequest, err := crdStatusToNCRequest(nodeNetConfig.Status) + ncRequest, err := CRDStatusToNCRequest(nodeNetConfig.Status) if err != nil { logger.Errorf("Error when converting nodeNetConfig status into CreateNetworkContainerRequest: %v", err) return err diff --git a/cns/singletenantcontroller/kubecontroller/crdtranslator.go b/cns/singletenantcontroller/kubecontroller/crdtranslator.go index f0aceb1dc0..04f35fbc3f 100644 --- a/cns/singletenantcontroller/kubecontroller/crdtranslator.go +++ b/cns/singletenantcontroller/kubecontroller/crdtranslator.go @@ -10,8 +10,8 @@ import ( nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" ) -// crdStatusToNCRequest translates a crd status to createnetworkcontainer request -func crdStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetworkContainerRequest, error) { +// CRDStatusToNCRequest translates a crd status to createnetworkcontainer request +func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetworkContainerRequest, error) { var ( ncRequest cns.CreateNetworkContainerRequest nc nnc.NetworkContainer diff --git a/cns/singletenantcontroller/kubecontroller/crdtranslator_test.go b/cns/singletenantcontroller/kubecontroller/crdtranslator_test.go index 5fb7711b6f..7fcb6fbffc 100644 --- a/cns/singletenantcontroller/kubecontroller/crdtranslator_test.go +++ b/cns/singletenantcontroller/kubecontroller/crdtranslator_test.go @@ -43,7 +43,7 @@ func TestStatusToNCRequestMalformedPrimaryIP(t *testing.T) { } // Test with malformed primary ip - _, err = crdStatusToNCRequest(status) + _, err = CRDStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with malformed ip to fail.") @@ -73,7 +73,7 @@ func TestStatusToNCRequestMalformedIPAssignment(t *testing.T) { } // Test with malformed ip assignment - _, err = crdStatusToNCRequest(status) + _, err = CRDStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with malformed ip assignment to fail.") @@ -103,7 +103,7 @@ func TestStatusToNCRequestPrimaryIPInCIDR(t *testing.T) { } // Test with primary ip not in CIDR form - _, err = crdStatusToNCRequest(status) + _, err = CRDStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with primary ip not CIDR, to fail.") @@ -133,7 +133,7 @@ func TestStatusToNCRequestIPAssignmentNotCIDR(t *testing.T) { } // Test with ip assignment not in CIDR form - _, err = crdStatusToNCRequest(status) + _, err = CRDStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with ip assignment not CIDR, to fail.") @@ -163,7 +163,7 @@ func TestStatusToNCRequestWithIncorrectSubnetAddressSpace(t *testing.T) { } // Test with ip assignment not in CIDR form - _, err = crdStatusToNCRequest(status) + _, err = CRDStatusToNCRequest(status) if err == nil { t.Fatalf("Expected translation of CRD status with ip assignment not CIDR, to fail.") @@ -200,7 +200,7 @@ func TestStatusToNCRequestSuccess(t *testing.T) { } // Test with ips formed correctly as CIDRs - ncRequest, err = crdStatusToNCRequest(status) + ncRequest, err = CRDStatusToNCRequest(status) if err != nil { t.Fatalf("Expected translation of CRD status to succeed, got error :%v", err) From 57dd73a039c1bbe044e6462ef46777025dfa1014 Mon Sep 17 00:00:00 2001 From: neaggarw Date: Wed, 23 Jun 2021 18:12:05 -0700 Subject: [PATCH 3/6] Rename the PodIp map --- cns/NetworkContainerContract.go | 19 +++++----- cns/api.go | 14 ++++---- cns/cnsclient/cli.go | 2 +- cns/cnsclient/cnsclient_test.go | 4 +-- cns/restserver/internalapi.go | 7 ++-- cns/restserver/internalapi_test.go | 22 +++++------- cns/restserver/ipam.go | 56 +++++++++++++---------------- cns/restserver/ipam_test.go | 58 +++++++++++++----------------- cns/restserver/restserver.go | 52 +++++++++++++-------------- cns/restserver/util.go | 16 ++++----- 10 files changed, 113 insertions(+), 137 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index e318181dc1..b501c84339 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -149,13 +149,8 @@ type PodInfo interface { Name() string // Namespace is the orchestrator pod namespace. Namespace() string - // String is a string rep of PodInfo. - String() string -} - -type KubernetesPodInfo struct { - PodName string - PodNamespace string + // OrchestratorContext is a JSON KubernetesPodInfo + OrchestratorContext() (json.RawMessage, error) } type KubernetesPodInfo struct { @@ -201,10 +196,12 @@ func (p *podInfo) Namespace() string { return p.PodNamespace } -// String is a string rep of PodInfo. -// String calls Key(). -func (p *podInfo) String() string { - return p.Key() +func (p *podInfo) OrchestratorContext() (json.RawMessage, error) { + jsonContext, err := json.Marshal(p.KubernetesPodInfo) + if err != nil { + return nil, fmt.Errorf("failed to marshal PodInfo, error: %w", err) + } + return jsonContext, nil } // NewPodInfo returns an implementation of PodInfo that returns the passed diff --git a/cns/api.go b/cns/api.go index 1e23388600..355784d502 100644 --- a/cns/api.go +++ b/cns/api.go @@ -50,16 +50,16 @@ type HTTPService interface { // This is used for KubernetesCRD orchestrator Type where NC has multiple ips. // This struct captures the state for SecondaryIPs associated to a given NC type IPConfigurationStatus struct { - NCID string - ID string //uuid - IPAddress string - State string - OrchestratorContext json.RawMessage + NCID string + ID string //uuid + IPAddress string + State string + PodInfo PodInfo } func (i IPConfigurationStatus) String() string { - return fmt.Sprintf("IPConfigurationStatus: Id: [%s], NcId: [%s], IpAddress: [%s], State: [%s], OrchestratorContext: [%s]", - i.ID, i.NCID, i.IPAddress, i.State, string(i.OrchestratorContext)) + return fmt.Sprintf("IPConfigurationStatus: Id: [%s], NcId: [%s], IpAddress: [%s], State: [%s], PodInfo: [%v]", + i.ID, i.NCID, i.IPAddress, i.State, i.PodInfo) } // SetEnvironmentRequest describes the Request to set the environment in CNS. diff --git a/cns/cnsclient/cli.go b/cns/cnsclient/cli.go index 3d91e0ef3f..a5aa5b6d2b 100644 --- a/cns/cnsclient/cli.go +++ b/cns/cnsclient/cli.go @@ -138,7 +138,7 @@ func getInMemory(client *CNSClient) error { } func printInMemoryStruct(data restserver.HttpRestServiceData) { - fmt.Println("PodIPIDByOrchestratorContext: ", data.PodIPIDByOrchestratorContext) + fmt.Println("PodIPIDByOrchestratorContext: ", data.PodIPIDByPodInterfaceKey) fmt.Println("PodIPConfigState: ", data.PodIPConfigState) fmt.Println("IPAMPoolMonitor: ", data.IPAMPoolMonitor) } diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index 51fda866f6..d1a2cd6e89 100644 --- a/cns/cnsclient/cnsclient_test.go +++ b/cns/cnsclient/cnsclient_test.go @@ -357,7 +357,7 @@ func TestCNSClientDebugAPI(t *testing.T) { t.Errorf("Get in-memory http REST Struct failed %+v", err) } - if len(inmemory.HttpRestServiceData.PodIPIDByOrchestratorContext) < 1 { + if len(inmemory.HttpRestServiceData.PodIPIDByPodInterfaceKey) < 1 { t.Errorf("OrchestratorContext map is expected but not returned") } @@ -392,7 +392,7 @@ func TestCNSClientDebugAPI(t *testing.T) { } t.Logf("In-memory Data: ") - t.Logf("PodIPIDByOrchestratorContext: %+v", inmemory.HttpRestServiceData.PodIPIDByOrchestratorContext) + t.Logf("PodIPIDByOrchestratorContext: %+v", inmemory.HttpRestServiceData.PodIPIDByPodInterfaceKey) t.Logf("PodIPConfigState: %+v", inmemory.HttpRestServiceData.PodIPConfigState) t.Logf("IPAMPoolMonitor: %+v", inmemory.HttpRestServiceData.IPAMPoolMonitor) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index bf80fff407..8cc6e0ca97 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -230,14 +230,17 @@ func (service *HTTPRestService) ReconcileNCState(ncRequest *cns.CreateNetworkCon if podInfo, exists := podInfoByIp[secIpConfig.IPAddress]; exists { logger.Printf("SecondaryIP %+v is allocated to Pod. %+v, ncId: %s", secIpConfig, podInfo, ncRequest.NetworkContainerid) - jsonContext, err := json.Marshal(podInfo) + jsonContext, err := podInfo.OrchestratorContext() if err != nil { - logger.Errorf("Failed to marshal PodInfo, error: %v", err) + logger.Errorf("Failed to marshal KubernetesPodInfo, error: %v", err) return UnexpectedError } + ipconfigRequest := cns.IPConfigRequest{ DesiredIPAddress: secIpConfig.IPAddress, OrchestratorContext: jsonContext, + PodInterfaceID: podInfo.InterfaceID(), + InfraContainerID: podInfo.InfraContainerID(), } if _, err := requestIPConfigHelper(service, ipconfigRequest); err != nil { diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 9b5adbdfa6..edb65d578d 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -406,18 +406,13 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) } // Validate IP state - if ipStatus.OrchestratorContext != nil { - podInfo, err := cns.UnmarshalPodInfo(ipStatus.OrchestratorContext) - if err != nil { - t.Fatalf("Failed to add IPConfig to state: %+v with error: %v", ipStatus, err) - } - - if _, exists := svc.PodIPIDByOrchestratorContext[podInfo.Key()]; exists { + if ipStatus.PodInfo != nil { + if _, exists := svc.PodIPIDByPodInterfaceKey[ipStatus.PodInfo.Key()]; exists { if ipStatus.State != cns.Allocated { t.Fatalf("IPId: %s State is not Allocated, ipStatus: %+v", ipid, ipStatus) } } else { - t.Fatalf("Failed to find podContext for allocated ip: %+v, podinfo :%+v", ipStatus, podInfo) + t.Fatalf("Failed to find podContext for allocated ip: %+v, podinfo :%+v", ipStatus, ipStatus.PodInfo) } } else if ipStatus.State != expectedIPStatus { // Todo: Validate for pendingRelease as well @@ -474,12 +469,12 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon validateNetworkRequest(t, *ncRequest) } - if len(expectedAllocatedPods) != len(svc.PodIPIDByOrchestratorContext) { - t.Fatalf("Unexpected allocated pods, actual: %d, expected: %d", len(svc.PodIPIDByOrchestratorContext), len(expectedAllocatedPods)) + if len(expectedAllocatedPods) != len(svc.PodIPIDByPodInterfaceKey) { + t.Fatalf("Unexpected allocated pods, actual: %d, expected: %d", len(svc.PodIPIDByPodInterfaceKey), len(expectedAllocatedPods)) } for ipaddress, podInfo := range expectedAllocatedPods { - ipId := svc.PodIPIDByOrchestratorContext[podInfo.Key()] + ipId := svc.PodIPIDByPodInterfaceKey[podInfo.Key()] ipConfigstate := svc.PodIPConfigState[ipId] if ipConfigstate.State != cns.Allocated { @@ -492,9 +487,8 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon } // Valdate pod context - expectedPodInfo, _ := cns.UnmarshalPodInfo(ipConfigstate.OrchestratorContext) - if reflect.DeepEqual(expectedPodInfo, podInfo) != true { - t.Fatalf("OrchestrationContext: is not same, expected: %+v, actual %+v", expectedPodInfo, podInfo) + if reflect.DeepEqual(ipConfigstate.PodInfo, podInfo) != true { + t.Fatalf("OrchestrationContext: is not same, expected: %+v, actual %+v", ipConfigstate.PodInfo, podInfo) } // Validate this IP belongs to a valid NCRequest diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 03346130b2..e29eec1bbc 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -4,8 +4,6 @@ package restserver import ( - "bytes" - "encoding/json" "fmt" "net/http" "strconv" @@ -93,7 +91,7 @@ func (service *HTTPRestService) MarkIPAsPendingRelease(totalIpsToRelease int) (m for uuid, existingIpConfig := range service.PodIPConfigState { if existingIpConfig.State == cns.PendingProgramming { - updatedIpConfig, err := service.updateIPConfigState(uuid, cns.PendingRelease, existingIpConfig.OrchestratorContext) + updatedIpConfig, err := service.updateIPConfigState(uuid, cns.PendingRelease, existingIpConfig.PodInfo) if err != nil { return nil, err } @@ -108,7 +106,7 @@ func (service *HTTPRestService) MarkIPAsPendingRelease(totalIpsToRelease int) (m // if not all expected IPs are set to PendingRelease, then check the Available IPs for uuid, existingIpConfig := range service.PodIPConfigState { if existingIpConfig.State == cns.Available { - updatedIpConfig, err := service.updateIPConfigState(uuid, cns.PendingRelease, existingIpConfig.OrchestratorContext) + updatedIpConfig, err := service.updateIPConfigState(uuid, cns.PendingRelease, existingIpConfig.PodInfo) if err != nil { return nil, err } @@ -125,11 +123,11 @@ func (service *HTTPRestService) MarkIPAsPendingRelease(totalIpsToRelease int) (m return pendingReleasedIps, nil } -func (service *HTTPRestService) updateIPConfigState(ipId string, updatedState string, orchestratorContext json.RawMessage) (cns.IPConfigurationStatus, error) { +func (service *HTTPRestService) updateIPConfigState(ipId string, updatedState string, podInfo cns.PodInfo) (cns.IPConfigurationStatus, error) { if ipConfig, found := service.PodIPConfigState[ipId]; found { - logger.Printf("[updateIPConfigState] Changing IpId [%s] state to [%s], orchestratorContext [%s]. Current config [%+v]", ipId, updatedState, string(orchestratorContext), ipConfig) + logger.Printf("[updateIPConfigState] Changing IpId [%s] state to [%s], podInfo [%+v]. Current config [%+v]", ipId, updatedState, podInfo, ipConfig) ipConfig.State = updatedState - ipConfig.OrchestratorContext = orchestratorContext + ipConfig.PodInfo = podInfo service.PodIPConfigState[ipId] = ipConfig return ipConfig, nil } @@ -205,7 +203,7 @@ func (service *HTTPRestService) getPodIPIDByOrchestratorContexthandler(w http.Re func (service *HTTPRestService) GetPodIPIDByOrchestratorContext() map[string]string { service.RLock() defer service.RUnlock() - return service.PodIPIDByOrchestratorContext + return service.PodIPIDByPodInterfaceKey } func (service *HTTPRestService) GetHTTPRestDataHandler(w http.ResponseWriter, r *http.Request) { @@ -234,9 +232,9 @@ func (service *HTTPRestService) GetHTTPStruct() HttpRestServiceData { defer service.RUnlock() return HttpRestServiceData{ - PodIPIDByOrchestratorContext: service.PodIPIDByOrchestratorContext, - PodIPConfigState: service.PodIPConfigState, - IPAMPoolMonitor: service.IPAMPoolMonitor.GetStateSnapshot(), + PodIPIDByPodInterfaceKey: service.PodIPIDByPodInterfaceKey, + PodIPConfigState: service.PodIPConfigState, + IPAMPoolMonitor: service.IPAMPoolMonitor.GetStateSnapshot(), } } @@ -350,13 +348,13 @@ func filterIPConfigMap(toBeAdded map[string]cns.IPConfigurationStatus, f func(cn } //SetIPConfigAsAllocated takes a lock of the service, and sets the ipconfig in the CNS state as allocated, does not take a lock -func (service *HTTPRestService) setIPConfigAsAllocated(ipconfig cns.IPConfigurationStatus, podInfo cns.PodInfo, marshalledOrchestratorContext json.RawMessage) (cns.IPConfigurationStatus, error) { - ipconfig, err := service.updateIPConfigState(ipconfig.ID, cns.Allocated, marshalledOrchestratorContext) +func (service *HTTPRestService) setIPConfigAsAllocated(ipconfig cns.IPConfigurationStatus, podInfo cns.PodInfo) (cns.IPConfigurationStatus, error) { + ipconfig, err := service.updateIPConfigState(ipconfig.ID, cns.Allocated, podInfo) if err != nil { return cns.IPConfigurationStatus{}, err } - service.PodIPIDByOrchestratorContext[podInfo.Key()] = ipconfig.ID + service.PodIPIDByPodInterfaceKey[podInfo.Key()] = ipconfig.ID return ipconfig, nil } @@ -367,7 +365,7 @@ func (service *HTTPRestService) setIPConfigAsAvailable(ipconfig cns.IPConfigurat return cns.IPConfigurationStatus{}, err } - delete(service.PodIPIDByOrchestratorContext, podInfo.Key()) + delete(service.PodIPIDByPodInterfaceKey, podInfo.Key()) logger.Printf("[setIPConfigAsAvailable] Deleted outdated pod info %s from PodIPIDByOrchestratorContext since IP %s with ID %s will be released and set as Available", podInfo.Key(), ipconfig.IPAddress, ipconfig.ID) return ipconfig, nil @@ -380,7 +378,7 @@ func (service *HTTPRestService) releaseIPConfig(podInfo cns.PodInfo) error { service.Lock() defer service.Unlock() - ipID := service.PodIPIDByOrchestratorContext[podInfo.Key()] + ipID := service.PodIPIDByPodInterfaceKey[podInfo.Key()] if ipID != "" { if ipconfig, isExist := service.PodIPConfigState[ipID]; isExist { logger.Printf("[releaseIPConfig] Releasing IP %+v for pod %+v", ipconfig.IPAddress, podInfo) @@ -432,7 +430,7 @@ func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.PodInfo) (cns.Po service.RLock() defer service.RUnlock() - ipID := service.PodIPIDByOrchestratorContext[podInfo.Key()] + ipID := service.PodIPIDByPodInterfaceKey[podInfo.Key()] if ipID != "" { if ipState, isExist := service.PodIPConfigState[ipID]; isExist { err := service.populateIpConfigInfoUntransacted(ipState, &podIpInfo) @@ -446,31 +444,27 @@ func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.PodInfo) (cns.Po return podIpInfo, isExist, nil } -func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.PodInfo, desiredIPAddress string, orchestratorContext json.RawMessage) (cns.PodIpInfo, error) { +func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.PodInfo, desiredIpAddress string) (cns.PodIpInfo, error) { var podIpInfo cns.PodIpInfo service.Lock() defer service.Unlock() found := false for _, ipConfig := range service.PodIPConfigState { - if ipConfig.IPAddress == desiredIPAddress { + if ipConfig.IPAddress == desiredIpAddress { if ipConfig.State == cns.Allocated { // This IP has already been allocated, if it is allocated to same pod, then return the same // IPconfiguration - if bytes.Equal(orchestratorContext, ipConfig.OrchestratorContext) == true { + if ipConfig.PodInfo.Key() == podInfo.Key() { logger.Printf("[AllocateDesiredIPConfig]: IP Config [%+v] is already allocated to this Pod [%+v]", ipConfig, podInfo) found = true } else { - pInfo, err := cns.UnmarshalPodInfo(ipConfig.OrchestratorContext) - if err != nil { - return podIpInfo, fmt.Errorf("[AllocateDesiredIPConfig] Failed to unmarshal IPState [%+v] OrchestratorContext, err: %v", ipConfig, err) - } - return podIpInfo, fmt.Errorf("[AllocateDesiredIPConfig] Desired IP is already allocated %+v to Pod: %+v, requested for pod %+v", ipConfig, pInfo, podInfo) + return podIpInfo, fmt.Errorf("[AllocateDesiredIPConfig] Desired IP is already allocated %+v, requested for pod %+v", ipConfig, podInfo) } } else if ipConfig.State == cns.Available || ipConfig.State == cns.PendingProgramming { // This race can happen during restart, where CNS state is lost and thus we have lost the NC programmed version // As part of reconcile, we mark IPs as Allocated which are already allocated to PODs (listed from APIServer) - _, err := service.setIPConfigAsAllocated(ipConfig, podInfo, orchestratorContext) + _, err := service.setIPConfigAsAllocated(ipConfig, podInfo) if err != nil { return podIpInfo, err } @@ -488,7 +482,7 @@ func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.PodInfo, des return podIpInfo, fmt.Errorf("Requested IP not found in pool") } -func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.PodInfo, orchestratorContext json.RawMessage) (cns.PodIpInfo, error) { +func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.PodInfo) (cns.PodIpInfo, error) { var podIpInfo cns.PodIpInfo service.Lock() @@ -496,7 +490,7 @@ func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.PodInfo for _, ipState := range service.PodIPConfigState { if ipState.State == cns.Available { - _, err := service.setIPConfigAsAllocated(ipState, podInfo, orchestratorContext) + _, err := service.setIPConfigAsAllocated(ipState, podInfo) if err != nil { return podIpInfo, err } @@ -522,7 +516,7 @@ func requestIPConfigHelper(service *HTTPRestService, req cns.IPConfigRequest) (c // check if ipconfig already allocated for this pod and return if exists or error // if error, ipstate is nil, if exists, ipstate is not nil and error is nil - podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) + podInfo, err := cns.NewPodInfoFromIPConfigRequest(req) if err != nil { return podIpInfo, err } @@ -533,9 +527,9 @@ func requestIPConfigHelper(service *HTTPRestService, req cns.IPConfigRequest) (c // return desired IPConfig if req.DesiredIPAddress != "" { - return service.AllocateDesiredIPConfig(podInfo, req.DesiredIPAddress, req.OrchestratorContext) + return service.AllocateDesiredIPConfig(podInfo, req.DesiredIPAddress) } // return any free IPConfig - return service.AllocateAnyAvailableIPConfig(podInfo, req.OrchestratorContext) + return service.AllocateAnyAvailableIPConfig(podInfo) } diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index a93d361ad7..d6f887dbf9 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -4,8 +4,6 @@ package restserver import ( - "encoding/json" - "fmt" "reflect" "strconv" "testing" @@ -109,22 +107,21 @@ func requestIpAddressAndGetState(t *testing.T, req cns.IPConfigRequest) (cns.IPC return ipState, err } - ipId := svc.PodIPIDByOrchestratorContext[podInfo.Key()] + ipId := svc.PodIPIDByPodInterfaceKey[podInfo.Key()] ipState = svc.PodIPConfigState[ipId] return ipState, err } -func NewPodStateWithOrchestratorContext(ipaddress, id, ncid, state string, prefixLength uint8, ncVersion int, orchestratorContext cns.PodInfo) (cns.IPConfigurationStatus, error) { +func NewPodStateWithOrchestratorContext(ipaddress, id, ncid, state string, prefixLength uint8, ncVersion int, podInfo cns.PodInfo) (cns.IPConfigurationStatus, error) { ipconfig := newSecondaryIPConfig(ipaddress, ncVersion) - b, err := json.Marshal(orchestratorContext) return cns.IPConfigurationStatus{ - IPAddress: ipconfig.IPAddress, - ID: id, - NCID: ncid, - State: state, - OrchestratorContext: b, - }, err + IPAddress: ipconfig.IPAddress, + ID: id, + NCID: ncid, + State: state, + PodInfo: podInfo, + }, nil } // Test function to populate the IPConfigState @@ -146,12 +143,7 @@ func UpdatePodIpConfigState(t *testing.T, svc *HTTPRestService, ipconfigs map[st // update ipconfigs to expected state for ipId, ipconfig := range ipconfigs { if ipconfig.State == cns.Allocated { - podInfo, err := cns.UnmarshalPodInfo(ipconfig.OrchestratorContext) - if err != nil { - return fmt.Errorf("Failed to add IPConfig to state: %+v with error: %v", ipconfig, err) - } - - svc.PodIPIDByOrchestratorContext[podInfo.Key()] = ipId + svc.PodIPIDByPodInterfaceKey[ipconfig.PodInfo.Key()] = ipId svc.PodIPConfigState[ipId] = ipconfig } } @@ -169,7 +161,7 @@ func TestIPAMGetAvailableIPConfig(t *testing.T) { UpdatePodIpConfigState(t, svc, ipconfigs) req := cns.IPConfigRequest{} - b, _ := json.Marshal(testPod1Info) + b, _ := testPod1Info.OrchestratorContext() req.OrchestratorContext = b actualstate, err := requestIpAddressAndGetState(t, req) @@ -178,7 +170,7 @@ func TestIPAMGetAvailableIPConfig(t *testing.T) { } desiredState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, 0) - desiredState.OrchestratorContext = b + desiredState.PodInfo = testPod1Info if reflect.DeepEqual(desiredState, actualstate) != true { t.Fatalf("Desired state not matching actual state, expected: %+v, actual: %+v", desiredState, actualstate) @@ -190,7 +182,7 @@ func TestIPAMGetNextAvailableIPConfig(t *testing.T) { svc := getTestService() // Add already allocated pod ip to state - svc.PodIPIDByOrchestratorContext[testPod1Info.Key()] = testPod1GUID + svc.PodIPIDByPodInterfaceKey[testPod1Info.Key()] = testPod1GUID state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available, 0) @@ -204,7 +196,7 @@ func TestIPAMGetNextAvailableIPConfig(t *testing.T) { } req := cns.IPConfigRequest{} - b, _ := json.Marshal(testPod2Info) + b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b actualstate, err := requestIpAddressAndGetState(t, req) @@ -233,7 +225,7 @@ func TestIPAMGetAlreadyAllocatedIPConfigForSamePod(t *testing.T) { } req := cns.IPConfigRequest{} - b, _ := json.Marshal(testPod1Info) + b, _ := testPod1Info.OrchestratorContext() req.OrchestratorContext = b actualstate, err := requestIpAddressAndGetState(t, req) @@ -263,7 +255,7 @@ func TestIPAMAttemptToRequestIPNotFoundInPool(t *testing.T) { } req := cns.IPConfigRequest{} - b, _ := json.Marshal(testPod2Info) + b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = testIP2 @@ -288,7 +280,7 @@ func TestIPAMGetDesiredIPConfigWithSpecfiedIP(t *testing.T) { } req := cns.IPConfigRequest{} - b, _ := json.Marshal(testPod1Info) + b, _ := testPod1Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = testIP1 @@ -298,7 +290,7 @@ func TestIPAMGetDesiredIPConfigWithSpecfiedIP(t *testing.T) { } desiredState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, 0) - desiredState.OrchestratorContext = b + desiredState.PodInfo = testPod1Info if reflect.DeepEqual(desiredState, actualstate) != true { t.Fatalf("Desired state not matching actual state, expected: %+v, actual: %+v", desiredState, actualstate) @@ -320,7 +312,7 @@ func TestIPAMFailToGetDesiredIPConfigWithAlreadyAllocatedSpecfiedIP(t *testing.T // request the already allocated ip with a new context req := cns.IPConfigRequest{} - b, _ := json.Marshal(testPod2Info) + b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = testIP1 @@ -348,7 +340,7 @@ func TestIPAMFailToGetIPWhenAllIPsAreAllocated(t *testing.T) { // request the already allocated ip with a new context req := cns.IPConfigRequest{} - b, _ := json.Marshal(testPod3Info) + b, _ := testPod3Info.OrchestratorContext() req.OrchestratorContext = b _, err = requestIpAddressAndGetState(t, req) @@ -379,7 +371,7 @@ func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { // Use TestPodInfo2 to request TestIP1, which has already been allocated req := cns.IPConfigRequest{} - b, _ := json.Marshal(testPod2Info) + b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = desiredIpAddress @@ -396,7 +388,7 @@ func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { // Rerequest req = cns.IPConfigRequest{} - b, _ = json.Marshal(testPod2Info) + b, _ = testPod2Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = desiredIpAddress @@ -408,7 +400,7 @@ func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { desiredState, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) // want first available Pod IP State desiredState.IPAddress = desiredIpAddress - desiredState.OrchestratorContext = b + desiredState.PodInfo = testPod2Info if reflect.DeepEqual(desiredState, actualstate) != true { t.Fatalf("Desired state not matching actual state, expected: %+v, actual: %+v", state1, actualstate) @@ -488,7 +480,7 @@ func TestAvailableIPConfigs(t *testing.T) { validateIpState(t, allocatedIps, desiredAllocatedIpConfigs) req := cns.IPConfigRequest{} - b, _ := json.Marshal(testPod1Info) + b, _ := testPod1Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = state1.IPAddress @@ -502,7 +494,7 @@ func TestAvailableIPConfigs(t *testing.T) { validateIpState(t, availableIps, desiredAvailableIps) desiredState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, 0) - desiredState.OrchestratorContext = b + desiredState.PodInfo = testPod1Info desiredAllocatedIpConfigs[desiredState.ID] = desiredState allocatedIps = svc.GetAllocatedIPConfigs() validateIpState(t, allocatedIps, desiredAllocatedIpConfigs) @@ -666,7 +658,7 @@ func TestIPAMMarkExistingIPConfigAsPending(t *testing.T) { svc := getTestService() // Add already allocated pod ip to state - svc.PodIPIDByOrchestratorContext[testPod1Info.Key()] = testPod1GUID + svc.PodIPIDByPodInterfaceKey[testPod1Info.Key()] = testPod1GUID state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available, 0) diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 36ef96926a..9cd5714036 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -36,17 +36,17 @@ var ( // HTTPRestService represents http listener for CNS - Container Networking Service. type HTTPRestService struct { *cns.Service - dockerClient *dockerclient.DockerClient - imdsClient imdsclient.ImdsClientInterface - ipamClient *ipamclient.IpamClient - nmagentClient nmagentclient.NMAgentClientInterface - networkContainer *networkcontainers.NetworkContainers - PodIPIDByOrchestratorContext map[string]string // OrchestratorContext is key and value is Pod IP uuid. - PodIPConfigState map[string]cns.IPConfigurationStatus // seondaryipid(uuid) is key - IPAMPoolMonitor cns.IPAMPoolMonitor - routingTable *routes.RoutingTable - store store.KeyValueStore - state *httpRestServiceState + dockerClient *dockerclient.DockerClient + imdsClient imdsclient.ImdsClientInterface + ipamClient *ipamclient.IpamClient + nmagentClient nmagentclient.NMAgentClientInterface + networkContainer *networkcontainers.NetworkContainers + PodIPIDByPodInterfaceKey map[string]string // PodInterfaceId is key and value is Pod IP (SecondaryIP) uuid. + PodIPConfigState map[string]cns.IPConfigurationStatus // Secondary IP ID(uuid) is key + IPAMPoolMonitor cns.IPAMPoolMonitor + routingTable *routes.RoutingTable + store store.KeyValueStore + state *httpRestServiceState sync.RWMutex dncPartitionKey string } @@ -58,9 +58,9 @@ type GetHTTPServiceDataResponse struct { //struct to return in-memory httprest data in debug api type HttpRestServiceData struct { - PodIPIDByOrchestratorContext map[string]string // OrchestratorContext is key and value is Pod IP uuid. - PodIPConfigState map[string]cns.IPConfigurationStatus // secondaryipid(uuid) is key - IPAMPoolMonitor cns.IpamPoolMonitorStateSnapshot + PodIPIDByPodInterfaceKey map[string]string // PodInterfaceId is key and value is Pod IP uuid. + PodIPConfigState map[string]cns.IPConfigurationStatus // secondaryipid(uuid) is key + IPAMPoolMonitor cns.IpamPoolMonitorStateSnapshot } type Response struct { @@ -122,21 +122,21 @@ func NewHTTPRestService(config *common.ServiceConfig, imdsClientInterface imdscl serviceState.Networks = make(map[string]*networkInfo) serviceState.joinedNetworks = make(map[string]struct{}) - podIPIDByOrchestratorContext := make(map[string]string) + podIPIDByPodInterfaceKey := make(map[string]string) podIPConfigState := make(map[string]cns.IPConfigurationStatus) return &HTTPRestService{ - Service: service, - store: service.Service.Store, - dockerClient: dc, - imdsClient: imdsClient, - ipamClient: ic, - nmagentClient: nmagentClient, - networkContainer: nc, - PodIPIDByOrchestratorContext: podIPIDByOrchestratorContext, - PodIPConfigState: podIPConfigState, - routingTable: routingTable, - state: serviceState, + Service: service, + store: service.Service.Store, + dockerClient: dc, + imdsClient: imdsClient, + ipamClient: ic, + nmagentClient: nmagentClient, + networkContainer: nc, + PodIPIDByPodInterfaceKey: podIPIDByPodInterfaceKey, + PodIPConfigState: podIPConfigState, + routingTable: routingTable, + state: serviceState, }, nil } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 93b44af5b1..ef8f630d2e 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -224,11 +224,7 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN if exists { // pod ip exists, validate if state is not allocated, else fail if ipConfigStatus.State == cns.Allocated { - var expectedPodInfo cns.PodInfo - if len(ipConfigStatus.OrchestratorContext) != 0 { - expectedPodInfo, _ = cns.UnmarshalPodInfo(ipConfigStatus.OrchestratorContext) - } - errMsg := fmt.Sprintf("Failed to delete an Allocated IP %v, PodInfo %+v", ipConfigStatus, expectedPodInfo) + errMsg := fmt.Sprintf("Failed to delete an Allocated IP %v", ipConfigStatus) return InconsistentIPConfigState, errMsg } } @@ -280,11 +276,11 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVe } // add the new State ipconfigStatus := cns.IPConfigurationStatus{ - NCID: ncId, - ID: ipId, - IPAddress: ipconfig.IPAddress, - State: newIPCNSStatus, - OrchestratorContext: nil, + NCID: ncId, + ID: ipId, + IPAddress: ipconfig.IPAddress, + State: newIPCNSStatus, + PodInfo: nil, } logger.Printf("[Azure-Cns] Add IP %s as %s", ipconfig.IPAddress, newIPCNSStatus) From 2d0972ba5bde3dbfd9e9f1a46e66d7167d1cdf97 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Thu, 24 Jun 2021 11:15:14 -0700 Subject: [PATCH 4/6] address review comments --- cni/client/client_unit_test.go | 4 +- cns/NetworkContainerContract.go | 2 +- cns/NetworkContainerContract_test.go | 71 +++++++++++++++++++++-- cns/cnireconciler/podinfoprovider.go | 10 +++- cns/cnireconciler/podinfoprovider_test.go | 52 +++++++++++++++++ cns/cnireconciler/version.go | 10 +++- cns/cnireconciler/version_test.go | 52 +++++++++++++++++ cns/cnsclient/cnsclient_test.go | 2 +- cns/restserver/api_test.go | 8 +-- cns/restserver/internalapi_test.go | 39 ++++++++++++- cns/restserver/ipam.go | 4 +- cns/restserver/ipam_test.go | 51 ++++++++++++---- cns/restserver/util.go | 2 +- cns/service/main.go | 51 ++++++++-------- 14 files changed, 298 insertions(+), 60 deletions(-) create mode 100644 cns/cnireconciler/podinfoprovider_test.go create mode 100644 cns/cnireconciler/version_test.go diff --git a/cni/client/client_unit_test.go b/cni/client/client_unit_test.go index 8f506fc1ca..25f852bea6 100644 --- a/cni/client/client_unit_test.go +++ b/cni/client/client_unit_test.go @@ -18,7 +18,7 @@ func TestGetState(t *testing.T) { fakeexec, _ := testutils.GetFakeExecWithScripts(calls) - c := &AzureCNIClient{exec: fakeexec} + c := New(fakeexec) state, err := c.GetEndpointState() require.NoError(t, err) @@ -39,7 +39,7 @@ func TestGetVersion(t *testing.T) { fakeexec, _ := testutils.GetFakeExecWithScripts(calls) - c := &AzureCNIClient{exec: fakeexec} + c := New(fakeexec) version, err := c.GetVersion() require.NoError(t, err) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index b501c84339..47b177113b 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -236,7 +236,7 @@ func NewPodInfoFromIPConfigRequest(req IPConfigRequest) (PodInfo, error) { if err != nil { return nil, err } - if GlobalPodInfoScheme == InterfaceIDPodInfoScheme && req.InfraContainerID == "" { + if GlobalPodInfoScheme == InterfaceIDPodInfoScheme && req.PodInterfaceID == "" { return nil, fmt.Errorf("need interfaceID for pod info but request was empty") } p.(*podInfo).PodInfraContainerID = req.InfraContainerID diff --git a/cns/NetworkContainerContract_test.go b/cns/NetworkContainerContract_test.go index 7e96a7310b..f3ff13428f 100644 --- a/cns/NetworkContainerContract_test.go +++ b/cns/NetworkContainerContract_test.go @@ -10,9 +10,10 @@ import ( func TestUnmarshalPodInfo(t *testing.T) { marshalledKubernetesPodInfo, _ := json.Marshal(KubernetesPodInfo{PodName: "pod", PodNamespace: "namespace"}) tests := []struct { - name string - b []byte - want *podInfo + name string + b []byte + want *podInfo + wantErr bool }{ { name: "orchestrator context", @@ -34,10 +35,72 @@ func TestUnmarshalPodInfo(t *testing.T) { }, }, }, + { + name: "malformed", + b: []byte(`{{}`), + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := UnmarshalPodInfo(tt.b) + if tt.wantErr { + assert.Error(t, err) + return + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.want, got) + }) + } +} + +func TestNewPodInfoFromIPConfigRequest(t *testing.T) { + GlobalPodInfoScheme = InterfaceIDPodInfoScheme + defer func() { GlobalPodInfoScheme = KubernetesPodInfoScheme }() + tests := []struct { + name string + req IPConfigRequest + want PodInfo + wantErr bool + }{ + { + name: "full req", + req: IPConfigRequest{ + PodInterfaceID: "abcdef-eth0", + InfraContainerID: "abcdef", + OrchestratorContext: []byte(`{"PodName":"pod","PodNamespace":"namespace"}`), + }, + want: &podInfo{ + KubernetesPodInfo: KubernetesPodInfo{ + PodName: "pod", + PodNamespace: "namespace", + }, + PodInterfaceID: "abcdef-eth0", + PodInfraContainerID: "abcdef", + Version: InterfaceIDPodInfoScheme, + }, + }, + { + name: "empty interface id", + req: IPConfigRequest{ + InfraContainerID: "abcdef", + OrchestratorContext: []byte(`{"PodName":"pod","PodNamespace":"namespace"}`), + }, + want: &podInfo{}, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, _ := UnmarshalPodInfo(tt.b) + got, err := NewPodInfoFromIPConfigRequest(tt.req) + if tt.wantErr { + assert.Error(t, err) + return + } else { + assert.NoError(t, err) + } assert.Equal(t, tt.want, got) }) } diff --git a/cns/cnireconciler/podinfoprovider.go b/cns/cnireconciler/podinfoprovider.go index f4cf73f933..8e06f40273 100644 --- a/cns/cnireconciler/podinfoprovider.go +++ b/cns/cnireconciler/podinfoprovider.go @@ -1,6 +1,8 @@ package cnireconciler import ( + "fmt" + "github.com/Azure/azure-container-networking/cni/api" "github.com/Azure/azure-container-networking/cni/client" "github.com/Azure/azure-container-networking/cns" @@ -10,10 +12,14 @@ import ( // NewCNIPodInfoProvider returns an implementation of cns.PodInfoByIPProvider // that execs out to the CNI and uses the response to build the PodInfo map. func NewCNIPodInfoProvider() (cns.PodInfoByIPProvider, error) { - cli := client.New(exec.New()) + return newCNIPodInfoProvider(exec.New()) +} + +func newCNIPodInfoProvider(exec exec.Interface) (cns.PodInfoByIPProvider, error) { + cli := client.New(exec) state, err := cli.GetEndpointState() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to invoke CNI client.GetEndpointState(): %w", err) } return cns.PodInfoByIPProviderFunc(func() map[string]cns.PodInfo { return cniStateToPodInfoByIP(state) diff --git a/cns/cnireconciler/podinfoprovider_test.go b/cns/cnireconciler/podinfoprovider_test.go new file mode 100644 index 0000000000..f7bd57b3a4 --- /dev/null +++ b/cns/cnireconciler/podinfoprovider_test.go @@ -0,0 +1,52 @@ +package cnireconciler + +import ( + "testing" + + "github.com/Azure/azure-container-networking/cns" + testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/stretchr/testify/assert" + "k8s.io/utils/exec" +) + +func newCNIStateFakeExec(stdout string) exec.Interface { + calls := []testutils.TestCmd{ + {Cmd: []string{"./azure-vnet"}, Stdout: stdout}, + } + + fake, _ := testutils.GetFakeExecWithScripts(calls) + return fake +} + +func TestNewCNIPodInfoProvider(t *testing.T) { + tests := []struct { + name string + exec exec.Interface + want map[string]cns.PodInfo + wantErr bool + }{ + { + name: "good", + exec: newCNIStateFakeExec( + `{"ContainerInterfaces":{"3f813b02-eth0":{"PodName":"metrics-server-77c8679d7d-6ksdh","PodNamespace":"kube-system","PodEndpointID":"3f813b02-eth0","ContainerID":"3f813b029429b4e41a09ab33b6f6d365d2ed704017524c78d1d0dece33cdaf46","IPAddresses":[{"IP":"10.241.0.17","Mask":"//8AAA=="}]},"6e688597-eth0":{"PodName":"tunnelfront-5d96f9b987-65xbn","PodNamespace":"kube-system","PodEndpointID":"6e688597-eth0","ContainerID":"6e688597eafb97c83c84e402cc72b299bfb8aeb02021e4c99307a037352c0bed","IPAddresses":[{"IP":"10.241.0.13","Mask":"//8AAA=="}]}}}`, + ), + want: map[string]cns.PodInfo{ + "10.241.0.13": cns.NewPodInfo("6e688597eafb97c83c84e402cc72b299bfb8aeb02021e4c99307a037352c0bed", "6e688597-eth0", "tunnelfront-5d96f9b987-65xbn", "kube-system"), + "10.241.0.17": cns.NewPodInfo("3f813b029429b4e41a09ab33b6f6d365d2ed704017524c78d1d0dece33cdaf46", "3f813b02-eth0", "metrics-server-77c8679d7d-6ksdh", "kube-system"), + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := newCNIPodInfoProvider(tt.exec) + if tt.wantErr { + assert.Error(t, err) + return + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.want, got.PodInfoByIP()) + }) + } +} diff --git a/cns/cnireconciler/version.go b/cns/cnireconciler/version.go index c24c1a06e4..cbf090bb6b 100644 --- a/cns/cnireconciler/version.go +++ b/cns/cnireconciler/version.go @@ -1,6 +1,8 @@ package cnireconciler import ( + "fmt" + "github.com/Azure/azure-container-networking/cni/client" semver "github.com/hashicorp/go-version" "k8s.io/utils/exec" @@ -13,13 +15,17 @@ const cniDumpStateVer = "1.4.2" // state and returns the result of that test or an error. Will always // return false when there is an error. func IsDumpStateVer() (bool, error) { + return isDumpStateVer(exec.New()) +} + +func isDumpStateVer(exec exec.Interface) (bool, error) { needVer, err := semver.NewVersion(cniDumpStateVer) if err != nil { return false, err } - cnicli := client.New(exec.New()) + cnicli := client.New(exec) if ver, err := cnicli.GetVersion(); err != nil { - return false, err + return false, fmt.Errorf("failed to invoke CNI client.GetVersion(): %w", err) } else if ver.LessThan(needVer) { return false, nil } diff --git a/cns/cnireconciler/version_test.go b/cns/cnireconciler/version_test.go new file mode 100644 index 0000000000..be35cd1577 --- /dev/null +++ b/cns/cnireconciler/version_test.go @@ -0,0 +1,52 @@ +package cnireconciler + +import ( + "testing" + + testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/stretchr/testify/assert" + "k8s.io/utils/exec" +) + +func newCNIVersionFakeExec(ver string) exec.Interface { + calls := []testutils.TestCmd{ + {Cmd: []string{"./azure-vnet", "-v"}, Stdout: ver}, + } + + fake, _ := testutils.GetFakeExecWithScripts(calls) + return fake +} + +func TestIsDumpStateVer(t *testing.T) { + tests := []struct { + name string + exec exec.Interface + want bool + wantErr bool + }{ + { + name: "bad ver", + exec: newCNIVersionFakeExec(`Azure CNI Version v1.4.0-2-g984c5a5e-dirty`), + want: false, + wantErr: false, + }, + { + name: "good ver", + exec: newCNIVersionFakeExec(`Azure CNI Version v1.4.2`), + want: true, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := isDumpStateVer(tt.exec) + if tt.wantErr { + assert.Error(t, err) + return + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index d1a2cd6e89..00bdb457c2 100644 --- a/cns/cnsclient/cnsclient_test.go +++ b/cns/cnsclient/cnsclient_test.go @@ -225,7 +225,7 @@ func TestCNSClientRequestAndRelease(t *testing.T) { addTestStateToRestServer(t, secondaryIps) - podInfo := cns.NewPodInfo("", "", podName, podNamespace) + podInfo := cns.KubernetesPodInfo{PodName: podName, PodNamespace: podNamespace} orchestratorContext, err := json.Marshal(podInfo) if err != nil { t.Fatal(err) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index d9168fccb7..4e22f56df9 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -707,7 +707,7 @@ func createOrUpdateNetworkContainerWithParams(t *testing.T, params createOrUpdat ipSubnet.IPAddress = params.ncIP ipSubnet.PrefixLength = 24 ipConfig.IPSubnet = ipSubnet - podInfo := cns.NewPodInfo("", "", "testpod", "testpodnamespace") + podInfo := cns.KubernetesPodInfo{PodName: "testpod", PodNamespace: "testpodnamespace"} context, _ := json.Marshal(podInfo) info := &cns.CreateNetworkContainerRequest{ @@ -775,7 +775,7 @@ func deleteNetworkContainerWithParams(t *testing.T, params createOrUpdateNetwork func getNetworkContainerByContext(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetNetworkContainerResponse - podInfo := cns.NewPodInfo("", "", params.podName, params.podNamespace) + podInfo := cns.KubernetesPodInfo{PodName: params.podName, PodNamespace: params.podNamespace} podInfoBytes, err := json.Marshal(podInfo) getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} @@ -802,7 +802,7 @@ func getNetworkContainerByContext(t *testing.T, params createOrUpdateNetworkCont func getNonExistNetworkContainerByContext(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetNetworkContainerResponse - podInfo := cns.NewPodInfo("", "", params.podName, params.podNamespace) + podInfo := cns.KubernetesPodInfo{PodName: params.podName, PodNamespace: params.podNamespace} podInfoBytes, err := json.Marshal(podInfo) getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} @@ -829,7 +829,7 @@ func getNonExistNetworkContainerByContext(t *testing.T, params createOrUpdateNet func getNetworkContainerByContextExpectedError(t *testing.T, params createOrUpdateNetworkContainerParams) error { var body bytes.Buffer var resp cns.GetNetworkContainerResponse - podInfo := cns.NewPodInfo("", "", params.podName, params.podNamespace) + podInfo := cns.KubernetesPodInfo{PodName: params.podName, PodNamespace: params.podNamespace} podInfoBytes, err := json.Marshal(podInfo) getReq := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index edb65d578d..8fa48ed835 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -234,10 +234,43 @@ func TestReconcileNCWithExistingState(t *testing.T) { } req := generateNetworkContainerRequest(secondaryIPConfigs, "reconcileNc1", "-1") - expectedAllocatedPods := make(map[string]cns.PodInfo) - expectedAllocatedPods["10.0.0.6"] = cns.NewPodInfo("", "", "reconcilePod1", "PodNS1") + expectedAllocatedPods := map[string]cns.PodInfo{ + "10.0.0.6": cns.NewPodInfo("", "", "reconcilePod1", "PodNS1"), + "10.0.0.7": cns.NewPodInfo("", "", "reconcilePod2", "PodNS1"), + } + + expectedNcCount := len(svc.state.ContainerStatus) + returnCode := svc.ReconcileNCState(&req, expectedAllocatedPods, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) + if returnCode != Success { + t.Errorf("Unexpected failure on reconcile with no state %d", returnCode) + } + + validateNCStateAfterReconcile(t, &req, expectedNcCount+1, expectedAllocatedPods) +} + +func TestReconcileNCWithExistingStateFromInterfaceID(t *testing.T) { + restartService() + setEnv(t) + setOrchestratorTypeInternal(cns.KubernetesCRD) + cns.GlobalPodInfoScheme = cns.InterfaceIDPodInfoScheme + defer func() { cns.GlobalPodInfoScheme = cns.KubernetesPodInfoScheme }() + + secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) + + var startingIndex = 6 + for i := 0; i < 4; i++ { + ipaddress := "10.0.0." + strconv.Itoa(startingIndex) + secIpConfig := newSecondaryIPConfig(ipaddress, -1) + ipId := uuid.New() + secondaryIPConfigs[ipId.String()] = secIpConfig + startingIndex++ + } + req := generateNetworkContainerRequest(secondaryIPConfigs, "reconcileNc1", "-1") - expectedAllocatedPods["10.0.0.7"] = cns.NewPodInfo("", "", "reconcilePod2", "PodNS1") + expectedAllocatedPods := map[string]cns.PodInfo{ + "10.0.0.6": cns.NewPodInfo("reconc-eth0", "abcdef", "reconcilePod1", "PodNS1"), + "10.0.0.7": cns.NewPodInfo("reconc-eth0", "abcxyz", "reconcilePod2", "PodNS1"), + } expectedNcCount := len(svc.state.ContainerStatus) returnCode := svc.ReconcileNCState(&req, expectedAllocatedPods, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index e29eec1bbc..43df0ca0f6 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -75,7 +75,7 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r podInfo, resp.ReturnCode, resp.Message = service.validateIpConfigRequest(req) if err = service.releaseIPConfig(podInfo); err != nil { - resp.ReturnCode = NotFound + resp.ReturnCode = UnexpectedError resp.Message = err.Error() logger.Errorf("releaseIPConfigHandler releaseIPConfig failed because %v, release IP config info %s", resp.Message, req) return @@ -394,7 +394,7 @@ func (service *HTTPRestService) releaseIPConfig(podInfo cns.PodInfo) error { ipconfig.IPAddress, podInfo) } } else { - logger.Errorf("[releaseIPConfig] SetIPConfigAsAvailable failed to release, no allocation found for pod [%+v]", podInfo) + logger.Errorf("[releaseIPConfig] SetIPConfigAsAvailable ignoring request to release, no allocation found for pod [%+v]", podInfo) return nil } return nil diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index d6f887dbf9..68c57dbf8b 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -18,15 +18,15 @@ var ( testIP1 = "10.0.0.1" testPod1GUID = "898fb8f1-f93e-4c96-9c31-6b89098949a3" - testPod1Info = cns.NewPodInfo("", "", "testpod1", "testpod1namespace") + testPod1Info = cns.NewPodInfo("898fb8-eth0", testPod1GUID, "testpod1", "testpod1namespace") testIP2 = "10.0.0.2" testPod2GUID = "b21e1ee1-fb7e-4e6d-8c68-22ee5049944e" - testPod2Info = cns.NewPodInfo("", "", "testpod2", "testpod2namespace") + testPod2Info = cns.NewPodInfo("b21e1e-eth0", testPod2GUID, "testpod2", "testpod2namespace") testIP3 = "10.0.0.3" testPod3GUID = "718e04ac-5a13-4dce-84b3-040accaa9b41" - testPod3Info = cns.NewPodInfo("", "", "testpod3", "testpod3namespace") + testPod3Info = cns.NewPodInfo("718e04-eth0", testPod3GUID, "testpod3", "testpod3namespace") testIP4 = "10.0.0.4" testPod4GUID = "718e04ac-5a13-4dce-84b3-040accaa9b42" @@ -160,7 +160,10 @@ func TestIPAMGetAvailableIPConfig(t *testing.T) { } UpdatePodIpConfigState(t, svc, ipconfigs) - req := cns.IPConfigRequest{} + req := cns.IPConfigRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } b, _ := testPod1Info.OrchestratorContext() req.OrchestratorContext = b @@ -195,7 +198,10 @@ func TestIPAMGetNextAvailableIPConfig(t *testing.T) { t.Fatalf("Expected to not fail adding IP's to state: %+v", err) } - req := cns.IPConfigRequest{} + req := cns.IPConfigRequest{ + PodInterfaceID: testPod2Info.InterfaceID(), + InfraContainerID: testPod2Info.InfraContainerID(), + } b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b @@ -224,7 +230,10 @@ func TestIPAMGetAlreadyAllocatedIPConfigForSamePod(t *testing.T) { t.Fatalf("Expected to not fail adding IP's to state: %+v", err) } - req := cns.IPConfigRequest{} + req := cns.IPConfigRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } b, _ := testPod1Info.OrchestratorContext() req.OrchestratorContext = b @@ -254,7 +263,10 @@ func TestIPAMAttemptToRequestIPNotFoundInPool(t *testing.T) { t.Fatalf("Expected to not fail adding IP's to state: %+v", err) } - req := cns.IPConfigRequest{} + req := cns.IPConfigRequest{ + PodInterfaceID: testPod2Info.InterfaceID(), + InfraContainerID: testPod2Info.InfraContainerID(), + } b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = testIP2 @@ -279,7 +291,10 @@ func TestIPAMGetDesiredIPConfigWithSpecfiedIP(t *testing.T) { t.Fatalf("Expected to not fail adding IP's to state: %+v", err) } - req := cns.IPConfigRequest{} + req := cns.IPConfigRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } b, _ := testPod1Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = testIP1 @@ -311,7 +326,10 @@ func TestIPAMFailToGetDesiredIPConfigWithAlreadyAllocatedSpecfiedIP(t *testing.T } // request the already allocated ip with a new context - req := cns.IPConfigRequest{} + req := cns.IPConfigRequest{ + PodInterfaceID: testPod2Info.InterfaceID(), + InfraContainerID: testPod2Info.InfraContainerID(), + } b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = testIP1 @@ -370,7 +388,10 @@ func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { desiredIpAddress := testIP1 // Use TestPodInfo2 to request TestIP1, which has already been allocated - req := cns.IPConfigRequest{} + req := cns.IPConfigRequest{ + PodInterfaceID: testPod2Info.InterfaceID(), + InfraContainerID: testPod2Info.InfraContainerID(), + } b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = desiredIpAddress @@ -387,7 +408,10 @@ func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { } // Rerequest - req = cns.IPConfigRequest{} + req = cns.IPConfigRequest{ + PodInterfaceID: testPod2Info.InterfaceID(), + InfraContainerID: testPod2Info.InfraContainerID(), + } b, _ = testPod2Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = desiredIpAddress @@ -479,7 +503,10 @@ func TestAvailableIPConfigs(t *testing.T) { allocatedIps := svc.GetAllocatedIPConfigs() validateIpState(t, allocatedIps, desiredAllocatedIpConfigs) - req := cns.IPConfigRequest{} + req := cns.IPConfigRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } b, _ := testPod1Info.OrchestratorContext() req.OrchestratorContext = b req.DesiredIPAddress = state1.IPAddress diff --git a/cns/restserver/util.go b/cns/restserver/util.go index ef8f630d2e..aea915cd3d 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -395,7 +395,7 @@ func (service *HTTPRestService) getNetworkContainerResponse(req cns.GetNetworkCo return getNetworkContainerResponse } - containerID = service.state.ContainerIDByOrchestratorContext[podInfo.Key()] + containerID = service.state.ContainerIDByOrchestratorContext[podInfo.Name()+podInfo.Namespace()] } logger.Printf("containerid %v", containerID) diff --git a/cns/service/main.go b/cns/service/main.go index 5339996c17..9e217223f4 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -76,7 +76,6 @@ var args = acn.ArgumentList{ acn.OptEnvironmentFileIpam: 0, }, }, - { Name: acn.OptAPIServerURL, Shorthand: acn.OptAPIServerURLAlias, @@ -428,31 +427,6 @@ func main() { configuration.SetCNSConfigDefaults(&cnsconfig) logger.Printf("[Azure CNS] Read config :%+v", cnsconfig) - // We might be configured to reinitialize state from the CNI instead of the apiserver. - // If so, we should check that the the CNI is new enough to support the state commands, - // otherwise we fall back to the existing behavior. - if cnsconfig.InitializeFromCNI { - isGoodVer, err := cni.IsDumpStateVer() - if err != nil { - logger.Errorf("error checking CNI ver: %v", err) - } - - // override the prior config flag with the result of the ver check. - cnsconfig.InitializeFromCNI = isGoodVer - - if cnsconfig.InitializeFromCNI { - // Set the PodInfoVersion by initialization type, so that the - // PodInfo maps use the correct key schema - cns.GlobalPodInfoScheme = cns.InterfaceIDPodInfoScheme - } - } - if cnsconfig.InitializeFromCNI { - logger.Printf("Initializing from CNI") - } else { - logger.Printf("Initializing from Kubernetes") - } - logger.Printf("Set GlobalPodInfoScheme %v", cns.GlobalPodInfoScheme) - if cnsconfig.WireserverIP != "" { nmagentclient.WireserverIP = cnsconfig.WireserverIP } @@ -553,6 +527,31 @@ func main() { // Initialze state in if CNS is running in CRD mode // State must be initialized before we start HTTPRestService if config.ChannelMode == cns.CRD { + // We might be configured to reinitialize state from the CNI instead of the apiserver. + // If so, we should check that the the CNI is new enough to support the state commands, + // otherwise we fall back to the existing behavior. + if cnsconfig.InitializeFromCNI { + isGoodVer, err := cni.IsDumpStateVer() + if err != nil { + logger.Errorf("error checking CNI ver: %v", err) + } + + // override the prior config flag with the result of the ver check. + cnsconfig.InitializeFromCNI = isGoodVer + + if cnsconfig.InitializeFromCNI { + // Set the PodInfoVersion by initialization type, so that the + // PodInfo maps use the correct key schema + cns.GlobalPodInfoScheme = cns.InterfaceIDPodInfoScheme + } + } + if cnsconfig.InitializeFromCNI { + logger.Printf("Initializing from CNI") + } else { + logger.Printf("Initializing from Kubernetes") + } + logger.Printf("Set GlobalPodInfoScheme %v", cns.GlobalPodInfoScheme) + err = InitializeCRDState(httpRestService, cnsconfig) if err != nil { logger.Errorf("Failed to start CRD Controller, err:%v.\n", err) From c6fe644e836362d917b21dc3023e691d0322d58e Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Thu, 24 Jun 2021 11:53:21 -0700 Subject: [PATCH 5/6] fix test --- cns/restserver/internalapi_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 8fa48ed835..9c34a5cc4a 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -268,8 +268,8 @@ func TestReconcileNCWithExistingStateFromInterfaceID(t *testing.T) { req := generateNetworkContainerRequest(secondaryIPConfigs, "reconcileNc1", "-1") expectedAllocatedPods := map[string]cns.PodInfo{ - "10.0.0.6": cns.NewPodInfo("reconc-eth0", "abcdef", "reconcilePod1", "PodNS1"), - "10.0.0.7": cns.NewPodInfo("reconc-eth0", "abcxyz", "reconcilePod2", "PodNS1"), + "10.0.0.6": cns.NewPodInfo("abcdef", "recon1-eth0", "reconcilePod1", "PodNS1"), + "10.0.0.7": cns.NewPodInfo("abcxyz", "recon2-eth0", "reconcilePod2", "PodNS1"), } expectedNcCount := len(svc.state.ContainerStatus) From fbb472aef2ebc5981a17b27ec33da273af3c1111 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Fri, 25 Jun 2021 19:28:42 -0700 Subject: [PATCH 6/6] fix version check --- cns/cnireconciler/version.go | 11 +++++------ cns/cnireconciler/version_test.go | 12 ++++++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/cns/cnireconciler/version.go b/cns/cnireconciler/version.go index cbf090bb6b..17b0d1124e 100644 --- a/cns/cnireconciler/version.go +++ b/cns/cnireconciler/version.go @@ -8,7 +8,7 @@ import ( "k8s.io/utils/exec" ) -const cniDumpStateVer = "1.4.2" +const lastCNIWithoutDumpStateVer = "1.4.1" // IsDumpStateVer checks if the CNI executable is a version that // has the dump state command required to initialize CNS from CNI @@ -19,15 +19,14 @@ func IsDumpStateVer() (bool, error) { } func isDumpStateVer(exec exec.Interface) (bool, error) { - needVer, err := semver.NewVersion(cniDumpStateVer) + needVer, err := semver.NewVersion(lastCNIWithoutDumpStateVer) if err != nil { return false, err } cnicli := client.New(exec) - if ver, err := cnicli.GetVersion(); err != nil { + ver, err := cnicli.GetVersion() + if err != nil { return false, fmt.Errorf("failed to invoke CNI client.GetVersion(): %w", err) - } else if ver.LessThan(needVer) { - return false, nil } - return true, nil + return ver.GreaterThan(needVer), nil } diff --git a/cns/cnireconciler/version_test.go b/cns/cnireconciler/version_test.go index be35cd1577..79775aa6c8 100644 --- a/cns/cnireconciler/version_test.go +++ b/cns/cnireconciler/version_test.go @@ -26,6 +26,12 @@ func TestIsDumpStateVer(t *testing.T) { }{ { name: "bad ver", + exec: newCNIVersionFakeExec(`Azure CNI Version v1.4.1`), + want: false, + wantErr: false, + }, + { + name: "bad dirty ver", exec: newCNIVersionFakeExec(`Azure CNI Version v1.4.0-2-g984c5a5e-dirty`), want: false, wantErr: false, @@ -36,6 +42,12 @@ func TestIsDumpStateVer(t *testing.T) { want: true, wantErr: false, }, + { + name: "good dirty ver", + exec: newCNIVersionFakeExec(`Azure CNI Version v1.4.2-7-g7b97e1eb`), + want: true, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {