diff --git a/operator/go.mod b/operator/go.mod index b34d9574b..b0efe6589 100644 --- a/operator/go.mod +++ b/operator/go.mod @@ -4,13 +4,13 @@ go 1.26.1 require ( github.com/ai-dynamo/grove/operator/api v0.0.0 - github.com/ai-dynamo/grove/operator/client v0.0.0-00010101000000-000000000000 github.com/ai-dynamo/grove/scheduler/api v0.0.0 github.com/docker/docker v28.3.3+incompatible github.com/go-logr/logr v1.4.3 github.com/go-logr/zapr v1.3.0 github.com/kai-scheduler/KAI-scheduler v0.14.0 github.com/open-policy-agent/cert-controller v0.14.0 + github.com/prometheus/client_golang v1.23.2 github.com/samber/lo v1.52.0 github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 @@ -87,6 +87,7 @@ require ( github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/klauspost/compress v1.18.0 // indirect + github.com/kylelemons/godebug v1.1.0 // indirect github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/lib/pq v1.10.9 // indirect @@ -113,7 +114,6 @@ require ( github.com/peterbourgon/diskv v2.0.1+incompatible // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/prometheus/client_golang v1.23.2 // indirect github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.67.5 // indirect github.com/prometheus/procfs v0.20.1 // indirect diff --git a/operator/internal/controller/podclique/reconciler.go b/operator/internal/controller/podclique/reconciler.go index a4599d258..05c01e68e 100644 --- a/operator/internal/controller/podclique/reconciler.go +++ b/operator/internal/controller/podclique/reconciler.go @@ -28,6 +28,7 @@ import ( pclqcomponent "github.com/ai-dynamo/grove/operator/internal/controller/podclique/components" ctrlutils "github.com/ai-dynamo/grove/operator/internal/controller/utils" "github.com/ai-dynamo/grove/operator/internal/expect" + grovemetrics "github.com/ai-dynamo/grove/operator/internal/metrics" "github.com/ai-dynamo/grove/operator/internal/scheduler" "k8s.io/client-go/tools/record" @@ -84,9 +85,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } reconcileSpecFlowResult := r.reconcileSpec(ctx, logger, pclq) - if statusReconcileResult := r.reconcileStatus(ctx, logger, pclq); ctrlcommon.ShortCircuitReconcileFlow(statusReconcileResult) { - return statusReconcileResult.Result() + statusReconcileResult := r.reconcileStatus(ctx, logger, pclq) + + if ctrlcommon.ShortCircuitReconcileFlow(statusReconcileResult) { + result, err := statusReconcileResult.Result() + grovemetrics.RecordReconcileError(controllerName, err) + return result, err } - return reconcileSpecFlowResult.Result() + result, err := reconcileSpecFlowResult.Result() + grovemetrics.RecordReconcileError(controllerName, err) + return result, err } diff --git a/operator/internal/controller/podclique/reconcilestatus.go b/operator/internal/controller/podclique/reconcilestatus.go index 74ded2305..e74261423 100644 --- a/operator/internal/controller/podclique/reconcilestatus.go +++ b/operator/internal/controller/podclique/reconcilestatus.go @@ -26,12 +26,14 @@ import ( internalconstants "github.com/ai-dynamo/grove/operator/internal/constants" ctrlcommon "github.com/ai-dynamo/grove/operator/internal/controller/common" componentutils "github.com/ai-dynamo/grove/operator/internal/controller/common/component/utils" + grovemetrics "github.com/ai-dynamo/grove/operator/internal/metrics" k8sutils "github.com/ai-dynamo/grove/operator/internal/utils/kubernetes" "github.com/go-logr/logr" "github.com/samber/lo" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -100,6 +102,9 @@ func (r *Reconciler) reconcileStatus(ctx context.Context, logger logr.Logger, pc // update the PodClique status. if err := r.client.Status().Patch(ctx, pclq, patch); err != nil { + if apierrors.IsConflict(err) { + grovemetrics.RecordStatusUpdateConflict(controllerName, "PodClique") + } logger.Error(err, "failed to update PodClique status") return ctrlcommon.ReconcileWithErrors("failed to update PodClique status", err) } diff --git a/operator/internal/controller/podcliquescalinggroup/reconciler.go b/operator/internal/controller/podcliquescalinggroup/reconciler.go index e0b1c1a6a..0659e5b1c 100644 --- a/operator/internal/controller/podcliquescalinggroup/reconciler.go +++ b/operator/internal/controller/podcliquescalinggroup/reconciler.go @@ -27,6 +27,7 @@ import ( componentutils "github.com/ai-dynamo/grove/operator/internal/controller/common/component/utils" pcsgcomponent "github.com/ai-dynamo/grove/operator/internal/controller/podcliquescalinggroup/components" ctrlutils "github.com/ai-dynamo/grove/operator/internal/controller/utils" + grovemetrics "github.com/ai-dynamo/grove/operator/internal/metrics" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -82,12 +83,18 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu deletionOrSpecReconcileFlowResult = r.reconcileSpec(ctx, specLog, pcsg) } - if statusReconcileResult := r.reconcileStatus(ctx, logger, ctrlclient.ObjectKeyFromObject(pcsg)); ctrlcommon.ShortCircuitReconcileFlow(statusReconcileResult) { - return statusReconcileResult.Result() + statusReconcileResult := r.reconcileStatus(ctx, logger, ctrlclient.ObjectKeyFromObject(pcsg)) + + if ctrlcommon.ShortCircuitReconcileFlow(statusReconcileResult) { + result, err := statusReconcileResult.Result() + grovemetrics.RecordReconcileError(controllerName, err) + return result, err } if ctrlcommon.ShortCircuitReconcileFlow(deletionOrSpecReconcileFlowResult) { - return deletionOrSpecReconcileFlowResult.Result() + result, err := deletionOrSpecReconcileFlowResult.Result() + grovemetrics.RecordReconcileError(controllerName, err) + return result, err } return ctrlcommon.DoNotRequeue().Result() diff --git a/operator/internal/controller/podcliquescalinggroup/reconcilestatus.go b/operator/internal/controller/podcliquescalinggroup/reconcilestatus.go index 4682478be..153e68343 100644 --- a/operator/internal/controller/podcliquescalinggroup/reconcilestatus.go +++ b/operator/internal/controller/podcliquescalinggroup/reconcilestatus.go @@ -29,12 +29,14 @@ import ( ctrlcommon "github.com/ai-dynamo/grove/operator/internal/controller/common" componentutils "github.com/ai-dynamo/grove/operator/internal/controller/common/component/utils" ctrlutils "github.com/ai-dynamo/grove/operator/internal/controller/utils" + grovemetrics "github.com/ai-dynamo/grove/operator/internal/metrics" k8sutils "github.com/ai-dynamo/grove/operator/internal/utils/kubernetes" "github.com/go-logr/logr" "github.com/samber/lo" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -97,6 +99,9 @@ func (r *Reconciler) reconcileStatus(ctx context.Context, logger logr.Logger, pc } if err = r.client.Status().Patch(ctx, pcsg, patchObj); err != nil { + if apierrors.IsConflict(err) { + grovemetrics.RecordStatusUpdateConflict(controllerName, "PodCliqueScalingGroup") + } logger.Error(err, "failed to update PodCliqueScalingGroup status") return ctrlcommon.ReconcileWithErrors("failed to update the status with label selector and replicas", err) } diff --git a/operator/internal/controller/podcliqueset/reconciler.go b/operator/internal/controller/podcliqueset/reconciler.go index 39bac1ef3..1b71ea6ad 100644 --- a/operator/internal/controller/podcliqueset/reconciler.go +++ b/operator/internal/controller/podcliqueset/reconciler.go @@ -27,6 +27,7 @@ import ( "github.com/ai-dynamo/grove/operator/internal/controller/common/component" pcscomponent "github.com/ai-dynamo/grove/operator/internal/controller/podcliqueset/components" ctrlutils "github.com/ai-dynamo/grove/operator/internal/controller/utils" + grovemetrics "github.com/ai-dynamo/grove/operator/internal/metrics" "github.com/ai-dynamo/grove/operator/internal/scheduler" "github.com/go-logr/logr" @@ -74,11 +75,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } reconcileSpecFlowResult := r.reconcileSpec(ctx, logger, pcs) - if statusReconcileResult := r.reconcileStatus(ctx, logger, pcs); ctrlcommon.ShortCircuitReconcileFlow(statusReconcileResult) { - return statusReconcileResult.Result() + statusReconcileResult := r.reconcileStatus(ctx, logger, pcs) + + if ctrlcommon.ShortCircuitReconcileFlow(statusReconcileResult) { + result, err := statusReconcileResult.Result() + grovemetrics.RecordReconcileError(controllerName, err) + return result, err } - return reconcileSpecFlowResult.Result() + result, err := reconcileSpecFlowResult.Result() + grovemetrics.RecordReconcileError(controllerName, err) + return result, err } // reconcileDelete handles PodCliqueSet deletion when a deletion timestamp is set. diff --git a/operator/internal/controller/podcliqueset/reconcilestatus.go b/operator/internal/controller/podcliqueset/reconcilestatus.go index ca16fc02c..8f4120a2a 100644 --- a/operator/internal/controller/podcliqueset/reconcilestatus.go +++ b/operator/internal/controller/podcliqueset/reconcilestatus.go @@ -28,6 +28,7 @@ import ( "github.com/ai-dynamo/grove/operator/internal/clustertopology" ctrlcommon "github.com/ai-dynamo/grove/operator/internal/controller/common" componentutils "github.com/ai-dynamo/grove/operator/internal/controller/common/component/utils" + grovemetrics "github.com/ai-dynamo/grove/operator/internal/metrics" k8sutils "github.com/ai-dynamo/grove/operator/internal/utils/kubernetes" "github.com/go-logr/logr" @@ -67,6 +68,9 @@ func (r *Reconciler) reconcileStatus(ctx context.Context, logger logr.Logger, pc // Update the PodCliqueSet status if err = r.client.Status().Update(ctx, pcs); err != nil { + if apierrors.IsConflict(err) { + grovemetrics.RecordStatusUpdateConflict(controllerName, "PodCliqueSet") + } return ctrlcommon.ReconcileWithErrors("failed to update PodCliqueSet status", err) } return ctrlcommon.ContinueReconcile() diff --git a/operator/internal/metrics/metrics.go b/operator/internal/metrics/metrics.go new file mode 100644 index 000000000..9ce8f1d3a --- /dev/null +++ b/operator/internal/metrics/metrics.go @@ -0,0 +1,106 @@ +// /* +// Copyright 2025 The Grove Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// */ + +// Package metrics provides Prometheus metrics for Grove operator controllers. +// All metrics use the grove_operator namespace prefix and are registered on +// the controller-runtime global registry so they are served on the existing +// metrics endpoint without additional configuration. +// +// Existing controller-runtime built-ins (controller_runtime_reconcile_total, +// controller_runtime_reconcile_time_seconds, controller_runtime_active_workers) +// cover reconcile count, duration, and in-flight tracking. Only signals absent +// from those built-ins are added here. +package metrics + +import ( + "errors" + "net" + + "github.com/prometheus/client_golang/prometheus" + apierrors "k8s.io/apimachinery/pkg/api/errors" + ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +const namespace = "grove_operator" + +var ( + // statusUpdateConflictTotal counts 409 Conflict errors on Status Update/Patch calls, + // partitioned by controller and the kind of resource being updated. + // controller_runtime_reconcile_total cannot attribute conflicts to a specific status-update + // target kind, so this metric fills that gap. + statusUpdateConflictTotal = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: namespace, + Name: "status_update_conflict_total", + Help: "Total 409 Conflict errors on status Update/Patch per controller, partitioned by target resource kind.", + }, + []string{"controller", "target_kind"}, + ) + + // reconcileErrorsTotal counts reconcile errors partitioned by controller and error type, + // enabling transient vs. persistent error breakdown that controller_runtime_reconcile_total + // does not provide. + reconcileErrorsTotal = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: namespace, + Name: "reconcile_errors_total", + Help: "Total reconcile errors per controller, partitioned by error type (conflict, not_found, invalid, transient, server_error, unknown).", + }, + []string{"controller", "error_type"}, + ) +) + +func init() { + ctrlmetrics.Registry.MustRegister( + statusUpdateConflictTotal, + reconcileErrorsTotal, + ) +} + +// RecordStatusUpdateConflict records a 409 Conflict on a status Update or Patch call. +// targetKind is the Kind of the resource being updated (e.g. "PodCliqueSet"). +func RecordStatusUpdateConflict(controller, targetKind string) { + statusUpdateConflictTotal.WithLabelValues(controller, targetKind).Inc() +} + +// RecordReconcileError categorises err and increments reconcile_errors_total. +// It is a no-op when err is nil. +func RecordReconcileError(controller string, err error) { + if err == nil { + return + } + reconcileErrorsTotal.WithLabelValues(controller, errorType(err)).Inc() +} + +func errorType(err error) string { + switch { + case apierrors.IsConflict(err): + return "conflict" + case apierrors.IsNotFound(err): + return "not_found" + case apierrors.IsInvalid(err): + return "invalid" + case apierrors.IsServerTimeout(err) || apierrors.IsTimeout(err) || apierrors.IsTooManyRequests(err): + return "transient" + case apierrors.IsInternalError(err) || apierrors.IsServiceUnavailable(err): + return "server_error" + } + var netErr *net.OpError + if errors.As(err, &netErr) { + return "transient" + } + return "unknown" +} diff --git a/operator/internal/metrics/metrics_test.go b/operator/internal/metrics/metrics_test.go new file mode 100644 index 000000000..ddb554920 --- /dev/null +++ b/operator/internal/metrics/metrics_test.go @@ -0,0 +1,79 @@ +// /* +// Copyright 2025 The Grove Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// */ + +package metrics + +import ( + "fmt" + "testing" + + "github.com/prometheus/client_golang/prometheus/testutil" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +func TestRecordStatusUpdateConflict(t *testing.T) { + ctrl, kind := "pcs-test-conflict", "PodCliqueSet" + before := testutil.ToFloat64(statusUpdateConflictTotal.WithLabelValues(ctrl, kind)) + RecordStatusUpdateConflict(ctrl, kind) + after := testutil.ToFloat64(statusUpdateConflictTotal.WithLabelValues(ctrl, kind)) + if after-before != 1 { + t.Errorf("status_update_conflict_total delta = %v, want 1", after-before) + } +} + +func TestRecordReconcileError_Conflict(t *testing.T) { + ctrl := "pcs-test-err-conflict" + err := apierrors.NewConflict(schema.GroupResource{Group: "core.grove.io", Resource: "podcliqueset"}, "obj", fmt.Errorf("modified")) + before := testutil.ToFloat64(reconcileErrorsTotal.WithLabelValues(ctrl, "conflict")) + RecordReconcileError(ctrl, err) + after := testutil.ToFloat64(reconcileErrorsTotal.WithLabelValues(ctrl, "conflict")) + if after-before != 1 { + t.Errorf("reconcile_errors_total{error_type=conflict} delta = %v, want 1", after-before) + } +} + +func TestRecordReconcileError_NotFound(t *testing.T) { + ctrl := "pcs-test-err-notfound" + err := apierrors.NewNotFound(schema.GroupResource{Resource: "podcliqueset"}, "obj") + before := testutil.ToFloat64(reconcileErrorsTotal.WithLabelValues(ctrl, "not_found")) + RecordReconcileError(ctrl, err) + after := testutil.ToFloat64(reconcileErrorsTotal.WithLabelValues(ctrl, "not_found")) + if after-before != 1 { + t.Errorf("reconcile_errors_total{error_type=not_found} delta = %v, want 1", after-before) + } +} + +func TestRecordReconcileError_Unknown(t *testing.T) { + ctrl := "pcs-test-err-unknown" + before := testutil.ToFloat64(reconcileErrorsTotal.WithLabelValues(ctrl, "unknown")) + RecordReconcileError(ctrl, fmt.Errorf("some unexpected error")) + after := testutil.ToFloat64(reconcileErrorsTotal.WithLabelValues(ctrl, "unknown")) + if after-before != 1 { + t.Errorf("reconcile_errors_total{error_type=unknown} delta = %v, want 1", after-before) + } +} + +func TestRecordReconcileError_Nil(t *testing.T) { + // Calling with nil should be a no-op; verify no panic and counter stays zero. + ctrl := "pcs-test-err-nil" + before := testutil.CollectAndCount(reconcileErrorsTotal) + RecordReconcileError(ctrl, nil) + after := testutil.CollectAndCount(reconcileErrorsTotal) + if after != before { + t.Errorf("reconcile_errors_total series count changed on nil error: before=%d after=%d", before, after) + } +}