Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pod Labels not applied to StatefulSet #511

Closed
endzyme opened this issue Nov 3, 2022 · 2 comments · Fixed by #521
Closed

Pod Labels not applied to StatefulSet #511

endzyme opened this issue Nov 3, 2022 · 2 comments · Fixed by #521

Comments

@endzyme
Copy link
Contributor

endzyme commented Nov 3, 2022

Description

The zookeepercluster.spec.pod.labels are not applied to the final pods or the StatefulSet created by the Zookeeper Operator.

Importance

should-have: If it's in the spec, it should work as intended or be removed.
blocker: Since the Solr Operator uses this Zookeeper Operator, it is currently blocking us from adding custom labels to Zookeeper Clusters created with the Solr Operator.

Location

Unsure, would need some help to track it down, but is likely related to #257.

Suggestions for an improvement

Taking into account the features or #257, this should have a merge between the two maps and apply the final merge to the template spec of the StatefulSet.

endzyme added a commit to endzyme/solr-operator that referenced this issue Dec 30, 2022
Zookeeper cluster CRD doesn't actually pass the pod.labels to the
StatefulSet pods, instead it passes all labels in spec.labels to the
statefulset. This problem is outlined in pravega/zookeeper-operator#511.

This change makes it so that any labels which are provided in the pod
labels are merged into the spec.labels of the zookeeper cluster crd.
This also prioritizes any needed labels provided by the Solr Operator
thus not allowing you to override any necessary labels like 'app' or
'release'.
endzyme added a commit to endzyme/solr-operator that referenced this issue Dec 30, 2022
Zookeeper cluster CRD doesn't actually pass the pod.labels to the
StatefulSet pods, instead it passes all labels in spec.labels to the
statefulset. This problem is outlined in pravega/zookeeper-operator#511.

This change makes it so that any labels which are provided in the pod
labels are merged into the spec.labels of the zookeeper cluster crd.
This also prioritizes any needed labels provided by the Solr Operator
thus not allowing you to override any necessary labels like 'app' or
'release'.
endzyme added a commit to endzyme/solr-operator that referenced this issue Dec 30, 2022
Zookeeper cluster CRD spec.pod.labels presently does not function as
intended. StatefulSet pods receive all their labels from spec.labels.
This is outlined in pravega/zookeeper-operator#511.

This change makes it so that any labels which are provided in the pod
labels are merged into the spec.labels of the Zookeeper cluster CRD.
This also prioritizes any labels provided by the Solr Operator, thus not
allowing you to override any important labels like 'app' or 'release'
when using the spec.pod.labels in the SolrCloud CRD.
@endzyme
Copy link
Contributor Author

endzyme commented Jan 3, 2023

@anishakj if you point me in the right direction, I can make a PR.

I am thinking there are two approaches.

  1. Remove the spec.pod.labels from the CRD
  2. Make the behavior the same as spec.labels and document which spec keys override one another
  3. Change the behavior so spec.labels only applies labels to statefulset and let spec.pod.labels be the place for pod label customization

Any thoughts or alternatives?

endzyme added a commit to endzyme/solr-operator that referenced this issue Jan 3, 2023
Zookeeper cluster CRD spec.pod.labels presently does not function as
intended. StatefulSet pods receive all their labels from spec.labels.
This is outlined in pravega/zookeeper-operator#511.

This change makes it so that any labels which are provided in the pod
labels are merged into the spec.labels of the Zookeeper cluster CRD.
This also prioritizes any labels provided by the Solr Operator, thus not
allowing you to override any important labels like 'app' or 'release'
when using the spec.pod.labels in the SolrCloud CRD.
endzyme added a commit to endzyme/solr-operator that referenced this issue Jan 3, 2023
Zookeeper cluster CRD spec.pod.labels presently does not function as
intended. StatefulSet pods receive all their labels from spec.labels.
This is outlined in pravega/zookeeper-operator#511.

This change makes it so that any labels which are provided in the pod
labels are merged into the spec.labels of the Zookeeper cluster CRD.
This also prioritizes any labels provided by the Solr Operator, thus not
allowing you to override any important labels like 'app' or 'release'
when using the spec.pod.labels in the SolrCloud CRD.

Co-authored-by: Josh Souza <[email protected]>
HoustonPutman pushed a commit to apache/solr-operator that referenced this issue Jan 3, 2023
Zookeeper cluster CRD spec.pod.labels presently does not function as
intended. StatefulSet pods receive all their labels from spec.labels.
This is outlined in pravega/zookeeper-operator#511.

This change makes it so that any labels which are provided in the pod
labels are merged into the spec.labels of the Zookeeper cluster CRD.
This also prioritizes any labels provided by the Solr Operator, thus not
allowing you to override any important labels like 'app' or 'release'
when using the spec.pod.labels in the SolrCloud CRD.

Co-authored-by: Josh Souza <[email protected]>
@endzyme
Copy link
Contributor Author

endzyme commented Jan 5, 2023

@anishakj if you point me in the right direction, I can make a PR.

I am thinking there are two approaches.

  1. Remove the spec.pod.labels from the CRD
  2. Make the behavior the same as spec.labels and document which spec keys override one another
  3. Change the behavior so spec.labels only applies labels to statefulset and let spec.pod.labels be the place for pod label customization

Any thoughts or alternatives?

I ended up taking approach 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant