diff --git a/api/cloud/common/const.go b/api/cloud/common/const.go index c81a72b..dcf0eff 100644 --- a/api/cloud/common/const.go +++ b/api/cloud/common/const.go @@ -3,4 +3,12 @@ package common const ( AnnotationKeyPrefixTemporalCloud = "cloud.temporal.io/" AnnotationKeyTemporalCloudHostPort = AnnotationKeyPrefixTemporalCloud + "hostport" + + AnnotationKeyDeletionPolicy = AnnotationKeyPrefixTemporalCloud + "deletion-policy" + // DeletionPolicyDelete deletes the Temporal Cloud resource, and removes + // the custom resource once Temporal Cloud has finished deleting it. + DeletionPolicyDelete = "delete" + // DeletionPolicyRetain leaves the Temporal Cloud resource in place and + // only removes the custom resource. + DeletionPolicyRetain = "retain" ) diff --git a/controller/namespace/delete.go b/controller/namespace/delete.go new file mode 100644 index 0000000..ef6fa77 --- /dev/null +++ b/controller/namespace/delete.go @@ -0,0 +1,193 @@ +package namespace + +import ( + "context" + "errors" + "fmt" + "time" + + ctrlreqlog "github.com/temporalio/kube-temporal/pkg/controller/request/log" + "github.com/temporalio/kube-temporal/pkg/controller/requeue" + "github.com/temporalio/kube-temporal/pkg/kube/annotation" + "github.com/temporalio/kube-temporal/pkg/kube/condition" + "github.com/temporalio/kube-temporal/pkg/types" + tcloudservicev1 "go.temporal.io/cloud-sdk/api/cloudservice/v1" + operationv1 "go.temporal.io/cloud-sdk/api/operation/v1" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + tcloudcommon "github.com/temporalio/cloud-operator/api/cloud/common" + tcloudv1alpha1 "github.com/temporalio/cloud-operator/api/cloud/v1alpha1" +) + +const ( + // ReasonResourceDeleteFailed is the Stalled condition reason when Temporal + // Cloud failed to delete the Namespace. + ReasonResourceDeleteFailed = "ResourceDeleteFailed" + + // deletePollInterval is how often to check on a delete in progress. + deletePollInterval = 15 * time.Second +) + +// ResourceDelete deletes the Temporal Cloud Namespace for the supplied +// [types.Resource], unless its deletion policy is retain. It returns nil once +// the Namespace is gone, and an error while the delete is in progress or has +// failed. +// +// TODO(Mechack): this relies on the kube-temporal Controller persisting the +// status set here (State and AsyncOperationID) and treating nil as "remove +// the finalizer" and a requeue error as "keep it and requeue". Today it passes +// desired and drops the result on error. Remove this once that's fixed. +// +// TODO(Mechack): status.id and status.resourceVersion are set by the create path. +func (r *Reconciler) ResourceDelete( + ctx context.Context, + desired types.Resource, +) error { + var err error + log := ctrlreqlog.FromContext(ctx) + exit := log.Trace("resource.delete") + defer func() { + exit(err) + }() + + policy, err := deletionPolicy(desired) + if err != nil { + return err + } + if policy == tcloudcommon.DeletionPolicyRetain { + return nil + } + + cr := desired.CR().(*tcloudv1alpha1.Namespace) + if cr.Status.ID == "" { + // The operator never created a Namespace in Temporal Cloud. + return nil + } + + tcc, err := r.CloudClientFor(ctx, desired) + if err != nil { + err = fmt.Errorf("failed getting tcloud client: %w", err) + return err + } + + // We may have already called DeleteNamespace for this resource. If so, + // check whether the delete operation has finished. + switch { + case cr.Status.State == tcloudcommon.ResourceStateDeleted: + return nil + case cr.Status.State == tcloudcommon.ResourceStateDeleting && + cr.Status.AsyncOperationID != "": + err = checkDelete(ctx, tcc, desired, cr) + return err + } + + // TODO(Mechack): uses the CR's last recorded resourceVersion. If it's stale, + // Temporal Cloud rejects the delete with "resource version mismatch". Switch to + // the version from a fresh GetNamespace if that becomes a problem. + resp, err := tcc.DeleteNamespace(ctx, &tcloudservicev1.DeleteNamespaceRequest{ + Namespace: cr.Status.ID, + ResourceVersion: cr.Status.ResourceVersion, + }) + if status.Code(err) == codes.NotFound { + // Already gone. + err = nil + return nil + } + if err != nil { + // The gRPC error includes Temporal Cloud's message, for example that + // deletion protection is enabled. + err = fmt.Errorf("failed deleting tcloud namespace %s: %w", cr.Status.ID, err) + condition.SetStalled( + desired, metav1.ConditionTrue, err.Error(), ReasonResourceDeleteFailed, + ) + return err + } + + cr.Status.State = tcloudcommon.ResourceStateDeleting + cr.Status.AsyncOperationID = resp.GetAsyncOperation().GetId() + err = requeue.NeededAfter( + fmt.Errorf( + "deleting tcloud namespace %s (async operation %s)", + cr.Status.ID, cr.Status.AsyncOperationID, + ), + deletePollInterval, + ) + return err +} + +// checkDelete checks the async operation for a delete already sent to +// Temporal Cloud. It returns nil once the operation is fulfilled, a requeue +// error while it is running, and an error with the Stalled condition set if +// it failed. +func checkDelete( + ctx context.Context, + tcc tcloudservicev1.CloudServiceClient, + desired types.Resource, + cr *tcloudv1alpha1.Namespace, +) error { + resp, err := tcc.GetAsyncOperation(ctx, &tcloudservicev1.GetAsyncOperationRequest{ + AsyncOperationId: cr.Status.AsyncOperationID, + }) + if err != nil { + return fmt.Errorf( + "failed getting tcloud async operation %s: %w", + cr.Status.AsyncOperationID, err, + ) + } + op := resp.GetAsyncOperation() + switch op.GetState() { + case operationv1.AsyncOperation_STATE_FULFILLED: + return nil + case operationv1.AsyncOperation_STATE_FAILED, + operationv1.AsyncOperation_STATE_REJECTED, + operationv1.AsyncOperation_STATE_CANCELLED: + msg := fmt.Sprintf( + "deleting tcloud namespace %s failed: async operation %s is %s: %s", + cr.Status.ID, op.GetId(), op.GetState(), op.GetFailureReason(), + ) + condition.SetStalled( + desired, metav1.ConditionTrue, msg, ReasonResourceDeleteFailed, + ) + return errors.New(msg) + default: + // Pending. + return requeue.NeededAfter( + fmt.Errorf( + "waiting for tcloud async operation %s (%s)", + op.GetId(), op.GetState(), + ), + deletePollInterval, + ) + } +} + +// deletionPolicy returns the subject's deletion policy from its +// cloud.temporal.io/deletion-policy annotation. An unset annotation means +// delete. Any other value is an error and never falls back to delete. +// +// TODO(Mechack/10/26/2026): once adoption is supported, the default for an adopted +// Namespace becomes retain. An explicit delete is honored either way. +func deletionPolicy(subject types.Resource) (string, error) { + key := tcloudcommon.AnnotationKeyDeletionPolicy + v, ok := annotation.Value(subject, key) + if !ok { + // annotation.Value reports false when more than one key matches + // case-insensitively, so check that the key is really absent. + if annotation.HasKey(subject, key) { + return "", fmt.Errorf("more than one %s annotation", key) + } + return tcloudcommon.DeletionPolicyDelete, nil + } + switch v { + case tcloudcommon.DeletionPolicyDelete, tcloudcommon.DeletionPolicyRetain: + return v, nil + default: + return "", fmt.Errorf( + "invalid %s annotation %q: must be %q or %q", + key, v, + tcloudcommon.DeletionPolicyDelete, tcloudcommon.DeletionPolicyRetain, + ) + } +} diff --git a/controller/namespace/reconcile.go b/controller/namespace/reconcile.go index 49b3fe6..aa0273e 100644 --- a/controller/namespace/reconcile.go +++ b/controller/namespace/reconcile.go @@ -160,33 +160,6 @@ func (r *Reconciler) ResourceSync( return err } -// ResourceDelete deletes any managed external resources for the supplied -// [types.Resource]. Note that the Reconciler does NOT need to remove the -// finalizer on the associated [client.Object]. The Controller handles that. -// All the Reconciler needs to do is delete any owned resources or -// externally-managed resources associated with the supplied [types.Resource]. -func (r *Reconciler) ResourceDelete( - ctx context.Context, - desired types.Resource, -) error { - var err error - log := ctrlreqlog.FromContext(ctx) - exit := log.Trace("resource.delete") - defer func() { - exit(err) - }() - - tcc, err := r.CloudClientFor(ctx, desired) - if err != nil { - return fmt.Errorf("failed getting tcloud client: %w", err) - } - _ = tcc - - // TODO: call the Temporal Cloud API to delete the Namespace. - - return err -} - // compile-time check that Reconciler satisfies the [types.Reconciler] // contract. var _ types.Reconciler = (*Reconciler)(nil) diff --git a/pkg/testutil/temporal/cloud/fake/server/namespace.go b/pkg/testutil/temporal/cloud/fake/server/namespace.go index 95e126a..93f5e4b 100644 --- a/pkg/testutil/temporal/cloud/fake/server/namespace.go +++ b/pkg/testutil/temporal/cloud/fake/server/namespace.go @@ -61,3 +61,69 @@ func (s fakeServer) GetNamespace( } return s.UnimplementedCloudServiceServer.GetNamespace(ctx, req) } + +type DeleteNamespaceFunc func( + context.Context, + *tcloudservicev1.DeleteNamespaceRequest, +) (*tcloudservicev1.DeleteNamespaceResponse, error) + +// WithDeleteNamespace the fake server's DeleteNamespace function +func WithDeleteNamespace(fn DeleteNamespaceFunc) NewOption { + return func(s *fakeServer) { + s.deleteNamespaceFunc = fn + } +} + +// OnDeleteNamespace lets tests validate arguments passed to DeleteNamespace +// and set its response. +func OnDeleteNamespace( + asserter func(*tcloudservicev1.DeleteNamespaceRequest), + returner func() (*tcloudservicev1.DeleteNamespaceResponse, error), +) NewOption { + return func(s *fakeServer) { + fn := func( + ctx context.Context, req *tcloudservicev1.DeleteNamespaceRequest, + ) (*tcloudservicev1.DeleteNamespaceResponse, error) { + if asserter != nil { + asserter(req) + } + if returner == nil { + return &tcloudservicev1.DeleteNamespaceResponse{}, nil + } + resp, err := returner() + if resp == nil && err == nil { + resp = &tcloudservicev1.DeleteNamespaceResponse{} + } + return resp, err + } + s.deleteNamespaceFunc = fn + } +} + +// OnDeleteNamespaceReturn sets the response returned by DeleteNamespace. +func OnDeleteNamespaceReturn( + resp *tcloudservicev1.DeleteNamespaceResponse, + err error, +) NewOption { + if resp == nil && err == nil { + resp = &tcloudservicev1.DeleteNamespaceResponse{} + } + return func(s *fakeServer) { + fn := func( + context.Context, *tcloudservicev1.DeleteNamespaceRequest, + ) (*tcloudservicev1.DeleteNamespaceResponse, error) { + return resp, err + } + s.deleteNamespaceFunc = fn + } +} + +func (s fakeServer) DeleteNamespace( + ctx context.Context, + req *tcloudservicev1.DeleteNamespaceRequest, +) (*tcloudservicev1.DeleteNamespaceResponse, error) { + if s.deleteNamespaceFunc != nil { + return s.deleteNamespaceFunc(ctx, req) + } + return s.UnimplementedCloudServiceServer.DeleteNamespace(ctx, req) +} diff --git a/pkg/testutil/temporal/cloud/fake/server/operation.go b/pkg/testutil/temporal/cloud/fake/server/operation.go new file mode 100644 index 0000000..dd5094e --- /dev/null +++ b/pkg/testutil/temporal/cloud/fake/server/operation.go @@ -0,0 +1,74 @@ +package server + +import ( + "context" + + tcloudservicev1 "go.temporal.io/cloud-sdk/api/cloudservice/v1" +) + +type GetAsyncOperationFunc func( + context.Context, + *tcloudservicev1.GetAsyncOperationRequest, +) (*tcloudservicev1.GetAsyncOperationResponse, error) + +// WithGetAsyncOperation sets the fake server's GetAsyncOperation function +// override. +func WithGetAsyncOperation(fn GetAsyncOperationFunc) NewOption { + return func(s *fakeServer) { + s.getAsyncOperationFunc = fn + } +} + +// OnGetAsyncOperation lets tests validate arguments passed to GetAsyncOperation +// and set its response. +func OnGetAsyncOperation( + asserter func(*tcloudservicev1.GetAsyncOperationRequest), + returner func() (*tcloudservicev1.GetAsyncOperationResponse, error), +) NewOption { + return func(s *fakeServer) { + fn := func( + ctx context.Context, req *tcloudservicev1.GetAsyncOperationRequest, + ) (*tcloudservicev1.GetAsyncOperationResponse, error) { + if asserter != nil { + asserter(req) + } + if returner == nil { + return &tcloudservicev1.GetAsyncOperationResponse{}, nil + } + resp, err := returner() + if resp == nil && err == nil { + resp = &tcloudservicev1.GetAsyncOperationResponse{} + } + return resp, err + } + s.getAsyncOperationFunc = fn + } +} + +// OnGetAsyncOperationReturn sets the response returned by GetAsyncOperation. +func OnGetAsyncOperationReturn( + resp *tcloudservicev1.GetAsyncOperationResponse, + err error, +) NewOption { + if resp == nil && err == nil { + resp = &tcloudservicev1.GetAsyncOperationResponse{} + } + return func(s *fakeServer) { + fn := func( + context.Context, *tcloudservicev1.GetAsyncOperationRequest, + ) (*tcloudservicev1.GetAsyncOperationResponse, error) { + return resp, err + } + s.getAsyncOperationFunc = fn + } +} + +func (s fakeServer) GetAsyncOperation( + ctx context.Context, + req *tcloudservicev1.GetAsyncOperationRequest, +) (*tcloudservicev1.GetAsyncOperationResponse, error) { + if s.getAsyncOperationFunc != nil { + return s.getAsyncOperationFunc(ctx, req) + } + return s.UnimplementedCloudServiceServer.GetAsyncOperation(ctx, req) +} diff --git a/pkg/testutil/temporal/cloud/fake/server/server.go b/pkg/testutil/temporal/cloud/fake/server/server.go index cc57d0f..22b829f 100644 --- a/pkg/testutil/temporal/cloud/fake/server/server.go +++ b/pkg/testutil/temporal/cloud/fake/server/server.go @@ -14,7 +14,9 @@ type fakeServer struct { lis net.Listener tcloudservicev1.UnimplementedCloudServiceServer - getNamespaceFunc GetNamespaceFunc + getNamespaceFunc GetNamespaceFunc + deleteNamespaceFunc DeleteNamespaceFunc + getAsyncOperationFunc GetAsyncOperationFunc } // HostPort returns the address of the TCP localhost port that the fake server