-
Notifications
You must be signed in to change notification settings - Fork 203
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
feat(helm): add support for NetworkPolicies in v2 Helm chart #3164
Conversation
@microsoft-github-policy-service agree company="Adfinis" |
b765eb0
to
92165f2
Compare
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.
Small comment, but other than that looks good to me.
We need to add another label to the pod as well, but we'll do that out of band. I'll file a separate issue for it.
@tongpu is this still something you're driving? If not we can probably take it over and get it merged. Let us know. |
I've lost track of it as life was rather busy recently. I'll push the proposed changes by next week the latest. |
0e0b4df
to
291a9a3
Compare
@matthchr Thanks for the reminder. I've pushed the remaining changes and I think the PR should be ready now. |
/ok-to-test sha=291a9a3 |
# Destination CIDR for talking to the Kubernetes API | ||
kubernetesApiCIDR: 0.0.0.0/0 | ||
# Destination CIDR for talking to MySQL servers | ||
kubernetesApiCIDR: 0.0.0.0/0 |
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.
copy/paste error (caught by CI): This should be mysqlCIDR
, and the one below postgresqlCIDR
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.
Fixed. One should not commit on a Friday afternoon.
/ok-to-test sha=be6ebf1 |
protocol: TCP | ||
to: | ||
- ipBlock: | ||
cidr: {{ .Values.networkpolicies.kubernetesApiCIDR }} |
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.
Ahh, I missed CI was also having issue with networkpolicies
vs networkPolicies
.
It's supposed to be networkPolicies
but a few of these here have a lowercase p
/ok-to-test sha=11672eb |
2 similar comments
/ok-to-test sha=11672eb |
/ok-to-test sha=11672eb |
/ok-to-test sha=94ffb6f |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
live mysql test timed out.. retrying |
/ok-to-test sha=94ffb6f |
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.
Doh, forgot to approve earlier.
Thanks for the contribution @tongpu! |
Closes #3160
What this PR does / why we need it:
Adds support for NetworkPolicies to ASO v2 Helm chart
Special notes for your reviewer:
How does this PR make you feel:
If applicable: