[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator
tagarr commented on code in PR #420: URL: https://github.com/apache/flink-kubernetes-operator/pull/420#discussion_r1023704151 ## helm/flink-kubernetes-operator/templates/webhook.yaml: ## @@ -85,7 +85,7 @@ metadata: cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/flink-operator-serving-cert name: flink-operator-{{ .Release.Namespace }}-webhook-configuration webhooks: -- name: flinkoperator.flink.apache.org +- name: validationwebhook.flink.apache.org Review Comment: the webhook names need to be unique -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator
tagarr commented on code in PR #420: URL: https://github.com/apache/flink-kubernetes-operator/pull/420#discussion_r1023719290 ## helm/flink-kubernetes-operator/templates/webhook.yaml: ## @@ -85,7 +85,7 @@ metadata: cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/flink-operator-serving-cert name: flink-operator-{{ .Release.Namespace }}-webhook-configuration webhooks: -- name: flinkoperator.flink.apache.org +- name: validationwebhook.flink.apache.org Review Comment: will do -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator
tagarr commented on code in PR #420: URL: https://github.com/apache/flink-kubernetes-operator/pull/420#discussion_r1023837986 ## helm/flink-kubernetes-operator/templates/webhook.yaml: ## @@ -121,7 +121,7 @@ metadata: cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/flink-operator-serving-cert name: flink-operator-{{ .Release.Namespace }}-webhook-configuration webhooks: - - name: flinkoperator.flink.apache.org + - name: mutationwebhook.flink.apache.org Review Comment: it has been changed -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator
tagarr commented on code in PR #420: URL: https://github.com/apache/flink-kubernetes-operator/pull/420#discussion_r1024106606 ## flink-kubernetes-webhook/pom.xml: ## @@ -96,6 +96,12 @@ under the License. ${okhttp.version} test + +org.mockito Review Comment: @morhidi Understand, I looked at using https://junit-pioneer.org/docs/environment-variables/ which has some nice capabilities to change system env vars, but noticed when running it I get `This extension uses reflection to mutate JDK-internal state, which is fragile` warnings when running in intellij. Do you think it would be ok to add a method to EnvUtils e.g. setEnvironment(Map env) and use this to override System env vars ? Or can you think of something better ? -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator
tagarr commented on code in PR #420: URL: https://github.com/apache/flink-kubernetes-operator/pull/420#discussion_r1024130690 ## flink-kubernetes-webhook/pom.xml: ## @@ -96,6 +96,12 @@ under the License. ${okhttp.version} test + +org.mockito Review Comment: Actually noticed there is a method in TestUtils, am I ok to use 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator
tagarr commented on code in PR #420: URL: https://github.com/apache/flink-kubernetes-operator/pull/420#discussion_r1024148468 ## helm/flink-kubernetes-operator/templates/flink-operator.yaml: ## @@ -79,7 +79,13 @@ spec: {{- end }} env: - name: OPERATOR_NAMESPACE - value: {{ .Release.Namespace }} + valueFrom: +fieldRef: + fieldPath: metadata.namespace +- name: WATCH_NAMESPACES + valueFrom: +fieldRef: + fieldPath: metadata.annotations['olm.targetNamespaces'] Review Comment: I can remove this and work out how to inject it into the csv if you prefer ? -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator
tagarr commented on code in PR #420: URL: https://github.com/apache/flink-kubernetes-operator/pull/420#discussion_r1026448880 ## helm/flink-kubernetes-operator/templates/flink-operator.yaml: ## @@ -79,7 +79,13 @@ spec: {{- end }} env: - name: OPERATOR_NAMESPACE - value: {{ .Release.Namespace }} + valueFrom: +fieldRef: + fieldPath: metadata.namespace +- name: WATCH_NAMESPACES + valueFrom: +fieldRef: + fieldPath: metadata.annotations['olm.targetNamespaces'] Review Comment: done -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator
tagarr commented on code in PR #420: URL: https://github.com/apache/flink-kubernetes-operator/pull/420#discussion_r1038965294 ## helm/flink-kubernetes-operator/templates/flink-operator.yaml: ## @@ -79,7 +79,9 @@ spec: {{- end }} env: - name: OPERATOR_NAMESPACE Review Comment: So it is a more consistent way of defining the operator namespace and works for all install methods. That's why I made the change. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org