NPM Prometheus Unit Tests

npm/ipsm/ipsm_test.go

@@ -13,7 +13,6 @@ import (
 	testutils "github.com/Azure/azure-container-networking/test/utils"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"k8s.io/utils/exec"
 )
 
 const (
@@ -507,7 +506,38 @@ func TestDestroyNpmIpsets(t *testing.T) {
 	testSet1Name := util.AzureNpmPrefix + "123456"
 	testSet2Name := util.AzureNpmPrefix + "56543"
 
-	ipsMgr := NewIpsetManager(exec.New())
+	ipsetListFormat := `Name: %s
+	Type: hash:net
+	Revision: 6
+	Header: family inet hashsize 1024 maxelem 65536
+	Size in memory: 448
+	References: 0
+	Number of entries: 0
+	Members:
+
+	Name: %s
+	Type: hash:net
+	Revision: 6
+	Header: family inet hashsize 1024 maxelem 65536
+	Size in memory: 448
+	References: 0
+	Number of entries: 0
+	Members:`
+	ipsetListStdout := fmt.Sprintf(ipsetListFormat, testSet1Name, testSet2Name)
+
+	calls := []testutils.TestCmd{
+		{Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName(testSet1Name), "nethash"}},
+		{Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName(testSet2Name), "nethash"}},
+		{Cmd: []string{"ipset", "list"}, Stdout: ipsetListStdout},
+		{Cmd: []string{"ipset", "-F", "-exist", testSet1Name}},
+		{Cmd: []string{"ipset", "-X", "-exist", testSet1Name}},
+		{Cmd: []string{"ipset", "-F", "-exist", testSet2Name}},
+		{Cmd: []string{"ipset", "-X", "-exist", testSet2Name}},
+	}
+
+	fexec := testutils.GetFakeExecWithScripts(calls)
+	ipsMgr := NewIpsetManager(fexec)
+	defer testutils.VerifyCalls(t, fexec, calls)
 
 	execCount := resetPrometheusAndGetExecCount(t)
 	expectedSets := []expectedSetInfo{{0, testSet1Name}, {0, testSet1Name}}

npm/metrics/acl_rules_test.go

@@ -0,0 +1,24 @@
+package metrics
+
+import "testing"
+
+var (
+	numRulesMetric = &basicMetric{ResetNumACLRules, IncNumACLRules, DecNumACLRules, GetNumACLRules}
+	ruleExecMetric = &recordingMetric{RecordACLRuleExecTime, GetACLRuleExecCount}
+)
+
+func TestRecordACLRuleExecTime(t *testing.T) {
+	testStopAndRecord(t, ruleExecMetric)
+}
+
+func TestIncNumACLRules(t *testing.T) {
+	testIncMetric(t, numRulesMetric)
+}
+
+func TestDecNumACLRules(t *testing.T) {
+	testDecMetric(t, numRulesMetric)
+}
+
+func TestResetNumACLRules(t *testing.T) {
+	testResetMetric(t, numRulesMetric)
+}

npm/metrics/ipsets.go

@@ -32,7 +32,7 @@ func ResetNumIPSets() {
 // TODO might be more efficient to keep track of the count
 func NumIPSetsIsPositive() bool {
 	val, err := GetNumIPSets()
-	return val > 0 && err != nil
+	return val > 0 && err == nil
 }
 
 // RecordIPSetExecTime adds an observation of execution time for adding an IPSet.
@@ -42,6 +42,7 @@ func RecordIPSetExecTime(timer *Timer) {
 }
 
 // AddEntryToIPSet increments the number of entries for IPSet setName.
+// It doesn't ever update the number of IPSets.
 func AddEntryToIPSet(setName string) {
 	numIPSetEntries.Inc()
 	ipsetInventoryMap[setName]++ // adds the setName with value 1 if it doesn't exist
@@ -63,6 +64,7 @@ func RemoveEntryFromIPSet(setName string) {
 }
 
 // RemoveAllEntriesFromIPSet sets the number of entries for ipset setName to 0.
+// It doesn't ever update the number of IPSets.
 func RemoveAllEntriesFromIPSet(setName string) {
 	numIPSetEntries.Add(-getEntryCountForIPSet(setName))
 	delete(ipsetInventoryMap, setName)
@@ -76,6 +78,7 @@ func DeleteIPSet(setName string) {
 }
 
 // ResetIPSetEntries sets the number of entries to 0 for all IPSets.
+// It doesn't ever update the number of IPSets.
 func ResetIPSetEntries() {
 	numIPSetEntries.Set(0)
 	ipsetInventoryMap = make(map[string]int)
@@ -95,6 +98,8 @@ func GetNumIPSetEntries() (int, error) {
 
 // GetNumEntriesForIPSet returns the number entries for IPSet setName.
 // This function is slow.
+// TODO could use the map if this function needs to be faster.
+// If updated, replace GetNumEntriesForIPSet() with getVecValue() in assertEqualMapAndMetricElements() in ipsets_test.go
 func GetNumEntriesForIPSet(setName string) (int, error) {
 	labels := getIPSetInventoryLabels(setName)
 	return getVecValue(ipsetInventory, labels)

npm/metrics/ipsets_test.go

@@ -0,0 +1,198 @@
+package metrics
+
+import (
+	"testing"
+
+	"github.com/Azure/azure-container-networking/npm/metrics/promutil"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	testName1 = "test-set1"
+	testName2 = "test-set2"
+)
+
+var (
+	numSetsMetric = &basicMetric{ResetNumIPSets, IncNumIPSets, DecNumIPSets, GetNumIPSets}
+	setExecMetric = &recordingMetric{RecordIPSetExecTime, GetIPSetExecCount}
+)
+
+type testSet struct {
+	name       string
+	entryCount int
+}
+
+func TestRecordIPSetExecTime(t *testing.T) {
+	testStopAndRecord(t, setExecMetric)
+}
+
+func TestIncNumIPSets(t *testing.T) {
+	testIncMetric(t, numSetsMetric)
+}
+
+func TestDecNumIPSets(t *testing.T) {
+	testDecMetric(t, numSetsMetric)
+}
+
+func TestResetNumIPSets(t *testing.T) {
+	testResetMetric(t, numSetsMetric)
+}
+
+func TestSetNumIPSets(t *testing.T) {
+	ResetIPSetEntries()
+	SetNumIPSets(10)
+	assertMetricVal(t, numSetsMetric, 10)
+	SetNumIPSets(0)
+	assertMetricVal(t, numSetsMetric, 0)
+}
+
+func TestNumIPSetsIsPositive(t *testing.T) {
+	ResetIPSetEntries()
+	assertNotPositive(t)
+
+	IncNumIPSets()
+	DecNumIPSets()
+	assertNotPositive(t)
+
+	IncNumIPSets()
+	assertPositive(t)
+
+	SetNumIPSets(5)
+	assertPositive(t)
+
+	SetNumIPSets(0)
+	assertNotPositive(t)
+
+	DecNumIPSets()
+	assertNotPositive(t)
+}
+
+func assertNotPositive(t *testing.T) {
+	if NumIPSetsIsPositive() {
+		numSets, _ := GetNumIPSets()
+		require.FailNowf(t, "", "expected num IPSets not to be positive. Current number: %d", numSets)
+	}
+}
+
+func assertPositive(t *testing.T) {
+	if !NumIPSetsIsPositive() {
+		numSets, _ := GetNumIPSets()
+		require.FailNowf(t, "", "expected num IPSets to be positive. Current number: %d", numSets)
+	}
+}
+
+func TestAddEntryToIPSet(t *testing.T) {
+	ResetIPSetEntries()
+	AddEntryToIPSet(testName1)
+	AddEntryToIPSet(testName1)
+	AddEntryToIPSet(testName2)
+	assertNumEntriesAndCounts(t, &testSet{testName1, 2}, &testSet{testName2, 1})
+	assertMapIsGood(t)
+}
+
+func assertNumEntriesAndCounts(t *testing.T, sets ...*testSet) {
+	expectedTotal := 0
+	for _, set := range sets {
+		val, exists := ipsetInventoryMap[set.name]
+		if !exists {
+			require.FailNowf(t, "", "expected set %s to exist in map for ipset entries", set.name)
+		}
+		if set.entryCount != val {
+			require.FailNowf(t, "", "set %s has incorrect number of entries. Expected %d, got %d", set.name, set.entryCount, val)
+		}
+		expectedTotal += val
+	}
+
+	numEntries, err := GetNumIPSetEntries()
+	promutil.NotifyIfErrors(t, err)
+	if numEntries != expectedTotal {
+		require.FailNowf(t, "", "incorrect numver of entries. Expected %d, got %d", expectedTotal, numEntries)
+	}
+}
+
+func assertNotInMap(t *testing.T, setNames ...string) {
+	for _, setName := range setNames {
+		_, exists := ipsetInventoryMap[setName]
+		if exists {
+			require.FailNowf(t, "", "expected set %s to not exist in map for ipset entries", setName)
+		}
+	}
+}
+
+func assertMapIsGood(t *testing.T) {
+	assertEqualMapAndMetricElements(t)
+	assertNoZeroEntriesInMap(t)
+}
+
+// can't get all of a GaugeVec's labels, so need to trust code to delete labels in GaugeVec when done
+func assertEqualMapAndMetricElements(t *testing.T) {
+	for setName, mapVal := range ipsetInventoryMap {
+		val, err := GetNumEntriesForIPSet(setName)
+		promutil.NotifyIfErrors(t, err)
+		promutil.NotifyIfErrors(t, err)
+		if mapVal != val {
+			require.FailNowf(t, "", "set %s has incorrect number of entries. Metric has %d, map has %d", setName, val, mapVal)
+		}
+	}
+}
+
+func assertNoZeroEntriesInMap(t *testing.T) {
+	for setname, mapVal := range ipsetInventoryMap {
+		if mapVal <= 0 {
+			require.FailNowf(t, "", "expected all ipset entry counts to be positive, but got %d for set %s", mapVal, setname)
+		}
+	}
+}
+
+func TestRemoveEntryFromIPSet(t *testing.T) {
+	ResetIPSetEntries()
+	AddEntryToIPSet(testName1)
+	AddEntryToIPSet(testName1)
+	AddEntryToIPSet(testName2)
+	RemoveEntryFromIPSet(testName1)
+	assertNumEntriesAndCounts(t, &testSet{testName1, 1}, &testSet{testName2, 1})
+	assertMapIsGood(t)
+}
+
+func TestRemoveAllEntriesFromIPSet(t *testing.T) {
+	ResetIPSetEntries()
+	AddEntryToIPSet(testName1)
+	AddEntryToIPSet(testName1)
+	AddEntryToIPSet(testName2)
+	RemoveAllEntriesFromIPSet(testName1)
+	assertNotInMap(t, testName1)
+	assertNumEntriesAndCounts(t, &testSet{testName2, 1})
+	assertMapIsGood(t)
+}
+
+func TestDeleteIPSet(t *testing.T) {
+	ResetNumIPSets()
+	ResetIPSetEntries()
+	IncNumIPSets()
+	AddEntryToIPSet(testName1)
+	AddEntryToIPSet(testName1)
+	IncNumIPSets()
+	AddEntryToIPSet(testName2)
+	DeleteIPSet(testName1)
+	assertNumSets(t, 1)
+	assertNotInMap(t, testName1)
+	assertNumEntriesAndCounts(t, &testSet{testName2, 1})
+	assertMapIsGood(t)
+}
+
+func assertNumSets(t *testing.T, exectedVal int) {
+	numSets, err := GetNumIPSets()
+	promutil.NotifyIfErrors(t, err)
+	if numSets != exectedVal {
+		require.FailNowf(t, "", "incorrect number of ipsets. Expected %d, got %d", exectedVal, numSets)
+	}
+}
+
+func TestResetIPSetEntries(t *testing.T) {
+	AddEntryToIPSet(testName1)
+	AddEntryToIPSet(testName1)
+	AddEntryToIPSet(testName2)
+	ResetIPSetEntries()
+	assertNotInMap(t, testName1, testName2)
+	assertMapIsGood(t)
+}

npm/metrics/policies_test.go

@@ -0,0 +1,24 @@
+package metrics
+
+import "testing"
+
+var (
+	numPoliciesMetric = &basicMetric{ResetNumPolicies, IncNumPolicies, DecNumPolicies, GetNumPolicies}
+	policyExecMetric  = &recordingMetric{RecordPolicyExecTime, GetPolicyExecCount}
+)
+
+func TestRecordPolicyExecTime(t *testing.T) {
+	testStopAndRecord(t, policyExecMetric)
+}
+
+func TestIncNumPolicies(t *testing.T) {
+	testIncMetric(t, numPoliciesMetric)
+}
+
+func TestDecNumPolicies(t *testing.T) {
+	testDecMetric(t, numPoliciesMetric)
+}
+
+func TestResetNumPolicies(t *testing.T) {
+	testResetMetric(t, numPoliciesMetric)
+}

npm/metrics/prometheus_metrics.go → npm/metrics/prometheus-metrics.go

@@ -1,13 +1,11 @@
 package metrics
 
 import (
-	"fmt"
 	"net/http"
 
 	"github.com/Azure/azure-container-networking/log"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
-	dto "github.com/prometheus/client_model/go"
 )
 
 const namespace = "npm"
@@ -137,40 +135,3 @@ func createSummary(name string, helpMessage string, isNodeLevel bool) prometheus
 	register(summary, name, isNodeLevel)
 	return summary
 }
-
-// getValue returns a Gauge metric's value.
-// This function is slow.
-func getValue(gaugeMetric prometheus.Gauge) (int, error) {
-	dtoMetric, err := getDTOMetric(gaugeMetric)
-	if err != nil {
-		return 0, err
-	}
-	return int(dtoMetric.Gauge.GetValue()), nil
-}
-
-// getVecValue Gauge Vec metric's value, or 0 if the label doesn't exist for the metric.
-// This function is slow.
-func getVecValue(gaugeVecMetric *prometheus.GaugeVec, labels prometheus.Labels) (int, error) {
-	return getValue(gaugeVecMetric.With(labels))
-}
-
-// getCountValue the number of times a Summary metric has recorded an observation.
-// This function is slow.
-func getCountValue(summaryMetric prometheus.Summary) (int, error) {
-	dtoMetric, err := getDTOMetric(summaryMetric)
-	if err != nil {
-		return 0, err
-	}
-	return int(dtoMetric.Summary.GetSampleCount()), nil
-}
-
-func getDTOMetric(collector prometheus.Collector) (*dto.Metric, error) {
-	channel := make(chan prometheus.Metric, 1)
-	collector.Collect(channel)
-	metric := &dto.Metric{}
-	err := (<-channel).Write(metric)
-	if err != nil {
-		err = fmt.Errorf("error while extracting Prometheus metric value: %w", err)
-	}
-	return metric, err
-}