Skip to content

HDDS-12527. Separate S3 Gateway from MiniOzoneCluster #8058

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 1 commit into from
Mar 12, 2025
Merged

HDDS-12527. Separate S3 Gateway from MiniOzoneCluster #8058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 14 additions & 26 deletions hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneCluster.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.hadoop.ozone;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.util.concurrent.TimeoutException;
Expand All @@ -33,7 +34,6 @@
import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.recon.ReconServer;
import org.apache.hadoop.ozone.s3.Gateway;
import org.apache.hadoop.security.authentication.client.AuthenticationException;
import org.apache.ozone.test.GenericTestUtils;
import org.apache.ratis.util.ExitUtils;
Expand Down Expand Up @@ -145,13 +145,6 @@ void waitForPipelineTobeReady(HddsProtos.ReplicationFactor factor,
*/
ReconServer getReconServer();

/**
* Returns a {@link Gateway} instance.
*
* @return {@link Gateway} instance if it is initialized, otherwise null.
*/
Gateway getS3G();

/**
* Returns an {@link OzoneClient} to access the {@link MiniOzoneCluster}.
* The caller is responsible for closing the client after use.
Expand Down Expand Up @@ -225,21 +218,6 @@ void restartHddsDatanode(DatanodeDetails dn, boolean waitForDatanode)
*/
void stopRecon();

/**
* Start S3G.
*/
void startS3G();

/**
* Restart S3G.
*/
void restartS3G();

/**
* Stop S3G.
*/
void stopS3G();

/**
* Shutdown the MiniOzoneCluster and delete the storage dirs.
*/
Expand Down Expand Up @@ -295,13 +273,13 @@ abstract class Builder {
protected String omId = UUID.randomUUID().toString();

protected boolean includeRecon = false;
protected boolean includeS3G = false;

protected int numOfDatanodes = 3;
protected boolean startDataNodes = true;
protected CertificateClient certClient;
protected SecretKeyClient secretKeyClient;
protected DatanodeFactory dnFactory = UniformDatanodesFactory.newBuilder().build();
private final List<Service> services = new ArrayList<>();

protected Builder(OzoneConfiguration conf) {
this.conf = conf;
Expand Down Expand Up @@ -378,11 +356,15 @@ public Builder includeRecon(boolean include) {
return this;
}

public Builder includeS3G(boolean include) {
this.includeS3G = include;
public Builder addService(Service service) {
services.add(service);
return this;
}

public List<Service> getServices() {
return services;
}

/**
* Constructs and returns MiniOzoneCluster.
*
Expand All @@ -397,4 +379,10 @@ public Builder includeS3G(boolean include) {
interface DatanodeFactory extends CheckedFunction<OzoneConfiguration, OzoneConfiguration, IOException> {
// marker
}

/** Service to manage as part of the mini cluster. */
interface Service {
void start(OzoneConfiguration conf) throws Exception;
void stop() throws Exception;
}
}
89 changes: 24 additions & 65 deletions ...op-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,6 @@
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.recon.ConfigurationProvider;
import org.apache.hadoop.ozone.recon.ReconServer;
import org.apache.hadoop.ozone.s3.Gateway;
import org.apache.hadoop.ozone.s3.OzoneClientCache;
import org.apache.hadoop.ozone.s3.OzoneConfigurationHolder;
import org.apache.hadoop.ozone.s3.S3GatewayConfigKeys;
import org.apache.hadoop.security.authentication.client.AuthenticationException;
import org.apache.ozone.recon.schema.ReconSqlDbConfig;
import org.apache.ozone.test.GenericTestUtils;
Expand Down Expand Up @@ -116,7 +112,7 @@ public class MiniOzoneClusterImpl implements MiniOzoneCluster {
private OzoneManager ozoneManager;
private final List<HddsDatanodeService> hddsDatanodes;
private ReconServer reconServer;
private Gateway s3g;
private final List<Service> services;

// Timeout for the cluster to be ready
private int waitForClusterToBeReadyTimeout = 120000; // 2 min
Expand All @@ -132,15 +128,14 @@ private MiniOzoneClusterImpl(OzoneConfiguration conf,
OzoneManager ozoneManager,
StorageContainerManager scm,
List<HddsDatanodeService> hddsDatanodes,
ReconServer reconServer,
Gateway s3g) {
ReconServer reconServer, List<Service> services) {
this.conf = conf;
this.ozoneManager = ozoneManager;
this.scm = scm;
this.hddsDatanodes = hddsDatanodes;
this.reconServer = reconServer;
this.scmConfigurator = scmConfigurator;
this.s3g = s3g;
this.services = services;
}

/**
Expand All @@ -150,11 +145,12 @@ private MiniOzoneClusterImpl(OzoneConfiguration conf,
* OzoneManagers and StorageContainerManagers.
*/
MiniOzoneClusterImpl(OzoneConfiguration conf, SCMConfigurator scmConfigurator,
List<HddsDatanodeService> hddsDatanodes, ReconServer reconServer) {
List<HddsDatanodeService> hddsDatanodes, ReconServer reconServer, List<Service> services) {
this.scmConfigurator = scmConfigurator;
this.conf = conf;
this.hddsDatanodes = hddsDatanodes;
this.reconServer = reconServer;
this.services = services;
}

public SCMConfigurator getSCMConfigurator() {
Expand Down Expand Up @@ -264,11 +260,6 @@ public ReconServer getReconServer() {
return this.reconServer;
}

@Override
public Gateway getS3G() {
return this.s3g;
}

@Override
public int getHddsDatanodeIndex(DatanodeDetails dn) throws IOException {
for (HddsDatanodeService service : hddsDatanodes) {
Expand Down Expand Up @@ -420,7 +411,7 @@ public void stop() {
stopDatanodes(hddsDatanodes);
stopSCM(scm);
stopRecon(reconServer);
stopS3G(s3g);
stopServices(services);
}

private void startHddsDatanode(HddsDatanodeService datanode) {
Expand Down Expand Up @@ -460,21 +451,10 @@ public void stopRecon() {
stopRecon(reconServer);
}

@Override
public void startS3G() {
s3g = new Gateway();
s3g.execute(NO_ARGS);
}

@Override
public void restartS3G() {
stopS3G(s3g);
startS3G();
}

@Override
public void stopS3G() {
stopS3G(s3g);
public void startServices() throws Exception {
for (Service service : services) {
service.start(getConf());
}
}

private CertificateClient getCAClient() {
Expand Down Expand Up @@ -531,16 +511,18 @@ private static void stopRecon(ReconServer reconServer) {
}
}

private static void stopS3G(Gateway s3g) {
try {
if (s3g != null) {
LOG.info("Stopping S3G");
// TODO (HDDS-11539): Remove this workaround once the @PreDestroy issue is fixed
OzoneClientCache.closeClient();
s3g.stop();
private static void stopServices(List<Service> services) {
// stop in reverse order
List<Service> reverse = new ArrayList<>(services);
Collections.reverse(reverse);

for (Service service : reverse) {
try {
service.stop();
LOG.info("Stopped {}", service);
} catch (Exception e) {
LOG.error("Error stopping {}", service, e);
Comment on lines +516 to +524
Copy link
Member

@peterxcli peterxcli Mar 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

nit:

Suggested change
List<Service> reverse = new ArrayList<>(services);
Collections.reverse(reverse);
for (Service service : reverse) {
try {
service.stop();
LOG.info("Stopped {}", service);
} catch (Exception e) {
LOG.error("Error stopping {}", service, e);
ListIterator<Service> iterator = services.listIterator(services.size());
while (iterator.hasPrevious()) {
Service service = iterator.previous();
try {
service.stop();
LOG.info("Stopped {}", service);
} catch (Exception e) {
LOG.error("Error stopping {}", service, e);

However, this has worse readability, so please take it with a grain of salt.

Copy link
Contributor Author

@adoroszlai adoroszlai Mar 12, 2025

Choose a reason for hiding this comment

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

Thanks for the suggestion. I'll keep the original, because I don't expect many services (only Recon (HDDS-12568) and maybe S3G HA (HDDS-11679)).

This is just a precaution to allow dependent services to be added in dependency order.

}
} catch (Exception e) {
LOG.error("Exception while shutting down S3 Gateway.", e);
}
}

Expand All @@ -567,23 +549,22 @@ public MiniOzoneCluster build() throws IOException {
OzoneManager om = null;
ReconServer reconServer = null;
List<HddsDatanodeService> hddsDatanodes = Collections.emptyList();
Gateway s3g = null;
try {
scm = createAndStartSingleSCM();
om = createAndStartSingleOM();
s3g = createS3G();
reconServer = createRecon();
hddsDatanodes = createHddsDatanodes();

MiniOzoneClusterImpl cluster = new MiniOzoneClusterImpl(conf,
scmConfigurator, om, scm,
hddsDatanodes, reconServer, s3g);
hddsDatanodes, reconServer, getServices());

cluster.setCAClient(certClient);
cluster.setSecretKeyClient(secretKeyClient);
if (startDataNodes) {
cluster.startHddsDatanodes();
}
cluster.startServices();

prepareForNextBuild();
return cluster;
Expand All @@ -592,9 +573,7 @@ public MiniOzoneCluster build() throws IOException {
if (includeRecon) {
stopRecon(reconServer);
}
if (includeS3G) {
stopS3G(s3g);
}
stopServices(getServices());
if (startDataNodes) {
stopDatanodes(hddsDatanodes);
}
Expand Down Expand Up @@ -763,16 +742,6 @@ protected ReconServer createRecon() {
return reconServer;
}

protected Gateway createS3G() {
Gateway s3g = null;
if (includeS3G) {
configureS3G();
s3g = new Gateway();
s3g.execute(NO_ARGS);
}
return s3g;
}

/**
* Creates HddsDatanodeService(s) instance.
*
Expand Down Expand Up @@ -837,15 +806,5 @@ protected void configureRecon() {

ConfigurationProvider.setConfiguration(conf);
}

private void configureS3G() {
OzoneConfigurationHolder.resetConfiguration();

conf.set(S3GatewayConfigKeys.OZONE_S3G_HTTP_ADDRESS_KEY, localhostWithFreePort());
conf.set(S3GatewayConfigKeys.OZONE_S3G_HTTPS_ADDRESS_KEY, localhostWithFreePort());

OzoneConfigurationHolder.setConfiguration(conf);
}

}
}
3 changes: 2 additions & 1 deletion ...-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.apache.hadoop.ozone;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS;
import static org.apache.ozone.test.GenericTestUtils.PortAllocator.getFreePort;
Expand Down Expand Up @@ -86,7 +87,7 @@ public MiniOzoneHAClusterImpl(
List<HddsDatanodeService> hddsDatanodes,
String clusterPath,
ReconServer reconServer) {
super(conf, scmConfigurator, hddsDatanodes, reconServer);
super(conf, scmConfigurator, hddsDatanodes, reconServer, emptyList());
this.omhaService = omhaService;
this.scmhaService = scmhaService;
this.clusterMetaPath = clusterPath;
Expand Down
69 changes: 69 additions & 0 deletions hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/s3/S3GatewayService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* 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
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hadoop.ozone.s3;

import static org.apache.ozone.test.GenericTestUtils.PortAllocator.localhostWithFreePort;

import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.MiniOzoneCluster;
import org.apache.ratis.util.Preconditions;

/** S3 Gateway for {@link MiniOzoneCluster}. */
public class S3GatewayService implements MiniOzoneCluster.Service {

private static final String[] NO_ARGS = new String[0];

private Gateway s3g;

@Override
public void start(OzoneConfiguration conf) throws Exception {
Preconditions.assertNull(s3g, "S3 Gateway already started");
configureS3G(new OzoneConfiguration(conf));
s3g = new Gateway();
s3g.execute(NO_ARGS);
}

@Override
public void stop() throws Exception {
Preconditions.assertNotNull(s3g, "S3 Gateway not running");
s3g.stop();
// TODO (HDDS-11539): Remove this workaround once the @PreDestroy issue is fixed
OzoneClientCache.closeClient();
}

@Override
public String toString() {
final Gateway instance = s3g;
return instance != null
? "S3Gateway(http=" + instance.getHttpAddress() + ", https=" + instance.getHttpsAddress() + ")"
: "S3Gateway";
}

public OzoneConfiguration getConf() {
return OzoneConfigurationHolder.configuration();
}

private void configureS3G(OzoneConfiguration conf) {
OzoneConfigurationHolder.resetConfiguration();

conf.set(S3GatewayConfigKeys.OZONE_S3G_HTTP_ADDRESS_KEY, localhostWithFreePort());
conf.set(S3GatewayConfigKeys.OZONE_S3G_HTTPS_ADDRESS_KEY, localhostWithFreePort());

OzoneConfigurationHolder.setConfiguration(conf);
}
}
Loading