codify custom search attributes for TemporalNamespace #776

TheHiddenLayer · 2024-07-23T04:33:24Z

Closes #567

This PR codifies custom search attributes, so one can specify:

apiVersion: temporal.io/v1beta1
kind: TemporalNamespace
metadata:
  name: default
  namespace: demo
spec:
  customSearchAttributes:
    name1: value1
    name2: value2
    name3: value2
  // additional fields

TheHiddenLayer · 2024-07-23T04:35:18Z

TODO(@TheHiddenLayer) add tests?

TheHiddenLayer · 2024-07-24T23:17:02Z

Hi @alexandrevilain,

I'm experiencing various breakages running make test-e2e-dev.
I needed helm and buildx to properly build the container when running make test-e2e-dev. Thinking of updating the local development docs to include those deps. Also, do you prefer the local development docs stay minimal/brief? I noticed there's a lot of commands in the Makefile that aren't mentioned in the local development docs.
Any way you could give me some guidance on what tests you want to see for custom search attributes? This is actually my first time contributing to a k8s operator, and I'm referencing this blog post for the basics.

P.S. regarding:

I'm experiencing various breakages running make test-e2e-dev.

the error is different each time. Here's the error I happened to see this time:

I0724 23:18:56.709046     135 round_trippers.go:553] GET https://temporal-becb55e-control-plane:6443/healthz?timeout=10s  in 5 milliseconds
I0724 23:18:57.177835     135 round_trippers.go:553] GET https://temporal-becb55e-control-plane:6443/healthz?timeout=10s  in 2 milliseconds
I0724 23:18:57.664057     135 round_trippers.go:553] GET https://temporal-becb55e-control-plane:6443/healthz?timeout=10s  in 2 milliseconds
I0724 23:18:58.167209     135 round_trippers.go:553] GET https://temporal-becb55e-control-plane:6443/healthz?timeout=10s  in 3 milliseconds
I0724 23:19:06.648034     135 round_trippers.go:553] GET https://temporal-becb55e-control-plane:6443/healthz?timeout=10s  in 7993 milliseconds
[api-check] The API server is not healthy after 4m0.02803015s

Unfortunately, an error has occurred:
	context deadline exceeded

This error is likely caused by:
	- The kubelet is not running
	- The kubelet is unhealthy due to a misconfiguration of the node in some way (required cgroups disabled)

If you are on a systemd-powered system, you can try to troubleshoot the error with the following commands:
	- 'systemctl status kubelet'
	- 'journalctl -xeu kubelet'

Additionally, a control plane component may have crashed or exited when started by the container runtime.

TheHiddenLayer · 2024-07-25T03:25:50Z

This is running on an M3 apple silicon with colima

fails again, this time with a different error.

docker save temporal-operator > /tmp/temporal-operator.tar
OPERATOR_IMAGE_PATH=/tmp/temporal-operator.tar go test ./tests/e2e -v -timeout 60m -args "--v=4"
I0724 20:24:34.366456   59239 kind.go:157] Creating kind cluster temporal-fcc1c45
I0724 20:24:34.367030   59239 command.go:37] "Found Provider tooling already installed on the machine" command="kind"
I0724 20:24:34.574632   59239 kind.go:171] Launching:kind create cluster --name temporal-fcc1c45
F0724 20:24:35.841364   59239 env.go:375] Setup failure: failed to create kind cluster: exit status 1 : exit status 1: Creating cluster "temporal-fcc1c45" ...
 • Ensuring node image (kindest/node:v1.30.0) 🖼  ...
 ✓ Ensuring node image (kindest/node:v1.30.0) 🖼
 • Preparing nodes 📦   ...
 ✗ Preparing nodes 📦 
Deleted nodes: ["temporal-fcc1c45-control-plane"]
ERROR: failed to create cluster: could not find a log line that matches "Reached target .*Multi-User System.*|detected cgroup v1"
FAIL	github.com/alexandrevilain/temporal-operator/tests/e2e	2.034s
FAIL
make: *** [test-e2e-dev] Error 1

TheHiddenLayer · 2024-08-01T19:18:41Z

even while running the make test-e2e-dev on the main branch I get

=== RUN   TestPersistence/cassandra_persistence/Temporal_cluster_ready_after_upgrade#03
panic: test timed out after 1h0m0s

@alexandrevilain are you able to run the whole e3e test suite without any errors or timeouts?

TheHiddenLayer · 2024-08-07T05:01:13Z

controllers/temporalnamespace_controller.go

@@ -136,13 +140,126 @@ func (r *TemporalNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		}
 	}

+	err = r.reconcileCustomSearchAttributes(ctx, &logger, namespace, cluster)


@alexandrevilain is this the right place to start reconciling custom search attribtues?

I think I should wait for the namespace to be marked ready, before reconciling custom search attributes, right? But where should I put that waiting logic?

I think my current implementation is causing the TemporalNamespace_ready e2e test to fail

Looks like this is the good place for this :)

alexandrevilain · 2024-08-07T12:21:23Z

Sorry @TheHiddenLayer I'm currently having issues with internet connectivity while I'm in holidays. I can't even pull the cert-manager image ...

…tributes

alexandrevilain · 2024-08-08T19:39:07Z

Hi @TheHiddenLayer following PR succesfully ended e2e without any retry: #777

TheHiddenLayer · 2024-08-08T23:09:59Z

Hi @TheHiddenLayer following PR succesfully ended e2e without any retry: #777

Nice, I'm glad that it worked. It could be something with my logic then? Pls LMK what you think about this.

alexandrevilain · 2024-08-17T19:57:18Z

controllers/temporalnamespace_controller.go

@@ -58,16 +62,19 @@ type TemporalNamespaceReconciler struct {
 func (r *TemporalNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
 	logger := log.FromContext(ctx)

-	logger.Info("Starting reconciliation")
+	logger.Info("=== (NS) STARTING RECONCILIATION")


We're ok on the fast that those kind of logging will be removed once the code is working, right ?

Certainly. These logging statements are for my debugging. I pushed them up to keep my branch up to date. Unfortunately, they show up in the PR.

alexandrevilain · 2024-08-17T19:59:56Z

controllers/temporalnamespace_controller.go

@@ -136,13 +140,126 @@ func (r *TemporalNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		}
 	}

+	err = r.reconcileCustomSearchAttributes(ctx, &logger, namespace, cluster)


Looks like this is the good place for this :)

TheHiddenLayer · 2024-08-25T23:06:28Z

@alexandrevilain I haven't forgotten about this PR. I'm still planning to finish it off. I’ve been tied up lately with work. Thanks for your patience!

…tributes

sonarqubecloud · 2024-08-29T16:52:12Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

alexandrevilain · 2024-09-26T19:45:50Z

Hi @TheHiddenLayer !
Do you need some help for the last few things to fix ?

TheHiddenLayer · 2024-09-26T20:03:07Z

hey @alexandrevilain! the feature itself works as expected. I've done manual tests confirming:

Add an attr whose name exists with same type -> no change (b/c no reconciliation needed)
Add an attr whose name exists with different type -> properly returns error (b/c user first needs to delete existing attr with same name)
Add an attr with invalid/gibberish type -> correctly returns parsing error (invalid type)
Add an attr with ":" in the name -> adds attr correctly (this tests a potential bug in postgres where key might not work if it has a colon)
Add a brand new attr -> adds attr correctly
Remove an existing attr -> removes attr correctly

BUT I didn't deeply investigate why this doesn't pass some E2E tests, so any assistance on that would be great.

I wanted to filter out and run a few tests that were failing to isolate my debugging, but the option flags I passed into the test command didn't do anything. So, the fact that I had to run the entire E2E pipeline (30-50min on my computer) has made this arduous to debug.

sonarqubecloud · 2024-10-01T17:19:32Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

TheHiddenLayer · 2024-10-01T17:23:42Z

I'll clean up the comments/dead code/debugging log statements

TheHiddenLayer added 4 commits July 19, 2024 02:00

feat: codify custom search attributes in TemporalNamespace

408f89c

add guard condition for empty payload in operatorservice requests

895333f

pass logger by reference

c6d6336

add customSearchAttributes to an example TemporalNamespace Spec

49126d7

add feature mentioning that the operator manages custom search attrs

80f3935

TheHiddenLayer mentioned this pull request Aug 2, 2024

docs: enhance details & directions #779

Merged

add feature mentioning that the operator manages custom search attrs

511d1e6

TheHiddenLayer marked this pull request as ready for review August 6, 2024 21:32

fix typo

7e2f787

TheHiddenLayer commented Aug 7, 2024

View reviewed changes

extract map construction logic into separate func

a9eb384

TheHiddenLayer added 2 commits August 7, 2024 21:07

Merge branch 'alexandrevilain:main' into feat/codify-custom-search-at…

18964c6

…tributes

testing with log statements

db0a0ba

alexandrevilain reviewed Aug 17, 2024

View reviewed changes

Merge branch 'alexandrevilain:main' into feat/codify-custom-search-at…

8452659

…tributes

TheHiddenLayer marked this pull request as draft August 29, 2024 16:55

alexandrevilain added the feature label Sep 28, 2024

Merge branch 'main' into feat/codify-custom-search-attributes

0a188b7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

codify custom search attributes for TemporalNamespace #776

codify custom search attributes for TemporalNamespace #776

TheHiddenLayer commented Jul 23, 2024 •

edited

Loading

TheHiddenLayer commented Jul 23, 2024

TheHiddenLayer commented Jul 24, 2024 •

edited

Loading

TheHiddenLayer commented Jul 25, 2024 •

edited

Loading

TheHiddenLayer commented Aug 1, 2024

TheHiddenLayer Aug 7, 2024

TheHiddenLayer Aug 7, 2024

TheHiddenLayer Aug 7, 2024

alexandrevilain Aug 17, 2024

alexandrevilain commented Aug 7, 2024

alexandrevilain commented Aug 8, 2024

TheHiddenLayer commented Aug 8, 2024

alexandrevilain Aug 17, 2024

TheHiddenLayer Aug 19, 2024

alexandrevilain Aug 17, 2024

TheHiddenLayer commented Aug 25, 2024 •

edited

Loading

sonarqubecloud bot commented Aug 29, 2024

alexandrevilain commented Sep 26, 2024

TheHiddenLayer commented Sep 26, 2024 •

edited

Loading

sonarqubecloud bot commented Oct 1, 2024

TheHiddenLayer commented Oct 1, 2024

codify custom search attributes for TemporalNamespace #776

Are you sure you want to change the base?

codify custom search attributes for TemporalNamespace #776

Conversation

TheHiddenLayer commented Jul 23, 2024 • edited Loading

TheHiddenLayer commented Jul 23, 2024

TheHiddenLayer commented Jul 24, 2024 • edited Loading

TheHiddenLayer commented Jul 25, 2024 • edited Loading

TheHiddenLayer commented Aug 1, 2024

TheHiddenLayer Aug 7, 2024

Choose a reason for hiding this comment

TheHiddenLayer Aug 7, 2024

Choose a reason for hiding this comment

TheHiddenLayer Aug 7, 2024

Choose a reason for hiding this comment

alexandrevilain Aug 17, 2024

Choose a reason for hiding this comment

alexandrevilain commented Aug 7, 2024

alexandrevilain commented Aug 8, 2024

TheHiddenLayer commented Aug 8, 2024

alexandrevilain Aug 17, 2024

Choose a reason for hiding this comment

TheHiddenLayer Aug 19, 2024

Choose a reason for hiding this comment

alexandrevilain Aug 17, 2024

Choose a reason for hiding this comment

TheHiddenLayer commented Aug 25, 2024 • edited Loading

sonarqubecloud bot commented Aug 29, 2024

Quality Gate passed

alexandrevilain commented Sep 26, 2024

TheHiddenLayer commented Sep 26, 2024 • edited Loading

sonarqubecloud bot commented Oct 1, 2024

Quality Gate passed

TheHiddenLayer commented Oct 1, 2024

TheHiddenLayer commented Jul 23, 2024 •

edited

Loading

TheHiddenLayer commented Jul 24, 2024 •

edited

Loading

TheHiddenLayer commented Jul 25, 2024 •

edited

Loading

TheHiddenLayer commented Aug 25, 2024 •

edited

Loading

TheHiddenLayer commented Sep 26, 2024 •

edited

Loading