Skip to content

Commit c9b97dc

Browse files
liggittliggitt
committed
Drop round-trip annotations in HPA conversion
1 parent 33357ef commit c9b97dc

File tree

9 files changed

+949
-55
lines changed

9 files changed

+949
-55
lines changed

‎pkg/apis/autoscaling/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ go_library(
1010
srcs = [
1111
"annotations.go",
1212
"doc.go",
13+
"helpers.go",
1314
"register.go",
1415
"types.go",
1516
"zz_generated.deepcopy.go",

‎pkg/apis/autoscaling/helpers.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
Copyright 2020 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package autoscaling
18+
19+
// DropRoundTripHorizontalPodAutoscalerAnnotations removes any annotations used to serialize round-tripped fields from later API versions,
20+
// and returns false if no changes were made and the original input object was returned.
21+
// It should always be called when converting internal -> external versions, prior
22+
// to setting any of the custom annotations:
23+
//
24+
// annotations, copiedAnnotations := DropRoundTripHorizontalPodAutoscalerAnnotations(externalObj.Annotations)
25+
// externalObj.Annotations = annotations
26+
//
27+
// if internal.SomeField != nil {
28+
// if !copiedAnnotations {
29+
// externalObj.Annotations = DeepCopyStringMap(externalObj.Annotations)
30+
// copiedAnnotations = true
31+
// }
32+
// externalObj.Annotations[...] = json.Marshal(...)
33+
// }
34+
func DropRoundTripHorizontalPodAutoscalerAnnotations(in map[string]string) (out map[string]string, copied bool) {
35+
_, hasMetricsSpecs := in[MetricSpecsAnnotation]
36+
_, hasMetricsStatuses := in[MetricStatusesAnnotation]
37+
_, hasConditions := in[HorizontalPodAutoscalerConditionsAnnotation]
38+
if hasMetricsSpecs || hasMetricsStatuses || hasConditions {
39+
out = DeepCopyStringMap(in)
40+
delete(out, MetricSpecsAnnotation)
41+
delete(out, MetricStatusesAnnotation)
42+
delete(out, HorizontalPodAutoscalerConditionsAnnotation)
43+
return out, true
44+
}
45+
return in, false
46+
}
47+
48+
// DeepCopyStringMap returns a copy of the input map.
49+
// If input is nil, an empty map is returned.
50+
func DeepCopyStringMap(in map[string]string) map[string]string {
51+
out := make(map[string]string, len(in))
52+
for k, v := range in {
53+
out[k] = v
54+
}
55+
return out
56+
}

‎pkg/apis/autoscaling/v1/conversion.go

Lines changed: 45 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,10 @@ func Convert_autoscaling_HorizontalPodAutoscaler_To_v1_HorizontalPodAutoscaler(i
294294
return err
295295
}
296296

297+
// clear any pre-existing round-trip annotations to make sure the only ones set are ones we produced during conversion
298+
annotations, copiedAnnotations := autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations)
299+
out.Annotations = annotations
300+
297301
otherMetrics := make([]autoscalingv1.MetricSpec, 0, len(in.Spec.Metrics))
298302
for _, metric := range in.Spec.Metrics {
299303
if metric.Type == autoscaling.ResourceMetricSourceType && metric.Resource != nil && metric.Resource.Name == core.ResourceCPU && metric.Resource.Target.AverageUtilization != nil {
@@ -323,19 +327,16 @@ func Convert_autoscaling_HorizontalPodAutoscaler_To_v1_HorizontalPodAutoscaler(i
323327
}
324328
}
325329

326-
if len(otherMetrics) > 0 || len(in.Status.CurrentMetrics) > 0 || len(currentConditions) > 0 {
327-
old := out.Annotations
328-
out.Annotations = make(map[string]string, len(old)+3)
329-
for k, v := range old {
330-
out.Annotations[k] = v
331-
}
332-
}
333-
334330
if len(otherMetrics) > 0 {
335331
otherMetricsEnc, err := json.Marshal(otherMetrics)
336332
if err != nil {
337333
return err
338334
}
335+
// copy before mutating
336+
if !copiedAnnotations {
337+
copiedAnnotations = true
338+
out.Annotations = autoscaling.DeepCopyStringMap(out.Annotations)
339+
}
339340
out.Annotations[autoscaling.MetricSpecsAnnotation] = string(otherMetricsEnc)
340341
}
341342

@@ -344,6 +345,11 @@ func Convert_autoscaling_HorizontalPodAutoscaler_To_v1_HorizontalPodAutoscaler(i
344345
if err != nil {
345346
return err
346347
}
348+
// copy before mutating
349+
if !copiedAnnotations {
350+
copiedAnnotations = true
351+
out.Annotations = autoscaling.DeepCopyStringMap(out.Annotations)
352+
}
347353
out.Annotations[autoscaling.MetricStatusesAnnotation] = string(currentMetricsEnc)
348354
}
349355

@@ -352,6 +358,11 @@ func Convert_autoscaling_HorizontalPodAutoscaler_To_v1_HorizontalPodAutoscaler(i
352358
if err != nil {
353359
return err
354360
}
361+
// copy before mutating
362+
if !copiedAnnotations {
363+
copiedAnnotations = true
364+
out.Annotations = autoscaling.DeepCopyStringMap(out.Annotations)
365+
}
355366
out.Annotations[autoscaling.HorizontalPodAutoscalerConditionsAnnotation] = string(currentConditionsEnc)
356367
}
357368

@@ -365,38 +376,32 @@ func Convert_v1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(i
365376

366377
if otherMetricsEnc, hasOtherMetrics := out.Annotations[autoscaling.MetricSpecsAnnotation]; hasOtherMetrics {
367378
var otherMetrics []autoscalingv1.MetricSpec
368-
if err := json.Unmarshal([]byte(otherMetricsEnc), &otherMetrics); err != nil {
369-
return err
370-
}
371-
372-
// the normal Spec conversion could have populated out.Spec.Metrics with a single element, so deal with that
373-
outMetrics := make([]autoscaling.MetricSpec, len(otherMetrics)+len(out.Spec.Metrics))
374-
for i, metric := range otherMetrics {
375-
if err := Convert_v1_MetricSpec_To_autoscaling_MetricSpec(&metric, &outMetrics[i], s); err != nil {
376-
return err
379+
if err := json.Unmarshal([]byte(otherMetricsEnc), &otherMetrics); err == nil {
380+
// the normal Spec conversion could have populated out.Spec.Metrics with a single element, so deal with that
381+
outMetrics := make([]autoscaling.MetricSpec, len(otherMetrics)+len(out.Spec.Metrics))
382+
for i, metric := range otherMetrics {
383+
if err := Convert_v1_MetricSpec_To_autoscaling_MetricSpec(&metric, &outMetrics[i], s); err != nil {
384+
return err
385+
}
377386
}
387+
if out.Spec.Metrics != nil {
388+
outMetrics[len(otherMetrics)] = out.Spec.Metrics[0]
389+
}
390+
out.Spec.Metrics = outMetrics
378391
}
379-
if out.Spec.Metrics != nil {
380-
outMetrics[len(otherMetrics)] = out.Spec.Metrics[0]
381-
}
382-
out.Spec.Metrics = outMetrics
383-
delete(out.Annotations, autoscaling.MetricSpecsAnnotation)
384392
}
385393

386394
if currentMetricsEnc, hasCurrentMetrics := out.Annotations[autoscaling.MetricStatusesAnnotation]; hasCurrentMetrics {
387395
// ignore any existing status values -- the ones here have more information
388396
var currentMetrics []autoscalingv1.MetricStatus
389-
if err := json.Unmarshal([]byte(currentMetricsEnc), &currentMetrics); err != nil {
390-
return err
391-
}
392-
393-
out.Status.CurrentMetrics = make([]autoscaling.MetricStatus, len(currentMetrics))
394-
for i, currentMetric := range currentMetrics {
395-
if err := Convert_v1_MetricStatus_To_autoscaling_MetricStatus(&currentMetric, &out.Status.CurrentMetrics[i], s); err != nil {
396-
return err
397+
if err := json.Unmarshal([]byte(currentMetricsEnc), &currentMetrics); err == nil {
398+
out.Status.CurrentMetrics = make([]autoscaling.MetricStatus, len(currentMetrics))
399+
for i, currentMetric := range currentMetrics {
400+
if err := Convert_v1_MetricStatus_To_autoscaling_MetricStatus(&currentMetric, &out.Status.CurrentMetrics[i], s); err != nil {
401+
return err
402+
}
397403
}
398404
}
399-
delete(out.Annotations, autoscaling.MetricStatusesAnnotation)
400405
}
401406

402407
// autoscaling/v1 formerly had an implicit default applied in the controller. In v2beta1, we apply it explicitly.
@@ -420,19 +425,19 @@ func Convert_v1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(i
420425

421426
if currentConditionsEnc, hasCurrentConditions := out.Annotations[autoscaling.HorizontalPodAutoscalerConditionsAnnotation]; hasCurrentConditions {
422427
var currentConditions []autoscalingv1.HorizontalPodAutoscalerCondition
423-
if err := json.Unmarshal([]byte(currentConditionsEnc), &currentConditions); err != nil {
424-
return err
425-
}
426-
427-
out.Status.Conditions = make([]autoscaling.HorizontalPodAutoscalerCondition, len(currentConditions))
428-
for i, currentCondition := range currentConditions {
429-
if err := Convert_v1_HorizontalPodAutoscalerCondition_To_autoscaling_HorizontalPodAutoscalerCondition(&currentCondition, &out.Status.Conditions[i], s); err != nil {
430-
return err
428+
if err := json.Unmarshal([]byte(currentConditionsEnc), &currentConditions); err == nil {
429+
out.Status.Conditions = make([]autoscaling.HorizontalPodAutoscalerCondition, len(currentConditions))
430+
for i, currentCondition := range currentConditions {
431+
if err := Convert_v1_HorizontalPodAutoscalerCondition_To_autoscaling_HorizontalPodAutoscalerCondition(&currentCondition, &out.Status.Conditions[i], s); err != nil {
432+
return err
433+
}
431434
}
432435
}
433-
delete(out.Annotations, autoscaling.HorizontalPodAutoscalerConditionsAnnotation)
434436
}
435437

438+
// drop round-tripping annotations after converting to internal
439+
out.Annotations, _ = autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations)
440+
436441
return nil
437442
}
438443

‎pkg/apis/autoscaling/v2beta1/conversion.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,3 +289,30 @@ func Convert_v2beta1_PodsMetricStatus_To_autoscaling_PodsMetricStatus(in *autosc
289289
}
290290
return nil
291291
}
292+
293+
func Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler(in *autoscaling.HorizontalPodAutoscaler, out *autoscalingv2beta1.HorizontalPodAutoscaler, s conversion.Scope) error {
294+
if err := autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler(in, out, s); err != nil {
295+
return err
296+
}
297+
298+
// clear any pre-existing round-trip annotations to make sure the only ones set are ones we produced during conversion
299+
annotations, _ := autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations)
300+
out.Annotations = annotations
301+
302+
return nil
303+
}
304+
305+
func Convert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in *autoscalingv2beta1.HorizontalPodAutoscaler, out *autoscaling.HorizontalPodAutoscaler, s conversion.Scope) error {
306+
if err := autoConvert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in, out, s); err != nil {
307+
return err
308+
}
309+
310+
// drop round-tripping annotations after converting to internal
311+
out.Annotations, _ = autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations)
312+
313+
return nil
314+
}
315+
316+
func Convert_autoscaling_HorizontalPodAutoscalerSpec_To_v2beta1_HorizontalPodAutoscalerSpec(in *autoscaling.HorizontalPodAutoscalerSpec, out *autoscalingv2beta1.HorizontalPodAutoscalerSpec, s conversion.Scope) error {
317+
return autoConvert_autoscaling_HorizontalPodAutoscalerSpec_To_v2beta1_HorizontalPodAutoscalerSpec(in, out, s)
318+
}

‎pkg/apis/autoscaling/v2beta1/zz_generated.conversion.go

Lines changed: 15 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/apis/autoscaling/v2beta2/BUILD

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")
33
go_library(
44
name = "go_default_library",
55
srcs = [
6+
"conversion.go",
67
"defaults.go",
78
"doc.go",
89
"register.go",
@@ -13,8 +14,12 @@ go_library(
1314
visibility = ["//visibility:public"],
1415
deps = [
1516
"//pkg/apis/autoscaling:go_default_library",
17+
"//pkg/apis/core:go_default_library",
1618
"//staging/src/k8s.io/api/autoscaling/v2beta2:go_default_library",
1719
"//staging/src/k8s.io/api/core/v1:go_default_library",
20+
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
21+
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
22+
"//staging/src/k8s.io/apimachinery/pkg/conversion:go_default_library",
1823
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
1924
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
2025
],

‎pkg/apis/autoscaling/v2beta2/conversion.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
Copyright 2020 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v2beta2
18+
19+
import (
20+
autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2"
21+
22+
"k8s.io/apimachinery/pkg/conversion"
23+
"k8s.io/kubernetes/pkg/apis/autoscaling"
24+
)
25+
26+
func Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(in *autoscaling.HorizontalPodAutoscaler, out *autoscalingv2beta2.HorizontalPodAutoscaler, s conversion.Scope) error {
27+
if err := autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(in, out, s); err != nil {
28+
return err
29+
}
30+
// v2beta2 round-trips to internal without any serialized annotations, make sure any from other versions don't get serialized
31+
out.Annotations, _ = autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations)
32+
return nil
33+
}
34+
35+
func Convert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in *autoscalingv2beta2.HorizontalPodAutoscaler, out *autoscaling.HorizontalPodAutoscaler, s conversion.Scope) error {
36+
if err := autoConvert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in, out, s); err != nil {
37+
return err
38+
}
39+
// v2beta2 round-trips to internal without any serialized annotations, make sure any from other versions don't get serialized
40+
out.Annotations, _ = autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations)
41+
return nil
42+
}

‎pkg/apis/autoscaling/v2beta2/doc.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ limitations under the License.
1515
*/
1616

1717
// +k8s:conversion-gen=k8s.io/kubernetes/pkg/apis/autoscaling
18+
// +k8s:conversion-gen-external-types=k8s.io/api/autoscaling/v2beta2
1819
// +k8s:defaulter-gen=TypeMeta
1920

2021
package v2beta2 // import "k8s.io/kubernetes/pkg/apis/autoscaling/v2beta2"

0 commit comments

Comments
 (0)