diff --git a/cmd/atecontroller/internal/controllers/actortemplate_controller.go b/cmd/atecontroller/internal/controllers/actortemplate_controller.go index eacd45361..f5778fc5d 100644 --- a/cmd/atecontroller/internal/controllers/actortemplate_controller.go +++ b/cmd/atecontroller/internal/controllers/actortemplate_controller.go @@ -22,7 +22,6 @@ import ( "github.com/agent-substrate/substrate/internal/resources" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" - "github.com/google/uuid" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" k8errors "k8s.io/apimachinery/pkg/api/errors" @@ -80,7 +79,7 @@ func (r *ActorTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Reques switch at.Status.Phase { case atev1alpha1.PhaseInitial: - actorID := uuid.NewString() + actorName := string(at.UID) // Golden actors live in the reserved ate-golden system atespace. _, err := r.AteClient.CreateAtespace(ctx, &ateapipb.CreateAtespaceRequest{ @@ -98,19 +97,19 @@ func (r *ActorTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Reques Actor: &ateapipb.Actor{ Metadata: &ateapipb.ResourceMetadata{ Atespace: resources.GoldenActorAtespace, - Name: actorID, + Name: actorName, }, ActorTemplateNamespace: at.ObjectMeta.Namespace, ActorTemplateName: at.ObjectMeta.Name, }, } _, err = r.AteClient.CreateActor(ctx, createReq) - if err != nil { + if err != nil && status.Code(err) != codes.AlreadyExists { return ctrl.Result{}, fmt.Errorf("while creating golden actor: %w", err) } at.Status.Phase = atev1alpha1.PhaseResumeGoldenActor - at.Status.GoldenActorID = actorID + at.Status.GoldenActorID = actorName if err := r.Status().Update(ctx, at); err != nil { return ctrl.Result{}, err } diff --git a/cmd/atecontroller/internal/controllers/actortemplate_controller_test.go b/cmd/atecontroller/internal/controllers/actortemplate_controller_test.go index fa70af860..00d736cf5 100644 --- a/cmd/atecontroller/internal/controllers/actortemplate_controller_test.go +++ b/cmd/atecontroller/internal/controllers/actortemplate_controller_test.go @@ -15,8 +15,19 @@ package controllers import ( + "context" "testing" + "github.com/agent-substrate/substrate/pkg/proto/ateapipb" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" ) @@ -82,3 +93,142 @@ func TestGoldenSnapshotWarmupFor(t *testing.T) { }) } } + +type mockControlClient struct { + ateapipb.ControlClient + createAtespaceFn func(ctx context.Context, req *ateapipb.CreateAtespaceRequest, opts ...grpc.CallOption) (*ateapipb.Atespace, error) + createActorFn func(ctx context.Context, req *ateapipb.CreateActorRequest, opts ...grpc.CallOption) (*ateapipb.Actor, error) +} + +func (m *mockControlClient) CreateAtespace(ctx context.Context, req *ateapipb.CreateAtespaceRequest, opts ...grpc.CallOption) (*ateapipb.Atespace, error) { + if m.createAtespaceFn != nil { + return m.createAtespaceFn(ctx, req, opts...) + } + return &ateapipb.Atespace{}, nil +} + +func (m *mockControlClient) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequest, opts ...grpc.CallOption) (*ateapipb.Actor, error) { + if m.createActorFn != nil { + return m.createActorFn(ctx, req, opts...) + } + return &ateapipb.Actor{}, nil +} + +func TestActorTemplateReconciler_Reconcile_PhaseInitial(t *testing.T) { + scheme := runtime.NewScheme() + if err := atev1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add scheme: %v", err) + } + + const templateUID = "test-uid-12345" + const expectedActorName = templateUID + + t.Run("creates golden actor using template UID", func(t *testing.T) { + template := &atev1alpha1.ActorTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-template", + Namespace: "default", + UID: types.UID(templateUID), + }, + Status: atev1alpha1.ActorTemplateStatus{ + Phase: atev1alpha1.PhaseInitial, + }, + } + + fakeK8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&atev1alpha1.ActorTemplate{}). + WithObjects(template). + Build() + + var createdActorName string + fakeAteClient := &mockControlClient{ + createActorFn: func(ctx context.Context, req *ateapipb.CreateActorRequest, opts ...grpc.CallOption) (*ateapipb.Actor, error) { + createdActorName = req.GetActor().GetMetadata().GetName() + return &ateapipb.Actor{}, nil + }, + } + + reconciler := &ActorTemplateReconciler{ + Client: fakeK8sClient, + Scheme: scheme, + AteClient: fakeAteClient, + } + + ctx := context.Background() + req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "my-template", Namespace: "default"}} + res, err := reconciler.Reconcile(ctx, req) + if err != nil { + t.Fatalf("Reconcile returned error: %v", err) + } + if !res.IsZero() { + t.Errorf("unexpected requeue result: %v", res) + } + + if createdActorName != expectedActorName { + t.Errorf("created actor name = %q, want %q", createdActorName, expectedActorName) + } + + reconciledTemplate := &atev1alpha1.ActorTemplate{} + if err := fakeK8sClient.Get(ctx, req.NamespacedName, reconciledTemplate); err != nil { + t.Fatalf("failed to get reconciled ActorTemplate: %v", err) + } + + if reconciledTemplate.Status.GoldenActorID != expectedActorName { + t.Errorf("status.GoldenActorID = %q, want %q", reconciledTemplate.Status.GoldenActorID, expectedActorName) + } + if reconciledTemplate.Status.Phase != atev1alpha1.PhaseResumeGoldenActor { + t.Errorf("status.Phase = %q, want %q", reconciledTemplate.Status.Phase, atev1alpha1.PhaseResumeGoldenActor) + } + }) + + t.Run("handles AlreadyExists error when golden actor was created on prior attempt", func(t *testing.T) { + template := &atev1alpha1.ActorTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-template-retry", + Namespace: "default", + UID: types.UID(templateUID), + }, + Status: atev1alpha1.ActorTemplateStatus{ + Phase: atev1alpha1.PhaseInitial, + }, + } + + fakeK8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&atev1alpha1.ActorTemplate{}). + WithObjects(template). + Build() + + fakeAteClient := &mockControlClient{ + createActorFn: func(ctx context.Context, req *ateapipb.CreateActorRequest, opts ...grpc.CallOption) (*ateapipb.Actor, error) { + return nil, status.Error(codes.AlreadyExists, "actor already exists in ateapi") + }, + } + + reconciler := &ActorTemplateReconciler{ + Client: fakeK8sClient, + Scheme: scheme, + AteClient: fakeAteClient, + } + + ctx := context.Background() + req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "my-template-retry", Namespace: "default"}} + _, err := reconciler.Reconcile(ctx, req) + if err != nil { + t.Fatalf("Reconcile returned error on AlreadyExists retry: %v", err) + } + + reconciledTemplate := &atev1alpha1.ActorTemplate{} + if err := fakeK8sClient.Get(ctx, req.NamespacedName, reconciledTemplate); err != nil { + t.Fatalf("failed to get reconciled ActorTemplate: %v", err) + } + + if reconciledTemplate.Status.GoldenActorID != expectedActorName { + t.Errorf("status.GoldenActorID = %q, want %q", reconciledTemplate.Status.GoldenActorID, expectedActorName) + } + if reconciledTemplate.Status.Phase != atev1alpha1.PhaseResumeGoldenActor { + t.Errorf("status.Phase = %q, want %q", reconciledTemplate.Status.Phase, atev1alpha1.PhaseResumeGoldenActor) + } + }) +}