Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 31 additions & 1 deletion go/core/internal/controller/agent_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ package controller
import (
"context"

apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/events"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -44,6 +46,12 @@ type AgentController struct {
Scheme *runtime.Scheme
Reconciler reconciler.KagentReconciler
AdkTranslator agent_translator.AdkApiTranslator
// Client is used to fetch the reconciled object so Events can be emitted
// against it. Optional; event emission is skipped when nil.
Client client.Client
// Recorder emits Kubernetes Events on reconcile transitions. Optional;
// event emission is skipped when nil, keeping behaviour unchanged.
Recorder events.EventRecorder
}

// +kubebuilder:rbac:groups=kagent.dev,resources=agents,verbs=get;list;watch;create;update;patch;delete
Expand All @@ -59,7 +67,29 @@ type AgentController struct {

func (r *AgentController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
_ = log.FromContext(ctx)
return ctrl.Result{}, r.Reconciler.ReconcileKagentAgent(ctx, req)
if err := r.Reconciler.ReconcileKagentAgent(ctx, req); err != nil {
r.recordEvent(ctx, req, "Warning", "ReconcileFailed", "Reconcile",
"failed to reconcile Agent: %s", err.Error())
return ctrl.Result{}, err
}
r.recordEvent(ctx, req, "Normal", "Accepted", "Reconcile", "Agent reconciled successfully")
return ctrl.Result{}, nil
}

// recordEvent emits a Kubernetes Event against the reconciled Agent. It is a
// no-op when no Recorder/Client is wired or the object can no longer be fetched.
func (r *AgentController) recordEvent(ctx context.Context, req ctrl.Request, eventtype, reason, action, note string, args ...any) {
if r.Recorder == nil || r.Client == nil {
return
}
agent := &v1alpha2.Agent{}
if err := r.Client.Get(ctx, req.NamespacedName, agent); err != nil {
if !apierrors.IsNotFound(err) {
log.FromContext(ctx).V(1).Info("unable to fetch Agent for event recording", "error", err.Error())
}
return
}
r.Recorder.Eventf(agent, nil, eventtype, reason, action, note, args...)
}

// SetupWithManager sets up the controller with the Manager.
Expand Down
51 changes: 51 additions & 0 deletions go/core/internal/controller/mcp_server_tool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ import (
agent_translator "github.com/kagent-dev/kagent/go/core/internal/controller/translator/agent"

"github.com/kagent-dev/kmcp/api/v1alpha1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/tools/events"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"
Expand All @@ -41,31 +44,79 @@ var mcpServerGK = schema.GroupKind{Group: "kagent.dev", Kind: "MCPServer"}
type MCPServerToolController struct {
Scheme *runtime.Scheme
Reconciler reconciler.KagentReconciler
// Client is used to fetch the reconciled object so Events can be emitted
// against it. Optional; event emission is skipped when nil.
Client client.Client
// Recorder emits Kubernetes Events on reconcile transitions. Optional.
Recorder events.EventRecorder
}

// +kubebuilder:rbac:groups=kagent.dev,resources=mcpservers,verbs=get;list;watch

func (r *MCPServerToolController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
_ = log.FromContext(ctx)

// This controller requeues every 60s to refresh tool status. Only emit the
// success Event on a meaningful transition (a spec change not yet observed
// in status), not on every periodic refresh, to avoid flooding Events.
specChanged := r.hasPendingGeneration(ctx, req)

err := r.Reconciler.ReconcileKagentMCPServer(ctx, req)
if err != nil {
// Check if this is a validation error that requires user action
var validationErr *agent_translator.ValidationError
if errors.As(err, &validationErr) {
r.recordEvent(ctx, req, "Warning", "ValidationFailed", "Reconcile",
"MCPServer validation failed: %s", err.Error())
// Validation error - don't retry until MCPServer spec is updated
// Return empty result with no error to avoid exponential backoff
return ctrl.Result{}, nil
}
r.recordEvent(ctx, req, "Warning", "ReconcileFailed", "Reconcile",
"failed to reconcile MCPServer: %s", err.Error())
// Transient error - return error to trigger exponential backoff retry
return ctrl.Result{}, err
}
if specChanged {
r.recordEvent(ctx, req, "Normal", "ToolsDiscovered", "Reconcile", "MCPServer tools discovered successfully")
}
// Success - requeue after 60s to refresh tool server status
return ctrl.Result{
RequeueAfter: 60 * time.Second,
}, nil
}

// hasPendingGeneration reports whether the MCPServer has spec changes not yet
// reflected in status (generation != observedGeneration). It gates success
// Events to meaningful transitions. Returns true (emit) when the object can't
// be fetched, so events are not silently dropped.
func (r *MCPServerToolController) hasPendingGeneration(ctx context.Context, req ctrl.Request) bool {
if r.Client == nil {
return true
}
server := &v1alpha1.MCPServer{}
if err := r.Client.Get(ctx, req.NamespacedName, server); err != nil {
return true
}
return server.Generation != server.Status.ObservedGeneration
}

// recordEvent emits a Kubernetes Event against the reconciled MCPServer.
// No-op when no Recorder/Client is wired or the object cannot be fetched.
func (r *MCPServerToolController) recordEvent(ctx context.Context, req ctrl.Request, eventtype, reason, action, note string, args ...any) {
if r.Recorder == nil || r.Client == nil {
return
}
server := &v1alpha1.MCPServer{}
if err := r.Client.Get(ctx, req.NamespacedName, server); err != nil {
if !apierrors.IsNotFound(err) {
log.FromContext(ctx).V(1).Info("unable to fetch MCPServer for event recording", "error", err.Error())
}
return
}
r.Recorder.Eventf(server, nil, eventtype, reason, action, note, args...)
}

// SetupWithManager sets up the controller with the Manager.
func (r *MCPServerToolController) SetupWithManager(mgr ctrl.Manager) error {
if _, err := mgr.GetRESTMapper().RESTMapping(mcpServerGK); err != nil {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package controller

import (
"context"
"errors"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/client-go/tools/events"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/kagent-dev/kmcp/api/v1alpha1"

agenttranslator "github.com/kagent-dev/kagent/go/core/internal/controller/translator/agent"
)

// TestMCPServerToolController_RecordsEvents verifies the controller emits a
// Kubernetes Event with the expected type/reason for each reconcile outcome.
func TestMCPServerToolController_RecordsEvents(t *testing.T) {
ctx := context.Background()

scheme := runtime.NewScheme()
utilruntime.Must(v1alpha1.AddToScheme(scheme))

// Generation != ObservedGeneration => a pending spec change, so the success
// Event is emitted (see the no-op case below for the periodic-refresh path).
server := &v1alpha1.MCPServer{
ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "test-server", Generation: 1},
}
req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: "test", Name: "test-server"}}

testCases := []struct {
name string
reconcileErr error
wantEventType string
wantReason string
}{
{"success emits ToolsDiscovered", nil, "Normal", "ToolsDiscovered"},
{"validation error emits ValidationFailed", agenttranslator.NewValidationError("invalid port"), "Warning", "ValidationFailed"},
{"transient error emits ReconcileFailed", errors.New("database connection failed"), "Warning", "ReconcileFailed"},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(server).Build()
recorder := events.NewFakeRecorder(1)

controller := &MCPServerToolController{
Reconciler: &fakeReconciler{reconcileMCPServerError: tc.reconcileErr},
Client: cl,
Recorder: recorder,
}

if _, err := controller.Reconcile(ctx, req); tc.wantEventType == "Warning" && tc.wantReason == "ReconcileFailed" {
require.Error(t, err)
} else {
require.NoError(t, err)
}

select {
case got := <-recorder.Events:
assert.Contains(t, got, tc.wantEventType)
assert.Contains(t, got, tc.wantReason)
default:
t.Fatalf("expected an event with reason %q, got none", tc.wantReason)
}
})
}
}

// TestMCPServerToolController_SkipsSuccessEventOnPeriodicRefresh verifies that a
// successful reconcile with no pending spec change (generation ==
// observedGeneration, i.e. a periodic 60s refresh) does NOT emit a success
// Event, so the Events feed is not flooded.
func TestMCPServerToolController_SkipsSuccessEventOnPeriodicRefresh(t *testing.T) {
scheme := runtime.NewScheme()
utilruntime.Must(v1alpha1.AddToScheme(scheme))

server := &v1alpha1.MCPServer{
ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "test-server", Generation: 3},
Status: v1alpha1.MCPServerStatus{ObservedGeneration: 3},
}
cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(server).Build()
recorder := events.NewFakeRecorder(1)
controller := &MCPServerToolController{Reconciler: &fakeReconciler{}, Client: cl, Recorder: recorder}
req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: "test", Name: "test-server"}}

if _, err := controller.Reconcile(context.Background(), req); err != nil {
t.Fatalf("unexpected error: %v", err)
}
select {
case got := <-recorder.Events:
t.Fatalf("expected no success Event on a periodic refresh, got %q", got)
default:
}
}

// TestMCPServerToolController_NoRecorderNoPanic verifies event recording is a
// safe no-op when no Recorder/Client is wired (the default wiring).
func TestMCPServerToolController_NoRecorderNoPanic(t *testing.T) {
controller := &MCPServerToolController{Reconciler: &fakeReconciler{}}
req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: "test", Name: "test-server"}}
// Must not panic despite nil Recorder/Client.
if _, err := controller.Reconcile(context.Background(), req); err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
31 changes: 30 additions & 1 deletion go/core/internal/controller/modelconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import (
"github.com/kagent-dev/kagent/go/core/internal/controller/reconciler"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/events"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -44,6 +46,11 @@ var (
type ModelConfigController struct {
Scheme *runtime.Scheme
Reconciler reconciler.KagentReconciler
// Client is used to fetch the reconciled object so Events can be emitted
// against it. Optional; event emission is skipped when nil.
Client client.Client
// Recorder emits Kubernetes Events on reconcile transitions. Optional.
Recorder events.EventRecorder
}

// +kubebuilder:rbac:groups=kagent.dev,resources=modelconfigs,verbs=get;list;watch;create;update;patch;delete
Expand All @@ -53,7 +60,29 @@ type ModelConfigController struct {

func (r *ModelConfigController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
_ = log.FromContext(ctx)
return ctrl.Result{}, r.Reconciler.ReconcileKagentModelConfig(ctx, req)
if err := r.Reconciler.ReconcileKagentModelConfig(ctx, req); err != nil {
r.recordEvent(ctx, req, "Warning", "ReconcileFailed", "Reconcile",
"failed to reconcile ModelConfig: %s", err.Error())
return ctrl.Result{}, err
}
r.recordEvent(ctx, req, "Normal", "Accepted", "Reconcile", "ModelConfig reconciled successfully")
return ctrl.Result{}, nil
}

// recordEvent emits a Kubernetes Event against the reconciled ModelConfig.
// No-op when no Recorder/Client is wired or the object cannot be fetched.
func (r *ModelConfigController) recordEvent(ctx context.Context, req ctrl.Request, eventtype, reason, action, note string, args ...any) {
if r.Recorder == nil || r.Client == nil {
return
}
mc := &v1alpha2.ModelConfig{}
if err := r.Client.Get(ctx, req.NamespacedName, mc); err != nil {
if !apierrors.IsNotFound(err) {
log.FromContext(ctx).V(1).Info("unable to fetch ModelConfig for event recording", "error", err.Error())
}
return
}
r.Recorder.Eventf(mc, nil, eventtype, reason, action, note, args...)
}

// SetupWithManager sets up the controller with the Manager.
Expand Down
48 changes: 48 additions & 0 deletions go/core/internal/controller/remote_mcp_server_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import (
"github.com/kagent-dev/kagent/go/core/internal/controller/reconciler"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/events"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -44,6 +46,11 @@ var (
type RemoteMCPServerController struct {
Scheme *runtime.Scheme
Reconciler reconciler.KagentReconciler
// Client is used to fetch the reconciled object so Events can be emitted
// against it. Optional; event emission is skipped when nil.
Client client.Client
// Recorder emits Kubernetes Events on reconcile transitions. Optional.
Recorder events.EventRecorder
}

// +kubebuilder:rbac:groups=kagent.dev,resources=remotemcpservers,verbs=get;list;watch;create;update;patch;delete
Expand All @@ -54,17 +61,58 @@ type RemoteMCPServerController struct {
func (r *RemoteMCPServerController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
_ = log.FromContext(ctx)

// This controller requeues every 60s to refresh tool status. Only emit the
// success Event on a meaningful transition (a spec change not yet observed
// in status), not on every periodic refresh, to avoid flooding Events.
specChanged := r.hasPendingGeneration(ctx, req)

err := r.Reconciler.ReconcileKagentRemoteMCPServer(ctx, req)
if err != nil {
r.recordEvent(ctx, req, "Warning", "ReconcileFailed", "Reconcile",
"failed to reconcile RemoteMCPServer: %s", err.Error())
// Return zero result when there's an error - controller-runtime will handle backoff
return ctrl.Result{}, err
}
if specChanged {
r.recordEvent(ctx, req, "Normal", "Accepted", "Reconcile", "RemoteMCPServer reconciled successfully")
}
// Success - requeue after 60s to refresh tool server status
return ctrl.Result{
RequeueAfter: 60 * time.Second,
}, nil
}

// hasPendingGeneration reports whether the RemoteMCPServer has spec changes not
// yet reflected in status (generation != observedGeneration). It gates success
// Events to meaningful transitions. Returns true (emit) when the object can't
// be fetched, so events are not silently dropped.
func (r *RemoteMCPServerController) hasPendingGeneration(ctx context.Context, req ctrl.Request) bool {
if r.Client == nil {
return true
}
server := &v1alpha2.RemoteMCPServer{}
if err := r.Client.Get(ctx, req.NamespacedName, server); err != nil {
return true
}
return server.Generation != server.Status.ObservedGeneration
}

// recordEvent emits a Kubernetes Event against the reconciled RemoteMCPServer.
// No-op when no Recorder/Client is wired or the object cannot be fetched.
func (r *RemoteMCPServerController) recordEvent(ctx context.Context, req ctrl.Request, eventtype, reason, action, note string, args ...any) {
if r.Recorder == nil || r.Client == nil {
return
}
server := &v1alpha2.RemoteMCPServer{}
if err := r.Client.Get(ctx, req.NamespacedName, server); err != nil {
if !apierrors.IsNotFound(err) {
log.FromContext(ctx).V(1).Info("unable to fetch RemoteMCPServer for event recording", "error", err.Error())
}
return
}
r.Recorder.Eventf(server, nil, eventtype, reason, action, note, args...)
}

// SetupWithManager sets up the controller with the Manager.
func (r *RemoteMCPServerController) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
Expand Down
Loading
Loading