Merge pull request #109782 from danwinship/no-local-endpoints-metric

Don't increment "no local endpoints" metric when there are no remote endpoints
This commit is contained in:
Kubernetes Prow Robot 2022-05-05 05:02:20 -07:00 committed by GitHub
commit 50e1f70027
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 32 additions and 21 deletions

View File

@ -1354,24 +1354,19 @@ func (proxier *Proxier) syncProxyRules() {
if len(localEndpoints) != 0 { if len(localEndpoints) != 0 {
// Write rules jumping from localPolicyChain to localEndpointChains // Write rules jumping from localPolicyChain to localEndpointChains
proxier.writeServiceToEndpointRules(svcNameString, svcInfo, localPolicyChain, localEndpoints, args) proxier.writeServiceToEndpointRules(svcNameString, svcInfo, localPolicyChain, localEndpoints, args)
} else { } else if hasEndpoints {
if svcInfo.InternalPolicyLocal() && utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) { if svcInfo.InternalPolicyLocal() && utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) {
serviceNoLocalEndpointsTotalInternal++ serviceNoLocalEndpointsTotalInternal++
} }
if svcInfo.ExternalPolicyLocal() { if svcInfo.ExternalPolicyLocal() {
serviceNoLocalEndpointsTotalExternal++ serviceNoLocalEndpointsTotalExternal++
} }
if hasEndpoints {
// Blackhole all traffic since there are no local endpoints // Blackhole all traffic since there are no local endpoints
args = append(args[:0], proxier.natRules.Write(
"-A", string(localPolicyChain), "-A", string(localPolicyChain),
"-m", "comment", "--comment", "-m", "comment", "--comment",
fmt.Sprintf(`"%s has no local endpoints"`, svcNameString), fmt.Sprintf(`"%s has no local endpoints"`, svcNameString),
"-j", "-j", string(KubeMarkDropChain))
string(KubeMarkDropChain),
)
proxier.natRules.Write(args)
}
} }
} }
} }

View File

@ -7743,6 +7743,14 @@ func TestNoEndpointsMetric(t *testing.T) {
expectedSyncProxyRulesNoLocalEndpointsTotalInternal: 1, expectedSyncProxyRulesNoLocalEndpointsTotalInternal: 1,
expectedSyncProxyRulesNoLocalEndpointsTotalExternal: 1, expectedSyncProxyRulesNoLocalEndpointsTotalExternal: 1,
}, },
{
name: "both policies are set and there are no endpoints at all",
internalTrafficPolicy: &internalTrafficPolicyLocal,
externalTrafficPolicy: externalTrafficPolicyLocal,
endpoints: []endpoint{},
expectedSyncProxyRulesNoLocalEndpointsTotalInternal: 0,
expectedSyncProxyRulesNoLocalEndpointsTotalExternal: 0,
},
} }
for _, tc := range testCases { for _, tc := range testCases {

View File

@ -1990,7 +1990,7 @@ func (proxier *Proxier) syncEndpoint(svcPortName proxy.ServicePortName, onlyNode
if !ok { if !ok {
klog.InfoS("Unable to filter endpoints due to missing service info", "servicePortName", svcPortName) klog.InfoS("Unable to filter endpoints due to missing service info", "servicePortName", svcPortName)
} else { } else {
clusterEndpoints, localEndpoints, _, _ := proxy.CategorizeEndpoints(endpoints, svcInfo, proxier.nodeLabels) clusterEndpoints, localEndpoints, _, hasAnyEndpoints := proxy.CategorizeEndpoints(endpoints, svcInfo, proxier.nodeLabels)
if onlyNodeLocalEndpoints { if onlyNodeLocalEndpoints {
if len(localEndpoints) > 0 { if len(localEndpoints) > 0 {
endpoints = localEndpoints endpoints = localEndpoints
@ -2001,11 +2001,11 @@ func (proxier *Proxier) syncEndpoint(svcPortName proxy.ServicePortName, onlyNode
// will have the POD address and will be discarded. // will have the POD address and will be discarded.
endpoints = clusterEndpoints endpoints = clusterEndpoints
if svcInfo.InternalPolicyLocal() && utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) { if hasAnyEndpoints && svcInfo.InternalPolicyLocal() && utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) {
proxier.serviceNoLocalEndpointsInternal.Insert(svcPortName.NamespacedName.String()) proxier.serviceNoLocalEndpointsInternal.Insert(svcPortName.NamespacedName.String())
} }
if svcInfo.ExternalPolicyLocal() { if hasAnyEndpoints && svcInfo.ExternalPolicyLocal() {
proxier.serviceNoLocalEndpointsExternal.Insert(svcPortName.NamespacedName.String()) proxier.serviceNoLocalEndpointsExternal.Insert(svcPortName.NamespacedName.String())
} }
} }

View File

@ -5838,7 +5838,7 @@ func TestNoEndpointsMetric(t *testing.T) {
expectedSyncProxyRulesNoLocalEndpointsTotalExternal int expectedSyncProxyRulesNoLocalEndpointsTotalExternal int
}{ }{
{ {
name: "internalTrafficPolicy is set and there is non-zero local endpoints", name: "internalTrafficPolicy is set and there are local endpoints",
internalTrafficPolicy: &internalTrafficPolicyLocal, internalTrafficPolicy: &internalTrafficPolicyLocal,
endpoints: []endpoint{ endpoints: []endpoint{
{"10.0.1.1", testHostname}, {"10.0.1.1", testHostname},
@ -5847,7 +5847,7 @@ func TestNoEndpointsMetric(t *testing.T) {
}, },
}, },
{ {
name: "externalTrafficPolicy is set and there is non-zero local endpoints", name: "externalTrafficPolicy is set and there are local endpoints",
externalTrafficPolicy: externalTrafficPolicyLocal, externalTrafficPolicy: externalTrafficPolicyLocal,
endpoints: []endpoint{ endpoints: []endpoint{
{"10.0.1.1", testHostname}, {"10.0.1.1", testHostname},
@ -5856,7 +5856,7 @@ func TestNoEndpointsMetric(t *testing.T) {
}, },
}, },
{ {
name: "both policies are set and there is non-zero local endpoints", name: "both policies are set and there are local endpoints",
internalTrafficPolicy: &internalTrafficPolicyLocal, internalTrafficPolicy: &internalTrafficPolicyLocal,
externalTrafficPolicy: externalTrafficPolicyLocal, externalTrafficPolicy: externalTrafficPolicyLocal,
endpoints: []endpoint{ endpoints: []endpoint{
@ -5866,7 +5866,7 @@ func TestNoEndpointsMetric(t *testing.T) {
}, },
}, },
{ {
name: "internalTrafficPolicy is set and there is zero local endpoint", name: "internalTrafficPolicy is set and there are no local endpoints",
internalTrafficPolicy: &internalTrafficPolicyLocal, internalTrafficPolicy: &internalTrafficPolicyLocal,
endpoints: []endpoint{ endpoints: []endpoint{
{"10.0.1.1", "host0"}, {"10.0.1.1", "host0"},
@ -5876,7 +5876,7 @@ func TestNoEndpointsMetric(t *testing.T) {
expectedSyncProxyRulesNoLocalEndpointsTotalInternal: 1, expectedSyncProxyRulesNoLocalEndpointsTotalInternal: 1,
}, },
{ {
name: "externalTrafficPolicy is set and there is zero local endpoint", name: "externalTrafficPolicy is set and there are no local endpoints",
externalTrafficPolicy: externalTrafficPolicyLocal, externalTrafficPolicy: externalTrafficPolicyLocal,
endpoints: []endpoint{ endpoints: []endpoint{
{"10.0.1.1", "host0"}, {"10.0.1.1", "host0"},
@ -5886,7 +5886,7 @@ func TestNoEndpointsMetric(t *testing.T) {
expectedSyncProxyRulesNoLocalEndpointsTotalExternal: 1, expectedSyncProxyRulesNoLocalEndpointsTotalExternal: 1,
}, },
{ {
name: "Both policies are set and there is zero local endpoint", name: "Both policies are set and there are no local endpoints",
internalTrafficPolicy: &internalTrafficPolicyLocal, internalTrafficPolicy: &internalTrafficPolicyLocal,
externalTrafficPolicy: externalTrafficPolicyLocal, externalTrafficPolicy: externalTrafficPolicyLocal,
endpoints: []endpoint{ endpoints: []endpoint{
@ -5897,6 +5897,14 @@ func TestNoEndpointsMetric(t *testing.T) {
expectedSyncProxyRulesNoLocalEndpointsTotalInternal: 1, expectedSyncProxyRulesNoLocalEndpointsTotalInternal: 1,
expectedSyncProxyRulesNoLocalEndpointsTotalExternal: 1, expectedSyncProxyRulesNoLocalEndpointsTotalExternal: 1,
}, },
{
name: "Both policies are set and there are no endpoints at all",
internalTrafficPolicy: &internalTrafficPolicyLocal,
externalTrafficPolicy: externalTrafficPolicyLocal,
endpoints: []endpoint{},
expectedSyncProxyRulesNoLocalEndpointsTotalInternal: 0,
expectedSyncProxyRulesNoLocalEndpointsTotalExternal: 0,
},
} }
for _, tc := range testCases { for _, tc := range testCases {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, true)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, true)()