Skip to content

fix: handle HTTPScaledObject not found gracefully in scaler #1488

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.

Sign up for GitHub

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

Open
Fedosin wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: handle HTTPScaledObject not found gracefully in scaler #1488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ This changelog keeps track of work items that have been completed and are ready
### Fixes

- **General**: Fix `roundToNDigits` using `math.Round` instead of `math.Floor` to correctly round small negative floating-point errors to zero ([#1483](https://github.com/kedacore/http-add-on/pull/1483))
- **Scaler**: Handle HTTPScaledObject not found gracefully in `GetMetrics` to avoid error storms during rapid deployments ([#1487](https://github.com/kedacore/http-add-on/issues/1487))

### Deprecations

Expand Down
20 changes: 16 additions & 4 deletions scaler/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/go-logr/logr"
"github.com/kedacore/keda/v2/pkg/scalers/externalscaler"
"google.golang.org/protobuf/types/known/emptypb"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -200,16 +201,27 @@ func (e *impl) GetMetrics(ctx context.Context, metricRequest *externalscaler.Get
return nil, err
}

// generated the metric name for HTTPScaledObject
namespacedName := k8s.NamespacedNameFromNameAndNamespace(httpScaledObjectName, sor.Namespace)
metricName := MetricName(namespacedName)
Comment on lines +204 to +206
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the comment reads "generated the metric name..." but this code is generating it now; consider changing to "generate the metric name..." for grammatical correctness (same phrasing appears in a few places in this file).

Copilot uses AI. Check for mistakes.

httpso := &httpv1alpha1.HTTPScaledObject{}
if err := e.reader.Get(ctx, types.NamespacedName{Namespace: sor.Namespace, Name: httpScaledObjectName}, httpso); err != nil {
Copy link
Copy Markdown
Member

@linkvt linkvt Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpicks:

  • we have types.NamespacedName{Namespace: sor.Namespace, Name: httpScaledObjectName} here and above the k8s.NamespacedNameFromNameAndNamespace(...) which also returns a NamespacedName
  • not yet available in cache is maybe a bit specific?

if apierrors.IsNotFound(err) {
lggr.V(1).Info("HTTPScaledObject not yet available in cache, returning 0", "httpScaledObjectName", httpScaledObjectName, "namespace", sor.Namespace)
return &externalscaler.GetMetricsResponse{
MetricValues: []*externalscaler.MetricValue{
{
MetricName: metricName,
MetricValue: 0,
},
},
}, nil
}
Comment on lines 209 to +220
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new NotFound handling path (returning a 0 metric value with nil error) changes behavior in a way that's easy to regress. Please add a unit test that exercises e.reader.Get returning a NotFound (e.g., fake client with no HTTPScaledObject) and asserts GetMetrics returns MetricValue=0 with no error (and ideally that StreamIsActive/IsActive remains healthy).

Copilot uses AI. Check for mistakes.
lggr.Error(err, "unable to get HTTPScaledObject", "name", httpScaledObjectName, "namespace", sor.Namespace, "httpScaledObjectName", httpScaledObjectName)
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error log fields here are inconsistent and redundant: the key "name" is set to httpScaledObjectName, and "httpScaledObjectName" is also provided with the same value. Elsewhere in this file (e.g., GetMetricSpec) "name" refers to the ScaledObject name (sor.Name). Consider using a distinct key for the ScaledObject name (or set "name" to sor.Name) and keep only one field for the HTTPScaledObject name to avoid confusing logs.

Suggested change
lggr.Error(err, "unable to get HTTPScaledObject", "name", httpScaledObjectName, "namespace", sor.Namespace, "httpScaledObjectName", httpScaledObjectName)
lggr.Error(err, "unable to get HTTPScaledObject", "name", sor.Name, "namespace", sor.Namespace, "httpScaledObjectName", httpScaledObjectName)

Copilot uses AI. Check for mistakes.
return nil, err
}

// generated the metric name for HTTPScaledObject
namespacedName := k8s.NamespacedNameFromNameAndNamespace(httpScaledObjectName, sor.Namespace)
metricName := MetricName(namespacedName)

key := namespacedName.String()
count := e.pinger.counts()[key]

Expand Down
Loading