diff options
18 files changed, 443 insertions, 296 deletions
diff --git a/config-model/pom.xml b/config-model/pom.xml index d42d5af8975..dc7bec27a3b 100644 --- a/config-model/pom.xml +++ b/config-model/pom.xml @@ -288,6 +288,22 @@ <version>${project.version}</version> <scope>provided</scope> </dependency> + <dependency> + <groupId>biz.aQute.bnd</groupId> + <artifactId>biz.aQute.bndlib</artifactId> + <version>6.1.0</version> + <exclusions> + <exclusion> + <groupId>org.slf4j</groupId> + <artifactId>slf4j-api</artifactId> + </exclusion> + <exclusion> + <!-- These are not needed for our use of bndlib --> + <groupId>org.osgi</groupId> + <artifactId>*</artifactId> + </exclusion> + </exclusions> + </dependency> </dependencies> <build> diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java new file mode 100644 index 00000000000..b6b9190fedf --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java @@ -0,0 +1,144 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.model.application.validation; + +import aQute.bnd.header.Parameters; +import aQute.bnd.osgi.Domain; +import aQute.bnd.version.VersionRange; +import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.application.api.ComponentInfo; +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.path.Path; +import com.yahoo.vespa.model.VespaModel; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Predicate; +import java.util.jar.Attributes; +import java.util.jar.JarFile; +import java.util.jar.Manifest; +import java.util.logging.Level; + +/** + * A validator for bundles. Uses BND library for some of the validation. + * + * @author hmusum + * @author bjorncs + */ +public class BundleValidator extends Validator { + + public BundleValidator() {} + + @Override + public void validate(VespaModel model, DeployState deployState) { + ApplicationPackage app = deployState.getApplicationPackage(); + for (ComponentInfo info : app.getComponentsInfo(deployState.getVespaVersion())) { + try { + Path path = Path.fromString(info.getPathRelativeToAppDir()); + DeployLogger deployLogger = deployState.getDeployLogger(); + deployLogger.log(Level.FINE, String.format("Validating bundle at '%s'", path)); + JarFile jarFile = new JarFile(app.getFileReference(path)); + validateJarFile(deployLogger, jarFile); + } catch (IOException e) { + throw new IllegalArgumentException( + "Failed to validate JAR file '" + info.getPathRelativeToAppDir() + "'", e); + } + } + } + + void validateJarFile(DeployLogger deployLogger, JarFile jarFile) throws IOException { + Manifest manifest = jarFile.getManifest(); + String jarPath = jarFile.getName(); + if (manifest == null) { + throw new IllegalArgumentException("Non-existing or invalid manifest in " + jarPath); + } + validateManifest(deployLogger, jarPath, manifest); + } + + void validateManifest(DeployLogger deployLogger, String jarPath, Manifest mf) { + // Check for required OSGI headers + Attributes attributes = mf.getMainAttributes(); + HashSet<String> mfAttributes = new HashSet<>(); + for (Map.Entry<Object,Object> entry : attributes.entrySet()) { + mfAttributes.add(entry.getKey().toString()); + } + List<String> requiredOSGIHeaders = Arrays.asList( + "Bundle-ManifestVersion", "Bundle-Name", "Bundle-SymbolicName", "Bundle-Version"); + for (String header : requiredOSGIHeaders) { + if (!mfAttributes.contains(header)) { + throw new IllegalArgumentException("Required OSGI header '" + header + + "' was not found in manifest in '" + jarPath + "'"); + } + } + + if (attributes.getValue("Bundle-Version").endsWith(".SNAPSHOT")) { + deployLogger.logApplicationPackage(Level.WARNING, "Deploying snapshot bundle " + jarPath + + ".\nTo use this bundle, you must include the qualifier 'SNAPSHOT' in the version specification in services.xml."); + } + + if (attributes.getValue("Import-Package") != null) { + validateImportedPackages(deployLogger, jarPath, mf); + } + } + + private static void validateImportedPackages(DeployLogger deployLogger, String jarPath, Manifest manifest) { + Domain osgiHeaders = Domain.domain(manifest); + Parameters importPackage = osgiHeaders.getImportPackage(); + Map<DeprecatedArtifact, List<String>> deprecatedPackagesInUse = new HashMap<>(); + + importPackage.forEach((packageName, attrs) -> { + VersionRange versionRange = attrs.getVersion() != null + ? VersionRange.parseOSGiVersionRange(attrs.getVersion()) + : null; + + for (DeprecatedArtifact deprecatedArtifact : DeprecatedArtifact.values()) { + if (deprecatedArtifact.javaPackages.contains(packageName) + && (versionRange == null || deprecatedArtifact.versionDiscriminator.test(versionRange))) { + deprecatedPackagesInUse.computeIfAbsent(deprecatedArtifact, __ -> new ArrayList<>()) + .add(packageName); + } + } + }); + + deprecatedPackagesInUse.forEach((artifact, packagesInUse) -> { + deployLogger.logApplicationPackage(Level.WARNING, + String.format("For JAR file '%s': \n" + + "Manifest imports the following Java packages from '%s': %s. \n" + + "%s", + jarPath, artifact.name, packagesInUse, artifact.description)); + }); + } + + private enum DeprecatedArtifact { + ORG_JSON("org.json:json", + "The org.json library will no longer provided by jdisc runtime on Vespa 8. " + + "See https://docs.vespa.ai/en/vespa8-release-notes.html#container-runtime.", + Set.of("org.json")); + + final String name; + final Collection<String> javaPackages; + final Predicate<VersionRange> versionDiscriminator; + final String description; + + DeprecatedArtifact(String name, String description, Collection<String> javaPackages) { + this(name, description, __ -> true, javaPackages); + } + + DeprecatedArtifact(String name, + String description, + Predicate<VersionRange> versionDiscriminator, + Collection<String> javaPackages) { + this.name = name; + this.javaPackages = javaPackages; + this.versionDiscriminator = versionDiscriminator; + this.description = description; + } + } +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComponentValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComponentValidator.java deleted file mode 100644 index 21e396959a7..00000000000 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComponentValidator.java +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.application.validation; - -import com.yahoo.config.application.api.ApplicationPackage; -import com.yahoo.config.model.deploy.DeployState; -import com.yahoo.path.Path; -import com.yahoo.vespa.model.VespaModel; -import com.yahoo.config.application.api.ComponentInfo; -import com.yahoo.config.application.api.DeployLogger; -import java.io.IOException; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.jar.Attributes; -import java.util.jar.JarFile; -import java.util.jar.Manifest; -import java.util.logging.Level; -import java.util.zip.ZipException; - -/** - * A validator for bundles. Uses BND library for some of the validation (not active yet) - * - * @author hmusum - * @since 2010-11-11 - */ -public class ComponentValidator extends Validator { - private JarFile jarFile; - - public ComponentValidator() { - } - - public ComponentValidator(JarFile jarFile) { - this.jarFile = jarFile; - } - - @Override - public void validate(VespaModel model, DeployState deployState) { - ApplicationPackage app = deployState.getApplicationPackage(); - for (ComponentInfo info : app.getComponentsInfo(deployState.getVespaVersion())) { - try { - this.jarFile = new JarFile(app.getFileReference(Path.fromString(info.getPathRelativeToAppDir()))); - } catch (ZipException e) { - throw new IllegalArgumentException("Error opening jar file '" + info.getPathRelativeToAppDir() + - "'. Please check that this is a valid jar file"); - } catch (IOException e) { - e.printStackTrace(); - } - try { - validateAll(deployState.getDeployLogger()); - } catch (IOException e) { - e.printStackTrace(); - } - } - } - - public void validateAll(DeployLogger deployLogger) throws IOException { - validateOSGIHeaders(deployLogger); - } - - public void validateOSGIHeaders(DeployLogger deployLogger) throws IOException { - Manifest mf = jarFile.getManifest(); - if (mf == null) { - throw new IllegalArgumentException("Non-existing or invalid manifest in " + jarFile.getName()); - } - - // Check for required OSGI headers - Attributes attributes = mf.getMainAttributes(); - HashSet<String> mfAttributes = new HashSet<>(); - for (Object attributeSet : attributes.entrySet()) { - Map.Entry<Object, Object> e = (Map.Entry<Object, Object>) attributeSet; - mfAttributes.add(e.getKey().toString()); - } - List<String> requiredOSGIHeaders = Arrays.asList( - "Bundle-ManifestVersion", "Bundle-Name", "Bundle-SymbolicName", "Bundle-Version"); - for (String header : requiredOSGIHeaders) { - if (!mfAttributes.contains(header)) { - throw new IllegalArgumentException("Required OSGI header '" + header + - "' was not found in manifest in '" + jarFile.getName() + "'"); - } - } - - if (attributes.getValue("Bundle-Version").endsWith(".SNAPSHOT")) { - deployLogger.logApplicationPackage(Level.WARNING, "Deploying snapshot bundle " + jarFile.getName() + - ".\nTo use this bundle, you must include the qualifier 'SNAPSHOT' in the version specification in services.xml."); - } - } -} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java index 5215fdcb301..08dc73a1bd0 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java @@ -63,7 +63,7 @@ public class Validation { new RoutingSelectorValidator().validate(model, deployState); } new SchemasDirValidator().validate(model, deployState); - new ComponentValidator().validate(model, deployState); + new BundleValidator().validate(model, deployState); new SearchDataTypeValidator().validate(model, deployState); new ComplexAttributeFieldsValidator().validate(model, deployState); new StreamingValidator().validate(model, deployState); diff --git a/config-model/src/test/cfg/application/validation/testjars/manifest-producing-import-warnings.MF b/config-model/src/test/cfg/application/validation/testjars/manifest-producing-import-warnings.MF new file mode 100644 index 00000000000..760a9ecf00f --- /dev/null +++ b/config-model/src/test/cfg/application/validation/testjars/manifest-producing-import-warnings.MF @@ -0,0 +1,10 @@ +Manifest-Version: 1.0 +Export-Package: com.yahoo.vespa.test.myapp;version=1.0.0 +Bundle-ManifestVersion: 2 +Bundle-SymbolicName: my-bundle +Bundle-Version: 7.0.0 +Created-By: vespa container maven plugin +Bundle-Name: my-bundle +Bundle-Vendor: Yahoo! +Import-Package: org.json;version="[0.0.0,1)" + diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java new file mode 100644 index 00000000000..e2eae30d78d --- /dev/null +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java @@ -0,0 +1,70 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.model.application.validation; + +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.model.application.provider.BaseDeployLogger; +import org.junit.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.jar.JarFile; +import java.util.jar.Manifest; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class BundleValidatorTest { + private static final String JARS_DIR = "src/test/cfg/application/validation/testjars/"; + + @Test + public void basicBundleValidation() throws Exception { + // Valid jar file + JarFile ok = new JarFile(new File(JARS_DIR + "ok.jar")); + BundleValidator bundleValidator = new BundleValidator(); + bundleValidator.validateJarFile(new BaseDeployLogger(), ok); + + // No manifest + validateWithException("nomanifest.jar", "Non-existing or invalid manifest in " + JARS_DIR + "nomanifest.jar"); + } + + private void validateWithException(String jarName, String exceptionMessage) throws IOException { + try { + JarFile jarFile = new JarFile(JARS_DIR + jarName); + BundleValidator bundleValidator = new BundleValidator(); + bundleValidator.validateJarFile(new BaseDeployLogger(), jarFile); + assert (false); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), exceptionMessage); + } + } + + @Test + public void require_that_deploying_snapshot_bundle_gives_warning() throws IOException { + final StringBuffer buffer = new StringBuffer(); + + DeployLogger logger = createDeployLogger(buffer); + + new BundleValidator().validateJarFile(logger, new JarFile(JARS_DIR + "snapshot_bundle.jar")); + assertTrue(buffer.toString().contains("Deploying snapshot bundle")); + } + + @Test + public void outputs_deploy_warning_on_import_of_packages_from_deprecated_artifact() throws IOException { + final StringBuffer buffer = new StringBuffer(); + DeployLogger logger = createDeployLogger(buffer); + BundleValidator validator = new BundleValidator(); + Manifest manifest = new Manifest(Files.newInputStream(Paths.get(JARS_DIR + "/manifest-producing-import-warnings.MF"))); + validator.validateManifest(logger, "my-app-bundle.jar", manifest); + assertThat(buffer.toString()) + .contains("For JAR file 'my-app-bundle.jar': \n" + + "Manifest imports the following Java packages from 'org.json:json': [org.json]. \n" + + "The org.json library will no longer provided by jdisc runtime on Vespa 8. See https://docs.vespa.ai/en/vespa8-release-notes.html#container-runtime."); + } + + private DeployLogger createDeployLogger(StringBuffer buffer) { + return (__, message) -> buffer.append(message).append('\n'); + } +} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComponentValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComponentValidatorTest.java deleted file mode 100644 index a375621d391..00000000000 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComponentValidatorTest.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.application.validation; - -import org.junit.Test; - -import com.yahoo.config.application.api.DeployLogger; -import com.yahoo.config.model.application.provider.BaseDeployLogger; - -import java.io.File; -import java.io.IOException; -import java.util.jar.JarFile; -import java.util.logging.Level; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -public class ComponentValidatorTest { - private static final String JARS_DIR = "src/test/cfg/application/validation/testjars/"; - - @Test - public void basicComponentValidation() throws Exception { - // Valid jar file - JarFile ok = new JarFile(new File(JARS_DIR + "ok.jar")); - ComponentValidator componentValidator = new ComponentValidator(ok); - componentValidator.validateAll(new BaseDeployLogger()); - - // No manifest - validateWithException("nomanifest.jar", "Non-existing or invalid manifest in " + JARS_DIR + "nomanifest.jar"); - } - - private void validateWithException(String jarName, String exceptionMessage) throws IOException { - try { - JarFile jarFile = new JarFile(JARS_DIR + jarName); - ComponentValidator componentValidator = new ComponentValidator(jarFile); - componentValidator.validateAll(new BaseDeployLogger()); - assert (false); - } catch (IllegalArgumentException e) { - assertEquals(e.getMessage(), exceptionMessage); - } - } - - @Test - public void require_that_deploying_snapshot_bundle_gives_warning() throws IOException { - final StringBuffer buffer = new StringBuffer(); - - DeployLogger logger = new DeployLogger() { - @Override - public void log(Level level, String message) { - buffer.append(message).append('\n'); - } - }; - - new ComponentValidator(new JarFile(JARS_DIR + "snapshot_bundle.jar")).validateAll(logger); - assertTrue(buffer.toString().contains("Deploying snapshot bundle")); - } -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java index 47eabb0347e..a0e8d83fba1 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java @@ -101,7 +101,7 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { ConnectionPool connectionPool = (otherConfigServersInCluster.isEmpty()) ? FileDownloader.emptyConnectionPool() : new FileDistributionConnectionPool(configSourceSet, supervisor); - return new FileDownloader(connectionPool, supervisor, downloadDirectory, Duration.ofSeconds(30)); + return new FileDownloader(connectionPool, supervisor, downloadDirectory, Duration.ofSeconds(300)); } @Override diff --git a/container-test/pom.xml b/container-test/pom.xml index 377272d939c..7c739faad26 100644 --- a/container-test/pom.xml +++ b/container-test/pom.xml @@ -26,6 +26,10 @@ <groupId>org.apache.httpcomponents.client5</groupId> <artifactId>httpclient5</artifactId> </exclusion> + <exclusion> + <groupId>biz.aQute.bnd</groupId> + <artifactId>*</artifactId> + </exclusion> </exclusions> </dependency> diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Form.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Form.java new file mode 100644 index 00000000000..42e13d63c19 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Form.java @@ -0,0 +1,30 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.template; + +/** + * Public methods common to both Template and ListElement. + * + * @author hakonhall + */ +public interface Form { + /** Set the value of a variable, e.g. %{=color}. */ + Template set(String name, String value); + + /** Set the value of a variable and/or if-condition. */ + default Template set(String name, boolean value) { return set(name, Boolean.toString(value)); } + + default Template set(String name, int value) { return set(name, Integer.toString(value)); } + default Template set(String name, long value) { return set(name, Long.toString(value)); } + + default Template set(String name, String format, String first, String... rest) { + var args = new Object[1 + rest.length]; + args[0] = first; + System.arraycopy(rest, 0, args, 1, rest.length); + var value = String.format(format, args); + + return set(name, value); + } + + /** Add an instance of a list section after any previously added (for the given name) */ + ListElement add(String name); +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/ListElement.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/ListElement.java new file mode 100644 index 00000000000..24bb9ea6523 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/ListElement.java @@ -0,0 +1,17 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.template; + +/** + * @author hakonhall + */ +public class ListElement implements Form { + private final Template template; + + ListElement(Template template) { this.template = template; } + + @Override + public Template set(String name, String value) { return template.set(name, value); } + + @Override + public ListElement add(String name) { return new ListElement(template.addElement(name)); } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Template.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Template.java index 77d50a392c8..41e8c3e65ce 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Template.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Template.java @@ -1,8 +1,10 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.task.util.template; +import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; import com.yahoo.vespa.hosted.node.admin.task.util.text.CursorRange; +import java.nio.file.Path; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -29,10 +31,9 @@ import java.util.Optional; * * <p>To reuse a template, create the template and work on snapshots of that ({@link #snapshot()}).</p> * - * @see TemplateFile * @author hakonhall */ -public class Template { +public class Template implements Form { private Template parent = null; private final CursorRange range; private final List<Section> sections; @@ -40,8 +41,13 @@ public class Template { private final Map<String, String> values = new HashMap<>(); private final Map<String, ListSection> lists; - public static Template from(String text) { return from(text, new TemplateDescriptor()); } + public static Template at(Path path) { return at(path, new TemplateDescriptor()); } + public static Template at(Path path, TemplateDescriptor descriptor) { + String content = new UnixPath(path).readUtf8File(); + return Template.from(content, descriptor); + } + public static Template from(String text) { return from(text, new TemplateDescriptor()); } public static Template from(String text, TemplateDescriptor descriptor) { return TemplateParser.parse(text, descriptor).template(); } @@ -52,38 +58,15 @@ public class Template { this.lists = Map.copyOf(lists); } - /** Must be set (if there is a parent) before any other method. */ - void setParent(Template parent) { this.parent = parent; } - /** Set the value of a variable, e.g. %{=color}. */ + @Override public Template set(String name, String value) { values.put(name, value); return this; } - /** Set the value of a variable and/or if-condition. */ - public Template set(String name, boolean value) { return set(name, Boolean.toString(value)); } - - public Template set(String name, int value) { return set(name, Integer.toString(value)); } - public Template set(String name, long value) { return set(name, Long.toString(value)); } - - public Template set(String name, String format, String first, String... rest) { - var args = new Object[1 + rest.length]; - args[0] = first; - System.arraycopy(rest, 0, args, 1, rest.length); - var value = String.format(format, args); - - return set(name, value); - } - - /** Add an instance of a list section after any previously added (for the given name) */ - public Template add(String name) { - var section = lists.get(name); - if (section == null) { - throw new NoSuchNameTemplateException(range, name); - } - return section.add(); - } + @Override + public ListElement add(String name) { return new ListElement(addElement(name)); } public String render() { var buffer = new StringBuilder((int) (range.length() * 1.2 + 128)); @@ -102,6 +85,17 @@ public class Template { return template; } + /** Must be called (if there is a parent) before any other method. */ + void setParent(Template parent) { this.parent = parent; } + + Template addElement(String name) { + var section = lists.get(name); + if (section == null) { + throw new NoSuchNameTemplateException(range, name); + } + return section.add(); + } + Optional<String> getVariableValue(String name) { String value = values.get(name); if (value != null) return Optional.of(value); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateFile.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateFile.java deleted file mode 100644 index 0c1a26f4f65..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateFile.java +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.task.util.template; - -import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; - -import java.nio.file.Path; - -/** - * Parses a template file, see {@link Template} for details. - * - * @author hakonhall - */ -public class TemplateFile { - public static Template read(Path path) { return read(path, new TemplateDescriptor()); } - - public static Template read(Path path, TemplateDescriptor descriptor) { - String content = new UnixPath(path).readUtf8File(); - return Template.from(content, descriptor); - } -} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateFileTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateFileTest.java deleted file mode 100644 index 8c276ff0491..00000000000 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateFileTest.java +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.task.util.template; - -import org.junit.jupiter.api.Test; - -import java.nio.file.Path; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -/** - * @author hakonhall - */ -class TemplateFileTest { - @Test - void verifyVariableSection() { - Template template = getTemplate("template1.tmp"); - template.set("varname", "varvalue"); - assertEquals("variable section 'varvalue'\n" + - "end of text\n", template.render()); - } - - @Test - void verifySimpleListSection() { - Template template = getTemplate("template1.tmp"); - template.set("varname", "varvalue") - .add("listname") - .set("varname", "different varvalue") - .set("varname2", "varvalue2"); - assertEquals("variable section 'varvalue'\n" + - "same variable section 'different varvalue'\n" + - "different variable section 'varvalue2'\n" + - "between ends\n" + - "end of text\n", template.render()); - } - - @Test - void verifyNestedListSection() { - Template template = getTemplate("template2.tmp"); - Template A0 = template.add("listA"); - Template A0B0 = A0.add("listB"); - Template A0B1 = A0.add("listB"); - - Template A1 = template.add("listA"); - Template A1B0 = A1.add("listB"); - assertEquals("body A\n" + - "body B\n" + - "body B\n" + - "body A\n" + - "body B\n", - template.render()); - } - - @Test - void verifyVariableReferences() { - Template template = getTemplate("template3.tmp"); - template.set("varname", "varvalue") - .set("innerVarSetAtTop", "val2"); - template.add("l"); - template.add("l") - .set("varname", "varvalue2"); - assertEquals("varvalue\n" + - "varvalue\n" + - "inner varvalue\n" + - "val2\n" + - "inner varvalue2\n" + - "val2\n", - template.render()); - } - - private Template getTemplate(String filename) { - return TemplateFile.read(Path.of("src/test/resources/" + filename)); - } -}
\ No newline at end of file diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateTest.java index fb5f8e74b73..e010b9780c6 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateTest.java @@ -3,6 +3,8 @@ package com.yahoo.vespa.hosted.node.admin.task.util.template; import org.junit.jupiter.api.Test; +import java.nio.file.Path; + import static org.junit.jupiter.api.Assertions.assertEquals; /** @@ -88,4 +90,64 @@ public class TemplateTest { .set("area", "Norway"); assertEquals("hello worldhello Norway", snapshot.render()); } + + @Test + void verifyVariableSection() { + Template template = getTemplate("template1.tmp"); + template.set("varname", "varvalue"); + assertEquals("variable section 'varvalue'\n" + + "end of text\n", template.render()); + } + + @Test + void verifySimpleListSection() { + Template template = getTemplate("template1.tmp"); + template.set("varname", "varvalue") + .add("listname") + .set("varname", "different varvalue") + .set("varname2", "varvalue2"); + assertEquals("variable section 'varvalue'\n" + + "same variable section 'different varvalue'\n" + + "different variable section 'varvalue2'\n" + + "between ends\n" + + "end of text\n", template.render()); + } + + @Test + void verifyNestedListSection() { + Template template = getTemplate("template2.tmp"); + ListElement A0 = template.add("listA"); + ListElement A0B0 = A0.add("listB"); + ListElement A0B1 = A0.add("listB"); + + ListElement A1 = template.add("listA"); + ListElement A1B0 = A1.add("listB"); + assertEquals("body A\n" + + "body B\n" + + "body B\n" + + "body A\n" + + "body B\n", + template.render()); + } + + @Test + void verifyVariableReferences() { + Template template = getTemplate("template3.tmp"); + template.set("varname", "varvalue") + .set("innerVarSetAtTop", "val2"); + template.add("l"); + template.add("l") + .set("varname", "varvalue2"); + assertEquals("varvalue\n" + + "varvalue\n" + + "inner varvalue\n" + + "val2\n" + + "inner varvalue2\n" + + "val2\n", + template.render()); + } + + private Template getTemplate(String filename) { + return Template.at(Path.of("src/test/resources/" + filename)); + } } diff --git a/vespalib/src/tests/cpu_usage/cpu_usage_test.cpp b/vespalib/src/tests/cpu_usage/cpu_usage_test.cpp index c8835d82cd8..98a7bd780a7 100644 --- a/vespalib/src/tests/cpu_usage/cpu_usage_test.cpp +++ b/vespalib/src/tests/cpu_usage/cpu_usage_test.cpp @@ -32,7 +32,7 @@ void be_busy(duration d) { std::vector<duration> sample(const std::vector<Sampler*> &list) { std::vector<duration> result; result.reserve(list.size()); - for (Sampler *sampler: list) { + for (auto *sampler: list) { result.push_back(sampler->sample()); } return result; @@ -40,15 +40,15 @@ std::vector<duration> sample(const std::vector<Sampler*> &list) { //----------------------------------------------------------------------------- -TEST_MT_F("require that external thread-based CPU usage sampling works", 5, std::vector<Sampler*>(4, nullptr)) { +void verify_sampling(size_t thread_id, size_t num_threads, std::vector<Sampler*> &samplers, bool force_mock) { if (thread_id == 0) { TEST_BARRIER(); // #1 auto t0 = steady_clock::now(); - std::vector<duration> pre_usage = sample(f1); + std::vector<duration> pre_usage = sample(samplers); TEST_BARRIER(); // #2 TEST_BARRIER(); // #3 auto t1 = steady_clock::now(); - std::vector<duration> post_usage = sample(f1); + std::vector<duration> post_usage = sample(samplers); TEST_BARRIER(); // #4 double wall = to_s(t1 - t0); std::vector<double> load(4, 0.0); @@ -59,8 +59,9 @@ TEST_MT_F("require that external thread-based CPU usage sampling works", 5, std: fprintf(stderr, "loads: { %.2f, %.2f, %.2f, %.2f }\n", load[0], load[1], load[2], load[3]); } else { int idx = (thread_id - 1); - Sampler sampler; - f1[idx] = &sampler; + double target_load = double(thread_id - 1) / (num_threads - 2); + auto sampler = cpu_usage::create_thread_sampler(force_mock, target_load); + samplers[idx] = sampler.get(); TEST_BARRIER(); // #1 TEST_BARRIER(); // #2 for (size_t i = 0; i < loop_cnt; ++i) { @@ -73,10 +74,18 @@ TEST_MT_F("require that external thread-based CPU usage sampling works", 5, std: //----------------------------------------------------------------------------- +TEST_MT_F("require that dummy thread-based CPU usage sampling with known expected load works", 5, std::vector<Sampler*>(4, nullptr)) { + TEST_DO(verify_sampling(thread_id, num_threads, f1, true)); +} + +TEST_MT_F("require that external thread-based CPU usage sampling works", 5, std::vector<Sampler*>(4, nullptr)) { + TEST_DO(verify_sampling(thread_id, num_threads, f1, false)); +} + TEST("measure thread CPU clock overhead") { - Sampler sampler; + auto sampler = cpu_usage::create_thread_sampler(); duration d; - double min_time_us = BenchmarkTimer::benchmark([&d, &sampler]() noexcept { d = sampler.sample(); }, budget) * 1000000.0; + double min_time_us = BenchmarkTimer::benchmark([&d, &sampler]() noexcept { d = sampler->sample(); }, budget) * 1000000.0; fprintf(stderr, "approx overhead per sample (thread CPU clock): %f us\n", min_time_us); } diff --git a/vespalib/src/vespa/vespalib/util/cpu_usage.cpp b/vespalib/src/vespa/vespalib/util/cpu_usage.cpp index b5bd3ed76ac..4eee0a63870 100644 --- a/vespalib/src/vespa/vespalib/util/cpu_usage.cpp +++ b/vespalib/src/vespa/vespalib/util/cpu_usage.cpp @@ -2,23 +2,53 @@ #include "cpu_usage.h" #include "require.h" +#include <pthread.h> namespace vespalib { namespace cpu_usage { -ThreadSampler::ThreadSampler() - : _my_clock() -{ - REQUIRE_EQ(pthread_getcpuclockid(pthread_self(), &_my_clock), 0); -} +namespace { + +class DummyThreadSampler : public ThreadSampler { +private: + steady_time _start; + double _load; +public: + DummyThreadSampler(double load) : _start(steady_clock::now()), _load(load) {} + duration sample() const override { + return from_s(to_s(steady_clock::now() - _start) * _load); + } +}; + +#ifdef __linux__ + +class LinuxThreadSampler : public ThreadSampler { +private: + clockid_t _my_clock; +public: + LinuxThreadSampler() : _my_clock() { + REQUIRE_EQ(pthread_getcpuclockid(pthread_self(), &_my_clock), 0); + } + duration sample() const override { + timespec ts; + REQUIRE_EQ(clock_gettime(_my_clock, &ts), 0); + return from_timespec(ts); + } +}; + +#endif + +} // <unnamed> -duration -ThreadSampler::sample() const -{ - timespec ts; - REQUIRE_EQ(clock_gettime(_my_clock, &ts), 0); - return from_timespec(ts); +ThreadSampler::UP create_thread_sampler(bool force_mock_impl, double expected_load) { + if (force_mock_impl) { + return std::make_unique<DummyThreadSampler>(expected_load); + } +#ifdef __linux__ + return std::make_unique<LinuxThreadSampler>(); +#endif + return std::make_unique<DummyThreadSampler>(expected_load); } } // cpu_usage diff --git a/vespalib/src/vespa/vespalib/util/cpu_usage.h b/vespalib/src/vespa/vespalib/util/cpu_usage.h index 452e96ba0ff..09509a984b5 100644 --- a/vespalib/src/vespa/vespalib/util/cpu_usage.h +++ b/vespalib/src/vespa/vespalib/util/cpu_usage.h @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <pthread.h> #include <vespa/vespalib/util/time.h> +#include <memory> namespace vespalib { @@ -11,17 +11,15 @@ namespace cpu_usage { * Samples the total CPU usage of the thread that created it. Note * that this must not be used after thread termination. Enables * sampling the CPU usage of a thread from outside the thread. - * - * uses: pthread_self, pthread_getcpuclockid, clock_gettime **/ -class ThreadSampler { -private: - clockid_t _my_clock; -public: - ThreadSampler(); - duration sample() const; +struct ThreadSampler { + using UP = std::unique_ptr<ThreadSampler>; + virtual duration sample() const = 0; + virtual ~ThreadSampler() {} }; +ThreadSampler::UP create_thread_sampler(bool force_mock_impl = false, double expected_load = 0.16); + } // cpu_usage } // namespace |