-
Notifications
You must be signed in to change notification settings - Fork 615
HDDS-15639. Adjust Upgrade status command to call OM intead of SCM #10579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: HDDS-14496-zdu
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,32 +17,54 @@ | |
|
|
||
| package org.apache.hadoop.ozone.admin.upgrade; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.concurrent.Callable; | ||
| import org.apache.hadoop.hdds.cli.AbstractSubcommand; | ||
| import org.apache.hadoop.hdds.cli.HddsVersionProvider; | ||
| import org.apache.hadoop.hdds.protocol.proto.HddsProtos; | ||
| import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; | ||
| import org.apache.hadoop.hdds.scm.client.ScmClient; | ||
| import org.apache.hadoop.ozone.OzoneManagerVersion; | ||
| import org.apache.hadoop.ozone.admin.om.OmAddressOptions; | ||
| import org.apache.hadoop.ozone.client.rpc.RpcClient; | ||
| import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol; | ||
| import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; | ||
| import picocli.CommandLine; | ||
|
|
||
| /** | ||
| * Sub command to query the overall upgrade status of the cluster, returning information about the finalization | ||
| * status of SCM, the datanodes and OM. | ||
| * status of SCM, the datanodes and OM. The command makes a single call to OM which returns the status of the other | ||
| * components as well as itself. | ||
| */ | ||
| @CommandLine.Command( | ||
| name = "status", | ||
| description = "Show status of the cluster upgrade", | ||
| mixinStandardHelpOptions = true, | ||
| versionProvider = HddsVersionProvider.class) | ||
| public class StatusSubCommand extends ScmSubcommand { | ||
| public class StatusSubCommand extends AbstractSubcommand implements Callable<Integer> { | ||
|
|
||
| @CommandLine.Mixin | ||
| private OmAddressOptions.OptionalServiceIdOrHostMixin omAddressOptions; | ||
|
|
||
| @Override | ||
| public void execute(ScmClient client) throws IOException { | ||
| HddsProtos.UpgradeStatus status = client.queryUpgradeStatus(); | ||
|
|
||
| out().println("Upgrade status:"); | ||
| out().println(" SCM Finalized: " + status.getScmFinalized()); | ||
| out().println(" Datanodes finalized: " + status.getNumDatanodesFinalized()); | ||
| out().println(" Total Datanodes: " + status.getNumDatanodesTotal()); | ||
| out().println(" Should Finalize: " + status.getShouldFinalize()); | ||
| public Integer call() throws Exception { | ||
| try (OzoneManagerProtocol client = getClient()) { | ||
| OzoneManagerVersion omVersion = RpcClient.getOmVersion(client.getServiceInfo()); | ||
| if (!OzoneManagerVersion.ZDU.isSupportedBy(omVersion)) { | ||
| err().println("OM does not support ZDU. The cluster upgrade status should be queried with the pre ZDU " + | ||
| "commands, eg `ozone admin scm finalizationstatus` and `ozone admin om finalizationstatus`"); | ||
| return 1; | ||
| } | ||
| OzoneManagerProtocolProtos.QueryUpgradeStatusResponse status = client.queryUpgradeStatus(); | ||
|
|
||
| out().println("Upgrade status:"); | ||
| out().println(" OM Finalized: " + status.getOmFinalized()); | ||
| out().println(" SCM Finalized: " + status.getHddsStatus().getScmFinalized()); | ||
| out().println(" Datanodes finalized: " + status.getHddsStatus().getNumDatanodesFinalized()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it's worth to phrase it differently, as the first two line are true/false and this is the number of finalized datanodes, right?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, maybe just using a question mark for the boolean and colon for the count? There will be a json option added next. |
||
| out().println(" Total Datanodes: " + status.getHddsStatus().getNumDatanodesTotal()); | ||
| out().println(" Should Finalize: " + status.getHddsStatus().getShouldFinalize()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just an indicator to OM from SCM. I don't think we should display it to the user. Maybe if we add a |
||
| } | ||
| return 0; | ||
| } | ||
|
|
||
| protected OzoneManagerProtocol getClient() throws Exception { | ||
| return omAddressOptions.newClient(); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -17,18 +17,25 @@ | |||||
|
|
||||||
| package org.apache.hadoop.ozone.admin.upgrade; | ||||||
|
|
||||||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||||||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||||||
| import static org.mockito.Mockito.mock; | ||||||
| import static org.mockito.Mockito.never; | ||||||
| import static org.mockito.Mockito.verify; | ||||||
| import static org.mockito.Mockito.when; | ||||||
|
|
||||||
| import java.io.ByteArrayOutputStream; | ||||||
| import java.io.IOException; | ||||||
| import java.io.PrintStream; | ||||||
| import java.io.UnsupportedEncodingException; | ||||||
| import java.nio.charset.StandardCharsets; | ||||||
| import java.util.Collections; | ||||||
| import org.apache.hadoop.hdds.protocol.proto.HddsProtos; | ||||||
| import org.apache.hadoop.hdds.scm.client.ScmClient; | ||||||
| import org.apache.hadoop.ozone.OzoneManagerVersion; | ||||||
| import org.apache.hadoop.ozone.om.helpers.ServiceInfo; | ||||||
| import org.apache.hadoop.ozone.om.helpers.ServiceInfoEx; | ||||||
| import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol; | ||||||
| import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; | ||||||
| import org.junit.jupiter.api.AfterEach; | ||||||
| import org.junit.jupiter.api.BeforeEach; | ||||||
| import org.junit.jupiter.api.Test; | ||||||
|
|
@@ -42,40 +49,87 @@ public class TestStatusSubCommand { | |||||
| private static final String DEFAULT_ENCODING = StandardCharsets.UTF_8.name(); | ||||||
|
|
||||||
| private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); | ||||||
| private final ByteArrayOutputStream errContent = new ByteArrayOutputStream(); | ||||||
| private final PrintStream originalOut = System.out; | ||||||
| private final PrintStream originalErr = System.err; | ||||||
| private OzoneManagerProtocol omClient; | ||||||
| private StatusSubCommand cmd; | ||||||
|
|
||||||
| @BeforeEach | ||||||
| public void setup() throws UnsupportedEncodingException { | ||||||
| cmd = new StatusSubCommand(); | ||||||
| public void setup() throws IOException { | ||||||
| omClient = mock(OzoneManagerProtocol.class); | ||||||
| when(omClient.getServiceInfo()).thenReturn(serviceInfoWithVersion(OzoneManagerVersion.ZDU)); | ||||||
|
|
||||||
| cmd = new StatusSubCommand() { | ||||||
| @Override | ||||||
| protected OzoneManagerProtocol getClient() throws Exception { | ||||||
| return omClient; | ||||||
| } | ||||||
| }; | ||||||
| System.setOut(new PrintStream(outContent, false, DEFAULT_ENCODING)); | ||||||
| System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING)); | ||||||
| } | ||||||
|
|
||||||
| @AfterEach | ||||||
| public void tearDown() { | ||||||
| System.setOut(originalOut); | ||||||
| System.setErr(originalErr); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testStatusCommandPrintsUpgradeStatus() throws IOException { | ||||||
| ScmClient scmClient = mock(ScmClient.class); | ||||||
| HddsProtos.UpgradeStatus status = HddsProtos.UpgradeStatus.newBuilder() | ||||||
| public void testStatusCommandPrintsUpgradeStatus() throws Exception { | ||||||
| HddsProtos.UpgradeStatus hddsStatus = HddsProtos.UpgradeStatus.newBuilder() | ||||||
| .setScmFinalized(false) | ||||||
| .setNumDatanodesFinalized(1) | ||||||
| .setNumDatanodesTotal(3) | ||||||
| .setShouldFinalize(true) | ||||||
| .build(); | ||||||
| when(scmClient.queryUpgradeStatus()).thenReturn(status); | ||||||
|
|
||||||
| OzoneManagerProtocolProtos.QueryUpgradeStatusResponse response = | ||||||
| OzoneManagerProtocolProtos.QueryUpgradeStatusResponse.newBuilder() | ||||||
| .setOmFinalized(false) | ||||||
| .setHddsStatus(hddsStatus) | ||||||
| .build(); | ||||||
|
|
||||||
| when(omClient.queryUpgradeStatus()).thenReturn(response); | ||||||
| new CommandLine(cmd).parseArgs(); | ||||||
| cmd.execute(scmClient); | ||||||
| cmd.call(); | ||||||
|
|
||||||
| String output = outContent.toString(DEFAULT_ENCODING); | ||||||
| assertTrue(output.contains("Upgrade status:")); | ||||||
| assertTrue(output.contains("OM Finalized: false")); | ||||||
| assertTrue(output.contains("SCM Finalized: false")); | ||||||
| assertTrue(output.contains("Datanodes finalized: 1")); | ||||||
| assertTrue(output.contains("Total Datanodes: 3")); | ||||||
| assertTrue(output.contains("Should Finalize: true")); | ||||||
| verify(scmClient).queryUpgradeStatus(); | ||||||
| verify(omClient).queryUpgradeStatus(); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testStatusCommandPropagatesException() throws Exception { | ||||||
| when(omClient.queryUpgradeStatus()).thenThrow(new IOException("OM unavailable")); | ||||||
| new CommandLine(cmd).parseArgs(); | ||||||
| assertThrows(IOException.class, () -> cmd.call()); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testNonZduServerPrintsErrorAndReturnsNonZero() throws Exception { | ||||||
| when(omClient.getServiceInfo()).thenReturn(serviceInfoWithVersion(OzoneManagerVersion.DEFAULT_VERSION)); | ||||||
|
|
||||||
| new CommandLine(cmd).parseArgs(); | ||||||
| assertEquals(1, cmd.call()); | ||||||
|
|
||||||
| String errOutput = errContent.toString(DEFAULT_ENCODING); | ||||||
| assertTrue(errOutput.contains("OM does not support ZDU")); | ||||||
| verify(omClient, never()).finalizeUpgrade(); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this check
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| private ServiceInfoEx serviceInfoWithVersion(OzoneManagerVersion version) { | ||||||
| ServiceInfo serviceInfo = new ServiceInfo.Builder() | ||||||
| .setNodeType(HddsProtos.NodeType.OM) | ||||||
| .setHostname("localhost") | ||||||
| .setOmVersion(version) | ||||||
| .build(); | ||||||
| return new ServiceInfoEx(Collections.singletonList(serviceInfo), "", Collections.emptyList()); | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -158,6 +158,7 @@ enum Type { | |
| DeleteObjectTagging = 142; | ||
| SubmitSnapshotDiff = 143; | ||
| StartFinalizeUpgrade = 144; | ||
| QueryUpgradeStatus = 145; | ||
| } | ||
|
|
||
| enum SafeMode { | ||
|
|
@@ -217,7 +218,7 @@ message OMRequest { | |
| optional ServiceListRequest serviceListRequest = 51; | ||
| optional DBUpdatesRequest dbUpdatesRequest = 53; | ||
| optional FinalizeUpgradeRequest finalizeUpgradeRequest = 54 [deprecated = true]; | ||
| optional FinalizeUpgradeProgressRequest finalizeUpgradeProgressRequest = 55; | ||
| optional FinalizeUpgradeProgressRequest finalizeUpgradeProgressRequest = 55 [deprecated = true]; | ||
| optional PrepareRequest prepareRequest = 56; | ||
| optional PrepareStatusRequest prepareStatusRequest = 57; | ||
| optional CancelPrepareRequest cancelPrepareRequest = 58; | ||
|
|
@@ -311,6 +312,7 @@ message OMRequest { | |
|
|
||
| optional SubmitSnapshotDiffRequest submitSnapshotDiffRequest = 144; | ||
| optional StartFinalizeUpgradeRequest startFinalizeUpgradeRequest = 145; | ||
| optional QueryUpgradeStatusRequest queryUpgradeStatusRequest = 146; | ||
| } | ||
|
|
||
| message OMResponse { | ||
|
|
@@ -361,7 +363,7 @@ message OMResponse { | |
| optional ServiceListResponse ServiceListResponse = 51; | ||
| optional DBUpdatesResponse dbUpdatesResponse = 52; | ||
| optional FinalizeUpgradeResponse finalizeUpgradeResponse = 54 [deprecated = true]; | ||
| optional FinalizeUpgradeProgressResponse finalizeUpgradeProgressResponse = 55; | ||
| optional FinalizeUpgradeProgressResponse finalizeUpgradeProgressResponse = 55 [deprecated = true]; | ||
| optional PrepareResponse prepareResponse = 56; | ||
| optional PrepareStatusResponse prepareStatusResponse = 57; | ||
| optional CancelPrepareResponse cancelPrepareResponse = 58; | ||
|
|
@@ -447,6 +449,7 @@ message OMResponse { | |
|
|
||
| optional SubmitSnapshotDiffResponse submitSnapshotDiffResponse = 143; | ||
| optional StartFinalizeUpgradeResponse startFinalizeUpgradeResponse = 144; | ||
| optional QueryUpgradeStatusResponse queryUpgradeStatusResponse = 145; | ||
| } | ||
|
|
||
| enum Status { | ||
|
|
@@ -1640,6 +1643,15 @@ message StartFinalizeUpgradeRequest { | |
| message StartFinalizeUpgradeResponse { | ||
| } | ||
|
|
||
| message QueryUpgradeStatusRequest { | ||
| } | ||
|
|
||
| message QueryUpgradeStatusResponse { | ||
| required bool omFinalized = 1; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and its SCM equivalent will need the software and apparent versions as well for two use cases:
|
||
| required hadoop.hdds.UpgradeStatus hddsStatus = 2; | ||
|
Comment on lines
+1650
to
+1651
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For new messages we have been making everything |
||
| } | ||
|
|
||
| // deprecated, use QueryUpgradeStatusRequest/Response instead. Retained for wire compatibility. | ||
| message FinalizeUpgradeProgressRequest { | ||
| // Ignored by OM; retained for wire compatibility. | ||
| required string upgradeClientId = 1; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.