Skip to content

Commit

Permalink
HDDS-11878. Use CommandSpec to find top-level command. (#7575)
Browse files Browse the repository at this point in the history
  • Loading branch information
adoroszlai authored Dec 26, 2024
1 parent f34cf34 commit f125363
Show file tree
Hide file tree
Showing 22 changed files with 149 additions and 177 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ public Void call() throws Exception {
throw new MissingSubcommandException(cmd);
}

@Override
public OzoneConfiguration createOzoneConfiguration() {
OzoneConfiguration ozoneConf = new OzoneConfiguration();
if (configurationPath != null) {
Expand All @@ -119,6 +118,7 @@ public OzoneConfiguration createOzoneConfiguration() {
return ozoneConf;
}

@Override
public OzoneConfiguration getOzoneConf() {
if (conf == null) {
conf = createOzoneConfiguration();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ public interface GenericParentCommand {

boolean isVerbose();

OzoneConfiguration createOzoneConfiguration();
/** Returns a cached configuration, i.e. it is created only once, subsequent calls return the same instance. */
OzoneConfiguration getOzoneConf();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.hadoop.hdds.cli;

import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import picocli.CommandLine;

import static picocli.CommandLine.Spec.Target.MIXEE;

/** Base functionality for all Ozone CLI mixins. */
@CommandLine.Command
public abstract class AbstractMixin {

@CommandLine.Spec(MIXEE)
private CommandLine.Model.CommandSpec spec;

protected CommandLine.Model.CommandSpec spec() {
return spec;
}

protected GenericParentCommand rootCommand() {
return AbstractSubcommand.findRootCommand(spec);
}

protected OzoneConfiguration getOzoneConf() {
return rootCommand().getOzoneConf();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.hadoop.hdds.cli;

import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.ratis.util.MemoizedSupplier;
import picocli.CommandLine;

import java.util.function.Supplier;

/** Base functionality for all Ozone subcommands. */
@CommandLine.Command(
mixinStandardHelpOptions = true,
versionProvider = HddsVersionProvider.class
)
public abstract class AbstractSubcommand {

@CommandLine.Spec
private CommandLine.Model.CommandSpec spec;

private final Supplier<GenericParentCommand> rootSupplier =
MemoizedSupplier.valueOf(() -> findRootCommand(spec));

protected CommandLine.Model.CommandSpec spec() {
return spec;
}

/** Get the Ozone object annotated with {@link CommandLine.Command}) that was used to run this command.
* Usually this is some subclass of {@link GenericCli}, but in unit tests it could be any subcommand. */
protected GenericParentCommand rootCommand() {
return rootSupplier.get();
}

protected boolean isVerbose() {
return rootCommand().isVerbose();
}

/** @see GenericParentCommand#getOzoneConf() */
protected OzoneConfiguration getOzoneConf() {
return rootCommand().getOzoneConf();
}

static GenericParentCommand findRootCommand(CommandLine.Model.CommandSpec spec) {
Object root = spec.root().userObject();
return root instanceof GenericParentCommand
? (GenericParentCommand) root
: new NoParentCommand();
}

/** No-op implementation for unit tests, which may bypass creation of GenericCli object. */
private static class NoParentCommand implements GenericParentCommand {

private final OzoneConfiguration conf = new OzoneConfiguration();

@Override
public boolean isVerbose() {
return false;
}

@Override
public OzoneConfiguration getOzoneConf() {
return conf;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.hdds.HddsUtils;
import org.apache.hadoop.hdds.cli.GenericParentCommand;
import org.apache.hadoop.hdds.cli.AbstractMixin;
import org.apache.hadoop.hdds.conf.ConfigurationException;
import org.apache.hadoop.hdds.conf.MutableConfigurationSource;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
Expand All @@ -33,15 +33,11 @@

import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY;
import static org.apache.hadoop.hdds.utils.HddsServerUtil.getScmSecurityClient;
import static picocli.CommandLine.Spec.Target.MIXEE;

/**
* Defines command-line option for SCM address.
*/
public class ScmOption {

@CommandLine.Spec(MIXEE)
private CommandLine.Model.CommandSpec spec;
public class ScmOption extends AbstractMixin {

@CommandLine.Option(names = {"--scm"},
description = "The destination scm (host:port)")
Expand All @@ -53,9 +49,7 @@ public class ScmOption {
private String scmServiceId;

public ScmClient createScmClient() throws IOException {
GenericParentCommand parent = (GenericParentCommand)
spec.root().userObject();
OzoneConfiguration conf = parent.createOzoneConfiguration();
OzoneConfiguration conf = getOzoneConf();
checkAndSetSCMAddressArg(conf);

return new ContainerOperationClient(conf);
Expand Down Expand Up @@ -91,13 +85,10 @@ private void checkAndSetSCMAddressArg(MutableConfigurationSource conf) {

public SCMSecurityProtocol createScmSecurityClient() {
try {
GenericParentCommand parent = (GenericParentCommand)
spec.root().userObject();
return getScmSecurityClient(parent.createOzoneConfiguration());
return getScmSecurityClient(getOzoneConf());
} catch (IOException ex) {
throw new IllegalArgumentException(
"Can't create SCM Security client", ex);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package org.apache.hadoop.hdds.scm.cli;

import org.apache.hadoop.hdds.cli.AbstractSubcommand;
import org.apache.hadoop.hdds.scm.client.ScmClient;
import picocli.CommandLine;

Expand All @@ -26,7 +27,7 @@
/**
* Base class for admin commands that connect via SCM client.
*/
public abstract class ScmSubcommand implements Callable<Void> {
public abstract class ScmSubcommand extends AbstractSubcommand implements Callable<Void> {

@CommandLine.Mixin
private ScmOption scmOption;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@
import org.apache.hadoop.hdds.cli.AdminSubcommand;
import org.apache.hadoop.hdds.cli.GenericCli;
import org.apache.hadoop.hdds.cli.HddsVersionProvider;
import org.apache.hadoop.hdds.cli.OzoneAdmin;

import org.kohsuke.MetaInfServices;
import picocli.CommandLine.Command;
import picocli.CommandLine.ParentCommand;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Spec;

Expand All @@ -52,16 +50,9 @@ public class ContainerCommands implements Callable<Void>, AdminSubcommand {
@Spec
private CommandSpec spec;

@ParentCommand
private OzoneAdmin parent;

@Override
public Void call() throws Exception {
GenericCli.missingSubcommand(spec);
return null;
}

public OzoneAdmin getParent() {
return parent;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import java.util.Scanner;
import java.util.stream.Collectors;

import org.apache.hadoop.hdds.cli.GenericParentCommand;
import org.apache.hadoop.hdds.cli.HddsVersionProvider;
import org.apache.hadoop.hdds.client.ReplicationConfig;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
Expand All @@ -47,9 +46,7 @@
import org.apache.hadoop.hdds.server.JsonUtils;
import picocli.CommandLine;
import picocli.CommandLine.Command;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Parameters;
import picocli.CommandLine.Spec;

/**
* This is the handler that process container info command.
Expand All @@ -61,9 +58,6 @@
versionProvider = HddsVersionProvider.class)
public class InfoSubcommand extends ScmSubcommand {

@Spec
private CommandSpec spec;

@CommandLine.Option(names = { "--json" },
defaultValue = "false",
description = "Format output as JSON")
Expand Down Expand Up @@ -181,10 +175,7 @@ private void printDetails(ScmClient scmClient, long containerID,
} else {
// Print container report info.
System.out.printf("Container id: %s%n", containerID);
boolean verbose = spec != null
&& spec.root().userObject() instanceof GenericParentCommand
&& ((GenericParentCommand) spec.root().userObject()).isVerbose();
if (verbose) {
if (isVerbose()) {
System.out.printf("Pipeline Info: %s%n", container.getPipeline());
} else {
System.out.printf("Pipeline id: %s%n", container.getPipeline().getId().getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import picocli.CommandLine.Command;
import picocli.CommandLine.ParentCommand;
import picocli.CommandLine.Help.Visibility;
import picocli.CommandLine.Option;

Expand Down Expand Up @@ -82,9 +81,6 @@ public class ListSubcommand extends ScmSubcommand {

private static final ObjectWriter WRITER;

@ParentCommand
private ContainerCommands parent;

static {
ObjectMapper mapper = new ObjectMapper()
.registerModule(new JavaTimeModule())
Expand Down Expand Up @@ -116,7 +112,7 @@ public void execute(ScmClient scmClient) throws IOException {
replication, new OzoneConfiguration());
}

int maxCountAllowed = parent.getParent().getOzoneConf()
int maxCountAllowed = getOzoneConf()
.getInt(ScmConfigKeys.OZONE_SCM_CONTAINER_LIST_MAX_COUNT,
ScmConfigKeys.OZONE_SCM_CONTAINER_LIST_MAX_COUNT_DEFAULT);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ public class NSSummaryAdmin implements AdminSubcommand {
@CommandLine.ParentCommand
private OzoneAdmin parent;

public OzoneAdmin getParent() {
return parent;
}

private boolean isObjectStoreBucket(OzoneBucket bucket, ObjectStore objectStore) {
boolean enableFileSystemPaths = getOzoneConfig()
.getBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package org.apache.hadoop.ozone.debug.container;

import org.apache.hadoop.hdds.cli.AbstractSubcommand;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.container.common.impl.ContainerData;
import org.apache.hadoop.ozone.container.common.interfaces.Container;
Expand All @@ -39,14 +40,14 @@
name = "inspect",
description
= "Check the metadata of all container replicas on this datanode.")
public class InspectSubcommand implements Callable<Void> {
public class InspectSubcommand extends AbstractSubcommand implements Callable<Void> {

@CommandLine.ParentCommand
private ContainerCommands parent;

@Override
public Void call() throws IOException {
final OzoneConfiguration conf = parent.getOzoneConf();
final OzoneConfiguration conf = getOzoneConf();
parent.loadContainersFromVolumes();

final KeyValueContainerMetadataInspector inspector
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,4 @@ private Collection<String> getConfiguredServiceIds() {
public UserGroupInformation getUser() throws IOException {
return UserGroupInformation.getCurrentUser();
}

protected OzoneRepair getParent() {
return parent;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,4 @@ public Void call() throws Exception {
System.out.println(ozoneManagerClient.getQuotaRepairStatus());
return null;
}

protected QuotaRepair getParent() {
return parent;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,4 @@ public Void call() throws Exception {
}
return null;
}

protected QuotaRepair getParent() {
return parent;
}

}
Loading

0 comments on commit f125363

Please sign in to comment.