You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "Vince-Chenal (via GitHub)" <gi...@apache.org> on 2023/01/30 16:23:25 UTC
[GitHub] [flink-kubernetes-operator] Vince-Chenal opened a new pull request, #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Vince-Chenal opened a new pull request, #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519
## What is the purpose of the change
This pull request adds the possibility for helm chart users to specify resources block for operator pod on flink-webhook container.
Forgot to add this in previous PR... https://github.com/apache/flink-kubernetes-operator/pull/516
## Brief change log
- Allow configuring resources block for operator pod flink-webhook container
## Verifying this change
<!--
Please make sure both new and modified tests in this PR follows the conventions defined in our code quality guide: https://flink.apache.org/contributing/code-style-and-quality-common.html#testing
-->
This change is a trivial rework / code cleanup without any test coverage.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changes to the `CustomResourceDescriptors`: no
- Core observer or reconciler logic that is regularly executed: no
## Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? docs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] mbalassi commented on a diff in pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "mbalassi (via GitHub)" <gi...@apache.org>.
mbalassi commented on code in PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519#discussion_r1091304786
##########
docs/content/docs/operations/helm.md:
##########
@@ -74,7 +74,8 @@ The configurable parameters of the Helm chart and which default values as detail
| operatorPod.dnsPolicy | DNS policy to be used by the operator pod. | |
| operatorPod.dnsConfig | DNS configuration to be used by the operator pod. | |
| operatorPod.nodeSelector | Custom nodeSelector to be added to the operator pod. | |
-| operatorPod.resources | Custom resources block to be added to the operator pod. | |
+| operatorPod.resources | Custom resources block to be added to the operator pod on main container. | |
+| operatorPod.flinkWebhookResources | Custom resources block to be added to the operator pod on flink-webhook container. | |
Review Comment:
cc @Vince-Chenal
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] mbalassi commented on a diff in pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "mbalassi (via GitHub)" <gi...@apache.org>.
mbalassi commented on code in PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519#discussion_r1091304685
##########
docs/content/docs/operations/helm.md:
##########
@@ -74,7 +74,8 @@ The configurable parameters of the Helm chart and which default values as detail
| operatorPod.dnsPolicy | DNS policy to be used by the operator pod. | |
| operatorPod.dnsConfig | DNS configuration to be used by the operator pod. | |
| operatorPod.nodeSelector | Custom nodeSelector to be added to the operator pod. | |
-| operatorPod.resources | Custom resources block to be added to the operator pod. | |
+| operatorPod.resources | Custom resources block to be added to the operator pod on main container. | |
+| operatorPod.flinkWebhookResources | Custom resources block to be added to the operator pod on flink-webhook container. | |
Review Comment:
Let us do `webhookPod` then and include the `dnsConfig` please.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] retrry commented on a diff in pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "retrry (via GitHub)" <gi...@apache.org>.
retrry commented on code in PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519#discussion_r1090990567
##########
docs/content/docs/operations/helm.md:
##########
@@ -74,7 +74,8 @@ The configurable parameters of the Helm chart and which default values as detail
| operatorPod.dnsPolicy | DNS policy to be used by the operator pod. | |
| operatorPod.dnsConfig | DNS configuration to be used by the operator pod. | |
| operatorPod.nodeSelector | Custom nodeSelector to be added to the operator pod. | |
-| operatorPod.resources | Custom resources block to be added to the operator pod. | |
+| operatorPod.resources | Custom resources block to be added to the operator pod on main container. | |
+| operatorPod.flinkWebhookResources | Custom resources block to be added to the operator pod on flink-webhook container. | |
Review Comment:
Maybe there should be new key `webhookPod` to be consistent with `operatorPod`? Or jus use `operatorPod.resources` a some configuration for webhook container is used from `operatorPod` key?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora merged pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "gyfora (via GitHub)" <gi...@apache.org>.
gyfora merged PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] Vince-Chenal commented on a diff in pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "Vince-Chenal (via GitHub)" <gi...@apache.org>.
Vince-Chenal commented on code in PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519#discussion_r1093170731
##########
docs/content/docs/operations/helm.md:
##########
@@ -74,7 +74,8 @@ The configurable parameters of the Helm chart and which default values as detail
| operatorPod.dnsPolicy | DNS policy to be used by the operator pod. | |
| operatorPod.dnsConfig | DNS configuration to be used by the operator pod. | |
| operatorPod.nodeSelector | Custom nodeSelector to be added to the operator pod. | |
-| operatorPod.resources | Custom resources block to be added to the operator pod. | |
+| operatorPod.resources | Custom resources block to be added to the operator pod on main container. | |
+| operatorPod.flinkWebhookResources | Custom resources block to be added to the operator pod on flink-webhook container. | |
Review Comment:
I moved it to `.Values.operatorPod.webhook.resources`, does it sound ok for you ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] Vince-Chenal commented on a diff in pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "Vince-Chenal (via GitHub)" <gi...@apache.org>.
Vince-Chenal commented on code in PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519#discussion_r1091603421
##########
docs/content/docs/operations/helm.md:
##########
@@ -74,7 +74,8 @@ The configurable parameters of the Helm chart and which default values as detail
| operatorPod.dnsPolicy | DNS policy to be used by the operator pod. | |
| operatorPod.dnsConfig | DNS configuration to be used by the operator pod. | |
| operatorPod.nodeSelector | Custom nodeSelector to be added to the operator pod. | |
-| operatorPod.resources | Custom resources block to be added to the operator pod. | |
+| operatorPod.resources | Custom resources block to be added to the operator pod on main container. | |
+| operatorPod.flinkWebhookResources | Custom resources block to be added to the operator pod on flink-webhook container. | |
Review Comment:
In fact there are two containers within the same pod, that's why I decided to keep it within operatorPod field.
`operatorPod.resources` is already confusing as we don't know which container it configures.
what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] retrry commented on a diff in pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "retrry (via GitHub)" <gi...@apache.org>.
retrry commented on code in PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519#discussion_r1091033497
##########
docs/content/docs/operations/helm.md:
##########
@@ -74,7 +74,8 @@ The configurable parameters of the Helm chart and which default values as detail
| operatorPod.dnsPolicy | DNS policy to be used by the operator pod. | |
| operatorPod.dnsConfig | DNS configuration to be used by the operator pod. | |
| operatorPod.nodeSelector | Custom nodeSelector to be added to the operator pod. | |
-| operatorPod.resources | Custom resources block to be added to the operator pod. | |
+| operatorPod.resources | Custom resources block to be added to the operator pod on main container. | |
+| operatorPod.flinkWebhookResources | Custom resources block to be added to the operator pod on flink-webhook container. | |
Review Comment:
Should `webhookPod.dnsConfig`be created for consistency, as currently webhook container config uses `operatorPod.dnsConfig`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] retrry commented on a diff in pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "retrry (via GitHub)" <gi...@apache.org>.
retrry commented on code in PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519#discussion_r1090990567
##########
docs/content/docs/operations/helm.md:
##########
@@ -74,7 +74,8 @@ The configurable parameters of the Helm chart and which default values as detail
| operatorPod.dnsPolicy | DNS policy to be used by the operator pod. | |
| operatorPod.dnsConfig | DNS configuration to be used by the operator pod. | |
| operatorPod.nodeSelector | Custom nodeSelector to be added to the operator pod. | |
-| operatorPod.resources | Custom resources block to be added to the operator pod. | |
+| operatorPod.resources | Custom resources block to be added to the operator pod on main container. | |
+| operatorPod.flinkWebhookResources | Custom resources block to be added to the operator pod on flink-webhook container. | |
Review Comment:
Maybe there should be new key `webhookPod` to be consistent with `operatorPod`? Or jus use `operatorPod.resources` as a some configuration for webhook container is used from `operatorPod` key?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "morhidi (via GitHub)" <gi...@apache.org>.
morhidi commented on code in PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519#discussion_r1091006333
##########
docs/content/docs/operations/helm.md:
##########
@@ -74,7 +74,8 @@ The configurable parameters of the Helm chart and which default values as detail
| operatorPod.dnsPolicy | DNS policy to be used by the operator pod. | |
| operatorPod.dnsConfig | DNS configuration to be used by the operator pod. | |
| operatorPod.nodeSelector | Custom nodeSelector to be added to the operator pod. | |
-| operatorPod.resources | Custom resources block to be added to the operator pod. | |
+| operatorPod.resources | Custom resources block to be added to the operator pod on main container. | |
+| operatorPod.flinkWebhookResources | Custom resources block to be added to the operator pod on flink-webhook container. | |
Review Comment:
I vote for `webhookPod` for the sake of consistency
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] mbalassi commented on a diff in pull request #519: [FLINK-30832] helm/flink-kubernetes-operator - allow specifying resources for operator pod on flink-webhook container
Posted by "mbalassi (via GitHub)" <gi...@apache.org>.
mbalassi commented on code in PR #519:
URL: https://github.com/apache/flink-kubernetes-operator/pull/519#discussion_r1090916003
##########
docs/content/docs/operations/helm.md:
##########
@@ -74,7 +74,8 @@ The configurable parameters of the Helm chart and which default values as detail
| operatorPod.dnsPolicy | DNS policy to be used by the operator pod. | |
| operatorPod.dnsConfig | DNS configuration to be used by the operator pod. | |
| operatorPod.nodeSelector | Custom nodeSelector to be added to the operator pod. | |
-| operatorPod.resources | Custom resources block to be added to the operator pod. | |
+| operatorPod.resources | Custom resources block to be added to the operator pod on main container. | |
+| operatorPod.flinkWebhookResources | Custom resources block to be added to the operator pod on flink-webhook container. | |
Review Comment:
How about `webhook.resurces` instead? It is a bit more concise and would make sense to group it like that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org