From b42b46bff07fe4414ff935f2298643b88c3dfea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Go=C5=82=C4=85b?= Date: Tue, 12 Mar 2019 14:05:09 +0100 Subject: [PATCH] Delete support for v1beta1 API w/o deleting the API itself. --- .../pkg/recommender/input/cluster_feeder.go | 16 ++--- .../recommender/input/cluster_feeder_test.go | 60 ++----------------- 2 files changed, 10 insertions(+), 66 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go index d9923caf3f9..6f2ebf95189 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go @@ -17,7 +17,6 @@ limitations under the License. package input import ( - "flag" "fmt" "time" @@ -52,10 +51,6 @@ const ( defaultResyncPeriod time.Duration = 10 * time.Minute ) -var ( - beta1APIDeprecated = flag.Bool("beta1-api-deprecated", true, `If v1beta1 API objects should be marked as deprecated.`) -) - // ClusterStateFeeder can update state of ClusterState object. type ClusterStateFeeder interface { @@ -432,7 +427,7 @@ func (feeder *clusterStateFeeder) getSelector(vpa *vpa_types.VerticalPodAutoscal if selector != nil { if legacySelector != nil { return labels.Nothing(), []condition{ - {conditionType: vpa_types.ConfigUnsupported, delete: false, message: "Both targetRef and label selector defined. Please reomve label selector"}, + {conditionType: vpa_types.ConfigUnsupported, delete: false, message: "Both targetRef and label selector defined. Please remove label selector"}, {conditionType: vpa_types.ConfigDeprecated, delete: true}, } } @@ -442,13 +437,10 @@ func (feeder *clusterStateFeeder) getSelector(vpa *vpa_types.VerticalPodAutoscal } } if legacySelector != nil { - if *beta1APIDeprecated { - return legacySelector, []condition{ - {conditionType: vpa_types.ConfigUnsupported, delete: true}, - {conditionType: vpa_types.ConfigDeprecated, delete: false, message: "Deprecated label selector defined, please migrate to targetRef"}, - } + return labels.Nothing(), []condition{ + {conditionType: vpa_types.ConfigUnsupported, delete: false, message: "Label selector is no longer supported, please migrate to targetRef"}, + {conditionType: vpa_types.ConfigDeprecated, delete: true}, } - return legacySelector, []condition{} } msg := "Cannot read targetRef" if fetchErr != nil { diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go index 4593794d6be..907870f7601 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go @@ -38,10 +38,10 @@ func parseLabelSelector(selector string) labels.Selector { } var ( - deprecatedConditionText = "Deprecated label selector defined, please migrate to targetRef" - unsupportedConditionTextFromFetcher = "Cannot read targetRef. Reason: targetRef not defined" - unsupportedConditionNoExtraText = "Cannot read targetRef" - unsupportedConditionBothDefined = "Both targetRef and label selector defined. Please reomve label selector" + unsupportedConditionNoLongerSupported = "Label selector is no longer supported, please migrate to targetRef" + unsupportedConditionTextFromFetcher = "Cannot read targetRef. Reason: targetRef not defined" + unsupportedConditionNoExtraText = "Cannot read targetRef" + unsupportedConditionBothDefined = "Both targetRef and label selector defined. Please remove label selector" ) func TestLegacySelector(t *testing.T) { @@ -49,7 +49,6 @@ func TestLegacySelector(t *testing.T) { type testCase struct { legacySelector labels.Selector selector labels.Selector - beta1APIDeprecated bool fetchSelectorError error expectedSelector labels.Selector expectedConfigUnsupported *string @@ -60,7 +59,6 @@ func TestLegacySelector(t *testing.T) { { legacySelector: nil, selector: nil, - beta1APIDeprecated: true, fetchSelectorError: fmt.Errorf("targetRef not defined"), expectedSelector: labels.Nothing(), expectedConfigUnsupported: &unsupportedConditionTextFromFetcher, @@ -69,7 +67,6 @@ func TestLegacySelector(t *testing.T) { { legacySelector: nil, selector: nil, - beta1APIDeprecated: true, fetchSelectorError: nil, expectedSelector: labels.Nothing(), expectedConfigUnsupported: &unsupportedConditionNoExtraText, @@ -78,57 +75,14 @@ func TestLegacySelector(t *testing.T) { { legacySelector: parseLabelSelector("app = test"), selector: nil, - beta1APIDeprecated: true, fetchSelectorError: fmt.Errorf("targetRef not defined"), - expectedSelector: parseLabelSelector("app = test"), - expectedConfigUnsupported: nil, - expectedConfigDeprecated: &deprecatedConditionText, - }, { - legacySelector: nil, - selector: parseLabelSelector("app = test"), - beta1APIDeprecated: true, - fetchSelectorError: nil, - expectedSelector: parseLabelSelector("app = test"), - expectedConfigUnsupported: nil, - expectedConfigDeprecated: nil, - }, { - legacySelector: parseLabelSelector("app = test1"), - selector: parseLabelSelector("app = test2"), - beta1APIDeprecated: true, - fetchSelectorError: nil, expectedSelector: labels.Nothing(), - expectedConfigUnsupported: &unsupportedConditionBothDefined, - expectedConfigDeprecated: nil, - }, { - legacySelector: nil, - selector: nil, - beta1APIDeprecated: false, - fetchSelectorError: fmt.Errorf("targetRef not defined"), - expectedSelector: labels.Nothing(), - expectedConfigUnsupported: &unsupportedConditionTextFromFetcher, - expectedConfigDeprecated: nil, - }, - { - legacySelector: nil, - selector: nil, - beta1APIDeprecated: false, - fetchSelectorError: nil, - expectedSelector: labels.Nothing(), - expectedConfigUnsupported: &unsupportedConditionNoExtraText, - expectedConfigDeprecated: nil, - }, - { - legacySelector: parseLabelSelector("app = test"), - selector: nil, - beta1APIDeprecated: false, - fetchSelectorError: fmt.Errorf("targetRef not defined"), - expectedSelector: parseLabelSelector("app = test"), - expectedConfigUnsupported: nil, + expectedConfigUnsupported: &unsupportedConditionNoLongerSupported, expectedConfigDeprecated: nil, }, { + // the only valid option since v1beta1 removal legacySelector: nil, selector: parseLabelSelector("app = test"), - beta1APIDeprecated: false, fetchSelectorError: nil, expectedSelector: parseLabelSelector("app = test"), expectedConfigUnsupported: nil, @@ -136,7 +90,6 @@ func TestLegacySelector(t *testing.T) { }, { legacySelector: parseLabelSelector("app = test1"), selector: parseLabelSelector("app = test2"), - beta1APIDeprecated: false, fetchSelectorError: nil, expectedSelector: labels.Nothing(), expectedConfigUnsupported: &unsupportedConditionBothDefined, @@ -153,7 +106,6 @@ func TestLegacySelector(t *testing.T) { vpa := test.VerticalPodAutoscaler().WithName("testVpa").WithContainer("container").WithNamespace("testNamespace").Get() vpaLister := &test.VerticalPodAutoscalerListerMock{} vpaLister.On("List").Return([]*vpa_types.VerticalPodAutoscaler{vpa}, nil) - *beta1APIDeprecated = tc.beta1APIDeprecated legacyTargetSelectorFetcher := target_mock.NewMockVpaTargetSelectorFetcher(ctrl) targetSelectorFetcher := target_mock.NewMockVpaTargetSelectorFetcher(ctrl)