From af3594dfa5211dae5f413cfdf46c78dfc2dabdc4 Mon Sep 17 00:00:00 2001 From: Adam Malcontenti-Wilson Date: Wed, 11 May 2022 16:38:51 +1000 Subject: [PATCH] disruptioncontroller: check for scale subresource correctly --- pkg/controller/disruption/disruption.go | 20 ++++++++++++------- pkg/controller/disruption/disruption_test.go | 21 +++++++++++++++++++- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/pkg/controller/disruption/disruption.go b/pkg/controller/disruption/disruption.go index 3c8fcbab770..346a3224149 100644 --- a/pkg/controller/disruption/disruption.go +++ b/pkg/controller/disruption/disruption.go @@ -297,14 +297,13 @@ func (dc *DisruptionController) getScaleController(ctx context.Context, controll return nil, err } gr := mapping.Resource.GroupResource() - scale, err := dc.scaleNamespacer.Scales(namespace).Get(ctx, gr, controllerRef.Name, metav1.GetOptions{}) if err != nil { if errors.IsNotFound(err) { // The IsNotFound error can mean either that the resource does not exist, // or it exist but doesn't implement the scale subresource. We check which // situation we are facing so we can give an appropriate error message. - isScale, err := dc.implementsScale(gv, controllerRef.Kind) + isScale, err := dc.implementsScale(mapping.Resource) if err != nil { return nil, err } @@ -321,17 +320,24 @@ func (dc *DisruptionController) getScaleController(ctx context.Context, controll return &controllerAndScale{scale.UID, scale.Spec.Replicas}, nil } -func (dc *DisruptionController) implementsScale(gv schema.GroupVersion, kind string) (bool, error) { - resourceList, err := dc.discoveryClient.ServerResourcesForGroupVersion(gv.String()) +func (dc *DisruptionController) implementsScale(gvr schema.GroupVersionResource) (bool, error) { + resourceList, err := dc.discoveryClient.ServerResourcesForGroupVersion(gvr.GroupVersion().String()) if err != nil { return false, err } + + scaleSubresourceName := fmt.Sprintf("%s/scale", gvr.Resource) for _, resource := range resourceList.APIResources { - if resource.Kind != kind { + if resource.Name != scaleSubresourceName { continue } - if strings.HasSuffix(resource.Name, "/scale") { - return true, nil + + for _, scaleGv := range scaleclient.NewScaleConverter().ScaleVersions() { + if resource.Group == scaleGv.Group && + resource.Version == scaleGv.Version && + resource.Kind == "Scale" { + return true, nil + } } } return false, nil diff --git a/pkg/controller/disruption/disruption_test.go b/pkg/controller/disruption/disruption_test.go index b242b35d761..0a4220f6c93 100644 --- a/pkg/controller/disruption/disruption_test.go +++ b/pkg/controller/disruption/disruption_test.go @@ -663,6 +663,25 @@ func TestScaleFinderNoResource(t *testing.T) { expectError bool }{ "resource implements scale": { + apiResources: []metav1.APIResource{ + { + Kind: customGVK.Kind, + Name: resourceName + "/status", + }, + { + Kind: "Scale", + Group: autoscalingapi.GroupName, + Version: "v1", + Name: resourceName + "/scale", + }, + { + Kind: customGVK.Kind, + Name: resourceName, + }, + }, + expectError: false, + }, + "resource implements unsupported data format for scale subresource": { apiResources: []metav1.APIResource{ { Kind: customGVK.Kind, @@ -673,7 +692,7 @@ func TestScaleFinderNoResource(t *testing.T) { Name: resourceName + "/scale", }, }, - expectError: false, + expectError: true, }, "resource does not implement scale": { apiResources: []metav1.APIResource{