[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator

2022-12-04 Thread GitBox 


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

[GitHub] [flink-kubernetes-operator] tagarr commented on a diff in pull request #420: FLINK-29536 - Add WATCH_NAMESPACE env var to operator

2022-11-18 Thread GitBox 


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

2022-11-16 Thread GitBox 


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

2022-11-16 Thread GitBox 


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

2022-11-16 Thread GitBox 


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

2022-11-16 Thread GitBox 


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

2022-11-16 Thread GitBox 


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

2022-11-16 Thread GitBox 


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