HDDS-15197. Remove unnecessary BelongsToLayoutVersion annotations.

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java

@@ -17,9 +17,6 @@
 
 package org.apache.hadoop.hdds.protocol;
 
-import static org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature.HADOOP_PRC_PORTS_IN_DATANODEDETAILS;
-import static org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature.RATIS_DATASTREAM_PORT_IN_DATANODEDETAILS;
-import static org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature.WEBUI_PORTS_IN_DATANODEDETAILS;
 import static org.apache.hadoop.ozone.ClientVersion.VERSION_HANDLES_UNKNOWN_DN_PORTS;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
@@ -46,7 +43,6 @@
 import org.apache.hadoop.hdds.scm.net.NetConstants;
 import org.apache.hadoop.hdds.scm.net.NetUtils;
 import org.apache.hadoop.hdds.scm.net.NodeImpl;
-import org.apache.hadoop.hdds.upgrade.BelongsToHDDSLayoutVersion;
 import org.apache.hadoop.hdds.utils.db.Codec;
 import org.apache.hadoop.hdds.utils.db.DelegatedCodec;
 import org.apache.hadoop.hdds.utils.db.Proto2Codec;
@@ -1031,13 +1027,9 @@ public static final class Port {
      */
     public enum Name {
       STANDALONE, RATIS, REST, REPLICATION, RATIS_ADMIN, RATIS_SERVER,
-      @BelongsToHDDSLayoutVersion(RATIS_DATASTREAM_PORT_IN_DATANODEDETAILS)
       RATIS_DATASTREAM,
-      @BelongsToHDDSLayoutVersion(WEBUI_PORTS_IN_DATANODEDETAILS)
       HTTP,
-      @BelongsToHDDSLayoutVersion(WEBUI_PORTS_IN_DATANODEDETAILS)
       HTTPS,
-      @BelongsToHDDSLayoutVersion(HADOOP_PRC_PORTS_IN_DATANODEDETAILS)
       CLIENT_RPC;
 
       public static final Set<Name> ALL_PORTS = ImmutableSet.copyOf(

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/DatanodeIdYaml.java

@@ -20,21 +20,15 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.lang.reflect.Field;
 import java.nio.file.Files;
-import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.UUID;
-import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.collections4.MapUtils;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
 import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
 import org.apache.hadoop.hdds.server.YamlUtils;
-import org.apache.hadoop.hdds.upgrade.BelongsToHDDSLayoutVersion;
-import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature;
-import org.apache.hadoop.ozone.container.common.DatanodeLayoutStorage;
+import org.apache.hadoop.ozone.container.upgrade.VersionedDatanodeFeatures;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.yaml.snakeyaml.DumperOptions;
@@ -241,34 +235,6 @@ private static DatanodeDetailsYaml getDatanodeDetailsYaml(
       DatanodeDetails datanodeDetails, ConfigurationSource conf)
       throws IOException {
 
-    DatanodeLayoutStorage datanodeLayoutStorage
-        = new DatanodeLayoutStorage(conf, datanodeDetails.getUuidString());
-
-    Map<String, Integer> portDetails = new LinkedHashMap<>();
-    final List<DatanodeDetails.Port> ports = datanodeDetails.getPorts();
-    if (!CollectionUtils.isEmpty(ports)) {
-      for (DatanodeDetails.Port port : ports) {
-        Field f = null;
-        try {
-          f = DatanodeDetails.Port.Name.class
-              .getDeclaredField(port.getName().name());
-        } catch (NoSuchFieldException e) {
-          LOG.error("There is no such field as {} in {}", port.getName().name(),
-              DatanodeDetails.Port.Name.class);
-        }
-        if (f != null
-            && f.isAnnotationPresent(BelongsToHDDSLayoutVersion.class)) {
-          HDDSLayoutFeature layoutFeature
-              = f.getAnnotation(BelongsToHDDSLayoutVersion.class).value();
-          if (layoutFeature.layoutVersion() >
-              datanodeLayoutStorage.getApparentVersion()) {
-            continue;
-          }
-        }
-        portDetails.put(port.getName().toString(), port.getValue());
-      }
-    }
-
     String persistedOpString = null;
     if (datanodeDetails.getPersistedOpState() != null) {
       persistedOpString = datanodeDetails.getPersistedOpState().name();
@@ -281,7 +247,7 @@ private static DatanodeDetailsYaml getDatanodeDetailsYaml(
         datanodeDetails.getCertSerialId(),
         persistedOpString,
         datanodeDetails.getPersistedOpStateExpiryEpochSec(),
-        portDetails,
+        VersionedDatanodeFeatures.DatanodePorts.getPortsToPersist(datanodeDetails, conf),
         datanodeDetails.getInitialVersion(),
         datanodeDetails.getCurrentVersion());
   }

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/VersionedDatanodeFeatures.java

@@ -19,10 +19,17 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.commons.collections4.CollectionUtils;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature;
 import org.apache.hadoop.hdds.upgrade.HDDSLayoutVersionManager;
 import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.container.common.DatanodeLayoutStorage;
 import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration;
 import org.apache.hadoop.ozone.container.common.volume.StorageVolume;
 
@@ -148,4 +155,51 @@ public static boolean isFinalizedAndEnabled(ConfigurationSource conf) {
       return false;
     }
   }
+
+  /**
+   * Methods to handle persisting ports to be compatible with older Datanode versions that would fail on unknown port
+   * fields.
+   */
+  public static class DatanodePorts {
+    /**
+     * Version that must be present before persisting each port name to disk.
+     * Ports omitted from this map are always persisted.
+     */
+    private static final Map<DatanodeDetails.Port.Name, HDDSLayoutFeature> PORT_TO_VERSION = new HashMap<>();
+
+    static {
+      PORT_TO_VERSION.put(DatanodeDetails.Port.Name.RATIS_DATASTREAM,
+          HDDSLayoutFeature.RATIS_DATASTREAM_PORT_IN_DATANODEDETAILS);
+      PORT_TO_VERSION.put(
+          DatanodeDetails.Port.Name.HTTP,
+          HDDSLayoutFeature.WEBUI_PORTS_IN_DATANODEDETAILS);
+      PORT_TO_VERSION.put(
+          DatanodeDetails.Port.Name.HTTPS,
+          HDDSLayoutFeature.WEBUI_PORTS_IN_DATANODEDETAILS);
+      PORT_TO_VERSION.put(
+          DatanodeDetails.Port.Name.CLIENT_RPC,
+          HDDSLayoutFeature.HADOOP_PRC_PORTS_IN_DATANODEDETAILS);
+    }
+
+    private static boolean shouldPersistPort(DatanodeDetails.Port port, DatanodeLayoutStorage datanodeLayoutStorage) {
+      HDDSLayoutFeature portVersion = PORT_TO_VERSION.get(port.getName());
+      return portVersion == null || portVersion.isSupportedBy(datanodeLayoutStorage.getApparentVersion());
+    }
+
+    public static Map<String, Integer> getPortsToPersist(DatanodeDetails datanodeDetails, ConfigurationSource conf)
+        throws IOException {
+      DatanodeLayoutStorage datanodeLayoutStorage = new DatanodeLayoutStorage(conf, datanodeDetails.getUuidString());
+      Map<String, Integer> portDetails = new LinkedHashMap<>();
+
+      final List<DatanodeDetails.Port> ports = datanodeDetails.getPorts();
+      if (!CollectionUtils.isEmpty(ports)) {
+        for (DatanodeDetails.Port port : ports) {
+          if (shouldPersistPort(port, datanodeLayoutStorage)) {
+            portDetails.put(port.getName().toString(), port.getValue());
+          }
+        }
+      }
+      return portDetails;
+    }
+  }
 }

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeatureAspect.java

@@ -28,7 +28,6 @@
 import org.aspectj.lang.JoinPoint;
 import org.aspectj.lang.annotation.Aspect;
 import org.aspectj.lang.annotation.Before;
-import org.aspectj.lang.annotation.Pointcut;
 import org.aspectj.lang.reflect.MethodSignature;
 
 /**
@@ -78,30 +77,6 @@ private void checkIsAllowed(String operationName,
     }
   }
 
-  @Pointcut("execution(* " +
-      "org.apache.hadoop.ozone.om.request.OMClientRequest+.preExecute(..)) " +
-      "&& @this(org.apache.hadoop.ozone.om.upgrade.BelongsToLayoutVersion)")
-  public void omRequestPointCut() {
-  }
-
-  @Before("omRequestPointCut()")
-  public void beforeRequestApplyTxn(final JoinPoint joinPoint)
-      throws OMException {
-
-    BelongsToLayoutVersion annotation = joinPoint.getTarget().getClass()
-        .getAnnotation(BelongsToLayoutVersion.class);
-    if (annotation == null) {
-      return;
-    }
-
-    Object[] args = joinPoint.getArgs();
-    OzoneManager om = (OzoneManager) args[0];
-
-    LayoutFeature lf = annotation.value();
-    checkIsAllowed(joinPoint.getTarget().getClass().getSimpleName(),
-        om.getVersionManager(), lf);
-  }
-
   /**
    * Note: Without this, it occasionally throws NoSuchMethodError when running
    * the test.

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestOMLayoutFeatureAspect.java

@@ -86,25 +86,4 @@ public void testDisallowedUntilLayoutVersion() throws Throwable {
     assertThat(omException.getMessage())
         .contains("cannot be invoked before finalization");
   }
-
-  @Test
-  public void testPreExecuteLayoutCheck() {
-
-    OzoneManager om = mock(OzoneManager.class);
-    OMVersionManager ovm = mock(OMVersionManager.class);
-    when(ovm.isAllowed(any(ComponentVersion.class))).thenReturn(false);
-    when(om.getVersionManager()).thenReturn(ovm);
-
-    MockOmRequest mockOmRequest = new MockOmRequest();
-    OMLayoutFeatureAspect aspect = new OMLayoutFeatureAspect();
-
-    JoinPoint joinPoint = mock(JoinPoint.class);
-    when(joinPoint.getArgs()).thenReturn(new Object[]{om});
-    when(joinPoint.getTarget()).thenReturn(mockOmRequest);
-
-    OMException omException = assertThrows(OMException.class,
-        () -> aspect.beforeRequestApplyTxn(joinPoint));
-    assertThat(omException.getMessage())
-        .contains("cannot be invoked before finalization");
-  }
 }