Updating Topology Aware Hints to support "Auto" value for annotation

Previously only "auto" was supported, a value that was inconsistent with
Kubernetes naming patterns.
This commit is contained in:
Rob Scott
2021-03-31 15:58:04 -07:00
parent a651804427
commit 50b377fe4e
9 changed files with 78 additions and 21 deletions

View File

@@ -102,7 +102,7 @@ var (
Name: "endpointslices_changed_per_sync",
Help: "Number of EndpointSlices changed on each Service sync",
},
[]string{"topology"}, // either "auto" or "disabled"
[]string{"topology"}, // either "Auto" or "Disabled"
)
// EndpointSliceSyncs tracks the number of sync operations the controller

View File

@@ -330,9 +330,9 @@ func (r *reconciler) finalize(
metrics.EndpointSliceChanges.WithLabelValues("delete").Inc()
}
topologyLabel := "disabled"
topologyLabel := "Disabled"
if r.topologyCache != nil && hintsEnabled(service.Annotations) {
topologyLabel = "auto"
topologyLabel = "Auto"
}
numSlicesChanged := len(slicesToCreate) + len(slicesToUpdate) + len(slicesToDelete)

View File

@@ -1460,9 +1460,9 @@ func TestReconcileTopology(t *testing.T) {
slicesChangedPerSyncTopology: 0,
},
}, {
name: "topology enabled, hintsAnnotation==auto, more slices and endpoints",
name: "topology enabled, hintsAnnotation==Auto, more slices and endpoints",
topologyCacheEnabled: true,
hintsAnnotation: "auto",
hintsAnnotation: "Auto",
existingSlices: []*discovery.EndpointSlice{slicesByName["zone-a-c"], slicesByName["zone-a-b"]},
pods: append(slicePods["zone-a-c"], slicePods["zone-a-b"]...),
nodes: nodes,
@@ -1784,13 +1784,13 @@ func expectMetrics(t *testing.T, em expectedMetrics) {
t.Errorf("Expected endpointSliceChangesDeleted to be %d, got %v", em.numDeleted, actualDeleted)
}
actualSlicesChangedPerSync, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("disabled"))
actualSlicesChangedPerSync, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Disabled"))
handleErr(t, err, "slicesChangedPerSync")
if actualSlicesChangedPerSync != float64(em.slicesChangedPerSync) {
t.Errorf("Expected slicesChangedPerSync to be %d, got %v", em.slicesChangedPerSync, actualSlicesChangedPerSync)
}
actualSlicesChangedPerSyncTopology, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("auto"))
actualSlicesChangedPerSyncTopology, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Auto"))
handleErr(t, err, "slicesChangedPerSyncTopology")
if actualSlicesChangedPerSyncTopology != float64(em.slicesChangedPerSyncTopology) {
t.Errorf("Expected slicesChangedPerSyncTopology to be %d, got %v", em.slicesChangedPerSyncTopology, actualSlicesChangedPerSyncTopology)
@@ -1825,8 +1825,8 @@ func setupMetrics() {
metrics.EndpointSliceChanges.Delete(map[string]string{"operation": "create"})
metrics.EndpointSliceChanges.Delete(map[string]string{"operation": "update"})
metrics.EndpointSliceChanges.Delete(map[string]string{"operation": "delete"})
metrics.EndpointSlicesChangedPerSync.Delete(map[string]string{"topology": "disabled"})
metrics.EndpointSlicesChangedPerSync.Delete(map[string]string{"topology": "auto"})
metrics.EndpointSlicesChangedPerSync.Delete(map[string]string{"topology": "Disabled"})
metrics.EndpointSlicesChangedPerSync.Delete(map[string]string{"topology": "Auto"})
metrics.EndpointSliceSyncs.Delete(map[string]string{"result": "success"})
metrics.EndpointSliceSyncs.Delete(map[string]string{"result": "error"})
}

View File

@@ -386,11 +386,11 @@ func unchangedSlices(existingSlices, slicesToUpdate, slicesToDelete []*discovery
}
// hintsEnabled returns true if the provided annotations include a
// corev1.AnnotationTopologyAwareHints key with a value set to "auto".
// corev1.AnnotationTopologyAwareHints key with a value set to "Auto" or "auto".
func hintsEnabled(annotations map[string]string) bool {
val, ok := annotations[corev1.AnnotationTopologyAwareHints]
if !ok {
return false
}
return val == "auto"
return val == "Auto" || val == "auto"
}

View File

@@ -1165,3 +1165,46 @@ func TestSupportedServiceAddressType(t *testing.T) {
})
}
}
func Test_hintsEnabled(t *testing.T) {
testCases := []struct {
name string
annotations map[string]string
expectEnabled bool
}{{
name: "empty annotations",
expectEnabled: false,
}, {
name: "different annotations",
annotations: map[string]string{"topology-hints": "enabled"},
expectEnabled: false,
}, {
name: "annotation == enabled",
annotations: map[string]string{v1.AnnotationTopologyAwareHints: "enabled"},
expectEnabled: false,
}, {
name: "annotation == aUto",
annotations: map[string]string{v1.AnnotationTopologyAwareHints: "aUto"},
expectEnabled: false,
}, {
name: "annotation == auto",
annotations: map[string]string{v1.AnnotationTopologyAwareHints: "auto"},
expectEnabled: true,
}, {
name: "annotation == Auto",
annotations: map[string]string{v1.AnnotationTopologyAwareHints: "Auto"},
expectEnabled: true,
}, {
name: "annotation == disabled",
annotations: map[string]string{v1.AnnotationTopologyAwareHints: "disabled"},
expectEnabled: false,
}}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
actualEnabled := hintsEnabled(tc.annotations)
if actualEnabled != tc.expectEnabled {
t.Errorf("Expected %t, got %t", tc.expectEnabled, actualEnabled)
}
})
}
}