fapifta commented on a change in pull request #3211:
URL: https://github.com/apache/ozone/pull/3211#discussion_r833560801
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
##########
@@ -466,11 +466,11 @@
public static final String OZONE_CLIENT_TEST_OFS_BUCKET_LAYOUT_DEFAULT =
"FILE_SYSTEM_OPTIMIZED";
- public static final String OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY =
- "ozone.om.client.protocol.version";
- // The version of the protocol for Client (S3G/OFS) to OM Communication.
- // The protocol starts at 2.0.0 and a null or empty value for older versions.
- public static final String OZONE_OM_CLIENT_PROTOCOL_VERSION = "2.0.0";
+ public static final String OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY =
+ "ozone.client.required.om.version.min";
+
+ public static final String OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_DEFAULT =
+ OzoneManagerVersion.NEW_S3G_AUTH_SCHEME.name();
Review comment:
I would like to keep the semantics as it is if possible, I think this
config should not be here, and definitelly should not belong to the whole
client, as in the client we would like to ensure the forward and backward
compatibility, and we would not want to flat deny everything towards a certain
version of OM.
That change on the other hand should be in a separate JIRA, I am taking a
note now, and next week as I will have some time to work on the compat design
again, I will add this to the changes necessary to achieve the compat goals.
What do you think?
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java
##########
@@ -30,6 +30,7 @@
*/
public enum OzoneManagerVersion implements ComponentVersion {
DEFAULT_VERSION(0, "Initial version"),
+ NEW_S3G_AUTH_SCHEME(1, "New S3G auth scheme support is present in OM."),
Review comment:
You are right, this is a better name, I have changed it.
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -210,16 +210,19 @@ public RpcClient(ConfigurationSource conf, String
omServiceId)
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(isS3);
if (isS3) {
// S3 Auth works differently and needs OM version to be at 2.0.0
- String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
- if (!validateOmVersion(omVersion, serviceInfoEx.getServiceInfoList()))
{
+ OzoneManagerVersion minOmVersion = conf.getEnum(
Review comment:
I think this we should leave also as it is for now, I don't think on the
long run we would like to have flat denial for older than a specific OM
version, and this is a really special case, which I believe we should not
handle here, but in a more S3G specific manner. I have a plan on how to move
this out, but again I think that should be done in a separate JIRA.
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -210,16 +210,19 @@ public RpcClient(ConfigurationSource conf, String
omServiceId)
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(isS3);
if (isS3) {
// S3 Auth works differently and needs OM version to be at 2.0.0
- String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
- if (!validateOmVersion(omVersion, serviceInfoEx.getServiceInfoList()))
{
+ OzoneManagerVersion minOmVersion = conf.getEnum(
+ OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY,
+ OzoneManagerVersion.DEFAULT_VERSION);
+ if (!validateOmVersion(
+ minOmVersion, serviceInfoEx.getServiceInfoList())) {
if (LOG.isDebugEnabled()) {
for (ServiceInfo s : serviceInfoEx.getServiceInfoList()) {
LOG.debug("Node {} version {}", s.getHostname(),
- s.getProtobuf().getOMProtocolVersion());
+ s.getProtobuf().getOMVersion());
}
}
throw new RuntimeException("OmVersion expected "
- + omVersion
+ + minOmVersion
Review comment:
Rewrote the exception message to express that the minimum version
criteria was not met.
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -290,20 +293,26 @@ public XceiverClientFactory getXceiverClientManager() {
return xceiverClientManager;
}
- static boolean validateOmVersion(String expectedVersion,
+ static boolean validateOmVersion(OzoneManagerVersion expectedVersion,
Review comment:
done, thanks for spotting!
--
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]
