Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete support for v1beta1 API w/o deleting the API itself. #1783

Merged
merged 1 commit into from
Mar 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package input

import (
"flag"
"fmt"
"time"

Expand Down Expand Up @@ -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 {

Expand Down Expand Up @@ -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},
}
}
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,17 @@ 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) {

type testCase struct {
legacySelector labels.Selector
selector labels.Selector
beta1APIDeprecated bool
fetchSelectorError error
expectedSelector labels.Selector
expectedConfigUnsupported *string
Expand All @@ -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,
Expand All @@ -69,7 +67,6 @@ func TestLegacySelector(t *testing.T) {
{
legacySelector: nil,
selector: nil,
beta1APIDeprecated: true,
fetchSelectorError: nil,
expectedSelector: labels.Nothing(),
expectedConfigUnsupported: &unsupportedConditionNoExtraText,
Expand All @@ -78,65 +75,21 @@ 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,
expectedConfigDeprecated: nil,
}, {
legacySelector: parseLabelSelector("app = test1"),
selector: parseLabelSelector("app = test2"),
beta1APIDeprecated: false,
fetchSelectorError: nil,
expectedSelector: labels.Nothing(),
expectedConfigUnsupported: &unsupportedConditionBothDefined,
Expand All @@ -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)
Expand Down