fapifta commented on code in PR #6981:
URL: https://github.com/apache/ozone/pull/6981#discussion_r1731270510
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java:
##########
@@ -118,13 +118,13 @@ public class ECReconstructionCoordinator implements
Closeable {
private final OzoneClientConfig ozoneClientConfig;
public ECReconstructionCoordinator(
- ConfigurationSource conf, CertificateClient certificateClient,
+ ConfigurationSource conf, CertificateClient certClient,
Review Comment:
Can we avoid this parameter rename please?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java:
##########
@@ -1610,9 +1610,9 @@ private void persistSCMCertificates() throws IOException {
// TODO: see if we can avoid doing this during every restart.
if (primaryScmNodeId != null && !primaryScmNodeId.equals(
scmStorageConfig.getScmId())) {
+ getScmSecurityClientWithMaxRetry(configuration, getCurrentUser());
Review Comment:
I think we don't need this line.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java:
##########
@@ -373,76 +372,9 @@ public static List<String> getExistingSstFiles(File db)
throws IOException {
return sstList;
}
- /**
- * Build CA list which need to be passed to client.
- *
- * If certificate client is null, obtain the list of CA using SCM security
- * client, else it uses certificate client.
- * @return list of CA
- */
- public static List<String> buildCAList(CertificateClient certClient,
- ConfigurationSource configuration) throws IOException {
- long waitDuration =
- configuration.getTimeDuration(OZONE_SCM_CA_LIST_RETRY_INTERVAL,
- OZONE_SCM_CA_LIST_RETRY_INTERVAL_DEFAULT, TimeUnit.SECONDS);
- if (certClient != null) {
- if (!SCMHAUtils.isSCMHAEnabled(configuration)) {
- return generateCAList(certClient);
- } else {
- Collection<String> scmNodes = SCMHAUtils.getSCMNodeIds(configuration);
- int expectedCount = scmNodes.size() + 1;
- if (scmNodes.size() > 1) {
- // First check if cert client has ca list initialized.
- // This is being done, when this method is called multiple times we
- // don't make call to SCM, we return from in-memory.
- List<String> caCertPemList = certClient.getCAList();
- if (caCertPemList != null && caCertPemList.size() == expectedCount) {
- return caCertPemList;
- }
- return getCAListWithRetry(() ->
- waitForCACerts(certClient::updateCAList, expectedCount),
- waitDuration);
- } else {
- return generateCAList(certClient);
- }
- }
- } else {
- SCMSecurityProtocolClientSideTranslatorPB scmSecurityProtocolClient =
- HddsServerUtil.getScmSecurityClient(configuration);
- if (!SCMHAUtils.isSCMHAEnabled(configuration)) {
- List<String> caCertPemList = new ArrayList<>();
- SCMGetCertResponseProto scmGetCertResponseProto =
- scmSecurityProtocolClient.getCACert();
- if (scmGetCertResponseProto.hasX509Certificate()) {
- caCertPemList.add(scmGetCertResponseProto.getX509Certificate());
- }
- if (scmGetCertResponseProto.hasX509RootCACertificate()) {
-
caCertPemList.add(scmGetCertResponseProto.getX509RootCACertificate());
- }
- return caCertPemList;
- } else {
- Collection<String> scmNodes = SCMHAUtils.getSCMNodeIds(configuration);
- int expectedCount = scmNodes.size() + 1;
- if (scmNodes.size() > 1) {
- return getCAListWithRetry(() -> waitForCACerts(
- scmSecurityProtocolClient::listCACertificate,
- expectedCount), waitDuration);
- } else {
- return scmSecurityProtocolClient.listCACertificate();
- }
- }
- }
- }
-
- private static List<String> generateCAList(CertificateClient certClient)
- throws IOException {
- List<String> caCertPemList = new ArrayList<>();
- for (X509Certificate cert : certClient.getAllRootCaCerts()) {
- caCertPemList.add(CertificateCodec.getPEMEncodedString(cert));
- }
- for (X509Certificate cert : certClient.getAllCaCerts()) {
- caCertPemList.add(CertificateCodec.getPEMEncodedString(cert));
- }
+ private static List<X509Certificate> generateCAList(CertificateClient
certClient) {
Review Comment:
As I see this method is not used anymore, can you please remove it?
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java:
##########
Review Comment:
I don't think we need to change indentations here, at least we should not do
this as part of this 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: [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]
