add helm chart values for pod annotations and labels#118
add helm chart values for pod annotations and labels#118aolear-ss wants to merge 1 commit intostrimzi:mainfrom
Conversation
aolear-ss
force-pushed
the
main
branch
2 times, most recently
from
434f7d4 to
f97bbb4
Compare
scholzj
left a comment
scholzj
left a comment
There was a problem hiding this comment.
I left some comments. But this enhancmenet makes sense. Thanks for opening the PR.
| appVersion: "0.2.0" | ||
| description: "Strimzi Kafka Access Operator" | ||
| name: strimzi-access-operator | ||
| version: 0.1.0 | ||
| version: 0.2.0 |
There was a problem hiding this comment.
I don't think we are changing these here.
There was a problem hiding this comment.
i can revert this change. Is the chart versioning handled elsewhere or are we just overwriting a static 0.1.0 version?
There was a problem hiding this comment.
TBH, I think the release tooling might handle it? In any case, we do not do any version changes for changes like this.
There was a problem hiding this comment.
In the packaging I think there should be future version of the operator and in the released Chart.yaml should be the latest released version - 0.2.0 in this case. However, it seems that we are just updating these values in Drain Cleaner repository.
But this should be changed in different PR IMO, so if you can please revert these changes from this PR, thanks :)
I think we will need to add some step to our release guide or to some automation we have (nothing for this PR).
| | `podAnnotations` | Additional annotations to apply to Access Operator Pod | `{}` | | ||
| | `podLabels` | Additional labels to apply to Access Operator Pod | `{}` | |
There was a problem hiding this comment.
Should we follow the Cluster Operator Helm Chart and call it just labels and annotations? WDYT @katheris and @strimzi/maintainers?
There was a problem hiding this comment.
i went with podAnnotations and podLabels to leave room if a future requirement dictated needing to manage different annotations or labels for different objects managed by the chart such as the deployment or service account.
In my use case I don't require that, but I have seen other charts take that approach for allowing for the most flexibility. If desired I can add those different options to that chart as well, or whatever the rest of the maintainers would like to see
There was a problem hiding this comment.
taking another look at their chart, it looks like they have annotations, which targets the pod and deploymentAnnotations which targets the deployment.
So I'll wait to hear from the rest of the maintainers, but I think you're right to keep it consistent for strimzi, I should rename this to just annotations
There was a problem hiding this comment.
Thanks @aolear-ss for the changes, I would keep it consistent with the Cluster Operator.
Signed-off-by: Alex <alex.olear@scaledsense.com>
3 participants
This pull request adds additional options to the helm chart for customization of the operator deployment.
One example of where this is necessary is hosting the Strimzi Access Operator in an Azure Kubernetes Cluster with a layer 7 egress firewall to set the
kubernetes.azure.com/set-kube-service-host-fqdnannotation. More info: https://learn.microsoft.com/en-us/azure/aks/outbound-rules-control-egress#required-outbound-network-rules-and-fqdns-for-aks-clusters