feat: Add labels and annotations to pods.

cpitstick-latai commented 2 weeks ago

Currently, there is only one deployment, so avoid premature optimization by making these global (and follow the same pattern for imagePullSecrets)

Requires more aggressive usage of the strip-kustomize-helm.sh script because we have to preserve existing annotations and labels from the config/manager directory.

This PR

adds this new feature

Related Issues

Fixes #1234523

Notes

Follow-up Tasks

How to test

toddbaert commented 2 weeks ago

I'll give this and your other PRs a test tomorrow.

toddbaert commented 2 weeks ago

Hey @cpitstick-latai this is a great start! I was able to easily get the annotations/labels working locally after running make helm-release and setting some in my values.yaml. I think I see 2 issues:

NOT passing any values seems to break the install. It's not immediately obvious to me why, since your defaults seem correct. In any case, I see:

helm upgrade --install open-feature-operator  /home/todd/git/open-feature-operator/charts/open-feature-operator-v0.6.1.tgz  
Release "open-feature-operator" does not exist. Installing it now.
Error: YAML parse error on open-feature-operator/templates/apps_v1_deployment_open-feature-operator-controller-manager.yaml: error converting YAML to JSON: yaml: line 16: did not find expected key

This PR doesn't deal with the runtime pods created by the operator. I think the annotations/labels should be added there as well, similar to how we added imagePullSecret there. See https://github.com/open-feature/open-feature-operator/pull/655 and https://github.com/open-feature/open-feature-operator/pull/666

thisthat commented 2 weeks ago

Hey @cpitstick-latai this is a great start! I was able to easily get the annotations/labels working locally after running make helm-release and setting some in my values.yaml. I think I see 2 issues:

NOT passing any values seems to break the install. It's not immediately obvious to me why, since your defaults seem correct. In any case, I see:
helm upgrade --install open-feature-operator  /home/todd/git/open-feature-operator/charts/open-feature-operator-v0.6.1.tgz  
Release "open-feature-operator" does not exist. Installing it now.
Error: YAML parse error on open-feature-operator/templates/apps_v1_deployment_open-feature-operator-controller-manager.yaml: error converting YAML to JSON: yaml: line 16: did not find expected key
This PR doesn't deal with the runtime pods created by the operator. I think the annotations/labels should be added there as well, similar to how we added imagePullSecret there. See feat: add image pull secrets #655 and fix: handle multiple imagePullSecrets #666

This sounds that we need to implement a CI action for testing the Helm installation in the e2e tests 🤔

cpitstick-latai commented 2 weeks ago

I'm working on addressing the issues you've found.

As to the e2e helm test, that feels out of scope for this PR. HOWEVER, we should create an issue for it.

cpitstick-latai commented 2 weeks ago

I think I fixed the issue where empty dictionaries {} break.

Thinking about how to implement the runtime pod issue.

open-feature / open-feature-operator