Skip to content

Commit

Permalink
Merge pull request #1783 from kgolab/kg-vpa-remove-v1beta1
Browse files Browse the repository at this point in the history 
Delete support for v1beta1 API w/o deleting the API itself.
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Mar 12, 2019
2 parents 951239d + b42b46b commit 9733fe0
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 66 deletions.
16 changes: 4 additions & 12 deletions vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go
View file
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
60 changes: 6 additions & 54 deletions vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go
View file
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

0 comments on commit 9733fe0

Please sign in to comment.