Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.protobuf.ByteString;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.hdds.DatanodeVersion;
import org.apache.hadoop.hdds.HddsUtils;
import org.apache.hadoop.hdds.annotation.InterfaceAudience;
Expand Down Expand Up @@ -175,13 +178,41 @@ public void setIpAddress(String ip) {
this.ipAddress = StringWithByteString.valueOf(ip);
}

/**
* Resolves and validates the IP address of the datanode based on its hostname.
* If the resolved IP address differs from the current IP address,
* it updates the IP address to the newly resolved value.
*/
public void validateDatanodeIpAddress() {
final String oldIP = getIpAddress();
Copy link
Contributor

Choose a reason for hiding this comment

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

NullPointerException: Cannot invoke "org.apache.hadoop.ozone.util.StringWithByteString.getString()" because "this.ipAddress" is null

Sorry, it looks like we need to handle ipAddress == null case in getIpAddress() and getHostName().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

final String hostname = getHostName();
if (StringUtils.isBlank(hostname)) {
LOG.warn("Could not resolve IP address of datanode '{}'", this);
return;
}

try {
final String newIP = InetAddress.getByName(hostname).getHostAddress();
if (StringUtils.isBlank(newIP)) {
throw new UnknownHostException("New IP address is invalid: " + newIP);
}

if (!newIP.equals(oldIP)) {
LOG.info("Updating IP address of datanode {} to {}", this, newIP);
Copy link
Contributor

@jojochuang jojochuang Apr 1, 2025

Choose a reason for hiding this comment

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

this.toString() format is "UUID(hostname/old_ip_address)" so this log would become

Updating IP address of datanode UUID(hostname/old_ip_address) to new_ip_address

It may look confusing initially but okay.

setIpAddress(newIP);
}
} catch (UnknownHostException e) {
LOG.warn("Could not resolve IP address of datanode '{}'", this, e);
}
}

/**
* Returns IP address of DataNode.
*
* @return IP address
*/
public String getIpAddress() {
return ipAddress.getString();
return ipAddress == null ? null : ipAddress.getString();
}

/**
Expand All @@ -208,7 +239,7 @@ public void setHostName(String host) {
* @return Hostname
*/
public String getHostName() {
return hostName.getString();
return hostName == null ? null : hostName.getString();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import com.google.common.base.Preconditions;
import java.io.File;
import java.io.IOException;
import java.net.InetAddress;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -225,11 +224,10 @@ public String getNamespace() {
HddsServerUtil.initializeMetrics(conf, "HddsDatanode");
try {
String hostname = HddsUtils.getHostName(conf);
String ip = InetAddress.getByName(hostname).getHostAddress();
datanodeDetails = initializeDatanodeDetails();
datanodeDetails.setHostName(hostname);
serviceRuntimeInfo.setHostName(hostname);
datanodeDetails.setIpAddress(ip);
datanodeDetails.validateDatanodeIpAddress();
datanodeDetails.setVersion(
HddsVersionInfo.HDDS_VERSION_INFO.getVersion());
datanodeDetails.setSetupTime(Time.now());
Expand All @@ -238,7 +236,7 @@ public String getNamespace() {
TracingUtil.initTracing(
"HddsDatanodeService." + datanodeDetails.getUuidString()
.substring(0, 8), conf);
LOG.info("HddsDatanodeService host:{} ip:{}", hostname, ip);
LOG.info("HddsDatanodeService {}", datanodeDetails);
// Authenticate Hdds Datanode service if security is enabled
if (OzoneSecurityUtil.isSecurityEnabled(conf)) {
component = "dn-" + datanodeDetails.getUuidString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ public static synchronized void writeDatanodeDetailsTo(

/**
* Read {@link DatanodeDetails} from a local ID file.
* Use {@link DatanodeDetails#validateDatanodeIpAddress()} to ensure that the IP address matches with the hostname
*
* @param path ID file local path
* @return {@link DatanodeDetails}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,14 @@
import static org.apache.hadoop.ozone.container.ContainerTestHelper.getDummyCommandRequestProto;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.when;

import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.net.InetAddress;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
Expand All @@ -45,6 +49,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.MockedStatic;

/**
* Test for {@link ContainerUtils}.
Expand Down Expand Up @@ -91,34 +96,50 @@ public void testTarName() throws IOException {
public void testDatanodeIDPersistent(@TempDir File tempDir) throws Exception {
// Generate IDs for testing
DatanodeDetails id1 = randomDatanodeDetails();
id1.setPort(DatanodeDetails.newStandalonePort(1));
assertWriteRead(tempDir, id1);

// Add certificate serial id.
id1.setCertSerialId(String.valueOf(RandomUtils.secure().randomLong()));
assertWriteRead(tempDir, id1);

// Read should return an empty value if file doesn't exist
File nonExistFile = new File(tempDir, "non_exist.id");
assertThrows(IOException.class,
() -> ContainerUtils.readDatanodeDetailsFrom(nonExistFile));

// Read should fail if the file is malformed
File malformedFile = new File(tempDir, "malformed.id");
createMalformedIDFile(malformedFile);
assertThrows(IOException.class,
() -> ContainerUtils.readDatanodeDetailsFrom(malformedFile));

// Test upgrade scenario - protobuf file instead of yaml
File protoFile = new File(tempDir, "valid-proto.id");
try (OutputStream out = Files.newOutputStream(protoFile.toPath())) {
HddsProtos.DatanodeDetailsProto proto = id1.getProtoBufMessage();
proto.writeTo(out);
try (MockedStatic<InetAddress> mockedStaticInetAddress = mockStatic(InetAddress.class)) {
InetAddress mockedInetAddress = mock(InetAddress.class);
mockedStaticInetAddress.when(() -> InetAddress.getByName(id1.getHostName()))
.thenReturn(mockedInetAddress);

// If persisted ip address is different from resolved ip address,
// DatanodeDetails should return the persisted ip address.
// Upon validation of the ip address, DatanodeDetails should return the resolved ip address.
when(mockedInetAddress.getHostAddress())
.thenReturn("127.0.0.1");
assertWriteReadWithChangedIpAddress(tempDir, id1);

when(mockedInetAddress.getHostAddress())
.thenReturn(id1.getIpAddress());

id1.setPort(DatanodeDetails.newStandalonePort(1));
assertWriteRead(tempDir, id1);

// Add certificate serial id.
id1.setCertSerialId(String.valueOf(RandomUtils.secure().randomLong()));
assertWriteRead(tempDir, id1);

// Read should return an empty value if file doesn't exist
File nonExistFile = new File(tempDir, "non_exist.id");
assertThrows(IOException.class,
() -> ContainerUtils.readDatanodeDetailsFrom(nonExistFile));

// Read should fail if the file is malformed
File malformedFile = new File(tempDir, "malformed.id");
createMalformedIDFile(malformedFile);
assertThrows(IOException.class,
() -> ContainerUtils.readDatanodeDetailsFrom(malformedFile));

// Test upgrade scenario - protobuf file instead of yaml
File protoFile = new File(tempDir, "valid-proto.id");
try (OutputStream out = Files.newOutputStream(protoFile.toPath())) {
HddsProtos.DatanodeDetailsProto proto = id1.getProtoBufMessage();
proto.writeTo(out);
}
assertDetailsEquals(id1, ContainerUtils.readDatanodeDetailsFrom(protoFile));

id1.setInitialVersion(1);
assertWriteRead(tempDir, id1);
}
assertDetailsEquals(id1, ContainerUtils.readDatanodeDetailsFrom(protoFile));

id1.setInitialVersion(1);
assertWriteRead(tempDir, id1);
}

private void assertWriteRead(@TempDir File tempDir,
Expand All @@ -133,6 +154,17 @@ private void assertWriteRead(@TempDir File tempDir,
assertEquals(details.getCurrentVersion(), read.getCurrentVersion());
}

private void assertWriteReadWithChangedIpAddress(@TempDir File tempDir,
DatanodeDetails details) throws IOException {
// Write a single ID to the file and read it out
File file = new File(tempDir, "valid-values.id");
ContainerUtils.writeDatanodeDetailsTo(details, file, conf);
DatanodeDetails read = ContainerUtils.readDatanodeDetailsFrom(file);
assertEquals(details.getIpAddress(), read.getIpAddress());
read.validateDatanodeIpAddress();
assertEquals("127.0.0.1", read.getIpAddress());
}

private void createMalformedIDFile(File malformedFile)
throws IOException {
DatanodeDetails id = randomDatanodeDetails();
Expand All @@ -149,5 +181,6 @@ private static void assertDetailsEquals(DatanodeDetails expected,
assertEquals(expected.getCertSerialId(), actual.getCertSerialId());
assertEquals(expected.getProtoBufMessage(), actual.getProtoBufMessage());
assertEquals(expected.getInitialVersion(), actual.getInitialVersion());
assertEquals(expected.getIpAddress(), actual.getIpAddress());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ x-om:
services:
dn1:
<<: *datanode
hostname: dn1
networks:
net:
ipv4_address: 10.9.0.11
Expand All @@ -57,6 +58,7 @@ services:
- ../..:${OZONE_DIR}
dn2:
<<: *datanode
hostname: dn2
networks:
net:
ipv4_address: 10.9.0.12
Expand All @@ -65,6 +67,7 @@ services:
- ../..:${OZONE_DIR}
dn3:
<<: *datanode
hostname: dn3
networks:
net:
ipv4_address: 10.9.0.13
Expand Down