morhidi commented on code in PR #517:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/517#discussion_r1095776890
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/utils/IngressUtils.java:
##########
@@ -175,4 +233,16 @@ private static String addProtocol(String url) {
}
return url;
}
+
+ public static boolean ingressInNetworkingV1(KubernetesClient client) {
Review Comment:
Can we use a built in
https://docs.oracle.com/javase/9/docs/api/java/lang/module/ModuleDescriptor.Version.html
for this?
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/utils/IngressUtils.java:
##########
@@ -65,18 +65,35 @@ public static void updateIngressRules(
KubernetesClient client) {
if (spec.getIngress() != null) {
- Ingress ingress =
- new IngressBuilder()
- .withNewMetadata()
-
.withAnnotations(spec.getIngress().getAnnotations())
- .withName(objectMeta.getName())
- .withNamespace(objectMeta.getNamespace())
- .endMetadata()
- .withNewSpec()
-
.withIngressClassName(spec.getIngress().getClassName())
- .withRules(getIngressRule(objectMeta, spec,
effectiveConfig))
- .endSpec()
- .build();
+ HasMetadata ingress;
+ if (ingressInNetworkingV1(client)) {
+ ingress =
Review Comment:
Let's use a helper for each block here
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkDeploymentControllerTest.java:
##########
@@ -1005,18 +1005,42 @@ public void testIngressLifeCycle() throws Exception {
appWithIngress.getSpec().setIngress(ingressSpec);
testController.reconcile(appWithIngress, context);
testController.reconcile(appWithIngress, context);
- Ingress ingress =
- kubernetesClient
- .network()
- .v1()
- .ingresses()
-
.inNamespace(appWithIngress.getMetadata().getNamespace())
- .withName(appWithIngress.getMetadata().getName())
- .get();
- Assertions.assertNotNull(ingress);
- IngressRule ingressRule =
ingress.getSpec().getRules().stream().findFirst().get();
+
+ Ingress ingress = null;
+ io.fabric8.kubernetes.api.model.networking.v1beta1.Ingress
ingressV1beta1 = null;
+ if (IngressUtils.ingressInNetworkingV1(kubernetesClient)) {
+ ingress =
+ kubernetesClient
Review Comment:
Let's extract the initialisations into some helpers for better readability
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/utils/IngressUtilsTest.java:
##########
@@ -57,37 +57,71 @@ public void testIngress() {
// no ingress when ingressDomain is empty
IngressUtils.updateIngressRules(
appCluster.getMetadata(), appCluster.getSpec(), config,
client);
- assertNull(
Review Comment:
Maybe its time to split this test into smaller chunks :)
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkDeploymentControllerTest.java:
##########
@@ -1029,19 +1053,35 @@ public void testIngressLifeCycle() throws Exception {
appWithIngress.getSpec().setIngress(ingressSpec);
testController.reconcile(appWithIngress, context);
testController.reconcile(appWithIngress, context);
- ingress =
- kubernetesClient
- .network()
- .v1()
- .ingresses()
-
.inNamespace(appWithIngress.getMetadata().getNamespace())
- .withName(appWithIngress.getMetadata().getName())
- .get();
- Assertions.assertNotNull(ingress);
- ingressRule = ingress.getSpec().getRules().stream().findFirst().get();
- Assertions.assertNotNull(ingressRule);
+ if (IngressUtils.ingressInNetworkingV1(kubernetesClient)) {
+ ingress =
Review Comment:
extract these to helper too, pls
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/utils/IngressUtils.java:
##########
@@ -65,18 +65,35 @@ public static void updateIngressRules(
KubernetesClient client) {
if (spec.getIngress() != null) {
- Ingress ingress =
- new IngressBuilder()
- .withNewMetadata()
-
.withAnnotations(spec.getIngress().getAnnotations())
- .withName(objectMeta.getName())
- .withNamespace(objectMeta.getNamespace())
- .endMetadata()
- .withNewSpec()
-
.withIngressClassName(spec.getIngress().getClassName())
- .withRules(getIngressRule(objectMeta, spec,
effectiveConfig))
- .endSpec()
- .build();
+ HasMetadata ingress;
+ if (ingressInNetworkingV1(client)) {
+ ingress =
+ new IngressBuilder()
+ .withNewMetadata()
+
.withAnnotations(spec.getIngress().getAnnotations())
+ .withName(objectMeta.getName())
+ .withNamespace(objectMeta.getNamespace())
+ .endMetadata()
+ .withNewSpec()
+
.withIngressClassName(spec.getIngress().getClassName())
+ .withRules(getIngressRule(objectMeta, spec,
effectiveConfig))
+ .endSpec()
+ .build();
+ } else {
+ ingress =
Review Comment:
Likewise
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
