-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
[bitnami/kafka] * Allow podSelector from any namespaceSelector #28719
Conversation
4db8117
to
b37f7e1
Compare
Thank you for initiating this pull request. We appreciate your effort. Just a friendly reminder that it's important to sign your commits. Adding your signature certifies that you either authored the patch or have the necessary rights to contribute the changes. You can find detailed information on how to do this in the “Sign your work” section of our contributing guidelines. Feel free to reach out if you have any questions or need assistance with the signing process. |
Thanks @carrodher, i've signed the commit as per below but the DCO check still fails.. does it need a newline after or something? |
Following the instructions from the failing action should be enough, it is not needed to add new code. See https://github.com/bitnami/charts/pull/28719/checks?check_run_id=28440524888 |
* [bitnami/grafana-operator] Release 4.4.14 updating components versions Signed-off-by: Bitnami Containers <[email protected]> * Update CHANGELOG.md Signed-off-by: Bitnami Containers <[email protected]> --------- Signed-off-by: Bitnami Containers <[email protected]> Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
Thanks @carrodher. Fixed - should be ready for your review now |
Signed-off-by: Carlos Rodríguez Hernández <[email protected]>
Signed-off-by: Carlos Rodríguez Hernández <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
Just wondering if good to merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we apply the #26597 changes here I think you can cover your needs using the value |
Ok, thanks! when are you targeting release of #26597 |
Sorry, maybe I didn't explain it very well. #26597 is in our template but that's not mean that we will apply those changes in short term to all our charts. Taking the advantage that you are changing the NetworkPolicy, Do those changes fit your needs? If that the case, Could you apply those changes here instead the solution you adopted? |
@fmulero have tested this just now and it works, happy to close the PR if you wan't. In general it'd be good to strive for consistency across charts for this type of thing (ie Redis's netpol still works in line with this PR) |
Hi @tbotnz, I've just created the PR #29274 with the changes I mentioned. Sorry for being so picky but I prefer having those changes instead of these to avoid changing the behaviour in a short period of time mainly when the same value ( |
Description of the change
Allow podSelector from any namespaceSelector to make kafka network policy behave like #12607 in the kafka chart
Benefits
Possible drawbacks
Applicable issues
#12607 #28720
Additional information
Checklist
Chart.yaml
according to semver. This is not necessary when the changes only affect README.md files.README.md
using readme-generator-for-helm