From bc9c3139078d5802c2d6ce5fcc6976c3ac7dd17c Mon Sep 17 00:00:00 2001
From: Xiaoyu Yao <xyao@apache.org>
Date: Wed, 29 Mar 2017 12:19:31 -0700
Subject: [PATCH] HDFS-11567. Ozone: SCM: Support update container. Contributed
 by Weiwei Yang.

---
 .../container/common/impl/Dispatcher.java     | 30 ++++++++-
 .../common/interfaces/ContainerManager.java   | 11 +++
 .../ozone/container/ContainerTestHelper.java  | 38 +++++++++++
 .../common/impl/TestContainerPersistence.java | 67 +++++++++++++++++++
 .../ozoneimpl/TestOzoneContainer.java         | 23 +++++++
 5 files changed, 167 insertions(+), 2 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/impl/Dispatcher.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/impl/Dispatcher.java
index 93c9da69cf..fc72e733b2 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/impl/Dispatcher.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/impl/Dispatcher.java
@@ -163,8 +163,7 @@ public class Dispatcher implements ContainerDispatcher {
         return ContainerUtils.unsupportedRequest(msg);
 
       case UpdateContainer:
-        // TODO : Support Update Container.
-        return ContainerUtils.unsupportedRequest(msg);
+        return handleUpdateContainer(msg);
 
       case ReadContainer:
         return handleReadContainer(msg);
@@ -297,6 +296,33 @@ public class Dispatcher implements ContainerDispatcher {
     }
   }
 
+  /**
+   * Update an existing container with the new container data.
+   *
+   * @param msg Request
+   * @return ContainerCommandResponseProto
+   * @throws IOException
+   */
+  private ContainerCommandResponseProto handleUpdateContainer(
+      ContainerCommandRequestProto msg)
+      throws IOException {
+    if (!msg.hasUpdateContainer()) {
+      LOG.debug("Malformed read container request. trace ID: {}",
+          msg.getTraceID());
+      return ContainerUtils.malformedRequest(msg);
+    }
+
+    Pipeline pipeline = Pipeline.getFromProtoBuf(
+        msg.getUpdateContainer().getPipeline());
+    String containerName = msg.getUpdateContainer()
+        .getContainerData().getName();
+
+    ContainerData data = ContainerData.getFromProtBuf(
+        msg.getUpdateContainer().getContainerData());
+    this.containerManager.updateContainer(pipeline, containerName, data);
+    return ContainerUtils.getContainerResponse(msg);
+  }
+
   /**
    * Calls into container logic and returns appropriate response.
    *
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerManager.java
index e41cc3c88c..378adcf3bb 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerManager.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerManager.java
@@ -69,6 +69,17 @@ public interface ContainerManager extends RwLock {
   void deleteContainer(Pipeline pipeline, String containerName)
       throws StorageContainerException;
 
+  /**
+   * Update an existing container.
+   *
+   * @param pipeline container nodes
+   * @param containerName name of the container
+   * @param data container data
+   * @throws StorageContainerException
+   */
+  void updateContainer(Pipeline pipeline, String containerName,
+      ContainerData data) throws StorageContainerException;
+
   /**
    * As simple interface for container Iterations.
    *
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java
index df96db57fc..076ab825ab 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java
@@ -41,6 +41,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Random;
 import java.util.UUID;
+import java.util.Map;
 
 /**
  * Helpers for container tests.
@@ -292,6 +293,43 @@ public final class ContainerTestHelper {
     return request.build();
   }
 
+  /**
+   * Return an update container command for test purposes.
+   * Creates a container data based on the given meta data,
+   * and request to update an existing container with it.
+   *
+   * @param containerName
+   * @param metaData
+   * @return
+   * @throws IOException
+   */
+  public static ContainerCommandRequestProto getUpdateContainerRequest(
+      String containerName, Map<String, String> metaData) throws IOException {
+    ContainerProtos.UpdateContainerRequestProto.Builder updateRequestBuilder =
+        ContainerProtos.UpdateContainerRequestProto.newBuilder();
+    ContainerProtos.ContainerData.Builder containerData = ContainerProtos
+        .ContainerData.newBuilder();
+    containerData.setName(containerName);
+    String[] keys = metaData.keySet().toArray(new String[]{});
+    for(int i=0; i<keys.length; i++) {
+      ContainerProtos.KeyValue.Builder kvBuilder =
+          ContainerProtos.KeyValue.newBuilder();
+      kvBuilder.setKey(keys[i]);
+      kvBuilder.setValue(metaData.get(keys[i]));
+      containerData.addMetadata(i, kvBuilder.build());
+    }
+
+    updateRequestBuilder.setPipeline(
+        ContainerTestHelper.createSingleNodePipeline(containerName)
+            .getProtobufMessage());
+    updateRequestBuilder.setContainerData(containerData.build());
+
+    ContainerCommandRequestProto.Builder request =
+        ContainerCommandRequestProto.newBuilder();
+    request.setCmdType(ContainerProtos.Type.UpdateContainer);
+    request.setUpdateContainer(updateRequestBuilder.build());
+    return request.build();
+  }
   /**
    * Returns a create container response for test purposes. There are a bunch of
    * tests where we need to just send a request and get a reply.
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
index 1727cc9b8c..06d373319c 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
@@ -44,6 +44,7 @@ import org.junit.rules.ExpectedException;
 import org.junit.rules.Timeout;
 
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.IOException;
 import java.net.URL;
 import java.nio.file.DirectoryStream;
@@ -613,5 +614,71 @@ public class TestContainerPersistence {
     keyManager.deleteKey(pipeline, keyName);
   }
 
+  /**
+   * Tries to update an existing and non-existing container.
+   * Verifies container map and persistent data both updated.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testUpdateContainer() throws IOException {
+    String containerName = OzoneUtils.getRequestID();
+    ContainerData data = new ContainerData(containerName);
+    data.addMetadata("VOLUME", "shire");
+    data.addMetadata("owner)", "bilbo");
 
+    containerManager.createContainer(
+        createSingleNodePipeline(containerName),
+        data);
+
+    File orgContainerFile = containerManager.getContainerFile(data);
+    Assert.assertTrue(orgContainerFile.exists());
+
+    ContainerData newData = new ContainerData(containerName);
+    newData.addMetadata("VOLUME", "shire_new");
+    newData.addMetadata("owner)", "bilbo_new");
+
+    containerManager.updateContainer(
+        createSingleNodePipeline(containerName),
+        containerName,
+        newData);
+
+    Assert.assertEquals(1, containerManager.getContainerMap().size());
+    Assert.assertTrue(containerManager.getContainerMap()
+        .containsKey(containerName));
+
+    // Verify in-memory map
+    ContainerData actualNewData = containerManager.getContainerMap()
+        .get(containerName).getContainer();
+    Assert.assertEquals(actualNewData.getAllMetadata().get("VOLUME"),
+        "shire_new");
+    Assert.assertEquals(actualNewData.getAllMetadata().get("owner)"),
+        "bilbo_new");
+
+    // Verify container data on disk
+    File newContainerFile = containerManager.getContainerFile(actualNewData);
+    Assert.assertTrue("Container file should exist.",
+        newContainerFile.exists());
+    Assert.assertEquals("Container file should be in same location.",
+        orgContainerFile.getAbsolutePath(),
+        newContainerFile.getAbsolutePath());
+
+    try (FileInputStream newIn = new FileInputStream(newContainerFile)) {
+      ContainerProtos.ContainerData actualContainerDataProto =
+          ContainerProtos.ContainerData.parseDelimitedFrom(newIn);
+      ContainerData actualContainerData = ContainerData
+          .getFromProtBuf(actualContainerDataProto);
+      Assert.assertEquals(actualContainerData.getAllMetadata().get("VOLUME"),
+          "shire_new");
+      Assert.assertEquals(actualContainerData.getAllMetadata().get("owner)"),
+          "bilbo_new");
+    }
+
+    // Update a non-existing container
+    exception.expect(StorageContainerException.class);
+    exception.expectMessage("Container doesn't exist.");
+    containerManager.updateContainer(
+        createSingleNodePipeline("non_exist_container"),
+        "non_exist_container", newData);
+  }
 }
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java
index 1a40b34f8a..6c28388310 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java
@@ -32,6 +32,8 @@ import org.junit.Test;
 import org.junit.rules.Timeout;
 
 import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
 
 /**
  * Tests ozone containers.
@@ -177,6 +179,27 @@ public class TestOzoneContainer {
       Assert.assertNotNull(response);
       Assert.assertEquals(ContainerProtos.Result.SUCCESS, response.getResult());
       Assert.assertTrue(request.getTraceID().equals(response.getTraceID()));
+
+      //Update an existing container
+      Map<String, String> containerUpdate = new HashMap<String, String>();
+      containerUpdate.put("container_updated_key", "container_updated_value");
+      ContainerProtos.ContainerCommandRequestProto updateRequest1 =
+          ContainerTestHelper.getUpdateContainerRequest(
+              containerName, containerUpdate);
+      ContainerProtos.ContainerCommandResponseProto updateResponse1 =
+          client.sendCommand(updateRequest1);
+      Assert.assertNotNull(updateResponse1);
+      Assert.assertEquals(ContainerProtos.Result.SUCCESS,
+          response.getResult());
+
+      //Update an non-existing container
+      ContainerProtos.ContainerCommandRequestProto updateRequest2 =
+          ContainerTestHelper.getUpdateContainerRequest(
+              "non_exist_container", containerUpdate);
+      ContainerProtos.ContainerCommandResponseProto updateResponse2 =
+          client.sendCommand(updateRequest2);
+      Assert.assertEquals(ContainerProtos.Result.CONTAINER_NOT_FOUND,
+          updateResponse2.getResult());
     } finally {
       if (client != null) {
         client.close();