Test the IptablesRulesTotal metric in TestSyncProxyRulesRepeated
This required fixing a small bug in the metric, where it had previously been counting the "-X" lines that had been passed to iptables-restore to delete stale chains, rather than only counting the actual rules.
This commit is contained in:
		| @@ -1381,6 +1381,7 @@ func (proxier *Proxier) syncProxyRules() { | ||||
| 	// to run on hosts with lots of iptables rules, we don't bother to do this on | ||||
| 	// every sync in large clusters. (Stale chains will not be referenced by any | ||||
| 	// active rules, so they're harmless other than taking up memory.) | ||||
| 	deletedChains := 0 | ||||
| 	if !proxier.largeClusterMode || time.Since(proxier.lastIPTablesCleanup) > proxier.syncPeriod { | ||||
| 		var existingNATChains map[utiliptables.Chain]struct{} | ||||
|  | ||||
| @@ -1400,6 +1401,7 @@ func (proxier *Proxier) syncProxyRules() { | ||||
| 					// the chain. Then we can remove the chain. | ||||
| 					proxier.natChains.Write(utiliptables.MakeChainLine(chain)) | ||||
| 					proxier.natRules.Write("-X", chainString) | ||||
| 					deletedChains++ | ||||
| 				} | ||||
| 			} | ||||
| 			proxier.lastIPTablesCleanup = time.Now() | ||||
| @@ -1481,7 +1483,7 @@ func (proxier *Proxier) syncProxyRules() { | ||||
| 	) | ||||
|  | ||||
| 	metrics.IptablesRulesTotal.WithLabelValues(string(utiliptables.TableFilter)).Set(float64(proxier.filterRules.Lines())) | ||||
| 	metrics.IptablesRulesTotal.WithLabelValues(string(utiliptables.TableNAT)).Set(float64(proxier.natRules.Lines())) | ||||
| 	metrics.IptablesRulesTotal.WithLabelValues(string(utiliptables.TableNAT)).Set(float64(proxier.natRules.Lines() - deletedChains)) | ||||
|  | ||||
| 	// Sync rules. | ||||
| 	proxier.iptablesData.Reset() | ||||
|   | ||||
| @@ -604,6 +604,15 @@ func countRules(tableName utiliptables.Table, ruleData string) int { | ||||
| 	return rules | ||||
| } | ||||
|  | ||||
| func countRulesFromMetric(tableName utiliptables.Table) int { | ||||
| 	numRulesFloat, err := testutil.GetGaugeMetricValue(metrics.IptablesRulesTotal.WithLabelValues(string(tableName))) | ||||
| 	if err != nil { | ||||
| 		klog.ErrorS(err, "metrics are not registered?") | ||||
| 		return -1 | ||||
| 	} | ||||
| 	return int(numRulesFloat) | ||||
| } | ||||
|  | ||||
| // findAllMatches takes an array of lines and a pattern with one parenthesized group, and | ||||
| // returns a sorted array of all of the unique matches of the parenthesized group. | ||||
| func findAllMatches(lines []string, pattern string) []string { | ||||
| @@ -1962,12 +1971,7 @@ func TestOverallIPTablesRulesWithMultipleServices(t *testing.T) { | ||||
|  | ||||
| 	assertIPTablesRulesEqual(t, getLine(), true, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	natRulesMetric, err := testutil.GetGaugeMetricValue(metrics.IptablesRulesTotal.WithLabelValues(string(utiliptables.TableNAT))) | ||||
| 	if err != nil { | ||||
| 		t.Errorf("failed to get %s value, err: %v", metrics.IptablesRulesTotal.Name, err) | ||||
| 	} | ||||
| 	nNatRules := int(natRulesMetric) | ||||
|  | ||||
| 	nNatRules := countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	expectedNatRules := countRules(utiliptables.TableNAT, fp.iptablesData.String()) | ||||
|  | ||||
| 	if nNatRules != expectedNatRules { | ||||
| @@ -5461,22 +5465,14 @@ func TestProxierMetricsIptablesTotalRules(t *testing.T) { | ||||
| 	fp.syncProxyRules() | ||||
| 	iptablesData := fp.iptablesData.String() | ||||
|  | ||||
| 	nFilterRulesMetric, err := testutil.GetGaugeMetricValue(metrics.IptablesRulesTotal.WithLabelValues(string(utiliptables.TableFilter))) | ||||
| 	if err != nil { | ||||
| 		t.Errorf("failed to get %s value, err: %v", metrics.IptablesRulesTotal.Name, err) | ||||
| 	} | ||||
| 	nFilterRules := int(nFilterRulesMetric) | ||||
| 	nFilterRules := countRulesFromMetric(utiliptables.TableFilter) | ||||
| 	expectedFilterRules := countRules(utiliptables.TableFilter, iptablesData) | ||||
|  | ||||
| 	if nFilterRules != expectedFilterRules { | ||||
| 		t.Fatalf("Wrong number of filter rule: expected %d got %d\n%s", expectedFilterRules, nFilterRules, iptablesData) | ||||
| 	} | ||||
|  | ||||
| 	nNatRulesMetric, err := testutil.GetGaugeMetricValue(metrics.IptablesRulesTotal.WithLabelValues(string(utiliptables.TableNAT))) | ||||
| 	if err != nil { | ||||
| 		t.Errorf("failed to get %s value, err: %v", metrics.IptablesRulesTotal.Name, err) | ||||
| 	} | ||||
| 	nNatRules := int(nNatRulesMetric) | ||||
| 	nNatRules := countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	expectedNatRules := countRules(utiliptables.TableNAT, iptablesData) | ||||
|  | ||||
| 	if nNatRules != expectedNatRules { | ||||
| @@ -5502,22 +5498,14 @@ func TestProxierMetricsIptablesTotalRules(t *testing.T) { | ||||
| 	fp.syncProxyRules() | ||||
| 	iptablesData = fp.iptablesData.String() | ||||
|  | ||||
| 	nFilterRulesMetric, err = testutil.GetGaugeMetricValue(metrics.IptablesRulesTotal.WithLabelValues(string(utiliptables.TableFilter))) | ||||
| 	if err != nil { | ||||
| 		t.Errorf("failed to get %s value, err: %v", metrics.IptablesRulesTotal.Name, err) | ||||
| 	} | ||||
| 	nFilterRules = int(nFilterRulesMetric) | ||||
| 	nFilterRules = countRulesFromMetric(utiliptables.TableFilter) | ||||
| 	expectedFilterRules = countRules(utiliptables.TableFilter, iptablesData) | ||||
|  | ||||
| 	if nFilterRules != expectedFilterRules { | ||||
| 		t.Fatalf("Wrong number of filter rule: expected %d got %d\n%s", expectedFilterRules, nFilterRules, iptablesData) | ||||
| 	} | ||||
|  | ||||
| 	nNatRulesMetric, err = testutil.GetGaugeMetricValue(metrics.IptablesRulesTotal.WithLabelValues(string(utiliptables.TableNAT))) | ||||
| 	if err != nil { | ||||
| 		t.Errorf("failed to get %s value, err: %v", metrics.IptablesRulesTotal.Name, err) | ||||
| 	} | ||||
| 	nNatRules = int(nNatRulesMetric) | ||||
| 	nNatRules = countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	expectedNatRules = countRules(utiliptables.TableNAT, iptablesData) | ||||
|  | ||||
| 	if nNatRules != expectedNatRules { | ||||
| @@ -7705,6 +7693,12 @@ func TestSyncProxyRulesRepeated(t *testing.T) { | ||||
| 		`) | ||||
| 	assertIPTablesRulesEqual(t, getLine(), true, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	rulesSynced := countRules(utiliptables.TableNAT, expected) | ||||
| 	rulesSyncedMetric := countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	if rulesSyncedMetric != rulesSynced { | ||||
| 		t.Errorf("metric shows %d rules synced but iptables data shows %d", rulesSyncedMetric, rulesSynced) | ||||
| 	} | ||||
|  | ||||
| 	// Add a new service and its endpoints. (This will only sync the SVC and SEP rules | ||||
| 	// for the new service, not the existing ones.) | ||||
| 	makeServiceMap(fp, | ||||
| @@ -7771,6 +7765,12 @@ func TestSyncProxyRulesRepeated(t *testing.T) { | ||||
| 		`) | ||||
| 	assertIPTablesRulesEqual(t, getLine(), false, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	rulesSynced = countRules(utiliptables.TableNAT, expected) | ||||
| 	rulesSyncedMetric = countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	if rulesSyncedMetric != rulesSynced { | ||||
| 		t.Errorf("metric shows %d rules synced but iptables data shows %d", rulesSyncedMetric, rulesSynced) | ||||
| 	} | ||||
|  | ||||
| 	// Delete a service. (Won't update the other services.) | ||||
| 	fp.OnServiceDelete(svc2) | ||||
| 	fp.syncProxyRules() | ||||
| @@ -7808,6 +7808,12 @@ func TestSyncProxyRulesRepeated(t *testing.T) { | ||||
| 		`) | ||||
| 	assertIPTablesRulesEqual(t, getLine(), false, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	rulesSynced = countRules(utiliptables.TableNAT, expected) | ||||
| 	rulesSyncedMetric = countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	if rulesSyncedMetric != rulesSynced { | ||||
| 		t.Errorf("metric shows %d rules synced but iptables data shows %d", rulesSyncedMetric, rulesSynced) | ||||
| 	} | ||||
|  | ||||
| 	// Add a service, sync, then add its endpoints. (The first sync will be a no-op other | ||||
| 	// than adding the REJECT rule. The second sync will create the new service.) | ||||
| 	var svc4 *v1.Service | ||||
| @@ -7854,6 +7860,12 @@ func TestSyncProxyRulesRepeated(t *testing.T) { | ||||
| 		`) | ||||
| 	assertIPTablesRulesEqual(t, getLine(), false, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	rulesSynced = countRules(utiliptables.TableNAT, expected) | ||||
| 	rulesSyncedMetric = countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	if rulesSyncedMetric != rulesSynced { | ||||
| 		t.Errorf("metric shows %d rules synced but iptables data shows %d", rulesSyncedMetric, rulesSynced) | ||||
| 	} | ||||
|  | ||||
| 	populateEndpointSlices(fp, | ||||
| 		makeTestEndpointSlice("ns4", "svc4", 1, func(eps *discovery.EndpointSlice) { | ||||
| 			eps.AddressType = discovery.AddressTypeIPv4 | ||||
| @@ -7904,6 +7916,12 @@ func TestSyncProxyRulesRepeated(t *testing.T) { | ||||
| 		`) | ||||
| 	assertIPTablesRulesEqual(t, getLine(), false, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	rulesSynced = countRules(utiliptables.TableNAT, expected) | ||||
| 	rulesSyncedMetric = countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	if rulesSyncedMetric != rulesSynced { | ||||
| 		t.Errorf("metric shows %d rules synced but iptables data shows %d", rulesSyncedMetric, rulesSynced) | ||||
| 	} | ||||
|  | ||||
| 	// Change an endpoint of an existing service. This will cause its SVC and SEP | ||||
| 	// chains to be rewritten. | ||||
| 	eps3update := eps3.DeepCopy() | ||||
| @@ -7949,6 +7967,12 @@ func TestSyncProxyRulesRepeated(t *testing.T) { | ||||
| 		`) | ||||
| 	assertIPTablesRulesEqual(t, getLine(), false, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	rulesSynced = countRules(utiliptables.TableNAT, expected) | ||||
| 	rulesSyncedMetric = countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	if rulesSyncedMetric != rulesSynced { | ||||
| 		t.Errorf("metric shows %d rules synced but iptables data shows %d", rulesSyncedMetric, rulesSynced) | ||||
| 	} | ||||
|  | ||||
| 	// Add an endpoint to a service. This will cause its SVC and SEP chains to be rewritten. | ||||
| 	eps3update2 := eps3update.DeepCopy() | ||||
| 	eps3update2.Endpoints = append(eps3update2.Endpoints, discovery.Endpoint{Addresses: []string{"10.0.3.3"}}) | ||||
| @@ -7995,6 +8019,12 @@ func TestSyncProxyRulesRepeated(t *testing.T) { | ||||
| 		`) | ||||
| 	assertIPTablesRulesEqual(t, getLine(), false, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	rulesSynced = countRules(utiliptables.TableNAT, expected) | ||||
| 	rulesSyncedMetric = countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	if rulesSyncedMetric != rulesSynced { | ||||
| 		t.Errorf("metric shows %d rules synced but iptables data shows %d", rulesSyncedMetric, rulesSynced) | ||||
| 	} | ||||
|  | ||||
| 	// Sync with no new changes... This will not rewrite any SVC or SEP chains | ||||
| 	fp.syncProxyRules() | ||||
|  | ||||
| @@ -8028,6 +8058,12 @@ func TestSyncProxyRulesRepeated(t *testing.T) { | ||||
| 		`) | ||||
| 	assertIPTablesRulesEqual(t, getLine(), false, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	rulesSynced = countRules(utiliptables.TableNAT, expected) | ||||
| 	rulesSyncedMetric = countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	if rulesSyncedMetric != rulesSynced { | ||||
| 		t.Errorf("metric shows %d rules synced but iptables data shows %d", rulesSyncedMetric, rulesSynced) | ||||
| 	} | ||||
|  | ||||
| 	// Now force a partial resync error and ensure that it recovers correctly | ||||
| 	if fp.needFullSync { | ||||
| 		t.Fatalf("Proxier unexpectedly already needs a full sync?") | ||||
| @@ -8125,6 +8161,12 @@ func TestSyncProxyRulesRepeated(t *testing.T) { | ||||
| 		COMMIT | ||||
| 		`) | ||||
| 	assertIPTablesRulesEqual(t, getLine(), false, expected, fp.iptablesData.String()) | ||||
|  | ||||
| 	rulesSynced = countRules(utiliptables.TableNAT, expected) | ||||
| 	rulesSyncedMetric = countRulesFromMetric(utiliptables.TableNAT) | ||||
| 	if rulesSyncedMetric != rulesSynced { | ||||
| 		t.Errorf("metric shows %d rules synced but iptables data shows %d", rulesSyncedMetric, rulesSynced) | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func TestNoEndpointsMetric(t *testing.T) { | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Dan Winship
					Dan Winship