-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDDS-1469. Generate default configuration fragments based on annotations #773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
25dcb23
cef0f91
b347483
3a1991e
61a2bbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,16 +18,14 @@ | |
|
|
||
| package org.apache.hadoop.hdds.conf; | ||
|
|
||
| import org.apache.hadoop.classification.InterfaceAudience; | ||
| import org.apache.hadoop.conf.Configuration; | ||
|
|
||
| import javax.xml.bind.JAXBContext; | ||
| import javax.xml.bind.JAXBException; | ||
| import javax.xml.bind.Unmarshaller; | ||
| import javax.xml.bind.annotation.XmlAccessType; | ||
| import javax.xml.bind.annotation.XmlAccessorType; | ||
| import javax.xml.bind.annotation.XmlElement; | ||
| import javax.xml.bind.annotation.XmlRootElement; | ||
| import java.io.IOException; | ||
| import java.lang.reflect.InvocationTargetException; | ||
| import java.lang.reflect.Method; | ||
| import java.net.URL; | ||
|
|
@@ -36,6 +34,9 @@ | |
| import java.util.List; | ||
| import java.util.Properties; | ||
|
|
||
| import org.apache.hadoop.classification.InterfaceAudience; | ||
| import org.apache.hadoop.conf.Configuration; | ||
|
|
||
| /** | ||
| * Configuration for ozone. | ||
| */ | ||
|
|
@@ -47,12 +48,33 @@ public class OzoneConfiguration extends Configuration { | |
|
|
||
| public OzoneConfiguration() { | ||
| OzoneConfiguration.activate(); | ||
| loadDefaults(); | ||
| } | ||
|
|
||
| public OzoneConfiguration(Configuration conf) { | ||
| super(conf); | ||
| //load the configuration from the classloader of the original conf. | ||
| setClassLoader(conf.getClassLoader()); | ||
| if (!(conf instanceof OzoneConfiguration)) { | ||
| loadDefaults(); | ||
| } | ||
| } | ||
|
|
||
| private void loadDefaults() { | ||
| try { | ||
| //there could be multiple ozone-default-generated.xml files on the | ||
| // classpath, which are generated by the annotation processor. | ||
| // Here we add all of them to the list of the available configuration. | ||
| Enumeration<URL> generatedDefaults = | ||
| OzoneConfiguration.class.getClassLoader().getResources( | ||
| "ozone-default-generated.xml"); | ||
| while (generatedDefaults.hasMoreElements()) { | ||
| addResource(generatedDefaults.nextElement()); | ||
| } | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| } | ||
| addResource("ozone-site.xml"); | ||
| } | ||
|
|
||
| public List<Property> readPropertyFromXml(URL url) throws JAXBException { | ||
|
|
@@ -265,7 +287,6 @@ public static void activate() { | |
| Configuration.addDefaultResource("hdfs-default.xml"); | ||
| Configuration.addDefaultResource("hdfs-site.xml"); | ||
| Configuration.addDefaultResource("ozone-default.xml"); | ||
| Configuration.addDefaultResource("ozone-site.xml"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we still allow this over-ride?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should. But in fact I didn't remove the possibility to use ozone-site.xml I just moved it. The problem here is that the hadoop Configuration is not designed to be reused. We can use multiple With using configuration fragments we have multiple files with the same name (String) but with different URL (eg. jar://hadoop-ozone-ozone-manager.jar!/ozone-default-generated.xml). To support this, but keep the same precedence I modified the configuration loading: The last three lines are in the constructor of OzoneConfiguration and as I would like to load |
||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,28 +35,33 @@ public class SimpleConfiguration { | |
|
|
||
| private long waitTime = 1; | ||
|
|
||
| @Config(key = "address") | ||
| @Config(key = "address", defaultValue = "localhost", description = "Just " | ||
| + "for testing", tags = ConfigTag.MANAGEMENT) | ||
| public void setClientAddress(String clientAddress) { | ||
| this.clientAddress = clientAddress; | ||
| } | ||
|
|
||
| @Config(key = "bind.host") | ||
| @Config(key = "bind.host", defaultValue = "0.0.0.0", description = "Just " | ||
| + "for testing", tags = ConfigTag.MANAGEMENT) | ||
| public void setBindHost(String bindHost) { | ||
| this.bindHost = bindHost; | ||
| } | ||
|
|
||
| @Config(key = "enabled") | ||
| @Config(key = "enabled", defaultValue = "true", description = "Just for " | ||
| + "testing", tags = ConfigTag.MANAGEMENT) | ||
| public void setEnabled(boolean enabled) { | ||
| this.enabled = enabled; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: what does client. enabled mean?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just an example configuration (for unit tests) I invented a random boolean configuration. Name doesn't matter but I can use a more meaningful one. For example client.compression.enabled. |
||
| } | ||
|
|
||
| @Config(key = "port") | ||
| @Config(key = "port", defaultValue = "9878", description = "Just for " | ||
| + "testing", tags = ConfigTag.MANAGEMENT) | ||
| public void setPort(int port) { | ||
| this.port = port; | ||
| } | ||
|
|
||
| @Config(key = "wait", type = ConfigType.TIME, timeUnit = | ||
| TimeUnit.SECONDS) | ||
| TimeUnit.SECONDS, defaultValue = "10m", description = "Just for " | ||
| + "testing", tags = ConfigTag.MANAGEMENT) | ||
| public void setWaitTime(long waitTime) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice 👏 |
||
| this.waitTime = waitTime; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <!-- | ||
| Licensed 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. See accompanying LICENSE file. | ||
| --> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 | ||
| http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
| <parent> | ||
| <groupId>org.apache.hadoop</groupId> | ||
| <artifactId>hadoop-hdds</artifactId> | ||
| <version>0.5.0-SNAPSHOT</version> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it possible to inherit this value from the parent POM ? more a question for my understanding..
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is some kind of chicken egg problem. You should have an exact version for your parent pom as you don't know which parent should be downloaded to check the current version. What you can remove is the normal But as a solution the maven version plugin is introduced to search and replace both version and parent version in a consistent way so it's not a problem. |
||
| </parent> | ||
| <artifactId>hadoop-hdds-config</artifactId> | ||
| <version>0.5.0-SNAPSHOT</version> | ||
| <description>Apache Hadoop Distributed Data Store Config Tools</description> | ||
| <name>Apache Hadoop HDDS Config</name> | ||
| <packaging>jar</packaging> | ||
|
|
||
| <properties> | ||
|
|
||
| </properties> | ||
|
|
||
| <dependencies> | ||
|
|
||
| <dependency> | ||
| <groupId>junit</groupId> | ||
| <artifactId>junit</artifactId> | ||
| <scope>test</scope> | ||
| </dependency> | ||
|
|
||
| </dependencies> | ||
|
|
||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <artifactId>maven-compiler-plugin</artifactId> | ||
| <version>3.1</version> | ||
| <executions> | ||
| <execution> | ||
| <id>default-compile</id> | ||
| <phase>compile</phase> | ||
| <goals> | ||
| <goal>compile</goal> | ||
| </goals> | ||
| <configuration> | ||
| <!-- don't need to activate annotation processor (which may not be available yet) for compilation --> | ||
| <compilerArgument>-proc:none</compilerArgument> | ||
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| /** | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF 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 | ||
| * <p> | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * <p> | ||
| * 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 org.apache.hadoop.hdds.conf; | ||
|
|
||
| import javax.xml.parsers.DocumentBuilder; | ||
| import javax.xml.parsers.DocumentBuilderFactory; | ||
| import javax.xml.transform.OutputKeys; | ||
| import javax.xml.transform.Transformer; | ||
| import javax.xml.transform.TransformerException; | ||
| import javax.xml.transform.TransformerFactory; | ||
| import javax.xml.transform.dom.DOMSource; | ||
| import javax.xml.transform.stream.StreamResult; | ||
| import java.io.InputStream; | ||
| import java.io.Writer; | ||
| import java.util.Arrays; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| import org.w3c.dom.Document; | ||
| import org.w3c.dom.Element; | ||
|
|
||
| /** | ||
| * Simple DOM based config file writer. | ||
| * <p> | ||
| * This class can init/load existing ozone-default-generated.xml fragments | ||
| * and append new entries and write to the file system. | ||
| */ | ||
| public class ConfigFileAppender { | ||
|
|
||
| private Document document; | ||
|
|
||
| private final DocumentBuilder builder; | ||
|
|
||
| public ConfigFileAppender() { | ||
| try { | ||
| DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); | ||
| builder = factory.newDocumentBuilder(); | ||
| } catch (Exception ex) { | ||
| throw new ConfigurationException("Can initialize new configuration", ex); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Initialize a new ozone-site.xml structure with empty content. | ||
| */ | ||
| public void init() { | ||
| try { | ||
| document = builder.newDocument(); | ||
| document.appendChild(document.createElement("configuration")); | ||
| } catch (Exception ex) { | ||
| throw new ConfigurationException("Can initialize new configuration", ex); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Load existing ozone-site.xml content and parse the DOM tree. | ||
| */ | ||
| public void load(InputStream stream) { | ||
| try { | ||
| document = builder.parse(stream); | ||
| } catch (Exception ex) { | ||
| throw new ConfigurationException("Can't load existing configuration", ex); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Add configuration fragment. | ||
| */ | ||
| public void addConfig(String key, String defaultValue, String description, | ||
| ConfigTag[] tags) { | ||
| Element root = document.getDocumentElement(); | ||
| Element propertyElement = document.createElement("property"); | ||
|
|
||
| addXmlElement(propertyElement, "name", key); | ||
|
|
||
| addXmlElement(propertyElement, "value", defaultValue); | ||
|
|
||
| addXmlElement(propertyElement, "description", description); | ||
|
|
||
| String tagsAsString = Arrays.stream(tags).map(tag -> tag.name()) | ||
| .collect(Collectors.joining(", ")); | ||
|
|
||
| addXmlElement(propertyElement, "tag", tagsAsString); | ||
|
|
||
| root.appendChild(propertyElement); | ||
| } | ||
|
|
||
| private void addXmlElement(Element parentElement, String tagValue, | ||
| String textValue) { | ||
| Element element = document.createElement(tagValue); | ||
| element.appendChild(document.createTextNode(textValue)); | ||
| parentElement.appendChild(element); | ||
| } | ||
|
|
||
| /** | ||
| * Write out the XML content to a writer. | ||
| */ | ||
| public void write(Writer writer) { | ||
| try { | ||
| TransformerFactory transformerFactory = TransformerFactory.newInstance(); | ||
| Transformer transf = transformerFactory.newTransformer(); | ||
|
|
||
| transf.setOutputProperty(OutputKeys.ENCODING, "UTF-8"); | ||
| transf.setOutputProperty(OutputKeys.INDENT, "yes"); | ||
| transf | ||
| .setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2"); | ||
|
|
||
| transf.transform(new DOMSource(document), new StreamResult(writer)); | ||
| } catch (TransformerException e) { | ||
| throw new ConfigurationException("Can't write the configuration xml", e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I be a greedy pig and request that we also sort these keys (in another JIRA of course) before we write out the XML?