Copilot commented on code in PR #11954:
URL: https://github.com/apache/ozone/pull/11954#discussion_r4486152364


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/StorageContainerServiceProviderImpl.java:
##########
@@ -183,9 +171,63 @@ private RocksDBCheckpoint getRocksDBCheckpoint(String 
snapshotFileName, File tar
         snapshotFileName);
     reconUtils.untarCheckpointFile(targetFile, untarredDbDir);
     FileUtils.deleteQuietly(targetFile);
+    // SCM writes this flag last, so a checkpoint without it was cut short.
+    if (!ratisSnapshotComplete(untarredDbDir)) {
+      FileUtils.deleteQuietly(untarredDbDir.toFile());
+      throw new IOException("Incomplete SCM DB checkpoint: " + 
OZONE_RATIS_SNAPSHOT_COMPLETE_FLAG_NAME + " not found");
+    }
     return new RocksDBCheckpoint(untarredDbDir);
   }
 
+  /**
+   * Downloads the SCM DB checkpoint over HTTP(S), authenticated with SPNEGO 
when SCM requires it.
+   * SCM's inter-SCM gRPC endpoint is not used, because it accepts SCM 
certificates only.
+   */
+  private void downloadSCMDBCheckpoint(String url, File targetFile) throws 
IOException {
+    LOG.info("Downloading SCM DB checkpoint from {}", url);
+    URLConnectionFactory connectionFactory = 
URLConnectionFactory.newDefaultURLConnectionFactory(configuration);
+    boolean spnegoEnabled = 
"kerberos".equals(configuration.get(HDDS_SCM_HTTP_AUTH_TYPE, "simple"));
+    try {
+      SecurityUtil.doAsLoginUser(() -> {
+        try (InputStream inputStream = 
reconUtils.makeHttpCall(connectionFactory, url, spnegoEnabled)
+            .getInputStream()) {
+          FileUtils.copyInputStreamToFile(inputStream, targetFile);
+        }
+        return null;
+      });
+    } finally {
+      connectionFactory.destroy();
+    }
+  }
+
+  /**
+   * Returns the DB checkpoint URL of the SCM at {@code host}. The HTTP(S) 
port is taken from the address configured
+   * for the SCM node with that host, else from the common SCM HTTP(S) address.
+   */
+  private String getSCMDBCheckpointUrl(String host) {
+    boolean httpsEnabled = 
HttpConfig.getHttpPolicy(configuration).isHttpsEnabled();
+    SecurityConfig securityConfig = new SecurityConfig(configuration);
+    if (!httpsEnabled && securityConfig.isSecurityEnabled() && 
securityConfig.isGrpcTlsEnabled()) {
+      LOG.warn("Downloading the SCM DB unencrypted over HTTP although {} is 
true. Set {} to HTTPS_ONLY or "
+          + "HTTP_AND_HTTPS to encrypt it.", HDDS_GRPC_TLS_ENABLED, 
OZONE_HTTP_POLICY_KEY);
+    }
+    String addressKey = httpsEnabled ? OZONE_SCM_HTTPS_ADDRESS_KEY : 
OZONE_SCM_HTTP_ADDRESS_KEY;
+    List<String> addressKeys = new ArrayList<>();
+    String serviceId = HddsUtils.getScmServiceId(configuration);
+    if (serviceId != null) {
+      for (String nodeId : HddsUtils.getSCMNodeIds(configuration, serviceId)) {
+        if 
(host.equals(configuration.getTrimmed(ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY,
 serviceId, nodeId)))) {

Review Comment:
   `ozone.scm.address.<service>.<node>` accepts a `host:port` value (the SCM HA 
setup parses both forms), but this compares the raw configured value with the 
hostname extracted from the Ratis role. With a valid configuration such as 
`scm2.example.com:9894`, the comparison never matches, so the node-specific 
HTTP(S) address is skipped and the common/default port is used; Recon can then 
connect to the wrong SCM endpoint. Compare the normalized host component of the 
configured address and add a host:port regression case.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/StorageContainerServiceProviderImpl.java:
##########
@@ -183,9 +171,63 @@ private RocksDBCheckpoint getRocksDBCheckpoint(String 
snapshotFileName, File tar
         snapshotFileName);
     reconUtils.untarCheckpointFile(targetFile, untarredDbDir);
     FileUtils.deleteQuietly(targetFile);
+    // SCM writes this flag last, so a checkpoint without it was cut short.
+    if (!ratisSnapshotComplete(untarredDbDir)) {
+      FileUtils.deleteQuietly(untarredDbDir.toFile());
+      throw new IOException("Incomplete SCM DB checkpoint: " + 
OZONE_RATIS_SNAPSHOT_COMPLETE_FLAG_NAME + " not found");
+    }
     return new RocksDBCheckpoint(untarredDbDir);
   }
 
+  /**
+   * Downloads the SCM DB checkpoint over HTTP(S), authenticated with SPNEGO 
when SCM requires it.
+   * SCM's inter-SCM gRPC endpoint is not used, because it accepts SCM 
certificates only.
+   */
+  private void downloadSCMDBCheckpoint(String url, File targetFile) throws 
IOException {
+    LOG.info("Downloading SCM DB checkpoint from {}", url);
+    URLConnectionFactory connectionFactory = 
URLConnectionFactory.newDefaultURLConnectionFactory(configuration);
+    boolean spnegoEnabled = 
"kerberos".equals(configuration.get(HDDS_SCM_HTTP_AUTH_TYPE, "simple"));
+    try {
+      SecurityUtil.doAsLoginUser(() -> {
+        try (InputStream inputStream = 
reconUtils.makeHttpCall(connectionFactory, url, spnegoEnabled)
+            .getInputStream()) {
+          FileUtils.copyInputStreamToFile(inputStream, targetFile);
+        }
+        return null;
+      });
+    } finally {
+      connectionFactory.destroy();
+    }
+  }
+
+  /**
+   * Returns the DB checkpoint URL of the SCM at {@code host}. The HTTP(S) 
port is taken from the address configured
+   * for the SCM node with that host, else from the common SCM HTTP(S) address.
+   */
+  private String getSCMDBCheckpointUrl(String host) {
+    boolean httpsEnabled = 
HttpConfig.getHttpPolicy(configuration).isHttpsEnabled();
+    SecurityConfig securityConfig = new SecurityConfig(configuration);
+    if (!httpsEnabled && securityConfig.isSecurityEnabled() && 
securityConfig.isGrpcTlsEnabled()) {
+      LOG.warn("Downloading the SCM DB unencrypted over HTTP although {} is 
true. Set {} to HTTPS_ONLY or "
+          + "HTTP_AND_HTTPS to encrypt it.", HDDS_GRPC_TLS_ENABLED, 
OZONE_HTTP_POLICY_KEY);
+    }
+    String addressKey = httpsEnabled ? OZONE_SCM_HTTPS_ADDRESS_KEY : 
OZONE_SCM_HTTP_ADDRESS_KEY;
+    List<String> addressKeys = new ArrayList<>();
+    String serviceId = HddsUtils.getScmServiceId(configuration);
+    if (serviceId != null) {
+      for (String nodeId : HddsUtils.getSCMNodeIds(configuration, serviceId)) {
+        if 
(host.equals(configuration.getTrimmed(ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY,
 serviceId, nodeId)))) {
+          addressKeys.add(ConfUtils.addKeySuffixes(addressKey, serviceId, 
nodeId));

Review Comment:
   This lookup cannot reliably select the leader's per-node HTTP(S) address 
because it keys the node-specific configuration only by `role[0]` (the 
hostname). HA deployments can legitimately run multiple SCMs on the same host 
with different Ratis and HTTP ports (for example, the MiniOzone HA setup uses 
`localhost` for every node); in that case a leader on `scm2` matches the first 
`localhost` entry and Recon downloads from the wrong port. Resolve the 
node-specific address using the Ratis peer identity/port (or otherwise preserve 
the SCM node ID) rather than matching only the host, and normalize configured 
host values when matching IPv6 brackets or an optional port.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to