AMDEPYC / AMDEPYC/kubernetes-power-manager

CR validation errors cause infinite requeue loop

Đang mở
#27 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
bug
Ngôn ngữ chính
Go
Star
5
Fork
4
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

Right now, validation errors are handled incorrectly in many places in the code. These are unrecoverable errors, so they shouldn't cause the reconcile request to be requeued. But due to invalid handling of these errors, the request is requeued over and over. In addition, the status subresource often doesn't indicate any errors in such case. Example:
https://github.com/AMDEPYC/kubernetes-power-manager/blob/8284ff941610e879011532a9f0866835bb3be120/internal/controller/powerprofile_controller.go#L195-L199

`Requeue` is false by default. Setting it explicitly won't prevent requeuing of the reconcile request, done automatically by controller-runtime when `Reconcile()` returns an error. Also, in order for the error to be written to the `errors` field of the status subresource, the error must be assigned to the `err` variable defined in the outermost scope.

### Steps to reproduce
1. Create a PowerProfile that won't pass validation:
```shell
kubectl -n power-manager apply -f - <<'EOF'
apiVersion: power.intel.com/v1
kind: PowerProfile
metadata:
name: invalid
spec:
name: invalid
max: 1000
min: 2000
EOF
```

### Expected result

1. Request to reconcile isn't requeued.
2. `errors` field of the status subresource contains the validation error.

### Actual result

1. Request to reconcile is requeued over and over.
2. `errors` field of the status subresource doesn't contain any errors.

### Possible solution

Preferably, assign the error to the `err` variable and wrap it in a [`reconcile.TerminalError`](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/reconcile#TerminalError) before returning.

```go
if profile.Spec.Max < profile.Spec.Min {
// Make sure to use = instead of :=, otherwise you will shadow the existing variable.
err = fmt.Errorf("max frequency value cannot be lower than the min frequency value")
logger.Error(err, fmt.Sprintf("error creating the profile '%s'", profile.Spec.Name))
return ctrl.Result{}, reconcile.TerminalError(err)
}
```

Alternatively, assign the error to the `err` variable and log it, but don't propagate it to the caller.

```go
if profile.Spec.Max < profile.Spec.Min {
// Make sure to use = instead of :=, otherwise you will shadow the existing variable.
err = fmt.Errorf("max frequency value cannot be lower than the min frequency value")
logger.Error(err, fmt.Sprintf("error creating the profile '%s'", profile.Spec.Name))
return ctrl.Result{}, nil
}
```

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.