diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 3093dac23e2..6f27d23de74 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -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() diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 166f7e1ffbc..70d43200337 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -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) {