Repository navigation
Add implementation and helpers for ResourceSetLatest and ResourceSync - #34
Conversation
tomba7
left a comment
tomba7
left a comment
There was a problem hiding this comment.
I suggest we comment out TestReconciler_ResourceSetLatest for now and I can do that as a follow up after you merge. The fake cloud server looks like the right thing to use for this.
|
|
||
| // resourceStateFromProto converts a Temporal Cloud resource state to its CR | ||
| // name. Unspecified and unknown states map to "": the CR field is | ||
| // enum-validated, so an unrecognized value would fail the status patch. |
There was a problem hiding this comment.
This is a good candidate to go under a common covert package IMO
There was a problem hiding this comment.
the namespace specific proto helpers have been moved to v1alpha1/namespace.go. general purpose proto helpers that work regardless of temporal resource have been moved to api/cloud/common and kube-temporal.
| } | ||
| } | ||
|
|
||
| func timeFromProto(ts *timestamppb.Timestamp) *metav1.Time { |
|
|
||
| // resourceStatusFromProto returns the [cloudcommon.ResourceStatus] that | ||
| // represents the supplied Temporal Cloud Namespace. | ||
| func resourceStatusFromProto( |
There was a problem hiding this comment.
This is also a good shared candidate. Plus I wanted to call out making it have the shape recommended here in this proposal comment
| ctrlerrors.ErrResourceSync, id, state, | ||
| )) | ||
| } | ||
|
|
There was a problem hiding this comment.
Would only Active fall through here?
There was a problem hiding this comment.
Updated the switch statement, only Active falls through. All async operations are blocked on the Temporal server side unless the state is Active, so we gate on that here so those requests don't start in the first place.
| // async operation, and a namespace with an operation in flight rejects new | ||
| // ones, so after each call we requeue and let the next reconcile observe the | ||
| // result and take the next step. | ||
| func syncNamespace( |
There was a problem hiding this comment.
Would be good to add a few unit tests for this as there are a few important paths to cover here, esp the ones requiring a requeue with the right errors. But I'm fine doing it as a follow up.
There was a problem hiding this comment.
will do this in a follow-up pr.
…go, update ResourceSetLatest test
What was changed
Issue #8
Implements ResourceSetLatest and ResourceSync for the Namespace reconciler. The operator can now create a Temporal Cloud namespace from a CR and keep its spec and tags in sync.