Use labels hash in the custom metrics store

The current implementation of the metrics store for custom metrics uses
just the object name as key. When scenarios where two HPAs reference the
same object but have different metric calculation (e.g. ingresses with
different weights), kube-metrics-adapter calculates the two metrics but
always overrides one of the metrics when saving it to the store.

This commit implements the use of a labels hash in the custom metrics
store. This way, metrics are identified not just by the referenced
object, but also by its selectors.

Co-authored-by: Katyanna Moura <amelie.kn@gmail.com>
Signed-off-by: Jonathan Juares Beber <jonathanbeber@gmail.com>
This commit is contained in:
Jonathan Juares Beber 2021-11-22 21:23:12 +01:00
parent e600557636
commit 0bf8f5dd0f
No known key found for this signature in database
GPG Key ID: 41D3F4ACE4465751
3 changed files with 163 additions and 52 deletions

View File

@ -269,7 +269,7 @@ func (p *HPAProvider) collectMetrics(ctx context.Context) {
// GetMetricByName gets a single metric by name.
func (p *HPAProvider) GetMetricByName(name types.NamespacedName, info provider.CustomMetricInfo, metricSelector labels.Selector) (*custom_metrics.MetricValue, error) {
metric := p.metricStore.GetMetricsByName(name, info)
metric := p.metricStore.GetMetricsByName(name, info, metricSelector)
if metric == nil {
return nil, provider.NewMetricNotFoundForError(info.GroupResource, info.Metric, name.Name)
}

View File

@ -32,7 +32,7 @@ type externalMetricsStoredMetric struct {
// MetricStore is a simple in-memory Metrics Store for HPA metrics.
type MetricStore struct {
// metricName -> referencedResource -> objectNamespace -> objectName -> metric
customMetricsStore map[string]map[schema.GroupResource]map[string]map[string]customMetricsStoredMetric
customMetricsStore map[string]map[schema.GroupResource]map[string]map[string]map[string]customMetricsStoredMetric
// namespace -> metricName -> labels -> metric
externalMetricsStore map[string]map[string]map[string]externalMetricsStoredMetric
metricsTTLCalculator func() time.Time
@ -42,7 +42,7 @@ type MetricStore struct {
// NewMetricStore initializes an empty Metrics Store.
func NewMetricStore(ttlCalculator func() time.Time) *MetricStore {
return &MetricStore{
customMetricsStore: make(map[string]map[schema.GroupResource]map[string]map[string]customMetricsStoredMetric, 0),
customMetricsStore: make(map[string]map[schema.GroupResource]map[string]map[string]map[string]customMetricsStoredMetric, 0),
externalMetricsStore: make(map[string]map[string]map[string]externalMetricsStoredMetric, 0),
metricsTTLCalculator: ttlCalculator,
}
@ -119,12 +119,20 @@ func (s *MetricStore) insertCustomMetric(value custom_metrics.MetricValue) {
TTL: s.metricsTTLCalculator(), // TODO: make TTL configurable
}
selector := value.Metric.Selector
labelsKey := ""
if selector != nil {
labelsKey = hashLabelMap(selector.MatchLabels)
}
metrics, ok := s.customMetricsStore[value.Metric.Name]
if !ok {
s.customMetricsStore[value.Metric.Name] = map[schema.GroupResource]map[string]map[string]customMetricsStoredMetric{
s.customMetricsStore[value.Metric.Name] = map[schema.GroupResource]map[string]map[string]map[string]customMetricsStoredMetric{
groupResource: {
value.DescribedObject.Namespace: map[string]customMetricsStoredMetric{
value.DescribedObject.Name: metric,
value.DescribedObject.Namespace: map[string]map[string]customMetricsStoredMetric{
value.DescribedObject.Name: map[string]customMetricsStoredMetric{
labelsKey: metric,
},
},
},
}
@ -133,23 +141,35 @@ func (s *MetricStore) insertCustomMetric(value custom_metrics.MetricValue) {
group, ok := metrics[groupResource]
if !ok {
metrics[groupResource] = map[string]map[string]customMetricsStoredMetric{
metrics[groupResource] = map[string]map[string]map[string]customMetricsStoredMetric{
value.DescribedObject.Namespace: {
value.DescribedObject.Name: metric,
value.DescribedObject.Name: map[string]customMetricsStoredMetric{
labelsKey: metric,
},
},
}
return
}
// TODO: what if an empty namespace?
namespace, ok := group[value.DescribedObject.Namespace]
if !ok {
group[value.DescribedObject.Namespace] = map[string]customMetricsStoredMetric{
value.DescribedObject.Name: metric,
group[value.DescribedObject.Namespace] = map[string]map[string]customMetricsStoredMetric{
value.DescribedObject.Name: map[string]customMetricsStoredMetric{
labelsKey: metric,
},
}
return
}
namespace[value.DescribedObject.Name] = metric
object, ok := namespace[value.DescribedObject.Name]
if !ok {
namespace[value.DescribedObject.Name] = map[string]customMetricsStoredMetric{
labelsKey: metric,
}
}
object[labelsKey] = metric
}
// insertExternalMetric inserts an external metric into the store.
@ -192,6 +212,24 @@ func hashLabelMap(labels map[string]string) string {
return strings.Join(strLabels, ",")
}
func parseHashLabelMap(s string) labels.Set {
labels := map[string]string{}
if s == "" {
return labels
}
keyValues := strings.Split(s, ",")
for _, keyValue := range keyValues {
splittedKeyValue := strings.Split(keyValue, "=")
key, value := splittedKeyValue[0], splittedKeyValue[1]
labels[key] = value
}
return labels
}
// GetMetricsBySelector gets metric from the customMetricsStore using a label selector to
// find metrics for matching resources.
func (s *MetricStore) GetMetricsBySelector(namespace string, selector labels.Selector, info provider.CustomMetricInfo) *custom_metrics.MetricValueList {
@ -212,16 +250,20 @@ func (s *MetricStore) GetMetricsBySelector(namespace string, selector labels.Sel
if !info.Namespaced {
for _, metricMap := range group {
for _, metric := range metricMap {
if selector.Matches(labels.Set(metric.Value.Metric.Selector.MatchLabels)) {
matchedMetrics = append(matchedMetrics, metric.Value)
for _, metricObject := range metricMap {
for _, metric := range metricObject {
if selector.Matches(labels.Set(metric.Value.Metric.Selector.MatchLabels)) {
matchedMetrics = append(matchedMetrics, metric.Value)
}
}
}
}
} else if metricMap, ok := group[namespace]; ok {
for _, metric := range metricMap {
if metric.Value.Metric.Selector != nil && selector.Matches(labels.Set(metric.Value.Metric.Selector.MatchLabels)) {
matchedMetrics = append(matchedMetrics, metric.Value)
for _, metricObject := range metricMap {
for _, metric := range metricObject {
if metric.Value.Metric.Selector != nil && selector.Matches(labels.Set(metric.Value.Metric.Selector.MatchLabels)) {
matchedMetrics = append(matchedMetrics, metric.Value)
}
}
}
}
@ -230,7 +272,7 @@ func (s *MetricStore) GetMetricsBySelector(namespace string, selector labels.Sel
}
// GetMetricsByName looks up metrics in the customMetricsStore by resource name.
func (s *MetricStore) GetMetricsByName(name types.NamespacedName, info provider.CustomMetricInfo) *custom_metrics.MetricValue {
func (s *MetricStore) GetMetricsByName(name types.NamespacedName, info provider.CustomMetricInfo, selector labels.Selector) *custom_metrics.MetricValue {
s.RLock()
defer s.RUnlock()
@ -247,13 +289,21 @@ func (s *MetricStore) GetMetricsByName(name types.NamespacedName, info provider.
if !info.Namespaced {
// TODO: rethink no namespace queries
for _, metricMap := range group {
if metric, ok := metricMap[name.Name]; ok {
return &metric.Value
if metricObject, ok := metricMap[name.Name]; ok {
for metric, value := range metricObject {
if selector.Matches(parseHashLabelMap(metric)) {
return &value.Value
}
}
}
}
} else if metricMap, ok := group[name.Namespace]; ok {
if metric, ok := metricMap[name.Name]; ok {
return &metric.Value
if metricObject, ok := metricMap[name.Name]; ok {
for metric, value := range metricObject {
if selector.Matches(parseHashLabelMap(metric)) {
return &value.Value
}
}
}
}
@ -331,13 +381,18 @@ func (s *MetricStore) RemoveExpired() {
// cleanup custom metrics
for metricName, groups := range s.customMetricsStore {
for group, namespaces := range groups {
for namespace, resources := range namespaces {
for resource, metric := range resources {
if metric.TTL.Before(time.Now().UTC()) {
delete(resources, resource)
for namespace, objects := range namespaces {
for object, resources := range objects {
for resource, metric := range resources {
if metric.TTL.Before(time.Now().UTC()) {
delete(resources, resource)
}
}
if len(resources) == 0 {
delete(objects, object)
}
}
if len(resources) == 0 {
if len(objects) == 0 {
delete(namespaces, namespace)
}
}

View File

@ -18,8 +18,7 @@ import (
"k8s.io/metrics/pkg/apis/external_metrics"
)
func newMetricIdentifier(metricName string) custom_metrics.MetricIdentifier {
selector := metav1.LabelSelector{}
func newMetricIdentifier(metricName string, selector metav1.LabelSelector) custom_metrics.MetricIdentifier {
return custom_metrics.MetricIdentifier{
Name: metricName,
Selector: &selector,
@ -46,7 +45,7 @@ func TestInternalMetricStorage(t *testing.T) {
insert: collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -94,7 +93,7 @@ func TestInternalMetricStorage(t *testing.T) {
insert: collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -151,7 +150,7 @@ func TestInternalMetricStorage(t *testing.T) {
insert: collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("scalingschedulename"),
Metric: newMetricIdentifier("scalingschedulename", metav1.LabelSelector{}),
Value: *resource.NewQuantity(10, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -208,7 +207,7 @@ func TestInternalMetricStorage(t *testing.T) {
insert: collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("clusterscalingschedulename"),
Metric: newMetricIdentifier("clusterscalingschedulename", metav1.LabelSelector{}),
Value: *resource.NewQuantity(10, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -265,7 +264,7 @@ func TestInternalMetricStorage(t *testing.T) {
insert: collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -307,12 +306,72 @@ func TestInternalMetricStorage(t *testing.T) {
},
},
},
{
test: "insert/list/get an Ingress metric with match labels",
insert: collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{
MatchLabels: map[string]string{
"select_key": "select_value",
},
}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
Kind: "Ingress",
APIVersion: "extensions/v1beta1",
},
},
},
expectedFound: true,
list: []provider.CustomMetricInfo{
{
GroupResource: schema.GroupResource{
Group: "extensions",
Resource: "ingresses",
},
Namespaced: false,
Metric: "metric-per-unit",
},
},
byName: struct {
name types.NamespacedName
info provider.CustomMetricInfo
}{
name: types.NamespacedName{Name: "metricObject", Namespace: ""},
info: provider.CustomMetricInfo{
GroupResource: schema.GroupResource{
Group: "extensions",
Resource: "ingresses",
},
Namespaced: false,
Metric: "metric-per-unit",
},
},
byLabel: struct {
namespace string
selector labels.Selector
info provider.CustomMetricInfo
}{
namespace: "",
selector: labels.SelectorFromSet(labels.Set{"select_key": "select_value"}),
info: provider.CustomMetricInfo{
GroupResource: schema.GroupResource{
Group: "extensions",
Resource: "ingresses",
},
Namespaced: false,
Metric: "metric-per-unit",
},
},
},
{
test: "insert/list/get an Ingress metric",
insert: collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -368,7 +427,7 @@ func TestInternalMetricStorage(t *testing.T) {
insert: collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -436,18 +495,15 @@ func TestInternalMetricStorage(t *testing.T) {
require.Equal(t, tc.list, metricInfos)
// Get the metric by name
metric := metricsStore.GetMetricsByName(tc.byName.name, tc.byName.info)
metric := metricsStore.GetMetricsByName(tc.byName.name, tc.byName.info, tc.byLabel.selector)
if tc.expectedFound {
require.Equal(t, tc.insert.Custom, *metric)
metrics := metricsStore.GetMetricsBySelector(tc.byLabel.namespace, tc.byLabel.selector, tc.byLabel.info)
require.Equal(t, tc.insert.Custom, metrics.Items[0])
} else {
require.Nil(t, metric)
metrics := metricsStore.GetMetricsBySelector(tc.byLabel.namespace, tc.byLabel.selector, tc.byLabel.info)
require.Len(t, metrics.Items, 0)
}
})
}
@ -474,7 +530,7 @@ func TestMultipleMetricValues(t *testing.T) {
{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -486,7 +542,7 @@ func TestMultipleMetricValues(t *testing.T) {
{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(1, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -543,7 +599,7 @@ func TestMultipleMetricValues(t *testing.T) {
{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -556,7 +612,7 @@ func TestMultipleMetricValues(t *testing.T) {
{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(1, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -612,7 +668,7 @@ func TestMultipleMetricValues(t *testing.T) {
metricsStore.Insert(insert)
// Get the metric by name
metric := metricsStore.GetMetricsByName(tc.byName.name, tc.byName.info)
metric := metricsStore.GetMetricsByName(tc.byName.name, tc.byName.info, tc.byLabel.selector)
require.Equal(t, insert.Custom, *metric)
// Get the metric by label
@ -677,7 +733,7 @@ func TestCustomMetricsStorageErrors(t *testing.T) {
insert: collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -741,7 +797,7 @@ func TestCustomMetricsStorageErrors(t *testing.T) {
require.Equal(t, tc.list, metricInfos)
// Get the metric by name
metric := metricsStore.GetMetricsByName(tc.byName.name, tc.byName.info)
metric := metricsStore.GetMetricsByName(tc.byName.name, tc.byName.info, tc.byLabel.selector)
require.Nil(t, metric)
metrics := metricsStore.GetMetricsBySelector(tc.byLabel.namespace, tc.byLabel.selector, tc.byLabel.info)
@ -769,7 +825,7 @@ func TestCustomMetricsStorageErrors(t *testing.T) {
{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -782,7 +838,7 @@ func TestCustomMetricsStorageErrors(t *testing.T) {
{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(1, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -795,7 +851,7 @@ func TestCustomMetricsStorageErrors(t *testing.T) {
{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(1, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -1128,7 +1184,7 @@ func TestMetricsExpiration(t *testing.T) {
customMetric := collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",
@ -1167,7 +1223,7 @@ func TestMetricsNonExpiration(t *testing.T) {
customMetric := collector.CollectedMetric{
Type: autoscalingv2.MetricSourceType("Object"),
Custom: custom_metrics.MetricValue{
Metric: newMetricIdentifier("metric-per-unit"),
Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}),
Value: *resource.NewQuantity(0, ""),
DescribedObject: custom_metrics.ObjectReference{
Name: "metricObject",