Fix eclipse-jkube#253: Refactor JKubeServiceHub's BuildService electi…

…on mechanism + Use ServiceLoader to load all Build Services and pick up service which is applicable in current context. This requires BuildService implementations to have zero-arg constructors so I need to set the elements from setters instead. + Added isApplicable(), setJKubeServiceHub(), methods in BuildService interface Signed-off-by: Rohan Kumar <[email protected]>
rohanKanojia · Jul 2, 2021 · 379d5bf · 379d5bf
1 parent 2a2c30a
commit 379d5bf
Show file tree

Hide file tree

Showing 12 changed files with 157 additions and 89 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@ Usage:
 ./scripts/extract-changelog-for-version.sh 1.3.37 5
 ```
 ### 1.4.0-SNAPSHOT
+* Fix #253: Refactor JKubeServiceHub's BuildService election mechanism via ServiceLoader
 * Fix #741: Private constructor added to Utility classes
 * Fix #725: Upgrade HttpClient from 4.5.10 to 4.5.13
 * Fix #653: `k8s:watch` port-forward websocket error due to wrong arguments in PortForwardService

diff --git a/...e-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/BuildService.java b/...e-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/BuildService.java
@@ -48,4 +48,18 @@ public interface BuildService {
      */
     void postProcess(BuildServiceConfig config);
 
+    /**
+     * Check whether provided Build Service implementation is applicable in current context or not.
+     *
+     * @param jKubeServiceHub {@link JKubeServiceHub}
+     * @return boolean value specifying whether provided BuildService implementation should be used.
+     */
+    boolean isApplicable(JKubeServiceHub jKubeServiceHub);
+
+    /**
+     * Set {@link JKubeServiceHub}
+     *
+     * @param jKubeServiceHub {@link JKubeServiceHub}
+     */
+    void setJKubeServiceHub(JKubeServiceHub jKubeServiceHub);
 }
diff --git a/...it/config/service/src/main/java/org/eclipse/jkube/kit/config/service/JKubeServiceHub.java b/...it/config/service/src/main/java/org/eclipse/jkube/kit/config/service/JKubeServiceHub.java
@@ -14,8 +14,10 @@
 package org.eclipse.jkube.kit.config.service;
 
 import java.io.Closeable;
+import java.util.Iterator;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.ServiceLoader;
 
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.openshift.client.OpenShiftClient;
@@ -30,11 +32,8 @@
 import org.eclipse.jkube.kit.common.util.LazyBuilder;
 import org.eclipse.jkube.kit.config.access.ClusterAccess;
 import org.eclipse.jkube.kit.config.access.ClusterConfiguration;
-import org.eclipse.jkube.kit.config.image.build.JKubeBuildStrategy;
 import org.eclipse.jkube.kit.common.JKubeConfiguration;
 import org.eclipse.jkube.kit.config.resource.RuntimeMode;
-import org.eclipse.jkube.kit.config.service.kubernetes.DockerBuildService;
-import org.eclipse.jkube.kit.config.service.kubernetes.JibBuildService;
 import org.eclipse.jkube.kit.config.service.kubernetes.KubernetesUndeployService;
 import org.eclipse.jkube.kit.config.service.openshift.OpenshiftBuildService;
 import org.eclipse.jkube.kit.config.service.openshift.OpenshiftUndeployService;
@@ -53,6 +52,7 @@ public class JKubeServiceHub implements Closeable {
     @Getter
     @Setter
     private RuntimeMode platformMode;
+    @Getter
     private KitLogger log;
     @Getter
     private ServiceHub dockerServiceHub;
@@ -166,30 +166,29 @@ private void initClusterAccessAndLazyBuilders() {
             }
             return new KubernetesUndeployService(this, log);
         });
-        buildService = new LazyBuilder<>(() -> {
-            BuildService ret;
-            if (JKubeBuildStrategy.jib == buildServiceConfig.getJKubeBuildStrategy()) {
-                return new JibBuildService(JKubeServiceHub.this, log);
-            }
-            // Creating platform-dependent services
-            if (platformMode == RuntimeMode.OPENSHIFT) {
-                validateIfConnectedToCluster();
-                if (!isOpenShift(client)) {
-                    throw new IllegalStateException("OpenShift platform has been specified but OpenShift has not been detected!");
-                }
-                // OpenShift services
-                ret = new OpenshiftBuildService((OpenShiftClient) client, log, JKubeServiceHub.this);
-            } else {
-                // Kubernetes services
-                ret = new DockerBuildService(JKubeServiceHub.this);
-            }
-            return ret;
-        });
+        buildService = new LazyBuilder<>(this::resolveBuildService);
     }
 
     private void validateIfConnectedToCluster() {
         if (client == null) {
             throw new IllegalArgumentException("Connection to Cluster required. Please check if offline mode is set to false");
         }
     }
+
+    private BuildService resolveBuildService() {
+        ServiceLoader<BuildService> buildServices = ServiceLoader.load(BuildService.class, Thread.currentThread().getContextClassLoader());
+        Iterator<BuildService> buildServiceIterator = buildServices.iterator();
+
+        BuildService selectedBuildService = null;
+        while (buildServiceIterator.hasNext()) {
+            BuildService b = buildServiceIterator.next();
+            if (b.isApplicable(JKubeServiceHub.this)) {
+                selectedBuildService = b;
+                selectedBuildService.setJKubeServiceHub(JKubeServiceHub.this);
+                break;
+            }
+        }
+
+        return selectedBuildService;
+    }
 }
diff --git a/...ice/src/main/java/org/eclipse/jkube/kit/config/service/kubernetes/DockerBuildService.java b/...ice/src/main/java/org/eclipse/jkube/kit/config/service/kubernetes/DockerBuildService.java
@@ -19,6 +19,7 @@
 
 import org.eclipse.jkube.kit.config.image.ImageConfiguration;
 import org.eclipse.jkube.kit.common.RegistryConfig;
+import org.eclipse.jkube.kit.config.resource.RuntimeMode;
 import org.eclipse.jkube.kit.config.service.BuildService;
 import org.eclipse.jkube.kit.config.service.BuildServiceConfig;
 import org.eclipse.jkube.kit.config.service.JKubeServiceException;
@@ -32,7 +33,9 @@ public class DockerBuildService implements BuildService {
 
     private JKubeServiceHub jKubeServiceHub;
 
-    public DockerBuildService(JKubeServiceHub jKubeServiceHub) {
+    public DockerBuildService() { }
+
+    DockerBuildService(JKubeServiceHub jKubeServiceHub) {
         Objects.requireNonNull(jKubeServiceHub.getDockerServiceHub(), "dockerServiceHub");
         Objects.requireNonNull(jKubeServiceHub.getBuildServiceConfig(), "BuildServiceConfig is required");
         this.jKubeServiceHub = jKubeServiceHub;
@@ -68,4 +71,13 @@ public void postProcess(BuildServiceConfig config) {
         // No post processing required
     }
 
+    @Override
+    public boolean isApplicable(JKubeServiceHub jKubeServiceHub) {
+        return jKubeServiceHub.getRuntimeMode() == RuntimeMode.KUBERNETES;
+    }
+
+    @Override
+    public void setJKubeServiceHub(JKubeServiceHub jKubeServiceHub) {
+        this.jKubeServiceHub = jKubeServiceHub;
+    }
 }
diff --git a/...ervice/src/main/java/org/eclipse/jkube/kit/config/service/kubernetes/JibBuildService.java b/...ervice/src/main/java/org/eclipse/jkube/kit/config/service/kubernetes/JibBuildService.java
@@ -30,6 +30,7 @@
 import org.eclipse.jkube.kit.config.image.ImageName;
 import org.eclipse.jkube.kit.common.RegistryConfig;
 import org.eclipse.jkube.kit.common.JKubeConfiguration;
+import org.eclipse.jkube.kit.config.image.build.JKubeBuildStrategy;
 import org.eclipse.jkube.kit.config.service.BuildService;
 import org.eclipse.jkube.kit.config.service.BuildServiceConfig;
 import org.eclipse.jkube.kit.config.service.JKubeServiceException;
@@ -53,13 +54,14 @@ public class JibBuildService implements BuildService {
             "docker.io", "index.docker.io", "registry.hub.docker.com"
     );
     private static final String PUSH_REGISTRY = "jkube.docker.push.registry";
-    private final JKubeServiceHub jKubeServiceHub;
-    private final KitLogger log;
+    private JKubeServiceHub jKubeServiceHub;
+    private KitLogger log;
 
-    public JibBuildService(JKubeServiceHub jKubeServiceHub, KitLogger logger) {
+    public JibBuildService() { }
+
+    JibBuildService(JKubeServiceHub jKubeServiceHub) {
         Objects.requireNonNull(jKubeServiceHub.getBuildServiceConfig(), "BuildServiceConfig is required");
-        this.jKubeServiceHub = jKubeServiceHub;
-        this.log = logger;
+        setJKubeServiceHub(jKubeServiceHub);
     }
 
     @Override
@@ -118,6 +120,17 @@ public void postProcess(BuildServiceConfig config) {
         // No post processing required
     }
 
+    @Override
+    public boolean isApplicable(JKubeServiceHub jKubeServiceHub) {
+        return jKubeServiceHub.getBuildServiceConfig().getJKubeBuildStrategy() == JKubeBuildStrategy.jib;
+    }
+
+    @Override
+    public void setJKubeServiceHub(JKubeServiceHub jKubeServiceHub) {
+        this.jKubeServiceHub = jKubeServiceHub;
+        this.log = jKubeServiceHub.getLog();
+    }
+
     static ImageConfiguration prependRegistry(ImageConfiguration imageConfiguration, String registry) {
         ImageName imageName = new ImageName(imageConfiguration.getName());
         if (!imageName.hasRegistry() && registry != null) {

diff --git a/...e/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenshiftBuildService.java b/...e/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenshiftBuildService.java
@@ -39,12 +39,15 @@
 import org.eclipse.jkube.kit.common.util.IoUtil;
 import org.eclipse.jkube.kit.common.util.KubernetesHelper;
 import org.eclipse.jkube.kit.common.util.OpenshiftHelper;
+import org.eclipse.jkube.kit.config.access.ClusterAccess;
+import org.eclipse.jkube.kit.config.access.ClusterConfiguration;
 import org.eclipse.jkube.kit.config.image.ImageConfiguration;
 import org.eclipse.jkube.kit.config.image.ImageName;
 import org.eclipse.jkube.kit.common.RegistryConfig;
 import org.eclipse.jkube.kit.config.image.build.BuildConfiguration;
 import org.eclipse.jkube.kit.config.image.build.JKubeBuildStrategy;
 import org.eclipse.jkube.kit.common.JKubeConfiguration;
+import org.eclipse.jkube.kit.config.resource.RuntimeMode;
 import org.eclipse.jkube.kit.config.service.BuildService;
 import org.eclipse.jkube.kit.config.service.BuildServiceConfig;
 import org.eclipse.jkube.kit.config.service.JKubeServiceException;
@@ -84,6 +87,7 @@
 
 import static org.eclipse.jkube.kit.build.api.helper.BuildUtil.extractBaseFromConfiguration;
 
+import static org.eclipse.jkube.kit.common.util.OpenshiftHelper.isOpenShift;
 import static org.eclipse.jkube.kit.config.service.openshift.ImageStreamService.resolveImageStreamName;
 
 /**
@@ -99,28 +103,28 @@ public class OpenshiftBuildService implements BuildService {
     public static final String REQUESTS = "requests";
     public static final String LIMITS = "limits";
 
-    private final OpenShiftClient client;
-    private final KitLogger log;
-    private final JKubeServiceHub jKubeServiceHub;
-    private final BuildServiceConfig config;
+    private OpenShiftClient client;
+    private KitLogger log;
+    private JKubeServiceHub jKubeServiceHub;
+    private BuildServiceConfig config;
     private AuthConfigFactory authConfigFactory;
 
+    public OpenshiftBuildService() { }
 
-    public OpenshiftBuildService(OpenShiftClient client, KitLogger log, JKubeServiceHub jKubeServiceHub) {
-        Objects.requireNonNull(client, "client");
-        Objects.requireNonNull(log, "log");
-        Objects.requireNonNull(jKubeServiceHub.getConfiguration(), "JKubeConfiguration");
-        Objects.requireNonNull(jKubeServiceHub.getDockerServiceHub(), "dockerServiceHub");
-        Objects.requireNonNull(jKubeServiceHub.getBuildServiceConfig(), "config");
-
+    OpenshiftBuildService(OpenShiftClient client, JKubeServiceHub jKubeServiceHub) {
         this.client = client;
-        this.log = log;
-        this.jKubeServiceHub = jKubeServiceHub;
-        config = jKubeServiceHub.getBuildServiceConfig();
+        setJKubeServiceHub(jKubeServiceHub);
+    }
+
+    public void setOpenShiftClient(OpenShiftClient openShiftClient) {
+        this.client = openShiftClient;
     }
 
     @Override
     public void build(ImageConfiguration imageConfig) throws JKubeServiceException {
+        Objects.requireNonNull(jKubeServiceHub.getConfiguration(), "JKubeConfiguration");
+        Objects.requireNonNull(jKubeServiceHub.getDockerServiceHub(), "dockerServiceHub");
+        Objects.requireNonNull(jKubeServiceHub.getBuildServiceConfig(), "config");
         String buildName = null;
         try {
             final ImageConfiguration.ImageConfigurationBuilder applicableImageConfigBuilder = imageConfig.toBuilder();
@@ -137,6 +141,7 @@ public void build(ImageConfiguration imageConfig) throws JKubeServiceException {
             KubernetesListBuilder builder = new KubernetesListBuilder();
 
             // Check for buildconfig / imagestream / pullSecret and create them if necessary
+            initOpenShiftClient();
             String openshiftPullSecret = config.getOpenshiftPullSecret();
             final boolean usePullSecret = checkOrCreatePullSecret(client, builder, openshiftPullSecret, applicableImageConfig);
             if (usePullSecret) {
@@ -232,6 +237,18 @@ public void postProcess(BuildServiceConfig config) {
         config.attachArtifact("is", getImageStreamFile());
     }
 
+    @Override
+    public boolean isApplicable(JKubeServiceHub jKubeServiceHub) {
+        return jKubeServiceHub.getRuntimeMode() == RuntimeMode.OPENSHIFT;
+    }
+
+    @Override
+    public void setJKubeServiceHub(JKubeServiceHub jKubeServiceHub) {
+        this.jKubeServiceHub = jKubeServiceHub;
+        this.config = jKubeServiceHub.getBuildServiceConfig();
+        this.log = jKubeServiceHub.getLog();
+    }
+
     protected String updateOrCreateBuildConfig(BuildServiceConfig config, OpenShiftClient client, KubernetesListBuilder builder, ImageConfiguration imageConfig, String openshiftPullSecret) {
         ImageName imageName = new ImageName(imageConfig.getName());
         String buildName = getS2IBuildName(config, imageName);
@@ -776,4 +793,18 @@ private Map<String, Map<String, Quantity>> getRequestsAndLimits() {
         return keyToQuantityMap;
     }
 
+    private void initOpenShiftClient() {
+        if (client == null) {
+            ClusterAccess clusterAccess = jKubeServiceHub.getClusterAccess();
+            if (clusterAccess == null) {
+                clusterAccess = new ClusterAccess(log,
+                        ClusterConfiguration.from(System.getProperties(), jKubeServiceHub.getConfiguration().getProject().getProperties()).build());
+            }
+            client = clusterAccess.createDefaultClient();
+            if (!isOpenShift(client)) {
+                throw new IllegalStateException("OpenShift platform has been specified but OpenShift has not been detected!");
+            }
+        }
+    }
+
 }
diff --git a/...ce/src/main/resources/META-INF/services/org.eclipse.jkube.kit.config.service.BuildService b/...ce/src/main/resources/META-INF/services/org.eclipse.jkube.kit.config.service.BuildService
@@ -0,0 +1,3 @@
+org.eclipse.jkube.kit.config.service.kubernetes.JibBuildService
+org.eclipse.jkube.kit.config.service.kubernetes.DockerBuildService
+org.eclipse.jkube.kit.config.service.openshift.OpenshiftBuildService