[NPM] refactor: npmcache json marshaling

npm/http/server/server.go

@@ -1,18 +1,18 @@
 package server
 
 import (
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/http/pprof"
 	_ "net/http/pprof"
 
-	"github.com/Azure/azure-container-networking/npm/cache"
+	"github.com/Azure/azure-container-networking/log"
 	npmconfig "github.com/Azure/azure-container-networking/npm/config"
 	"github.com/Azure/azure-container-networking/npm/http/api"
 	"github.com/Azure/azure-container-networking/npm/metrics"
 	"k8s.io/klog"
 
-	"github.com/Azure/azure-container-networking/npm"
 	"github.com/gorilla/mux"
 )
 
@@ -21,7 +21,7 @@ type NPMRestServer struct {
 	router           *mux.Router
 }
 
-func NPMRestServerListenAndServe(config npmconfig.Config, npmEncoder npm.NetworkPolicyManagerEncoder) {
+func NPMRestServerListenAndServe(config npmconfig.Config, npmEncoder json.Marshaler) {
 	rs := NPMRestServer{}
 
 	rs.router = mux.NewRouter()
@@ -60,12 +60,16 @@ func NPMRestServerListenAndServe(config npmconfig.Config, npmEncoder npm.Network
 	klog.Errorf("Failed to start NPM HTTP Server with error: %+v", srv.ListenAndServe())
 }
 
-func (n *NPMRestServer) npmCacheHandler(npmEncoder npm.NetworkPolicyManagerEncoder) http.Handler {
+func (n *NPMRestServer) npmCacheHandler(npmCacheEncoder json.Marshaler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		err := cache.Encode(w, npmEncoder)
+		b, err := json.Marshal(npmCacheEncoder)
 		if err != nil {
 			http.Error(w, err.Error(), 500)
 			return
 		}
+		_, err = w.Write(b)
+		if err != nil {
+			log.Errorf("failed to write resp: %w", err)
+		}
 	})
 }

npm/http/server/server_test.go

@@ -1,44 +1,26 @@
 package server
 
 import (
+	"encoding/json"
+	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
-	"os"
 	"testing"
-	"time"
 
-	"github.com/Azure/azure-container-networking/npm/cache"
 	"github.com/Azure/azure-container-networking/npm/http/api"
 	"github.com/Azure/azure-container-networking/npm/ipsm"
 	"github.com/stretchr/testify/assert"
 
 	"github.com/Azure/azure-container-networking/npm"
-	k8sversion "k8s.io/apimachinery/pkg/version"
-	kubeinformers "k8s.io/client-go/informers"
-	k8sfake "k8s.io/client-go/kubernetes/fake"
-	fakeexec "k8s.io/utils/exec/testing"
 )
 
-func NPMEncoder() npm.NetworkPolicyManagerEncoder {
-	noResyncPeriodFunc := func() time.Duration { return 0 }
-	kubeclient := k8sfake.NewSimpleClientset()
-	kubeInformer := kubeinformers.NewSharedInformerFactory(kubeclient, noResyncPeriodFunc())
-	fakeK8sVersion := &k8sversion.Info{
-		GitVersion: "v1.20.2",
-	}
-	exec := &fakeexec.FakeExec{}
-	npmVersion := "npm-ut-test"
-
-	npmEncoder := npm.NewNetworkPolicyManager(kubeInformer, exec, npmVersion, fakeK8sVersion)
-	return npmEncoder
-}
-
 func TestGetNPMCacheHandler(t *testing.T) {
 	assert := assert.New(t)
 
-	npmEncoder := NPMEncoder()
+	nodeName := "nodename"
+	npmCacheEncoder := npm.CacheEncoder(nodeName)
 	n := &NPMRestServer{}
-	handler := n.npmCacheHandler(npmEncoder)
+	handler := n.npmCacheHandler(npmCacheEncoder)
 
 	req, err := http.NewRequest(http.MethodGet, api.NPMMgrPath, nil)
 	if err != nil {
@@ -53,14 +35,19 @@ func TestGetNPMCacheHandler(t *testing.T) {
 			status, http.StatusOK)
 	}
 
-	var actual *cache.NPMCache
-	actual, err = cache.Decode(rr.Body)
+	byteArray, err := ioutil.ReadAll(rr.Body)
 	if err != nil {
-		t.Fatal(err)
+		t.Errorf("failed to read response's data : %w", err)
+	}
+
+	actual := &npm.Cache{}
+	err = json.Unmarshal(byteArray, actual)
+	if err != nil {
+		t.Fatalf("failed to unmarshal %s due to %v", string(byteArray), err)
 	}
 
-	expected := &cache.NPMCache{
-		Nodename: os.Getenv("HOSTNAME"),
+	expected := &npm.Cache{
+		NodeName: nodeName,
 		NsMap:    make(map[string]*npm.Namespace),
 		PodMap:   make(map[string]*npm.NpmPod),
 		ListMap:  make(map[string]*ipsm.Ipset),

npm/ipsm/ipsm.go

@@ -15,6 +15,7 @@ import (
 	"github.com/Azure/azure-container-networking/log"
 	"github.com/Azure/azure-container-networking/npm/metrics"
 	"github.com/Azure/azure-container-networking/npm/util"
+	"github.com/pkg/errors"
 	utilexec "k8s.io/utils/exec"
 )
 
@@ -75,22 +76,28 @@ func NewIpsetManager(exec utilexec.Interface) *IpsetManager {
 	}
 }
 
-// Encode encodes listmap and setmap.
-// The ordering to encode them is important.
-// Do encode listMap first and then setMap.
-func (ipsMgr *IpsetManager) Encode(enc *json.Encoder) error {
+func (ipsMgr *IpsetManager) MarshalListMapJSON() ([]byte, error) {
 	ipsMgr.Lock()
 	defer ipsMgr.Unlock()
 
-	if err := enc.Encode(ipsMgr.listMap); err != nil {
-		return fmt.Errorf("failed to encode listMap %w", err)
+	listMapRaw, err := json.Marshal(ipsMgr.listMap)
+	if err != nil {
+		return nil, errors.Errorf("failed to marshal ListMap due to %v", err)
 	}
 
-	if err := enc.Encode(ipsMgr.setMap); err != nil {
-		return fmt.Errorf("failed to encode setMap %w", err)
+	return listMapRaw, nil
+}
+
+func (ipsMgr *IpsetManager) MarshalSetMapJSON() ([]byte, error) {
+	ipsMgr.Lock()
+	defer ipsMgr.Unlock()
+
+	setMapRaw, err := json.Marshal(ipsMgr.setMap)
+	if err != nil {
+		return nil, errors.Errorf("failed to marshal SetMap due to %v", err)
 	}
 
-	return nil
+	return setMapRaw, nil
 }
 
 // Exists checks if an element exists in setMap/listMap.

npm/ipsm/ipsm_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/Azure/azure-container-networking/npm/metrics/promutil"
 	"github.com/Azure/azure-container-networking/npm/util"
 	testutils "github.com/Azure/azure-container-networking/test/utils"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"k8s.io/utils/exec"
 )
@@ -531,6 +532,54 @@ func TestDestroyNpmIpsets(t *testing.T) {
 	}
 }
 
+func TestMarshalListMapJSON(t *testing.T) {
+	testListSet := "test-list"
+	calls := []testutils.TestCmd{
+		{Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName(testListSet), "setlist"}},
+	}
+
+	fexec := testutils.GetFakeExecWithScripts(calls)
+	ipsMgr := NewIpsetManager(fexec)
+	defer testutils.VerifyCalls(t, fexec, calls)
+
+	err := ipsMgr.createList(testListSet)
+	require.NoError(t, err)
+
+	listMapRaw, err := ipsMgr.MarshalListMapJSON()
+	require.NoError(t, err)
+	fmt.Println(string(listMapRaw))
+
+	expect := []byte(`{"test-list":{}}`)
+
+	fmt.Printf("%v\n", ipsMgr.listMap)
+	assert.ElementsMatch(t, expect, listMapRaw)
+}
+
+func TestMarshalSetMapJSON(t *testing.T) {
+	testSet := "test-set"
+	calls := []testutils.TestCmd{
+		{Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName(testSet), "nethash"}},
+	}
+
+	fexec := testutils.GetFakeExecWithScripts(calls)
+	ipsMgr := NewIpsetManager(fexec)
+	defer testutils.VerifyCalls(t, fexec, calls)
+
+	err := ipsMgr.createSet(testSet, []string{util.IpsetNetHashFlag})
+	require.NoError(t, err)
+
+	setMapRaw, err := ipsMgr.MarshalSetMapJSON()
+	require.NoError(t, err)
+	fmt.Println(string(setMapRaw))
+
+	expect := []byte(`{"test-set":{}}`)
+	for key, val := range ipsMgr.setMap {
+		fmt.Printf("key: %s value: %+v\n", key, val)
+	}
+
+	assert.ElementsMatch(t, expect, setMapRaw)
+}
+
 // Enable these tests once the the changes for ipsm are enabled
 /*
 const letters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"