From dd5be2a882b4f7979a6bfabe814410f3d9a6c1d8 Mon Sep 17 00:00:00 2001 From: mattjmcnaughton Date: Mon, 25 Sep 2017 09:43:04 -0400 Subject: [PATCH] Make unnecessary hpa public funcs private Previously `pkg.controller.podautoscaler.UnsafeConvertToVersion` was exported. However, it was never used outside of the `podautoscaler` package. Make it private to prevent confusion. Additionally, move the two private functions in `horizontal.go` to be with the other private functions at the bottom of the file - imho its more readable than having them directly at the top of the file, before the public type and function definitions. --- pkg/controller/podautoscaler/horizontal.go | 48 +++++++++---------- .../podautoscaler/horizontal_test.go | 2 +- .../podautoscaler/legacy_horizontal_test.go | 2 +- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go index 726301155f..e9b1fb432f 100644 --- a/pkg/controller/podautoscaler/horizontal.go +++ b/pkg/controller/podautoscaler/horizontal.go @@ -57,28 +57,6 @@ var ( scaleUpLimitMinimum = 4.0 ) -func calculateScaleUpLimit(currentReplicas int32) int32 { - return int32(math.Max(scaleUpLimitFactor*float64(currentReplicas), scaleUpLimitMinimum)) -} - -// UnsafeConvertToVersionVia is like api.Scheme.UnsafeConvertToVersion, but it does so via an internal version first. -// We use it since working with v2alpha1 is convenient here, but we want to use the v1 client (and -// can't just use the internal version). Note that conversion mutates the object, so you need to deepcopy -// *before* you call this if the input object came out of a shared cache. -func UnsafeConvertToVersionVia(obj runtime.Object, externalVersion schema.GroupVersion) (runtime.Object, error) { - objInt, err := api.Scheme.UnsafeConvertToVersion(obj, schema.GroupVersion{Group: externalVersion.Group, Version: runtime.APIVersionInternal}) - if err != nil { - return nil, fmt.Errorf("failed to convert the given object to the internal version: %v", err) - } - - objExt, err := api.Scheme.UnsafeConvertToVersion(objInt, externalVersion) - if err != nil { - return nil, fmt.Errorf("failed to convert the given object back to the external version: %v", err) - } - - return objExt, err -} - // HorizontalController is responsible for the synchronizing HPA objects stored // in the system with the actual deployments/replication controllers they // control. @@ -365,7 +343,7 @@ func (a *HorizontalController) reconcileAutoscaler(hpav1Shared *autoscalingv1.Ho // make a copy so that we never mutate the shared informer cache (conversion can mutate the object) hpav1 := hpav1Shared.DeepCopy() // then, convert to autoscaling/v2, which makes our lives easier when calculating metrics - hpaRaw, err := UnsafeConvertToVersionVia(hpav1, autoscalingv2.SchemeGroupVersion) + hpaRaw, err := unsafeConvertToVersionVia(hpav1, autoscalingv2.SchemeGroupVersion) if err != nil { a.eventRecorder.Event(hpav1, v1.EventTypeWarning, "FailedConvertHPA", err.Error()) return fmt.Errorf("failed to convert the given HPA to %s: %v", autoscalingv2.SchemeGroupVersion.String(), err) @@ -577,7 +555,7 @@ func (a *HorizontalController) updateStatusIfNeeded(oldStatus *autoscalingv2.Hor // updateStatus actually does the update request for the status of the given HPA func (a *HorizontalController) updateStatus(hpa *autoscalingv2.HorizontalPodAutoscaler) error { // convert back to autoscalingv1 - hpaRaw, err := UnsafeConvertToVersionVia(hpa, autoscalingv1.SchemeGroupVersion) + hpaRaw, err := unsafeConvertToVersionVia(hpa, autoscalingv1.SchemeGroupVersion) if err != nil { a.eventRecorder.Event(hpa, v1.EventTypeWarning, "FailedConvertHPA", err.Error()) return fmt.Errorf("failed to convert the given HPA to %s: %v", autoscalingv2.SchemeGroupVersion.String(), err) @@ -593,6 +571,24 @@ func (a *HorizontalController) updateStatus(hpa *autoscalingv2.HorizontalPodAuto return nil } +// unsafeConvertToVersionVia is like api.Scheme.UnsafeConvertToVersion, but it does so via an internal version first. +// We use it since working with v2alpha1 is convenient here, but we want to use the v1 client (and +// can't just use the internal version). Note that conversion mutates the object, so you need to deepcopy +// *before* you call this if the input object came out of a shared cache. +func unsafeConvertToVersionVia(obj runtime.Object, externalVersion schema.GroupVersion) (runtime.Object, error) { + objInt, err := api.Scheme.UnsafeConvertToVersion(obj, schema.GroupVersion{Group: externalVersion.Group, Version: runtime.APIVersionInternal}) + if err != nil { + return nil, fmt.Errorf("failed to convert the given object to the internal version: %v", err) + } + + objExt, err := api.Scheme.UnsafeConvertToVersion(objInt, externalVersion) + if err != nil { + return nil, fmt.Errorf("failed to convert the given object back to the external version: %v", err) + } + + return objExt, err +} + // setCondition sets the specific condition type on the given HPA to the specified value with the given reason // and message. The message and args are treated like a format string. The condition will be added if it is // not present. @@ -631,3 +627,7 @@ func setConditionInList(inputList []autoscalingv2.HorizontalPodAutoscalerConditi return resList } + +func calculateScaleUpLimit(currentReplicas int32) int32 { + return int32(math.Max(scaleUpLimitFactor*float64(currentReplicas), scaleUpLimitMinimum)) +} diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go index 98a48ea4f2..2d708318a3 100644 --- a/pkg/controller/podautoscaler/horizontal_test.go +++ b/pkg/controller/podautoscaler/horizontal_test.go @@ -232,7 +232,7 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa } // and... convert to autoscaling v1 to return the right type - objv1, err := UnsafeConvertToVersionVia(obj, autoscalingv1.SchemeGroupVersion) + objv1, err := unsafeConvertToVersionVia(obj, autoscalingv1.SchemeGroupVersion) if err != nil { return true, nil, err } diff --git a/pkg/controller/podautoscaler/legacy_horizontal_test.go b/pkg/controller/podautoscaler/legacy_horizontal_test.go index d1fb896f70..1a942fb100 100644 --- a/pkg/controller/podautoscaler/legacy_horizontal_test.go +++ b/pkg/controller/podautoscaler/legacy_horizontal_test.go @@ -200,7 +200,7 @@ func (tc *legacyTestCase) prepareTestClient(t *testing.T) *fake.Clientset { } // and... convert to autoscaling v1 to return the right type - objv1, err := UnsafeConvertToVersionVia(obj, autoscalingv1.SchemeGroupVersion) + objv1, err := unsafeConvertToVersionVia(obj, autoscalingv1.SchemeGroupVersion) if err != nil { return true, nil, err }