Implement values for hubble-relay to properly control sub chart values#11757
Implement values for hubble-relay to properly control sub chart values#11757tgraf merged 1 commit intocilium:masterfrom seanmwinn:cilium-11755
Conversation
|
Please set the appropriate release note label. |
michi-covalent
left a comment
There was a problem hiding this comment.
lgtm, i have a question regarding global vs local values files.
sayboras
left a comment
There was a problem hiding this comment.
You might need to change this as well https://github.com/cilium/cilium/blob/master/install/kubernetes/cilium/requirements.yaml#L10
Just want to express my opinion, I see the benefit of this change, which will help to move one step making sub charts are more self contrained and independent.
However, there is inter-depedency between sub-charts, and currently we are using global as controlling flag (e.g. hubble-cli chart checks global.hubble.enabled). So with change, the subcharts can be using values from other sub-charts directly, I feel this will make it hard to understand and manage.
|
@sayboras Thanks for your feedback. We are trying to focus more on your first point which indicates this will make the charts function in a manner that allows for proper control of sub-charts. The long-term goal is to minimize the use of global values and build sub-charts that are configurable, tunable, and expose the knobs that operators are used to having available such as update strategy, tolerations, annotation, affinity, and others. Today, the parent chart's values file holds little more than configmap values and is not implemented in a manner that meets basic best practices. |
@seanmwinn making sense to me 💯, thanks for your explaination 🎉 |
There was a problem hiding this comment.
what do think about moving these two blocks all the way up, right after preflight subchart values? I think it's easier for user to follow.
There was a problem hiding this comment.
nits: tag, pullpolicy are still refering to global values. I can send PR later if you are ok.
|
@seanmwinn there is another PR that's related to this: #11784 if we are including the hubble binary in the cilium docker image, does it mean we can simply get rid of the hubble-cli subchart? let's sync up with @rolinh . |
michi-covalent
left a comment
There was a problem hiding this comment.
i'm blocking this until we clarify if the hubble-cli chart is still needed after #11784
michi-covalent
left a comment
There was a problem hiding this comment.
lgtm, it might conflict with #11806
|
Commit f9427cb355b6b0f7e55d9809e4652374be6dda44 does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
|
test-me-please |
Fixes: #11755 Signed-off-by: Sean Winn <sean@isovalent.com>
|
test-me-please |
Implements hubble-relay values to properly control sub chart values using the parent chart's values file.
Documents values in the parent chart for overriding.
Fixes: #11755
Signed-off-by: Sean Winn sean@isovalent.com