Skip to content

HDDS-12777. Use module-specific name for generated config files #8475

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.

Sign up for GitHub

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

Merged
adoroszlai merged 6 commits into from
May 22, 2025
Merged

HDDS-12777. Use module-specific name for generated config files #8475

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
304775d
HDDS-12777. ozone-default is used as actual configuration
adoroszlai May 16, 2025
9471bba
simplify resource loading
adoroszlai May 17, 2025
9d210c7
set timeout for httpfs test
adoroszlai May 17, 2025
41eab9e
use Ozone startup msg in httpfs
adoroszlai May 17, 2025
1e8145a
Merge remote-tracking branch 'origin/master' into HDDS-12777
adoroszlai May 22, 2025
d9ab1a9
use list, add comments, add ozone-csi
adoroszlai May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions hadoop-hdds/client/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,16 @@

<build>
<plugins>
<plugin>
<groupId>com.coderplus.maven.plugins</groupId>
<artifactId>copy-rename-maven-plugin</artifactId>
<executions>
<execution>
<id>rename-generated-config</id>
<phase>process-classes</phase>
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
Expand Down
14 changes: 14 additions & 0 deletions hadoop-hdds/common/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,20 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.coderplus.maven.plugins</groupId>
<artifactId>copy-rename-maven-plugin</artifactId>
<executions>
<execution>
<id>rename-generated-config</id>
<phase>process-classes</phase>
</execution>
<execution>
<id>rename-generated-test-config</id>
<phase>process-test-classes</phase>
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
Expand Down
54 changes: 18 additions & 36 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CONTAINER_COPY_WORKDIR;

import com.google.common.base.Preconditions;
import java.io.IOException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -105,42 +104,10 @@ public static <T> T newInstanceOf(Class<T> configurationClass) {
}

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();
addResource(conf);
}
}

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-default.xml");
// Adding core-site here because properties from core-site are
// distributed to executors by spark driver. Ozone properties which are
// added to core-site, will be overridden by properties from adding Resource
// ozone-default.xml. So, adding core-site again will help to resolve
// this override issue.
Copy link
Contributor

@umamaheswararao umamaheswararao May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adoroszlai have checked the above comment why we added again core-site.xml here? Just wanted to make sure we are not skipping anything!

Copy link
Contributor Author

@adoroszlai adoroszlai May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@umamaheswararao Please see the "Remove the hack introduced by HDDS-3871" point in PR description.

Copy link
Contributor

@umamaheswararao umamaheswararao May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adoroszlai for pointing it.

addResource("core-site.xml");
addResource("ozone-site.xml");
}

public List<Property> readPropertyFromXml(URL url) throws JAXBException {
Expand Down Expand Up @@ -242,10 +209,25 @@ public boolean equals(Object obj) {
}
}

/** Add default resources. */
public static void activate() {
// adds the default resources
Configuration.addDefaultResource("hdfs-default.xml");
Configuration.addDefaultResource("hdfs-site.xml");
// core-default and core-site are added by parent class
addDefaultResource("hdfs-default.xml");
addDefaultResource("hdfs-site.xml");

// One generated file per module
addDefaultResource("hdds-common-default.xml");
Copy link
Contributor

@ChenSammi ChenSammi May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we extra the module name into an array or list, and append the "default.xml" automatically for each module, can let a comment telling about if there is any new module added in future, and the new module uses this annotation based property definition mechanism, module name should be added into this array?

Copy link
Contributor Author

@adoroszlai adoroszlai May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, waiting a bit for any other feedback.

Copy link
Contributor

@umamaheswararao umamaheswararao May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, now we generate these default xml files per module and load each file once right?

Copy link
Contributor Author

@adoroszlai adoroszlai May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Contributor

@umamaheswararao umamaheswararao May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works, thanks

addDefaultResource("hdds-client-default.xml");
addDefaultResource("hdds-container-service-default.xml");
addDefaultResource("hdds-server-framework-default.xml");
addDefaultResource("hdds-server-scm-default.xml");
addDefaultResource("ozone-common-default.xml");
addDefaultResource("ozone-manager-default.xml");
addDefaultResource("ozone-recon-default.xml");

// Non-generated configs
addDefaultResource("ozone-default.xml");
addDefaultResource("ozone-site.xml");
}

/**
Expand Down
4 changes: 2 additions & 2 deletions ...common/src/test/java/org/apache/hadoop/hdds/conf/TestGeneratedConfigurationOverwrite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@
public class TestGeneratedConfigurationOverwrite {

private final Path generatedConfigurationPath =
Paths.get("target/test-classes/ozone-default-generated.xml");
Paths.get("target/test-classes/hdds-common-default.xml");
private final Path generatedConfigurationPathBak =
Paths.get("target/test-classes/ozone-default-generated.xml.bak");
Paths.get("target/test-classes/hdds-common-default.xml.bak");

private OzoneConfiguration conf;

Expand Down
10 changes: 10 additions & 0 deletions hadoop-hdds/container-service/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,16 @@

<build>
<plugins>
<plugin>
<groupId>com.coderplus.maven.plugins</groupId>
<artifactId>copy-rename-maven-plugin</artifactId>
<executions>
<execution>
<id>rename-generated-config</id>
<phase>process-classes</phase>
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
Expand Down
14 changes: 14 additions & 0 deletions hadoop-hdds/framework/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,20 @@

<build>
<plugins>
<plugin>
<groupId>com.coderplus.maven.plugins</groupId>
<artifactId>copy-rename-maven-plugin</artifactId>
<executions>
<execution>
<id>rename-generated-config</id>
<phase>process-classes</phase>
</execution>
<execution>
<id>rename-generated-test-config</id>
<phase>process-test-classes</phase>
Copy link
Contributor

@ChenSammi ChenSammi May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the two modules, famework and common, who has this extra "process-test-classes", will both the two generated xml files be loaded during OzoneConfiguration activation?
If only first generated xml file will be loaded, how TestGeneratedConfigurationOverwrite.java can pass?

Copy link
Contributor Author

@adoroszlai adoroszlai May 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config file for test is created in target/test-classes (used in their own unit tests) and is added to ...-tests.jar (used by some other modules' tests).

</execution>
</executions>
</plugin>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
Expand Down
10 changes: 10 additions & 0 deletions hadoop-hdds/server-scm/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,16 @@
</testResource>
</testResources>
<plugins>
<plugin>
<groupId>com.coderplus.maven.plugins</groupId>
<artifactId>copy-rename-maven-plugin</artifactId>
<executions>
<execution>
<id>rename-generated-config</id>
<phase>process-classes</phase>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
Expand Down
33 changes: 1 addition & 32 deletions hadoop-ozone/client/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,40 +130,9 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths>
<path>
<groupId>org.apache.ozone</groupId>
<artifactId>hdds-config</artifactId>
<version>${hdds.version}</version>
</path>
</annotationProcessorPaths>
<annotationProcessors>
<annotationProcessor>org.apache.hadoop.hdds.conf.ConfigFileGenerator</annotationProcessor>
</annotationProcessors>
<proc>none</proc>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<executions>
<execution>
<id>ban-annotations</id>
<!-- override default restriction from root POM -->
<configuration>
<rules>
<restrictImports>
<reason>Only selected annotation processors are enabled, see configuration of maven-compiler-plugin.</reason>
<bannedImports>
<bannedImport>org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator</bannedImport>
<bannedImport>org.apache.hadoop.hdds.scm.metadata.Replicate</bannedImport>
<bannedImport>org.kohsuke.MetaInfServices</bannedImport>
</bannedImports>
</restrictImports>
</rules>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
10 changes: 10 additions & 0 deletions hadoop-ozone/common/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,16 @@
</resource>
</resources>
<plugins>
<plugin>
<groupId>com.coderplus.maven.plugins</groupId>
<artifactId>copy-rename-maven-plugin</artifactId>
<executions>
<execution>
<id>rename-generated-config</id>
<phase>process-classes</phase>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-maven-plugins</artifactId>
Expand Down
10 changes: 10 additions & 0 deletions hadoop-ozone/csi/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,16 @@

<build>
<plugins>
<plugin>
<groupId>com.coderplus.maven.plugins</groupId>
<artifactId>copy-rename-maven-plugin</artifactId>
<executions>
<execution>
<id>rename-generated-config</id>
<phase>process-classes</phase>
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.salesforce.servicelibs</groupId>
<artifactId>proto-backwards-compatibility</artifactId>
Expand Down
1 change: 1 addition & 0 deletions hadoop-ozone/dist/src/main/smoketest/httpfs/operations_tests.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Resource operations.robot
Resource ../lib/os.robot
Resource ../commonlib.robot
Suite Setup Generate volume
Test Timeout 2 minutes

*** Variables ***
${volume} generated
Expand Down
4 changes: 4 additions & 0 deletions hadoop-ozone/httpfsgateway/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@
<groupId>org.apache.ozone</groupId>
<artifactId>hdds-server-framework</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ozone</groupId>
<artifactId>ozone-common</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ozone</groupId>
<artifactId>ozone-filesystem-common</artifactId>
Expand Down
5 changes: 3 additions & 2 deletions ...ne/httpfsgateway/src/main/java/org/apache/ozone/fs/http/server/HttpFSServerWebServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@

package org.apache.ozone.fs.http.server;

import static org.apache.hadoop.util.StringUtils.startupShutdownMessage;
import static org.apache.hadoop.hdds.utils.HddsServerUtil.startupShutdownMessage;
import static org.apache.hadoop.ozone.util.OzoneVersionInfo.OZONE_VERSION_INFO;

import java.io.IOException;
import java.net.InetSocketAddress;
Expand Down Expand Up @@ -177,8 +178,8 @@ public URL getUrl() {
}

public static void main(String[] args) throws Exception {
startupShutdownMessage(HttpFSServerWebServer.class, args, LOG);
OzoneConfiguration conf = new OzoneConfiguration();
startupShutdownMessage(OZONE_VERSION_INFO, HttpFSServerWebServer.class, args, LOG, conf);
Configuration sslConf = SSLFactory.readSSLConfiguration(conf,
SSLFactory.Mode.SERVER);
HttpFSServerWebServer webServer =
Expand Down
12 changes: 2 additions & 10 deletions hadoop-ozone/insight/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,8 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths>
<path>
<groupId>org.apache.ozone</groupId>
<artifactId>hdds-config</artifactId>
<version>${hdds.version}</version>
</path>
</annotationProcessorPaths>
<annotationProcessors>
<annotationProcessor>org.apache.hadoop.hdds.conf.ConfigFileGenerator</annotationProcessor>
</annotationProcessors>
<!-- no annotation processor, ConfigGroup and Config are only used for finding annotation at runtime -->
<proc>none</proc>
</configuration>
</plugin>
<plugin>
Expand Down
53 changes: 10 additions & 43 deletions ...ne/insight/src/test/java/org/apache/hadoop/ozone/insight/TestConfigurationSubCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@
import java.io.PrintStream;
import java.io.UnsupportedEncodingException;
import java.nio.charset.StandardCharsets;
import org.apache.hadoop.hdds.conf.Config;
import org.apache.hadoop.hdds.conf.ConfigGroup;
import org.apache.hadoop.hdds.conf.ConfigTag;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.om.OmConfig;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -53,49 +51,18 @@ public void reset() {
@Test
public void testPrintConfig() throws UnsupportedEncodingException {
OzoneConfiguration conf = new OzoneConfiguration();
conf.set("ozone.scm.client.address", "omclient");
long customValue = OmConfig.Defaults.SERVER_LIST_MAX_SIZE + 50;
conf.setLong(OmConfig.Keys.SERVER_LIST_MAX_SIZE, customValue);
ConfigurationSubCommand subCommand = new ConfigurationSubCommand();

subCommand.printConfig(CustomConfig.class, conf);
subCommand.printConfig(OmConfig.class, conf);

final String output = out.toString(StandardCharsets.UTF_8.name());
assertThat(output).contains(">>> ozone.scm.client.address");
assertThat(output).contains("default: localhost");
assertThat(output).contains("current: omclient");
assertThat(output).contains(">>> ozone.scm.client.secure");
assertThat(output).contains("default: true");
assertThat(output).contains("current: true");
}

/**
* Example configuration parent.
*/
public static class ParentConfig {
@Config(key = "secure", defaultValue = "true", description = "Make "
+ "everything secure.", tags = ConfigTag.MANAGEMENT)
private boolean secure = true;

public boolean isSecure() {
return secure;
}
}

/**
* Example configuration.
*/
@ConfigGroup(prefix = "ozone.scm.client")
public static class CustomConfig extends ParentConfig {

@Config(key = "address", defaultValue = "localhost", description = "Client "
+ "address (To test string injection).", tags = ConfigTag.MANAGEMENT)
private String clientAddress;

public String getClientAddress() {
return clientAddress;
}

public void setClientAddress(String clientAddress) {
this.clientAddress = clientAddress;
}
assertThat(output).contains(">>> " + OmConfig.Keys.SERVER_LIST_MAX_SIZE);
assertThat(output).contains("default: " + OmConfig.Defaults.SERVER_LIST_MAX_SIZE);
assertThat(output).contains("current: " + customValue);
assertThat(output).contains(">>> " + OmConfig.Keys.ENABLE_FILESYSTEM_PATHS);
assertThat(output).contains("default: " + OmConfig.Defaults.ENABLE_FILESYSTEM_PATHS);
assertThat(output).contains("current: " + OmConfig.Defaults.ENABLE_FILESYSTEM_PATHS);
}
}
1 change: 0 additions & 1 deletion ...zone/integration-test-recon/src/test/java/org/apache/hadoop/ozone/recon/ReconService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.util.Objects;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.MiniOzoneCluster;
import org.apache.ozone.recon.schema.ReconSqlDbConfig;
import org.apache.ratis.util.Preconditions;

/** Recon for {@link MiniOzoneCluster}. */
Expand Down
Loading