From d35cc3bd7db4222cff7fea47177face3a5ff27fc Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Fri, 3 Jan 2020 16:39:20 +0530 Subject: [PATCH] Fix #1770: Provide support for setting BuildConfig memory/CPU requests and limits --- CHANGELOG.md | 1 + .../core/config/OpenshiftBuildConfig.java | 44 +++++++++++++++ .../maven/core/config/ResourceConfig.java | 12 +++- .../maven/core/service/BuildService.java | 23 ++++++++ .../openshift/OpenshiftBuildService.java | 56 +++++++++++++++---- .../kubernetes/KubernetesResourceUtil.java | 42 ++++++++++++++ .../openshift/OpenshiftBuildServiceTest.java | 37 ++++++++++++ .../inc/goals/build/_fabric8-build.adoc | 19 +++++++ .../maven/plugin/mojo/build/BuildMojo.java | 2 + 9 files changed, 225 insertions(+), 11 deletions(-) create mode 100644 core/src/main/java/io/fabric8/maven/core/config/OpenshiftBuildConfig.java diff --git a/CHANGELOG.md b/CHANGELOG.md index a69c657438..bb017f730f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ After this we will switch probably to real [Semantic Versioning 2.0.0](http://se * Fix #1695: IllegalArgumentException when Spring Boot application.yaml contains integer keys * Fix #797: spring-boot generator can not handle multi-profile configuration * Fix #1751: Build Names are suffixed with -s2i regardless of build strategy +* Fix #1770: Support for setting BuildConfig memory/cpu request and limits * Fix #1755: Spring boot enricher does not produce a proper heath check and liveness check path when "/" is used. * Feature: Check maven.compiler.target property for base image detection. * Fix: Enrichers should resolve relative paths against project directory, not working directory 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 new file mode 100644 index 0000000000..fc9d91fdc4 --- /dev/null +++ b/core/src/main/java/io/fabric8/maven/core/config/OpenshiftBuildConfig.java @@ -0,0 +1,44 @@ +/** + * Copyright 2016 Red Hat, Inc. + * + * Red Hat licenses this file to you under the Apache License, version + * 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package io.fabric8.maven.core.config; + +import java.util.Map; + +public class OpenshiftBuildConfig { + private Map limits; + private Map requests; + + public OpenshiftBuildConfig(Map limits, Map requests) { + this.limits = limits; + this.requests = requests; + } + + public Map getRequests() { + return requests; + } + + public void setRequests(Map requests) { + this.requests = requests; + } + + public Map getLimits() { + return limits; + } + + public void setLimits(Map resourceLimits) { + this.limits = resourceLimits; + } +} diff --git a/core/src/main/java/io/fabric8/maven/core/config/ResourceConfig.java b/core/src/main/java/io/fabric8/maven/core/config/ResourceConfig.java index 70bca8f602..d9ecb4fb8b 100644 --- a/core/src/main/java/io/fabric8/maven/core/config/ResourceConfig.java +++ b/core/src/main/java/io/fabric8/maven/core/config/ResourceConfig.java @@ -15,7 +15,6 @@ */ package io.fabric8.maven.core.config; -import io.fabric8.kubernetes.api.model.extensions.IngressRule; import org.apache.maven.plugins.annotations.Parameter; import java.util.List; @@ -97,6 +96,9 @@ public class ResourceConfig { */ private String routeDomain; + @Parameter + private OpenshiftBuildConfig openshiftBuildConfig; + public Optional> getEnv() { return Optional.ofNullable(env); } @@ -175,6 +177,9 @@ public List getRemotes() { public String getRouteDomain() { return routeDomain; } + public OpenshiftBuildConfig getOpenshiftBuildConfig() { + return openshiftBuildConfig; + } // ============================================================================================= public static class Builder { @@ -276,6 +281,11 @@ public Builder withRouteDomain(String routeDomain) { return this; } + public Builder withOpenshiftBuildConfig(OpenshiftBuildConfig openshiftBuildConfig) { + config.openshiftBuildConfig = openshiftBuildConfig; + return this; + } + public ResourceConfig build() { return config; } diff --git a/core/src/main/java/io/fabric8/maven/core/service/BuildService.java b/core/src/main/java/io/fabric8/maven/core/service/BuildService.java index f7bfca9696..2a4a6d929f 100644 --- a/core/src/main/java/io/fabric8/maven/core/service/BuildService.java +++ b/core/src/main/java/io/fabric8/maven/core/service/BuildService.java @@ -19,6 +19,7 @@ import io.fabric8.kubernetes.api.model.KubernetesListBuilder; import io.fabric8.maven.core.config.BuildRecreateMode; import io.fabric8.maven.core.config.OpenShiftBuildStrategy; +import io.fabric8.maven.core.config.ResourceConfig; import io.fabric8.maven.docker.config.ImageConfiguration; import io.fabric8.maven.docker.service.ImagePullManager; import io.fabric8.maven.docker.util.MojoParameters; @@ -72,6 +73,10 @@ class BuildServiceConfig { private boolean s2iImageStreamLookupPolicyLocal; + private ResourceConfig resourceConfig; + + private File resourceDir; + public BuildServiceConfig() { } @@ -121,6 +126,14 @@ public boolean isForcePullEnabled() { return forcePull; } + public ResourceConfig getResourceConfig() { + return resourceConfig; + } + + public File getResourceDir() { + return resourceDir; + } + public void attachArtifact(String classifier, File destFile) { if (attacher != null) { attacher.attach(classifier, destFile); @@ -198,6 +211,16 @@ public Builder imagePullManager(ImagePullManager imagePullManager) { return this; } + public Builder resourceConfig(ResourceConfig resourceConfig) { + config.resourceConfig = resourceConfig; + return this; + } + + public Builder resourceDir(File resourceDir) { + config.resourceDir = resourceDir; + return this; + } + public BuildServiceConfig build() { return config; } 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 index c1196c6da6..0d20054dbf 100644 --- 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 @@ -37,6 +37,7 @@ import io.fabric8.kubernetes.api.model.LocalObjectReferenceBuilder; import io.fabric8.kubernetes.api.model.ObjectReference; import io.fabric8.kubernetes.api.model.Pod; +import io.fabric8.kubernetes.api.model.Quantity; import io.fabric8.kubernetes.api.model.Status; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.Watch; @@ -67,6 +68,7 @@ import io.fabric8.openshift.api.model.Build; import io.fabric8.openshift.api.model.BuildConfig; 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; @@ -84,6 +86,7 @@ public class OpenshiftBuildService implements BuildService { private static final String DEFAULT_S2I_BUILD_SUFFIX = "-s2i"; + public static final String DEFAULT_S2I_SOURCE_TYPE = "Binary"; private final OpenShiftClient client; private final Logger log; @@ -199,7 +202,7 @@ public void postProcess(BuildServiceConfig config) { config.attachArtifact("is", getImageStreamFile(config)); } - private String updateOrCreateBuildConfig(BuildServiceConfig config, OpenShiftClient client, KubernetesListBuilder builder, ImageConfiguration imageConfig, String openshiftPullSecret) { + protected String updateOrCreateBuildConfig(BuildServiceConfig config, OpenShiftClient client, KubernetesListBuilder builder, ImageConfiguration imageConfig, String openshiftPullSecret) { ImageName imageName = new ImageName(imageConfig.getName()); String buildName = getS2IBuildName(config, imageName); String imageStreamName = getImageStreamName(imageName); @@ -236,8 +239,8 @@ private void validateSourceType(String buildName, BuildConfigSpec spec) { BuildSource source = spec.getSource(); if (source != null) { String sourceType = source.getType(); - if (!Objects.equals("Binary", sourceType)) { - log.warn("BuildServiceConfig %s is not of type: 'Binary' but is '%s' !", buildName, sourceType); + if (!Objects.equals(DEFAULT_S2I_SOURCE_TYPE, sourceType)) { + log.warn("BuildServiceConfig %s is not of type: '" + DEFAULT_S2I_SOURCE_TYPE + "' but is '%s' !", buildName, sourceType); } } } @@ -257,17 +260,50 @@ private String createBuildConfig(KubernetesListBuilder builder, String buildName .withNewMetadata() .withName(buildName) .endMetadata() - .withNewSpec() - .withNewSource() - .withType("Binary") - .endSource() - .withStrategy(buildStrategyResource) - .withOutput(buildOutput) - .endSpec() + .withSpec(getBuildConfigSpec(buildStrategyResource, buildOutput)) .endBuildConfigItem(); return buildName; } + private BuildConfigSpec getBuildConfigSpec(BuildStrategy buildStrategyResource, BuildOutput buildOutput) { + BuildConfigSpecBuilder specBuilder = null; + + // Check for BuildConfig resource fragment + File buildConfigResourceFragment = KubernetesResourceUtil.getResourceFragmentFromSource(config.getResourceDir(), config.getResourceConfig(), "buildconfig.yml", log); + if (buildConfigResourceFragment != null) { + BuildConfig buildConfigFragment = client.buildConfigs().load(buildConfigResourceFragment).get(); + specBuilder = new BuildConfigSpecBuilder(buildConfigFragment.getSpec()); + } else { + specBuilder = new BuildConfigSpecBuilder(); + } + + if (specBuilder.buildSource() == null) { + specBuilder.withNewSource() + .withType(DEFAULT_S2I_SOURCE_TYPE) + .endSource(); + } + + if (specBuilder.buildStrategy() == null) { + specBuilder.withStrategy(buildStrategyResource); + } + + if (specBuilder.buildOutput() == null) { + specBuilder.withOutput(buildOutput); + } + + if (config.getResourceConfig() != null && config.getResourceConfig().getOpenshiftBuildConfig() != null) { + Map limits = KubernetesResourceUtil.getQuantityFromString(config.getResourceConfig().getOpenshiftBuildConfig().getLimits()); + if (limits != null && !limits.isEmpty()) { + specBuilder.editOrNewResources().addToLimits(limits).endResources(); + } + Map requests = KubernetesResourceUtil.getQuantityFromString(config.getResourceConfig().getOpenshiftBuildConfig().getRequests()); + if (limits != null && !limits.isEmpty()) { + specBuilder.editOrNewResources().addToRequests(requests).endResources() ; + } + } + return specBuilder.build(); + } + private String updateBuildConfig(OpenShiftClient client, String buildName, BuildStrategy buildStrategy, BuildOutput buildOutput, BuildConfigSpec spec) { // lets check if the strategy or output has changed and if so lets update the BC diff --git a/core/src/main/java/io/fabric8/maven/core/util/kubernetes/KubernetesResourceUtil.java b/core/src/main/java/io/fabric8/maven/core/util/kubernetes/KubernetesResourceUtil.java index 10ac3be95c..4a04c300a1 100644 --- a/core/src/main/java/io/fabric8/maven/core/util/kubernetes/KubernetesResourceUtil.java +++ b/core/src/main/java/io/fabric8/maven/core/util/kubernetes/KubernetesResourceUtil.java @@ -40,6 +40,7 @@ import io.fabric8.kubernetes.api.model.PodSpecBuilder; import io.fabric8.kubernetes.api.model.PodStatus; import io.fabric8.kubernetes.api.model.PodTemplateSpec; +import io.fabric8.kubernetes.api.model.Quantity; import io.fabric8.kubernetes.api.model.ReplicationController; import io.fabric8.kubernetes.api.model.ReplicationControllerSpec; import io.fabric8.kubernetes.api.model.apps.DaemonSet; @@ -55,6 +56,7 @@ import io.fabric8.kubernetes.api.model.batch.JobSpec; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.internal.HasMetadataComparator; +import io.fabric8.maven.core.config.ResourceConfig; import io.fabric8.maven.core.util.FileUtil; import io.fabric8.maven.core.config.PlatformMode; import io.fabric8.maven.core.model.GroupArtifactVersion; @@ -1086,4 +1088,44 @@ private static boolean isLocalCustomisation(PodSpec podSpec) { } return true; } + + /** + * Get a specific resource fragment ending with some suffix + * + * @param resourceDirFinal resource directory + * @param resourceConfig resource config in case remote fragments are provided + * @param resourceNameSuffix resource name suffix + * @param log log object + * @return file if present or null + */ + public static File getResourceFragmentFromSource(File resourceDirFinal, ResourceConfig resourceConfig, String resourceNameSuffix, Logger log) { + if (resourceDirFinal != null) { + File[] resourceFiles = KubernetesResourceUtil.listResourceFragments(resourceDirFinal, resourceConfig != null ? resourceConfig.getRemotes() : null, log); + + if (resourceFiles != null) { + for (File file : resourceFiles) { + if (file.getName().endsWith(resourceNameSuffix)) { + return file; + } + } + } + } + return null; + } + + /** + * Get requests or limit objects from string hashmaps + * + * @param quantity hashmap of strings + * @return hashmap of string to quantity + */ + public static Map getQuantityFromString(Map quantity) { + Map stringQuantityMap = new HashMap<>(); + if (quantity != null && !quantity.isEmpty()) { + for (Map.Entry entry : quantity.entrySet()) { + stringQuantityMap.put(entry.getKey(), new Quantity(entry.getValue())); + } + } + return stringQuantityMap; + } } 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 index 2cbc94acba..80914e3075 100644 --- 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 @@ -19,6 +19,7 @@ import java.io.FileReader; import java.io.IOException; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -30,6 +31,8 @@ import io.fabric8.kubernetes.api.model.WatchEvent; import io.fabric8.maven.core.config.BuildRecreateMode; import io.fabric8.maven.core.config.OpenShiftBuildStrategy; +import io.fabric8.maven.core.config.OpenshiftBuildConfig; +import io.fabric8.maven.core.config.ResourceConfig; import io.fabric8.maven.core.service.BuildService; import io.fabric8.maven.core.service.Fabric8ServiceException; import io.fabric8.maven.core.util.WebServerEventCollector; @@ -65,6 +68,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static junit.framework.TestCase.assertNotNull; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -456,6 +460,39 @@ public void checkTarPackageSecret() throws Exception { }); } + @Test + public void testBuildConfigResourceConfig() throws Exception { + retryInMockServer(() -> { + Map limitsMap = new HashMap<>(); + limitsMap.put("cpu", "100m"); + limitsMap.put("memory", "256Mi"); + + BuildService.BuildServiceConfig config = defaultConfig + .resourceConfig(new ResourceConfig.Builder() + .withOpenshiftBuildConfig(new OpenshiftBuildConfig(limitsMap, null)).build()).build(); + OpenShiftMockServer mockServer = new OpenShiftMockServer(); + + OpenShiftClient client = mockServer.createOpenShiftClient(); + final OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + + ImageConfiguration imageWithEnv = new ImageConfiguration.Builder(image) + .buildConfig(new BuildImageConfiguration.Builder(image.getBuildConfiguration()) + .env(Collections.singletonMap("FOO", "BAR")) + .build() + ).build(); + + KubernetesListBuilder builder = new KubernetesListBuilder(); + service.createBuildArchive(imageWithEnv); + service.updateOrCreateBuildConfig(config, client, builder, imageWithEnv, null); + BuildConfig buildConfig = (BuildConfig) builder.buildFirstItem(); + assertNotNull(buildConfig); + assertNotNull(buildConfig.getSpec().getResources()); + assertEquals("256Mi", buildConfig.getSpec().getResources().getLimits().get("memory").getAmount()); + assertEquals("100m", buildConfig.getSpec().getResources().getLimits().get("cpu").getAmount()); + }); + + } + @FunctionalInterface private interface MockServerRetryable { void run() throws Fabric8ServiceException, MojoExecutionException, IOException; diff --git a/doc/src/main/asciidoc/inc/goals/build/_fabric8-build.adoc b/doc/src/main/asciidoc/inc/goals/build/_fabric8-build.adoc index 6962a7ae7a..ba9ae1e5e8 100644 --- a/doc/src/main/asciidoc/inc/goals/build/_fabric8-build.adoc +++ b/doc/src/main/asciidoc/inc/goals/build/_fabric8-build.adoc @@ -110,6 +110,25 @@ Both build strategies update an https://docs.openshift.com/enterprise/latest/arc The https://docs.openshift.com/enterprise/latest/dev_guide/builds.html#defining-a-buildconfig[Build Config] and https://docs.openshift.com/enterprise/latest/architecture/core_concepts/builds_and_image_streams.html#image-streams[Image streams] can be managed by this plugin. If they do not exist, they will be automatically created by `fabric8:build`. If they do already exist, they are reused, except when the `buildRecreate` configuration option (property `fabric8.build.recreate`) is set to a value as described in <>. Also if the provided build strategy is different than the one defined in the existing build configuration, the Build Config is edited to reflect the new type (which in turn removes all build associated with the previous build). +If you want to configure memory/cpu requests and limits related to `BuildConfig`, you can either provide them as in plugin configuration or as a resource fragment in `src/main/fabric8` directory. for XML configuration it needs to be done like this: +``` + + io.fabric8 + fabric8-maven-plugin + ${plugin.version} + + + + + 100m + 256Mi + + + + + +``` + This image stream created can then be directly referenced from https://docs.openshift.com/enterprise/latest/architecture/core_concepts/deployments.html#deployments-and-deployment-configurations[Deployment Configuration] objects created by <>. By default, image streams are created with a local lookup policy, so that they can be used also by other resources such as Deployments or StatefulSets. This behavior can be turned off by setting the `fabric8.s2i.imageStreamLookupPolicyLocal` property to `false` when building the project. diff --git a/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/BuildMojo.java b/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/BuildMojo.java index 1e7f27b68c..19da9f8bbb 100644 --- a/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/BuildMojo.java +++ b/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/BuildMojo.java @@ -312,6 +312,8 @@ protected BuildService.BuildServiceConfig getBuildServiceConfig() throws MojoExe .forcePullEnabled(forcePull) .imagePullManager(getImagePullManager(imagePullPolicy, autoPull)) .buildDirectory(project.getBuild().getDirectory()) + .resourceDir(ResourceDirCreator.getFinalResourceDir(resourceDir, environment)) + .resourceConfig(resources) .attacher((classifier, destFile) -> { if (destFile.exists()) { projectHelper.attachArtifact(project, "yml", classifier, destFile);