Fix fabric8io#1770: Fixed my mistake, remove constructor from Openshi…

…ftBuildConfig I accidentally added them for testing and forgot to retest it :-( . I apologize for inconvinience caused.
rohanKanojia · Feb 18, 2020 · a7f2e66 · a7f2e66
1 parent 4eea7a2
commit a7f2e66
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ Usage:
 
 ### 4.5-SNAPSHOT
 * Fix #1789: script to extract changelog information for notifications
+* Fix #1770: Fixed my mistake, remove constructors from OpenshiftBuildConfig which were causing PluginConfigurationException
 
 ### 4.4.0 (2020-02-13)
 * Fix #1572: Support maven --batch-mode option

diff --git a/core/src/main/java/io/fabric8/maven/core/config/OpenshiftBuildConfig.java b/core/src/main/java/io/fabric8/maven/core/config/OpenshiftBuildConfig.java
@@ -21,11 +21,6 @@ public class OpenshiftBuildConfig {
     private Map<String, String> limits;
     private Map<String, String> requests;
 
-    public OpenshiftBuildConfig(Map<String, String> limits, Map<String, String> requests) {
-        this.limits = limits;
-        this.requests = requests;
-    }
-
     public Map<String, String> getRequests() {
         return requests;
     }
@@ -41,4 +36,33 @@ public Map<String, String> getLimits() {
     public void setLimits(Map<String, String> resourceLimits) {
         this.limits = resourceLimits;
     }
+
+    public static class Builder {
+        private OpenshiftBuildConfig openshiftBuildConfig;
+
+        public Builder() {
+            this.openshiftBuildConfig = new OpenshiftBuildConfig();
+        }
+
+        public Builder(OpenshiftBuildConfig openshiftBuildConfig) {
+            if (openshiftBuildConfig != null) {
+                this.openshiftBuildConfig.limits = openshiftBuildConfig.limits;
+                this.openshiftBuildConfig.requests = openshiftBuildConfig.requests;
+            }
+        }
+
+        public Builder limits(Map<String, String> limits) {
+            this.openshiftBuildConfig.limits = limits;
+            return this;
+        }
+
+        public Builder requests(Map<String, String> requests) {
+            this.openshiftBuildConfig.requests = requests;
+            return this;
+        }
+
+        public OpenshiftBuildConfig build() {
+            return this.openshiftBuildConfig;
+        }
+    }
 }
diff --git a/core/src/main/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildService.java b/core/src/main/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildService.java
@@ -67,13 +67,15 @@
 import io.fabric8.maven.docker.util.Logger;
 import io.fabric8.openshift.api.model.Build;
 import io.fabric8.openshift.api.model.BuildConfig;
+import io.fabric8.openshift.api.model.BuildConfigFluent;
 import io.fabric8.openshift.api.model.BuildConfigSpec;
 import io.fabric8.openshift.api.model.BuildConfigSpecBuilder;
 import io.fabric8.openshift.api.model.BuildOutput;
 import io.fabric8.openshift.api.model.BuildOutputBuilder;
 import io.fabric8.openshift.api.model.BuildSource;
 import io.fabric8.openshift.api.model.BuildStrategy;
 import io.fabric8.openshift.api.model.BuildStrategyBuilder;
+import io.fabric8.openshift.api.model.DoneableBuildConfig;
 import io.fabric8.openshift.client.OpenShiftClient;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang3.StringUtils;
@@ -291,15 +293,12 @@ private BuildConfigSpec getBuildConfigSpec(BuildStrategy buildStrategyResource,
             specBuilder.withOutput(buildOutput);
         }
 
-        if (config.getResourceConfig() != null && config.getResourceConfig().getOpenshiftBuildConfig() != null) {
-            Map<String, Quantity> limits = KubernetesResourceUtil.getQuantityFromString(config.getResourceConfig().getOpenshiftBuildConfig().getLimits());
-            if (limits != null && !limits.isEmpty()) {
-                specBuilder.editOrNewResources().addToLimits(limits).endResources();
-            }
-            Map<String, Quantity> requests = KubernetesResourceUtil.getQuantityFromString(config.getResourceConfig().getOpenshiftBuildConfig().getRequests());
-            if (limits != null && !limits.isEmpty()) {
-                specBuilder.editOrNewResources().addToRequests(requests).endResources() ;
-            }
+        Map<String, Map<String, Quantity>> requestsLimitsMap = getRequestsAndLimits();
+        if (requestsLimitsMap.containsKey("requests")) {
+            specBuilder.editOrNewResources().addToRequests(requestsLimitsMap.get("requests")).endResources();
+        }
+        if (requestsLimitsMap.containsKey("limits")) {
+            specBuilder.editOrNewResources().addToLimits(requestsLimitsMap.get("limits")).endResources();
         }
         return specBuilder.build();
     }
@@ -309,12 +308,19 @@ private String updateBuildConfig(OpenShiftClient client, String buildName, Build
         // lets check if the strategy or output has changed and if so lets update the BC
         // e.g. the S2I builder image or the output tag and
         if (!Objects.equals(buildStrategy, spec.getStrategy()) || !Objects.equals(buildOutput, spec.getOutput())) {
-            client.buildConfigs().withName(buildName).edit()
-                    .editSpec()
-                    .withStrategy(buildStrategy)
-                    .withOutput(buildOutput)
-                    .endSpec()
-                    .done();
+            BuildConfigFluent.SpecNested<DoneableBuildConfig> doneableBuildConfig = client.buildConfigs().withName(buildName).edit().editSpec();
+            doneableBuildConfig.withStrategy(buildStrategy)
+                    .withOutput(buildOutput);
+
+            Map<String, Map<String, Quantity>> requestsLimitsMap = getRequestsAndLimits();
+            if (requestsLimitsMap.containsKey("requests")) {
+                doneableBuildConfig.editOrNewResources().addToRequests(requestsLimitsMap.get("requests")).endResources();
+            }
+            if (requestsLimitsMap.containsKey("limits")) {
+                doneableBuildConfig.editOrNewResources().addToLimits(requestsLimitsMap.get("limits")).endResources();
+            }
+
+            doneableBuildConfig.endSpec().done();
             log.info("Updating BuildServiceConfig %s for %s strategy", buildName, buildStrategy.getType());
         } else {
             log.info("Using BuildServiceConfig %s for %s strategy", buildName, buildStrategy.getType());
@@ -761,4 +767,19 @@ private String getMapValueWithDefault(Map<String, String> map, String field, Str
         return value != null ? value : defaultValue;
     }
 
+    private Map<String, Map<String, Quantity>> getRequestsAndLimits() {
+        Map<String, Map<String, Quantity>> keyToQuantityMap = new HashMap<>();
+        if (config.getResourceConfig() != null && config.getResourceConfig().getOpenshiftBuildConfig() != null) {
+            Map<String, Quantity> limits = KubernetesResourceUtil.getQuantityFromString(config.getResourceConfig().getOpenshiftBuildConfig().getLimits());
+            if (!limits.isEmpty()) {
+                keyToQuantityMap.put("limits", limits);
+            }
+            Map<String, Quantity> requests = KubernetesResourceUtil.getQuantityFromString(config.getResourceConfig().getOpenshiftBuildConfig().getRequests());
+            if (!requests.isEmpty()) {
+                keyToQuantityMap.put("requests", requests);
+            }
+        }
+        return keyToQuantityMap;
+    }
+
 }
diff --git a/core/src/test/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildServiceTest.java b/core/src/test/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildServiceTest.java
@@ -469,7 +469,7 @@ public void testBuildConfigResourceConfig() throws Exception {
 
             BuildService.BuildServiceConfig config = defaultConfig
                     .resourceConfig(new ResourceConfig.Builder()
-                    .withOpenshiftBuildConfig(new OpenshiftBuildConfig(limitsMap, null)).build()).build();
+                    .withOpenshiftBuildConfig(new OpenshiftBuildConfig.Builder().limits(limitsMap).build()).build()).build();
             OpenShiftMockServer mockServer = new OpenShiftMockServer();
 
             OpenShiftClient client = mockServer.createOpenShiftClient();