You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2023/03/23 04:33:34 UTC
[GitHub] [ozone] swamirishi opened a new pull request, #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
swamirishi opened a new pull request, #4455:
URL: https://github.com/apache/ozone/pull/4455
## What changes were proposed in this pull request?
Currently s3Gateway fails to come up if "ozone.metadata.dirs" is not defined. Since S3Gateway doesn't need any metadir, we can set a tmp dir path which can be deleted when the process is stopped.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8253
## How was this patch tested?
Manually tested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1145698632
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +75,17 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_METADATA_DIRS))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory("ozone_s3g_tmpdir");
Review Comment:
Can this be set relative to the current working directory instead of `/tmp` which it would default to on most systems?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1169112790
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +76,25 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_HTTP_BASEDIR))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1150226756
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +77,25 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_METADATA_DIRS))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+ "ozone_s3g_tmp_meta_dir");
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+ try {
+ FileUtils.deleteDirectory(tmpMetaDir.toFile());
+ } catch (IOException e) {
+ LOG.error("Failed to cleanup temporary S3 Gateway Metadir {}",
+ tmpMetaDir.toFile().getAbsolutePath(), e);
+ }
+ }));
+ ozoneConfiguration.set(HddsConfigKeys.OZONE_METADATA_DIRS,
+ tmpMetaDir.toFile().getAbsolutePath());
+ }
Review Comment:
> Please extract this logic to a method.
This is still TODO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4455:
URL: https://github.com/apache/ozone/pull/4455#issuecomment-1512583470
/ready
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #4455: HDDS-8253. Set ozone.metadata.dirs to temporary dir if not defined in S3 Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #4455:
URL: https://github.com/apache/ozone/pull/4455
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1169109126
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +77,25 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_METADATA_DIRS))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+ "ozone_s3g_tmp_meta_dir");
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
Review Comment:
@kerneltime Should I do the web ui change as part of another PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1146609860
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +75,17 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_METADATA_DIRS))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory("ozone_s3g_tmpdir");
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1146820937
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +75,17 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
Review Comment:
Got it thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1162342584
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +77,25 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_METADATA_DIRS))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+ "ozone_s3g_tmp_meta_dir");
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
Review Comment:
@swamirishi do you plan to address this? Also, we can change this PR to change the logic for all the webUIs to see the $CWD.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1155403774
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +76,25 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_HTTP_BASEDIR))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
Review Comment:
Update comment, setting to tmp directory from current working directory, `cwd`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1169110460
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -63,13 +69,35 @@ public static void main(String[] args) throws Exception {
new Gateway().run(args);
}
+ private void setHttpBaseDir(OzoneConfiguration ozoneConfiguration)
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1150226485
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +77,25 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_METADATA_DIRS))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+ "ozone_s3g_tmp_meta_dir");
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
Review Comment:
Please use `ShutdownHookManager` to add the shutdown hook. Also, I think it should be executed after the other hook (which stops the server), so use a lower than default priority.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +77,25 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_METADATA_DIRS))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+ "ozone_s3g_tmp_meta_dir");
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+ try {
+ FileUtils.deleteDirectory(tmpMetaDir.toFile());
+ } catch (IOException e) {
+ LOG.error("Failed to cleanup temporary S3 Gateway Metadir {}",
+ tmpMetaDir.toFile().getAbsolutePath(), e);
+ }
+ }));
+ ozoneConfiguration.set(HddsConfigKeys.OZONE_METADATA_DIRS,
+ tmpMetaDir.toFile().getAbsolutePath());
+ }
Review Comment:
This is still TODO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1146612373
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +75,17 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
Review Comment:
ServerUtils.getOzoneMetaDirPath throws an exception here. It would be better to set the value if not set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1169112054
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -63,13 +69,35 @@ public static void main(String[] args) throws Exception {
new Gateway().run(args);
}
+ private void setHttpBaseDir(OzoneConfiguration ozoneConfiguration)
+ throws IOException {
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_HTTP_BASEDIR))) {
+ //Setting ozone.http.basedir if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+ "ozone_s3g_tmp_base_dir");
+ ShutdownHookManager.get().addShutdownHook(() -> {
+ try {
+ FileUtils.deleteDirectory(tmpMetaDir.toFile());
+ } catch (IOException e) {
+ LOG.error("Failed to cleanup temporary S3 Gateway Metadir {}",
+ tmpMetaDir.toFile().getAbsolutePath(), e);
+ }
+ }, 0);
+ ozoneConfiguration.set(OzoneConfigKeys.OZONE_HTTP_BASEDIR,
+ tmpMetaDir.toFile().getAbsolutePath());
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1168992447
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -63,13 +69,35 @@ public static void main(String[] args) throws Exception {
new Gateway().run(args);
}
+ private void setHttpBaseDir(OzoneConfiguration ozoneConfiguration)
Review Comment:
```
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java
72: 'ozoneConfiguration' hides a field.
```
I guess the parameter is unnecessary, it can use the field directly.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -63,13 +69,35 @@ public static void main(String[] args) throws Exception {
new Gateway().run(args);
}
+ private void setHttpBaseDir(OzoneConfiguration ozoneConfiguration)
+ throws IOException {
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_HTTP_BASEDIR))) {
+ //Setting ozone.http.basedir if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+ "ozone_s3g_tmp_base_dir");
+ ShutdownHookManager.get().addShutdownHook(() -> {
+ try {
+ FileUtils.deleteDirectory(tmpMetaDir.toFile());
+ } catch (IOException e) {
+ LOG.error("Failed to cleanup temporary S3 Gateway Metadir {}",
+ tmpMetaDir.toFile().getAbsolutePath(), e);
+ }
+ }, 0);
+ ozoneConfiguration.set(OzoneConfigKeys.OZONE_HTTP_BASEDIR,
+ tmpMetaDir.toFile().getAbsolutePath());
Review Comment:
Nit: Please extract `tmpMetaDir.toFile()`, it's executed 3 times.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4455:
URL: https://github.com/apache/ozone/pull/4455#issuecomment-1502716423
> CI workflow has errors in the integration tests, re-run to see if resolves
Since there are changes requested outstanding, CI does not need to be re-run with the current state of the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1147356876
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +77,25 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_METADATA_DIRS))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+ "ozone_s3g_tmp_meta_dir");
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+ try {
+ FileUtils.deleteDirectory(tmpMetaDir.toFile());
+ } catch (IOException e) {
+ LOG.error("Failed to cleanup temporary S3 Gateway Metadir {}",
+ tmpMetaDir.toFile().getAbsolutePath(), e);
+ }
+ }));
+ ozoneConfiguration.set(HddsConfigKeys.OZONE_METADATA_DIRS,
+ tmpMetaDir.toFile().getAbsolutePath());
+ }
Review Comment:
Please extract this logic to a method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4455:
URL: https://github.com/apache/ozone/pull/4455#issuecomment-1489772447
Thanks @swamirishi for updating the patch to set `OZONE_HTTP_BASEDIR`. I think you missed the other two comments:
https://github.com/apache/ozone/pull/4455#discussion_r1147356876
https://github.com/apache/ozone/pull/4455#discussion_r1150226485
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4455: HDDS-8253. Set ozone.metadata.dirs to temporary dir if not defined in S3 Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4455:
URL: https://github.com/apache/ozone/pull/4455#issuecomment-1512837777
Thanks @swamirishi for the patch, @kerneltime, @neils-dev, @sadanand48 for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4455:
URL: https://github.com/apache/ozone/pull/4455#issuecomment-1512010995
Thanks @swamirishi for the updates. Latest commit introduced another checkstyle failure:
```
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java
23: Unused import - java.nio.file.Path.
```
(Hint: it can be run locally simply by `hadoop-ozone/dev-support/checks/checkstyle.sh`.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4455:
URL: https://github.com/apache/ozone/pull/4455#issuecomment-1511104968
/pending extract method, use `ShutdownHookManager`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4455:
URL: https://github.com/apache/ozone/pull/4455#issuecomment-1502688614
Thanks @swamirishi . Following up on this, just have the minor comment regarding fixing the comment in the code to set to current working dir if not set. Also, the CI workflow has errors in the integration tests, re-run to see if resolves.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #4455:
URL: https://github.com/apache/ozone/pull/4455#issuecomment-1480627510
I took a deeper look into this, this was due to the change in https://github.com/apache/ozone/pull/4308 which impacts S3G as S3 G does configure a webui that displays some basic AWS CLI usage. One option could be to set the temporary folder to be relative to the current working directory for S3G. The main issue that is being avoided in #4308 is the cleanup of the landing page content due to clean up of `/tmp` folder.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1146034668
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +75,17 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
Review Comment:
nit : can use `ServerUtils.getOzoneMetaDirPath(ozoneConfiguration)` instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #4455: HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #4455:
URL: https://github.com/apache/ozone/pull/4455#discussion_r1169113601
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java:
##########
@@ -70,6 +77,25 @@ public Void call() throws Exception {
OzoneConfigurationHolder.setConfiguration(ozoneConfiguration);
UserGroupInformation.setConfiguration(ozoneConfiguration);
loginS3GUser(ozoneConfiguration);
+
+ if (StringUtils.isEmpty(ozoneConfiguration.get(
+ OzoneConfigKeys.OZONE_METADATA_DIRS))) {
+ //Setting ozone.metadata.dirs if not set so that server setup doesn't
+ // fail.
+ Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+ "ozone_s3g_tmp_meta_dir");
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+ try {
+ FileUtils.deleteDirectory(tmpMetaDir.toFile());
+ } catch (IOException e) {
+ LOG.error("Failed to cleanup temporary S3 Gateway Metadir {}",
+ tmpMetaDir.toFile().getAbsolutePath(), e);
+ }
+ }));
+ ozoneConfiguration.set(HddsConfigKeys.OZONE_METADATA_DIRS,
+ tmpMetaDir.toFile().getAbsolutePath());
+ }
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org