fix shared storage (#4042)

docs/en_US/Tutorial/ExperimentConfig.rst

@@ -890,14 +890,7 @@ storageAccountKey
 
 Optional. String.
 
-Required if using AzureBlob storage and ``resourceGroupName`` not set. The azure storage account key.
-
-resourceGroupName
-^^^^^^^^^^^^^^^^^
-
-Optional. String.
-
-Required if using AzureBlob storage and ``storageAccountKey`` not set. The resource group that AzureBlob container belongs to.
+Required if using AzureBlob storage. The azure storage account key.
 
 containerName
 ^^^^^^^^^^^^^

docs/en_US/Tutorial/HowToUseSharedStorage.rst

@@ -11,7 +11,7 @@ And we will develop more practical features in the future based on shared storag
 
 .. note::
     Shared storage is currently in the experimental stage. We suggest use AzureBlob under Ubuntu/CentOS/RHEL, and NFS under Ubuntu/CentOS/RHEL/Fedora/Debian for remote.
-    And make sure your local machine can mount NFS or fuse AzureBlob and has ``sudo`` permission on your remote runtime. We only support shared storage under training service with reuse mode for now.
+    And make sure your local machine can mount NFS or fuse AzureBlob and the machine used in training service has ``sudo`` permission without password. We only support shared storage under training service with reuse mode for now.
 
 Example
 -------
@@ -21,18 +21,26 @@ If you want to use AzureBlob, add below to your config. Full config file see :gi
 
     sharedStorage:
         storageType: AzureBlob
+        # please set localMountPoint as absolute path and localMountPoint should outside the code directory
+        # because nni will copy user code to localMountPoint
         localMountPoint: ${your/local/mount/point}
+        # remoteMountPoint is the mount point on training service machine, it can be set as both absolute path and relative path
+        # make sure you have `sudo` permission without password on training service machine
         remoteMountPoint: ${your/remote/mount/point}
         storageAccountName: ${replace_to_your_storageAccountName}
         storageAccountKey: ${replace_to_your_storageAccountKey}
-        # If you did not set storageAccountKey, you need use `az login` with Azure CLI at first and set resourceGroupName.
-        # resourceGroupName: ${replace_to_your_resourceGroupName}
         containerName: ${replace_to_your_containerName}
         # usermount means you have already mount this storage on localMountPoint
         # nnimount means nni will try to mount this storage on localMountPoint
         # nomount means storage will not mount in local machine, will support partial storages in the future 
         localMounted: nnimount
 
+You can find ``storageAccountName``, ``storageAccountKey``, ``containerName`` on azure storage account portal.
+
+.. image:: ../../img/azure_storage.png
+   :target: ../../img/azure_storage.png
+   :alt: 
+
 If you want to use NFS, add below to your config. Full config file see :githublink:`mnist-sharedstorage/config_nfs.yml <examples/trials/mnist-sharedstorage/config_nfs.yml>`.
 
 .. code-block:: yaml

docs/en_US/reference/experiment_config.rst

@@ -926,17 +926,6 @@ Azure storage account key.
 
 type: ``Optional[str]``
 
-When not set storageAccountKey, should use ``az login`` with Azure CLI at first and set `resourceGroupName`_.
-
-
-resourceGroupName
-"""""""""""""""""
-
-Resource group that AzureBlob container belongs to.
-
-type: ``Optional[str]``
-
-Required if ``storageAccountKey`` not set.
 
 containerName
 """""""""""""

examples/trials/mnist-sharedstorage/config_azureblob.yml

@@ -24,12 +24,14 @@ trainingService:
                             #   C:/Users/USERNAME/.conda/envs/ENVNAME;C:/Users/USERNAME/.conda/envs/ENVNAME/Scripts;C:/Users/USERNAME/.conda/envs/ENVNAME/Library/bin
 sharedStorage:
   storageType: AzureBlob
+  # please set localMountPoint as absolute path and localMountPoint should outside the code directory
+  # because nni will copy user code to localMountPoint
   localMountPoint: ${your/local/mount/point}
+  # remoteMountPoint is the mount point on training service machine, it can be set as both absolute path and relative path
+  # make sure you have `sudo` permission without password on training service machine
   remoteMountPoint: ${your/remote/mount/point}
   storageAccountName: ${replace_to_your_storageAccountName}
   storageAccountKey: ${replace_to_your_storageAccountKey}
-  # If you did not set storageAccountKey, you need use `az login` with Azure CLI at first and set resourceGroupName.
-  # resourceGroupName: ${replace_to_your_resourceGroupName}
   containerName: ${replace_to_your_containerName}
   # usermount means you have already mount this storage on localMountPoint
   # nnimount means nni will try to mount this storage on localMountPoint

nni/experiment/config/common.py

@@ -49,10 +49,22 @@ class TrainingServiceConfig(ConfigBase):
 @dataclass(init=False)
 class SharedStorageConfig(ConfigBase):
     storage_type: str
-    local_mount_point: str
+    local_mount_point: PathLike
     remote_mount_point: str
     local_mounted: str
-
+    storage_account_name: Optional[str] = None
+    storage_account_key: Optional[str] = None
+    container_name: Optional[str] = None
+    nfs_server: Optional[str] = None
+    exported_directory: Optional[str] = None
+
+    def __init__(self, *, _base_path: Optional[Path], **kwargs):
+        kwargs = {util.case_insensitive(key): value for key, value in kwargs.items()}
+        if 'localmountpoint' in kwargs:
+            kwargs['localmountpoint'] = Path(kwargs['localmountpoint']).expanduser()
+            if not kwargs['localmountpoint'].is_absolute():
+                raise ValueError('localMountPoint can only be set as an absolute path.')
+        super().__init__(_base_path=_base_path, **kwargs)
 
 @dataclass(init=False)
 class ExperimentConfig(ConfigBase):
@@ -101,6 +113,8 @@ class ExperimentConfig(ConfigBase):
         for algo_type in ['tuner', 'assessor', 'advisor']:
             if isinstance(kwargs.get(algo_type), dict):
                 setattr(self, algo_type, _AlgorithmConfig(**kwargs.pop(algo_type)))
+        if isinstance(kwargs.get('sharedstorage'), dict):
+            setattr(self, 'shared_storage', SharedStorageConfig(_base_path=base_path, **kwargs.pop('sharedstorage')))
 
     def canonical(self):
         ret = super().canonical()

nni/tools/nnictl/config_schema.py

@@ -158,7 +158,6 @@ common_schema = {
         Optional('storageAccountName'): setType('storageAccountName', str),
         Optional('storageAccountKey'): setType('storageAccountKey', str),
         Optional('containerName'): setType('containerName', str),
-        Optional('resourceGroupName'): setType('resourceGroupName', str),
         Optional('localMounted'): setChoice('localMounted', 'usermount', 'nnimount', 'nomount')
     }
 }

test/config/sharedstorage_test/config_sharedstorage_remote_azureblob.yml

@@ -25,8 +25,6 @@ sharedStorage:
   remoteMountPoint: ${your/remote/mount/point}
   storageAccountName: ${replace_to_your_storageAccountName}
   storageAccountKey: ${replace_to_your_storageAccountKey}
-  # If you did not set storageAccountKey, you need use `az login` with Azure CLI at first and set resourceGroupName.
-  # resourceGroupName: ${replace_to_your_resourceGroupName}
   containerName: ${replace_to_your_containerName}
   # usermount means you have already mount this storage on localMountPoint
   # nnimount means nni will try to mount this storage on localMountPoint

ts/nni_manager/common/experimentConfig.ts

@@ -148,7 +148,6 @@ export interface NfsConfig extends SharedStorageConfig {
 export interface AzureBlobConfig extends SharedStorageConfig {
     storageAccountName: string;
     storageAccountKey?: string;
-    resourceGroupName?: string;
     containerName: string;
 }
 

ts/nni_manager/rest_server/restValidationSchemas.ts

@@ -211,7 +211,6 @@ export namespace ValidationSchemas {
                 storageAccountName: joi.string(),
                 storageAccountKey: joi.string(),
                 containerName: joi.string(),
-                resourceGroupName: joi.string(),
                 localMounted: joi.string()
             })
         }

ts/nni_manager/training_service/reusable/shared_storages/azureblobStorageService.ts

@@ -79,11 +79,9 @@ export class AzureBlobSharedStorageService extends SharedStorageService {
         this.storageAccountName = azureblobConfig.storageAccountName;
         this.containerName = azureblobConfig.containerName;
         if (azureblobConfig.storageAccountKey !== undefined) {
-            this.storageAccountKey =azureblobConfig.storageAccountKey;
-        } else if (azureblobConfig.resourceGroupName !== undefined) {
-            await this.setAccountKey(azureblobConfig.resourceGroupName);
+            this.storageAccountKey = azureblobConfig.storageAccountKey;
         } else {
-            const errorMessage = `${this.storageType} Shared Storage: must set one of 'storageAccountKey' or 'resourceGroupName'.`;
+            const errorMessage = `${this.storageType} Shared Storage: must set 'storageAccountKey'.`;
             this.log.error(errorMessage);
             return Promise.reject(errorMessage);
         }
@@ -174,21 +172,6 @@ export class AzureBlobSharedStorageService extends SharedStorageService {
         return Promise.resolve();
     }
 
-    private async setAccountKey(resourceGroupName: string): Promise<void> {
-        try {
-            const result = await cpp.exec(`az storage account keys list --resource-group ${resourceGroupName} --account-name ${this.storageAccountName} --query "[0].value" | tr -d '"'`);
-            if (result.stderr) {
-                throw Error(result.stderr);
-            } else {
-                this.storageAccountKey = result.stdout.trim();
-            }
-        } catch (error) {
-            const errorMessage: string = `${this.storageType} Shared Storage: get account key failed, error is ${error}`;
-            this.log.error(errorMessage);
-            return Promise.reject(errorMessage);
-        }
-    }
-
     public async cleanUp(): Promise<void> {
         if (this.localMounted !== 'nnimount') {
             return Promise.resolve();