You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by John Burwell <jb...@basho.com> on 2012/11/19 07:37:28 UTC

Review Request: S3-backed Secondary Storage

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/
-----------------------------------------------------------

Review request for cloudstack.


Description
-------

Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.

For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).


This addresses bug CLOUDSTACK-509.


Diffs
-----

  .gitignore 9daca15 
  api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
  api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
  api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
  api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
  api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
  api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
  api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
  api/src/com/cloud/api/ApiConstants.java 78a3ded 
  api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
  api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
  api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
  api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
  api/src/com/cloud/resource/ResourceService.java 1065453 
  api/src/com/cloud/storage/S3.java PRE-CREATION 
  build/package.xml 09ed939 
  client/WEB-INF/classes/resources/messages.properties 626e44a 
  client/tomcatconf/commands.properties.in 149547e 
  console-proxy/scripts/_run.sh e408378 
  core/pom.xml 15f0f7b 
  core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 2dc7950 
  core/src/com/cloud/storage/S3VO.java PRE-CREATION 
  core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
  core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
  core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
  patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
  plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
  pom.xml 08f61f5 
  scripts/vm/hypervisor/xenserver/s3xen.py PRE-CREATION 
  scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
  scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
  scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
  server/pom.xml cd4fbbe 
  server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
  server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
  server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
  server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
  server/src/com/cloud/configuration/Config.java 66ac276 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
  server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
  server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
  server/src/com/cloud/servlet/ConsoleProxyPasswordBasedEncryptor.java 2638c8b 
  server/src/com/cloud/storage/StorageManagerImpl.java e252633 
  server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
  server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
  server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
  server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
  server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
  server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
  server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
  server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
  server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
  server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
  server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
  server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
  setup/db/create-schema.sql fff084e 
  setup/db/db/schema-40to410.sql PRE-CREATION 
  tools/apidoc/gen_toc.py eeaf2a2 
  tools/marvin/marvin/cloudstackConnection.py bd8a5b2 
  tools/marvin/marvin/deployDataCenter.py bdf08cc 
  ui/dictionary.jsp b80e296 
  ui/scripts/cloudStack.js de3bd73 
  ui/scripts/sharedFunctions.js b6b3ef8 
  ui/scripts/system.js 9e3932f 
  ui/scripts/templates.js 74511f0 
  utils/pom.xml 2e4e74f 
  utils/src/com/cloud/utils/DateUtil.java be1627d 
  utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
  utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
  utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
  utils/src/com/cloud/utils/nio/NioConnection.java 50e6a88 

Diff: https://reviews.apache.org/r/8123/diff/


Testing
-------

I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:

   * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
   * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
   * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.


Screenshots
-----------

S3 Configuration Form
  https://reviews.apache.org/r/8123/s/13/
S3 Enable Menu on the Zone Tab
  https://reviews.apache.org/r/8123/s/14/


Thanks,

John Burwell


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <ro...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13556
-----------------------------------------------------------


John, can you fix indendations at some places, trailing whitespaces etc. see:
http://docs.cloudstack.org/CloudStack_Documentation/Design_Documents/Coding_Conventions

- Rohit Yadav


On Nov. 19, 2012, 6:37 a.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2012, 6:37 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 9daca15 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   console-proxy/scripts/_run.sh e408378 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 2dc7950 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 08f61f5 
>   scripts/vm/hypervisor/xenserver/s3xen.py PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml cd4fbbe 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/servlet/ConsoleProxyPasswordBasedEncryptor.java 2638c8b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py bd8a5b2 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 2e4e74f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
>   utils/src/com/cloud/utils/nio/NioConnection.java 50e6a88 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.

> On Nov. 19, 2012, 8:03 a.m., Rohit Yadav wrote:
> > John, can you fix indendations at some places, trailing whitespaces etc. see:
> > http://docs.cloudstack.org/CloudStack_Documentation/Design_Documents/Coding_Conventions

Rohit,

Which files are have the issue?

Thanks,
-John


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13557
-----------------------------------------------------------


On Nov. 19, 2012, 6:37 a.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2012, 6:37 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 9daca15 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   console-proxy/scripts/_run.sh e408378 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 2dc7950 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 08f61f5 
>   scripts/vm/hypervisor/xenserver/s3xen.py PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml cd4fbbe 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/servlet/ConsoleProxyPasswordBasedEncryptor.java 2638c8b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py bd8a5b2 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 2e4e74f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
>   utils/src/com/cloud/utils/nio/NioConnection.java 50e6a88 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <ro...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13557
-----------------------------------------------------------


John, can you fix indendations at some places, trailing whitespaces etc. see:
http://docs.cloudstack.org/CloudStack_Documentation/Design_Documents/Coding_Conventions

- Rohit Yadav


On Nov. 19, 2012, 6:37 a.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2012, 6:37 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 9daca15 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   console-proxy/scripts/_run.sh e408378 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 2dc7950 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 08f61f5 
>   scripts/vm/hypervisor/xenserver/s3xen.py PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml cd4fbbe 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/servlet/ConsoleProxyPasswordBasedEncryptor.java 2638c8b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py bd8a5b2 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 2e4e74f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
>   utils/src/com/cloud/utils/nio/NioConnection.java 50e6a88 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.

> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote:
> > John, thanks for patch. Much better now. Can you check, why the code that configure db in the testclient is commented? tools/marvin/marvin/deployDataCenter.py
> > For the actual functional review, Edison can help on storage and Alena/Kishan on API/DB layers.

Rohit,

I removed the code (which I forgot to delete) because, as I understand it, Marvin no longer connects to the database during testing but uses the API.  If it is no longer necessary, removing the requirement for remote access to the database on development machines the tool much lighter weight.

Thanks,
-John


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13904
-----------------------------------------------------------


On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 9:44 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 33f95c7 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 4a4276e 
>   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml e3308d8 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 6bed67f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by Prasanna Santhanam <Pr...@citrix.com>.

> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote:
> > John, thanks for patch. Much better now. Can you check, why the code that configure db in the testclient is commented? tools/marvin/marvin/deployDataCenter.py
> > For the actual functional review, Edison can help on storage and Alena/Kishan on API/DB layers.
> 
> John Burwell wrote:
>     Rohit,
>     
>     I removed the code (which I forgot to delete) because, as I understand it, Marvin no longer connects to the database during testing but uses the API.  If it is no longer necessary, removing the requirement for remote access to the database on development machines the tool much lighter weight.
>     
>     Thanks,
>     -John
> 
> Rohit Yadav wrote:
>     John, we need that. May be for this particular patch these are not required. But marvin needs to be generic, and should be able to provide testClient with db settings configured, these can be used by some kind of testing, for ex. usage event, we cannot be always sure if an API is working for example. That's why I want them, please uncomment them:
>     
>             #dbSvr = self.config.dbSvr	
>             #self.testClient.dbConfigure(dbSvr.dbSvr, dbSvr.port, dbSvr.user, \
>             #                            dbSvr.passwd, dbSvr.db)
> 
> John Burwell wrote:
>     Rohit,
>     
>     What if I modified the patch not attempt a database connection if no settings are provided?  This would allow test runs that do not require a database connection to omit the configuration process while leaving it available for those that do.
>     
>     Thanks,
>     -John 
>

Yes, that will do it. If the dbSvr settings are provided in the json config make the connection to create a dbClient or else return None.


- Prasanna


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13904
-----------------------------------------------------------


On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 9:44 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 33f95c7 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 4a4276e 
>   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml e3308d8 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 6bed67f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.

> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote:
> > John, thanks for patch. Much better now. Can you check, why the code that configure db in the testclient is commented? tools/marvin/marvin/deployDataCenter.py
> > For the actual functional review, Edison can help on storage and Alena/Kishan on API/DB layers.
> 
> John Burwell wrote:
>     Rohit,
>     
>     I removed the code (which I forgot to delete) because, as I understand it, Marvin no longer connects to the database during testing but uses the API.  If it is no longer necessary, removing the requirement for remote access to the database on development machines the tool much lighter weight.
>     
>     Thanks,
>     -John
> 
> Rohit Yadav wrote:
>     John, we need that. May be for this particular patch these are not required. But marvin needs to be generic, and should be able to provide testClient with db settings configured, these can be used by some kind of testing, for ex. usage event, we cannot be always sure if an API is working for example. That's why I want them, please uncomment them:
>     
>             #dbSvr = self.config.dbSvr	
>             #self.testClient.dbConfigure(dbSvr.dbSvr, dbSvr.port, dbSvr.user, \
>             #                            dbSvr.passwd, dbSvr.db)

Rohit,

What if I modified the patch not attempt a database connection if no settings are provided?  This would allow test runs that do not require a database connection to omit the configuration process while leaving it available for those that do.

Thanks,
-John 


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13904
-----------------------------------------------------------


On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 9:44 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 33f95c7 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 4a4276e 
>   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml e3308d8 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 6bed67f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <ro...@citrix.com>.

> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote:
> > John, thanks for patch. Much better now. Can you check, why the code that configure db in the testclient is commented? tools/marvin/marvin/deployDataCenter.py
> > For the actual functional review, Edison can help on storage and Alena/Kishan on API/DB layers.
> 
> John Burwell wrote:
>     Rohit,
>     
>     I removed the code (which I forgot to delete) because, as I understand it, Marvin no longer connects to the database during testing but uses the API.  If it is no longer necessary, removing the requirement for remote access to the database on development machines the tool much lighter weight.
>     
>     Thanks,
>     -John

John, we need that. May be for this particular patch these are not required. But marvin needs to be generic, and should be able to provide testClient with db settings configured, these can be used by some kind of testing, for ex. usage event, we cannot be always sure if an API is working for example. That's why I want them, please uncomment them:

        #dbSvr = self.config.dbSvr	
        #self.testClient.dbConfigure(dbSvr.dbSvr, dbSvr.port, dbSvr.user, \
        #                            dbSvr.passwd, dbSvr.db)


- Rohit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13904
-----------------------------------------------------------


On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 9:44 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 33f95c7 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 4a4276e 
>   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml e3308d8 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 6bed67f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <ro...@citrix.com>.

> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote:
> > John, thanks for patch. Much better now. Can you check, why the code that configure db in the testclient is commented? tools/marvin/marvin/deployDataCenter.py
> > For the actual functional review, Edison can help on storage and Alena/Kishan on API/DB layers.
> 
> John Burwell wrote:
>     Rohit,
>     
>     I removed the code (which I forgot to delete) because, as I understand it, Marvin no longer connects to the database during testing but uses the API.  If it is no longer necessary, removing the requirement for remote access to the database on development machines the tool much lighter weight.
>     
>     Thanks,
>     -John
> 
> Rohit Yadav wrote:
>     John, we need that. May be for this particular patch these are not required. But marvin needs to be generic, and should be able to provide testClient with db settings configured, these can be used by some kind of testing, for ex. usage event, we cannot be always sure if an API is working for example. That's why I want them, please uncomment them:
>     
>             #dbSvr = self.config.dbSvr	
>             #self.testClient.dbConfigure(dbSvr.dbSvr, dbSvr.port, dbSvr.user, \
>             #                            dbSvr.passwd, dbSvr.db)
> 
> John Burwell wrote:
>     Rohit,
>     
>     What if I modified the patch not attempt a database connection if no settings are provided?  This would allow test runs that do not require a database connection to omit the configuration process while leaving it available for those that do.
>     
>     Thanks,
>     -John 
>
> 
> Prasanna Santhanam wrote:
>     Yes, that will do it. If the dbSvr settings are provided in the json config make the connection to create a dbClient or else return None.

Alright let's do one more round of review, pl. add Edison as the reviewer.


- Rohit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13904
-----------------------------------------------------------


On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 9:44 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 33f95c7 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 4a4276e 
>   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml e3308d8 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 6bed67f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <ro...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13904
-----------------------------------------------------------


John, thanks for patch. Much better now. Can you check, why the code that configure db in the testclient is commented? tools/marvin/marvin/deployDataCenter.py
For the actual functional review, Edison can help on storage and Alena/Kishan on API/DB layers.

- Rohit Yadav


On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 9:44 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 33f95c7 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 4a4276e 
>   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml e3308d8 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 6bed67f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by edison su <ed...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review14496
-----------------------------------------------------------

Ship it!


Ship It!

- edison su


On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 9:44 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 33f95c7 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 4a4276e 
>   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml e3308d8 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 6bed67f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.
Edison,

The ConfigurationManagerImpl import change was result of a merge conflict resolution.  During that process, I noticed that the imports didn't conform to the projects guidelines, so I organized them as part of the resolution process.

I have resolved the issue with the pom.xml change.  The change submitted was the result of an upstream merge conflict resolution.  However, as I reviewed the changes in my branch and master, the content was identical.  Therefore, I reverted pom.xml in my branch to master, regenerated the patch, and submitted it via review board [1].

Thanks again for your help,
-John

[1] https://reviews.apache.org/r/8123/

On Dec 19, 2012, at 4:58 PM, Edison Su <Ed...@citrix.com> wrote:

> If I click https://reviews.apache.org/r/8123/diff/5/,  I get:
> The patch to 'pom.xml' didn't apply cleanly. The temporary files have been left in '/tmp/reviewboard.gkgGgx' for debugging purposes. `patch` returned: patching file /tmp/reviewboard.gkgGgx/tmpvi_JO8 Reversed (or previously applied) patch detected! Assume -R? [n] Apply anyway? [n] Skipping patch. 1 out of 1 hunk ignored -- saving rejects to file /tmp/reviewboard.gkgGgx/tmpvi_JO8-new.rej
> 
> In diff5, there is lot of changes on java import in ConfigurationManagerImpl, while there is no code change in the class itself, 
> 
> Something like:
> +++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java
> @@ -16,35 +16,110 @@
> // under the License.
> package com.cloud.configuration;
> 
> +import java.net.URI;
> +import java.sql.PreparedStatement;
> +import java.sql.ResultSet;
> +import java.sql.SQLException;
> +import java.util.ArrayList;
> +import java.util.Collection;
> +import java.util.HashMap;
> +import java.util.HashSet;
> +import java.util.Hashtable;
> +import java.util.Iterator;
> +import java.util.List;
> +import java.util.Map;
> +import java.util.Set;
> +import java.util.UUID;
> +
> +import javax.ejb.Local;
> +import javax.naming.ConfigurationException;
> +import javax.naming.Context;
> +import javax.naming.NamingException;
> +import javax.naming.directory.DirContext;
> +import javax.naming.directory.InitialDirContext;
> +
> +import org.apache.log4j.Logger;
> +
> import com.cloud.acl.SecurityChecker;
> import com.cloud.alert.AlertManager;
> import com.cloud.api.ApiConstants.LDAPParams;
> import com.cloud.api.ApiDBUtils;
> -import com.cloud.api.commands.*;
> +import com.cloud.api.commands.CreateDiskOfferingCmd;
> +import com.cloud.api.commands.CreateNetworkOfferingCmd;
> +import com.cloud.api.commands.CreateServiceOfferingCmd;
> +import com.cloud.api.commands.CreateVlanIpRangeCmd;
> +import com.cloud.api.commands.CreateZoneCmd;
> +import com.cloud.api.commands.DeleteDiskOfferingCmd;
> +import com.cloud.api.commands.DeleteNetworkOfferingCmd;
> +import com.cloud.api.commands.DeletePodCmd;
> +import com.cloud.api.commands.DeleteServiceOfferingCmd;
> +import com.cloud.api.commands.DeleteVlanIpRangeCmd;
> +import com.cloud.api.commands.DeleteZoneCmd;
> +import com.cloud.api.commands.LDAPConfigCmd;
> +import com.cloud.api.commands.LDAPRemoveCmd;
> +import com.cloud.api.commands.ListNetworkOfferingsCmd;
> +import com.cloud.api.commands.UpdateCfgCmd;
> +import com.cloud.api.commands.UpdateDiskOfferingCmd;
> +import com.cloud.api.commands.UpdateNetworkOfferingCmd;
> +import com.cloud.api.commands.UpdatePodCmd;
> +import com.cloud.api.commands.UpdateServiceOfferingCmd;
> +import com.cloud.api.commands.UpdateZoneCmd;
> import com.cloud.capacity.dao.CapacityDao;
> import com.cloud.configuration.Resource.ResourceType;
> import com.cloud.configuration.dao.ConfigurationDao;
> -import com.cloud.dc.*;
> +import com.cloud.dc.AccountVlanMapVO;
> +import com.cloud.dc.ClusterVO;
> +import com.cloud.dc.DataCenter;
> import com.cloud.dc.DataCenter.NetworkType;
> +import com.cloud.dc.DataCenterIpAddressVO;
> +import com.cloud.dc.DataCenterLinkLocalIpAddressVO;
> +import com.cloud.dc.DataCenterVO;
> +import com.cloud.dc.HostPodVO;
> +import com.cloud.dc.Pod;
> +import com.cloud.dc.PodVlanMapVO;
> +import com.cloud.dc.Vlan;
> import com.cloud.dc.Vlan.VlanType;
> -import com.cloud.dc.dao.*;
> +import com.cloud.dc.VlanVO;
> +import com.cloud.dc.dao.AccountVlanMapDao;
> +import com.cloud.dc.dao.ClusterDao;
> +import com.cloud.dc.dao.DataCenterDao;
> +import com.cloud.dc.dao.DataCenterIpAddressDao;
> +import com.cloud.dc.dao.DataCenterLinkLocalIpAddressDaoImpl;
> +import com.cloud.dc.dao.HostPodDao;
> +import com.cloud.dc.dao.PodVlanMapDao;
> +import com.cloud.dc.dao.VlanDao;
> import com.cloud.deploy.DataCenterDeployment;
> import com.cloud.domain.Domain;
> import com.cloud.domain.DomainVO;
> import com.cloud.domain.dao.DomainDao;
> import com.cloud.event.ActionEvent;
> import com.cloud.event.EventTypes;
> -import com.cloud.exception.*;
> +import com.cloud.exception.ConcurrentOperationException;
> +import com.cloud.exception.InsufficientCapacityException;
> +import com.cloud.exception.InvalidParameterValueException;
> +import com.cloud.exception.PermissionDeniedException;
> +import com.cloud.exception.ResourceAllocationException;
> +import com.cloud.exception.ResourceUnavailableException;
> import com.cloud.host.HostVO;
> import com.cloud.hypervisor.Hypervisor.HypervisorType;
> -import com.cloud.network.*;
> +import com.cloud.network.IPAddressVO;
> +import com.cloud.network.Network;
> import com.cloud.network.Network.Capability;
> import com.cloud.network.Network.GuestType;
> import com.cloud.network.Network.Provider;
> import com.cloud.network.Network.Service;
> +import com.cloud.network.NetworkManager;
> +import com.cloud.network.NetworkVO;
> import com.cloud.network.Networks.BroadcastDomainType;
> import com.cloud.network.Networks.TrafficType;
> -import com.cloud.network.dao.*;
> +import com.cloud.network.PhysicalNetwork;
> +import com.cloud.network.PhysicalNetworkVO;
> +import com.cloud.network.dao.FirewallRulesDao;
> +import com.cloud.network.dao.IPAddressDao;
> +import com.cloud.network.dao.NetworkDao;
> +import com.cloud.network.dao.PhysicalNetworkDao;
> +import com.cloud.network.dao.PhysicalNetworkTrafficTypeDao;
> +import com.cloud.network.dao.PhysicalNetworkTrafficTypeVO;
> import com.cloud.network.vpc.VpcManager;
> import com.cloud.offering.DiskOffering;
> import com.cloud.offering.NetworkOffering;
> 
> 
> Are all these imports are necessary?
> 
> 
> 
>> -----Original Message-----
>> From: John Burwell [mailto:noreply@reviews.apache.org] On Behalf Of John
>> Burwell
>> Sent: Wednesday, December 19, 2012 1:40 PM
>> To: Edison Su
>> Cc: cloudstack; John Burwell
>> Subject: Re: Review Request: S3-backed Secondary Storage
>> 
>> 
>> 
>>> On Dec. 17, 2012, 5:51 p.m., edison su wrote:
>>>> Hi John, thanks for the patch. Few comments: 1. I can't apply the patch
>> against master top, there is conflict in pom.xml. 2. Could you remove the
>> unnecessary change in ConfigurationManagerImpl file?
>> 
>> Edison,
>> 
>> Which change in ConfigurationManagerImpl is unnecessary?  Looking
>> through the diff, I cant find any unnecessary changes.  Also, can you specify
>> the merge issue you are encountering with pom.xml?  I just merged my
>> feature branch with cloudstack/master and had no conflicts.
>> 
>> Thanks,
>> -John
>> 
>> 
>> - John
>> 
>> 
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/8123/#review14588
>> -----------------------------------------------------------
>> 
>> 
>> On Dec. 14, 2012, 11:54 p.m., John Burwell wrote:
>>> 
>>> -----------------------------------------------------------
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/8123/
>>> -----------------------------------------------------------
>>> 
>>> (Updated Dec. 14, 2012, 11:54 p.m.)
>>> 
>>> 
>>> Review request for cloudstack and edison su.
>>> 
>>> 
>>> Description
>>> -------
>>> 
>>> Backs NFS-based secondary storage with an S3-compatible object store.
>> Periodically, a reaper thread synchronizes templates and ISOs stored on a
>> NFS secondary storage mount with a configured S3 object store. It also
>> pushes snapshots to the object store when they are created and downloads
>> them in other zones on-demand. In addition to permitting the use of
>> commodity or IaaS storage solutions for static assets, it provides a means of
>> automatically synchronizing template and ISO assets across multiple zones.
>>> 
>>> For more information about the design of the patch, please see the design
>> document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-
>> backed+Secondary+Storage).
>>> 
>>> 
>>> This addresses bug CLOUDSTACK-509.
>>> 
>>> 
>>> Diffs
>>> -----
>>> 
>>>  api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION
>>>  build/package.xml 09ed939
>>>  pom.xml 4a4276e
>>>  server/src/com/cloud/configuration/ConfigurationManagerImpl.java
>> ef940e8
>>>  server/src/com/cloud/server/ManagementServerImpl.java 117be57
>>>  server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913
>>>  server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8
>>>  server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION
>>>  tools/marvin/marvin/cloudstackConnection.py c805213
>>>  tools/marvin/marvin/deployDataCenter.py bdf08cc
>>>  utils/src/com/cloud/utils/db/DbUtil.java feef7b3
>>> 
>>> Diff: https://reviews.apache.org/r/8123/diff/
>>> 
>>> 
>>> Testing
>>> -------
>>> 
>>> I am submitting patch to begin the feedback process while we complete
>> integration testing.  I have verified that it does not interfere with normal
>> CloudStack operations when S3-backed Secondary Storage is disabled (the
>> default setting) .  I have successfully tested operation of single zone
>> template and ISO scenarios on devcloud described in the design document.  I
>> am currently working through some issues in our multi-zone test
>> environment to complete all scenarios described.  The following are the
>> known deficiencies of the current implementation which I plan to correct in a
>> subsequent patch:
>>> 
>>>   * Cross zone garbage collection: When a global asset is deleted from one
>> zone's secondary storage, it is not deleted from the secondary storage of
>> other zones which have downloaded it from the object store
>>>   * S3 Configuration Update: The API only supports adding an object store
>> configuration.  Users should be able to edit the access key, secret key,
>> connection timeout. max error retries, and socket timeout.
>>>   * Multi-threaded Uploads: Permit the upload of multiple assets to the
>> object store simultaneously to decrease the propagation latency across all
>> zones.
>>> 
>>> 
>>> Screenshots
>>> -----------
>>> 
>>> S3 Configuration Form
>>>  https://reviews.apache.org/r/8123/s/13/
>>> S3 Enable Menu on the Zone Tab
>>>  https://reviews.apache.org/r/8123/s/14/
>>> 
>>> 
>>> Thanks,
>>> 
>>> John Burwell
>>> 
>>> 
> 


RE: Review Request: S3-backed Secondary Storage

Posted by Edison Su <Ed...@citrix.com>.
If I click https://reviews.apache.org/r/8123/diff/5/,  I get:
The patch to 'pom.xml' didn't apply cleanly. The temporary files have been left in '/tmp/reviewboard.gkgGgx' for debugging purposes. `patch` returned: patching file /tmp/reviewboard.gkgGgx/tmpvi_JO8 Reversed (or previously applied) patch detected! Assume -R? [n] Apply anyway? [n] Skipping patch. 1 out of 1 hunk ignored -- saving rejects to file /tmp/reviewboard.gkgGgx/tmpvi_JO8-new.rej

In diff5, there is lot of changes on java import in ConfigurationManagerImpl, while there is no code change in the class itself, 

Something like:
+++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -16,35 +16,110 @@
 // under the License.
 package com.cloud.configuration;
 
+import java.net.URI;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Hashtable;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+
+import javax.ejb.Local;
+import javax.naming.ConfigurationException;
+import javax.naming.Context;
+import javax.naming.NamingException;
+import javax.naming.directory.DirContext;
+import javax.naming.directory.InitialDirContext;
+
+import org.apache.log4j.Logger;
+
 import com.cloud.acl.SecurityChecker;
 import com.cloud.alert.AlertManager;
 import com.cloud.api.ApiConstants.LDAPParams;
 import com.cloud.api.ApiDBUtils;
-import com.cloud.api.commands.*;
+import com.cloud.api.commands.CreateDiskOfferingCmd;
+import com.cloud.api.commands.CreateNetworkOfferingCmd;
+import com.cloud.api.commands.CreateServiceOfferingCmd;
+import com.cloud.api.commands.CreateVlanIpRangeCmd;
+import com.cloud.api.commands.CreateZoneCmd;
+import com.cloud.api.commands.DeleteDiskOfferingCmd;
+import com.cloud.api.commands.DeleteNetworkOfferingCmd;
+import com.cloud.api.commands.DeletePodCmd;
+import com.cloud.api.commands.DeleteServiceOfferingCmd;
+import com.cloud.api.commands.DeleteVlanIpRangeCmd;
+import com.cloud.api.commands.DeleteZoneCmd;
+import com.cloud.api.commands.LDAPConfigCmd;
+import com.cloud.api.commands.LDAPRemoveCmd;
+import com.cloud.api.commands.ListNetworkOfferingsCmd;
+import com.cloud.api.commands.UpdateCfgCmd;
+import com.cloud.api.commands.UpdateDiskOfferingCmd;
+import com.cloud.api.commands.UpdateNetworkOfferingCmd;
+import com.cloud.api.commands.UpdatePodCmd;
+import com.cloud.api.commands.UpdateServiceOfferingCmd;
+import com.cloud.api.commands.UpdateZoneCmd;
 import com.cloud.capacity.dao.CapacityDao;
 import com.cloud.configuration.Resource.ResourceType;
 import com.cloud.configuration.dao.ConfigurationDao;
-import com.cloud.dc.*;
+import com.cloud.dc.AccountVlanMapVO;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.DataCenter;
 import com.cloud.dc.DataCenter.NetworkType;
+import com.cloud.dc.DataCenterIpAddressVO;
+import com.cloud.dc.DataCenterLinkLocalIpAddressVO;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.HostPodVO;
+import com.cloud.dc.Pod;
+import com.cloud.dc.PodVlanMapVO;
+import com.cloud.dc.Vlan;
 import com.cloud.dc.Vlan.VlanType;
-import com.cloud.dc.dao.*;
+import com.cloud.dc.VlanVO;
+import com.cloud.dc.dao.AccountVlanMapDao;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.dc.dao.DataCenterIpAddressDao;
+import com.cloud.dc.dao.DataCenterLinkLocalIpAddressDaoImpl;
+import com.cloud.dc.dao.HostPodDao;
+import com.cloud.dc.dao.PodVlanMapDao;
+import com.cloud.dc.dao.VlanDao;
 import com.cloud.deploy.DataCenterDeployment;
 import com.cloud.domain.Domain;
 import com.cloud.domain.DomainVO;
 import com.cloud.domain.dao.DomainDao;
 import com.cloud.event.ActionEvent;
 import com.cloud.event.EventTypes;
-import com.cloud.exception.*;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.PermissionDeniedException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.host.HostVO;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import com.cloud.network.*;
+import com.cloud.network.IPAddressVO;
+import com.cloud.network.Network;
 import com.cloud.network.Network.Capability;
 import com.cloud.network.Network.GuestType;
 import com.cloud.network.Network.Provider;
 import com.cloud.network.Network.Service;
+import com.cloud.network.NetworkManager;
+import com.cloud.network.NetworkVO;
 import com.cloud.network.Networks.BroadcastDomainType;
 import com.cloud.network.Networks.TrafficType;
-import com.cloud.network.dao.*;
+import com.cloud.network.PhysicalNetwork;
+import com.cloud.network.PhysicalNetworkVO;
+import com.cloud.network.dao.FirewallRulesDao;
+import com.cloud.network.dao.IPAddressDao;
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.dao.PhysicalNetworkDao;
+import com.cloud.network.dao.PhysicalNetworkTrafficTypeDao;
+import com.cloud.network.dao.PhysicalNetworkTrafficTypeVO;
 import com.cloud.network.vpc.VpcManager;
 import com.cloud.offering.DiskOffering;
 import com.cloud.offering.NetworkOffering;


Are all these imports are necessary?



> -----Original Message-----
> From: John Burwell [mailto:noreply@reviews.apache.org] On Behalf Of John
> Burwell
> Sent: Wednesday, December 19, 2012 1:40 PM
> To: Edison Su
> Cc: cloudstack; John Burwell
> Subject: Re: Review Request: S3-backed Secondary Storage
> 
> 
> 
> > On Dec. 17, 2012, 5:51 p.m., edison su wrote:
> > > Hi John, thanks for the patch. Few comments: 1. I can't apply the patch
> against master top, there is conflict in pom.xml. 2. Could you remove the
> unnecessary change in ConfigurationManagerImpl file?
> 
> Edison,
> 
> Which change in ConfigurationManagerImpl is unnecessary?  Looking
> through the diff, I cant find any unnecessary changes.  Also, can you specify
> the merge issue you are encountering with pom.xml?  I just merged my
> feature branch with cloudstack/master and had no conflicts.
> 
> Thanks,
> -John
> 
> 
> - John
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/#review14588
> -----------------------------------------------------------
> 
> 
> On Dec. 14, 2012, 11:54 p.m., John Burwell wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/8123/
> > -----------------------------------------------------------
> >
> > (Updated Dec. 14, 2012, 11:54 p.m.)
> >
> >
> > Review request for cloudstack and edison su.
> >
> >
> > Description
> > -------
> >
> > Backs NFS-based secondary storage with an S3-compatible object store.
> Periodically, a reaper thread synchronizes templates and ISOs stored on a
> NFS secondary storage mount with a configured S3 object store. It also
> pushes snapshots to the object store when they are created and downloads
> them in other zones on-demand. In addition to permitting the use of
> commodity or IaaS storage solutions for static assets, it provides a means of
> automatically synchronizing template and ISO assets across multiple zones.
> >
> > For more information about the design of the patch, please see the design
> document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-
> backed+Secondary+Storage).
> >
> >
> > This addresses bug CLOUDSTACK-509.
> >
> >
> > Diffs
> > -----
> >
> >   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION
> >   build/package.xml 09ed939
> >   pom.xml 4a4276e
> >   server/src/com/cloud/configuration/ConfigurationManagerImpl.java
> ef940e8
> >   server/src/com/cloud/server/ManagementServerImpl.java 117be57
> >   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913
> >   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8
> >   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION
> >   tools/marvin/marvin/cloudstackConnection.py c805213
> >   tools/marvin/marvin/deployDataCenter.py bdf08cc
> >   utils/src/com/cloud/utils/db/DbUtil.java feef7b3
> >
> > Diff: https://reviews.apache.org/r/8123/diff/
> >
> >
> > Testing
> > -------
> >
> > I am submitting patch to begin the feedback process while we complete
> integration testing.  I have verified that it does not interfere with normal
> CloudStack operations when S3-backed Secondary Storage is disabled (the
> default setting) .  I have successfully tested operation of single zone
> template and ISO scenarios on devcloud described in the design document.  I
> am currently working through some issues in our multi-zone test
> environment to complete all scenarios described.  The following are the
> known deficiencies of the current implementation which I plan to correct in a
> subsequent patch:
> >
> >    * Cross zone garbage collection: When a global asset is deleted from one
> zone's secondary storage, it is not deleted from the secondary storage of
> other zones which have downloaded it from the object store
> >    * S3 Configuration Update: The API only supports adding an object store
> configuration.  Users should be able to edit the access key, secret key,
> connection timeout. max error retries, and socket timeout.
> >    * Multi-threaded Uploads: Permit the upload of multiple assets to the
> object store simultaneously to decrease the propagation latency across all
> zones.
> >
> >
> > Screenshots
> > -----------
> >
> > S3 Configuration Form
> >   https://reviews.apache.org/r/8123/s/13/
> > S3 Enable Menu on the Zone Tab
> >   https://reviews.apache.org/r/8123/s/14/
> >
> >
> > Thanks,
> >
> > John Burwell
> >
> >


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.

> On Dec. 17, 2012, 5:51 p.m., edison su wrote:
> > Hi John, thanks for the patch. Few comments: 1. I can't apply the patch against master top, there is conflict in pom.xml. 2. Could you remove the unnecessary change in ConfigurationManagerImpl file?

Edison,

Which change in ConfigurationManagerImpl is unnecessary?  Looking through the diff, I cant find any unnecessary changes.  Also, can you specify the merge issue you are encountering with pom.xml?  I just merged my feature branch with cloudstack/master and had no conflicts.

Thanks,
-John


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review14588
-----------------------------------------------------------


On Dec. 14, 2012, 11:54 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2012, 11:54 p.m.)
> 
> 
> Review request for cloudstack and edison su.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   build/package.xml 09ed939 
>   pom.xml 4a4276e 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/server/ManagementServerImpl.java 117be57 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   utils/src/com/cloud/utils/db/DbUtil.java feef7b3 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by edison su <ed...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review14588
-----------------------------------------------------------


Hi John, thanks for the patch. Few comments: 1. I can't apply the patch against master top, there is conflict in pom.xml. 2. Could you remove the unnecessary change in ConfigurationManagerImpl file? 

- edison su


On Dec. 14, 2012, 11:54 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2012, 11:54 p.m.)
> 
> 
> Review request for cloudstack and edison su.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   build/package.xml 09ed939 
>   pom.xml 4a4276e 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/server/ManagementServerImpl.java 117be57 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   utils/src/com/cloud/utils/db/DbUtil.java feef7b3 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review15168
-----------------------------------------------------------


Already shipped, pl. close it as submitted. Thanks.

- Rohit Yadav


On Dec. 21, 2012, 12:10 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Dec. 21, 2012, 12:10 p.m.)
> 
> 
> Review request for cloudstack and edison su.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   build/package.xml 09ed939 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/server/ManagementServerImpl.java 117be57 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/configGenerator.py 11fbce9 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   utils/src/com/cloud/utils/db/DbUtil.java feef7b3 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by edison su <ed...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review15014
-----------------------------------------------------------

Ship it!


Ship It!

- edison su


On Dec. 21, 2012, 12:10 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Dec. 21, 2012, 12:10 p.m.)
> 
> 
> Review request for cloudstack and edison su.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   build/package.xml 09ed939 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/server/ManagementServerImpl.java 117be57 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/configGenerator.py 11fbce9 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   utils/src/com/cloud/utils/db/DbUtil.java feef7b3 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/
-----------------------------------------------------------

(Updated Dec. 21, 2012, 12:10 p.m.)


Review request for cloudstack and edison su.


Changes
-------

Fixes a merge conflict on pom.xml.  The change in the previous patch attempt to fix an upstream merge conflict.  On further review, cloudstack/master's content was identical.  Therefore, the change was reverted to masteer's version.


Description
-------

Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.

For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).


This addresses bug CLOUDSTACK-509.


Diffs (updated)
-----

  api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
  build/package.xml 09ed939 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
  server/src/com/cloud/server/ManagementServerImpl.java 117be57 
  server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
  server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
  server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
  tools/marvin/marvin/cloudstackConnection.py c805213 
  tools/marvin/marvin/configGenerator.py 11fbce9 
  tools/marvin/marvin/deployDataCenter.py bdf08cc 
  utils/src/com/cloud/utils/db/DbUtil.java feef7b3 

Diff: https://reviews.apache.org/r/8123/diff/


Testing
-------

I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:

   * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
   * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
   * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.


Screenshots
-----------

S3 Configuration Form
  https://reviews.apache.org/r/8123/s/13/
S3 Enable Menu on the Zone Tab
  https://reviews.apache.org/r/8123/s/14/


Thanks,

John Burwell


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/
-----------------------------------------------------------

(Updated Dec. 14, 2012, 11:54 p.m.)


Review request for cloudstack and edison su.


Changes
-------

Add parent diff for the sake of completeness.


Description
-------

Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.

For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).


This addresses bug CLOUDSTACK-509.


Diffs (updated)
-----

  api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
  build/package.xml 09ed939 
  pom.xml 4a4276e 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
  server/src/com/cloud/server/ManagementServerImpl.java 117be57 
  server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
  server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
  server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
  tools/marvin/marvin/cloudstackConnection.py c805213 
  tools/marvin/marvin/deployDataCenter.py bdf08cc 
  utils/src/com/cloud/utils/db/DbUtil.java feef7b3 

Diff: https://reviews.apache.org/r/8123/diff/


Testing
-------

I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:

   * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
   * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
   * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.


Screenshots
-----------

S3 Configuration Form
  https://reviews.apache.org/r/8123/s/13/
S3 Enable Menu on the Zone Tab
  https://reviews.apache.org/r/8123/s/14/


Thanks,

John Burwell


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/
-----------------------------------------------------------

(Updated Dec. 14, 2012, 11:48 p.m.)


Review request for cloudstack and edison su.


Description
-------

Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.

For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).


This addresses bug CLOUDSTACK-509.


Diffs
-----

  api/src/com/cloud/api/commands/ListS3sCmd.java 507053d 
  build/package.xml 09ed939 
  pom.xml e5aa024 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java 2d7dfe2 
  server/src/com/cloud/server/ManagementServerImpl.java 117be57 
  server/src/com/cloud/storage/dao/VMTemplateDao.java 1284ba1 
  server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 5c71f1b 
  server/src/com/cloud/storage/s3/S3ManagerImpl.java 6b07254 
  tools/marvin/marvin/cloudstackConnection.py 8c4e325 
  tools/marvin/marvin/deployDataCenter.py 01235fe 
  utils/src/com/cloud/utils/db/DbUtil.java feef7b3 

Diff: https://reviews.apache.org/r/8123/diff/


Testing
-------

I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:

   * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
   * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
   * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.


Screenshots
-----------

S3 Configuration Form
  https://reviews.apache.org/r/8123/s/13/
S3 Enable Menu on the Zone Tab
  https://reviews.apache.org/r/8123/s/14/


Thanks,

John Burwell


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <ro...@citrix.com>.

> On Dec. 14, 2012, 9:58 p.m., Rohit Yadav wrote:
> > John, don't put all the patches on one review. Pl. close this one as submitted and open a new review (possibly backed by a jira issue) for your changes:
> > Makes the Marvin database configuration optional
> > Adds S3 template search into ManagementServerImpl#listTemplates
> >
> 
> John Burwell wrote:
>     Rohit,
>     
>     The original patch was prematurely merged into master.  I had re-opened the review previous to the merge requesting a hold until I push the response to feedback.  Since this patch includes feedback to complete this review, I attached per my previous comment.  All of this work pertains to the CLOUDSTACK-509 ticket.
>     
>     Thanks,
>     -John

I get it what you're doing. Just saying even if they are related to the premature commit, you should open a new review per commit.
Let's wait for Edison to review and merge now.


- Rohit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review14524
-----------------------------------------------------------


On Dec. 14, 2012, 11:54 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2012, 11:54 p.m.)
> 
> 
> Review request for cloudstack and edison su.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   build/package.xml 09ed939 
>   pom.xml 4a4276e 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/server/ManagementServerImpl.java 117be57 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   utils/src/com/cloud/utils/db/DbUtil.java feef7b3 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.

> On Dec. 14, 2012, 9:58 p.m., Rohit Yadav wrote:
> > John, don't put all the patches on one review. Pl. close this one as submitted and open a new review (possibly backed by a jira issue) for your changes:
> > Makes the Marvin database configuration optional
> > Adds S3 template search into ManagementServerImpl#listTemplates
> >

Rohit,

The original patch was prematurely merged into master.  I had re-opened the review previous to the merge requesting a hold until I push the response to feedback.  Since this patch includes feedback to complete this review, I attached per my previous comment.  All of this work pertains to the CLOUDSTACK-509 ticket.

Thanks,
-John


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review14524
-----------------------------------------------------------


On Dec. 14, 2012, 11:48 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2012, 11:48 p.m.)
> 
> 
> Review request for cloudstack and edison su.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/api/commands/ListS3sCmd.java 507053d 
>   build/package.xml 09ed939 
>   pom.xml e5aa024 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java 2d7dfe2 
>   server/src/com/cloud/server/ManagementServerImpl.java 117be57 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java 1284ba1 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 5c71f1b 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java 6b07254 
>   tools/marvin/marvin/cloudstackConnection.py 8c4e325 
>   tools/marvin/marvin/deployDataCenter.py 01235fe 
>   utils/src/com/cloud/utils/db/DbUtil.java feef7b3 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <ro...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review14524
-----------------------------------------------------------


John, don't put all the patches on one review. Pl. close this one as submitted and open a new review (possibly backed by a jira issue) for your changes:
Makes the Marvin database configuration optional
Adds S3 template search into ManagementServerImpl#listTemplates


- Rohit Yadav


On Dec. 14, 2012, 9:49 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2012, 9:49 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/api/commands/ListS3sCmd.java 507053d 
>   build/package.xml 09ed939 
>   pom.xml e5aa024 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java 2d7dfe2 
>   server/src/com/cloud/server/ManagementServerImpl.java 117be57 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java 1284ba1 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 5c71f1b 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java 6b07254 
>   tools/marvin/marvin/cloudstackConnection.py 8c4e325 
>   tools/marvin/marvin/deployDataCenter.py 01235fe 
>   utils/src/com/cloud/utils/db/DbUtil.java feef7b3 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/
-----------------------------------------------------------

(Updated Dec. 14, 2012, 9:49 p.m.)


Review request for cloudstack.


Changes
-------

Makes the Marvin database configuration optional
Adds S3 template search into ManagementServerImpl#listTemplates


Description
-------

Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.

For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).


This addresses bug CLOUDSTACK-509.


Diffs (updated)
-----

  api/src/com/cloud/api/commands/ListS3sCmd.java 507053d 
  build/package.xml 09ed939 
  pom.xml e5aa024 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java 2d7dfe2 
  server/src/com/cloud/server/ManagementServerImpl.java 117be57 
  server/src/com/cloud/storage/dao/VMTemplateDao.java 1284ba1 
  server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 5c71f1b 
  server/src/com/cloud/storage/s3/S3ManagerImpl.java 6b07254 
  tools/marvin/marvin/cloudstackConnection.py 8c4e325 
  tools/marvin/marvin/deployDataCenter.py 01235fe 
  utils/src/com/cloud/utils/db/DbUtil.java feef7b3 

Diff: https://reviews.apache.org/r/8123/diff/


Testing
-------

I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:

   * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
   * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
   * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.


Screenshots
-----------

S3 Configuration Form
  https://reviews.apache.org/r/8123/s/13/
S3 Enable Menu on the Zone Tab
  https://reviews.apache.org/r/8123/s/14/


Thanks,

John Burwell


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <ro...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review14508
-----------------------------------------------------------


Was applied on master:
commit b70c1a5a84ee014d980c07555d341f16ed515cb8
Author:     Edison Su <ed...@citrix.com>
AuthorDate: Thu Dec 13 23:11:30 2012 -0800
Commit:     Edison Su <ed...@citrix.com>
CommitDate: Thu Dec 13 23:18:37 2012 -0800

    Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stor~

Note; now that review board does not strip off git formatted patch, we should git am -s <patch> to retain original author signature and timestamp.

- Rohit Yadav


On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 9:44 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 33f95c7 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 4a4276e 
>   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml e3308d8 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 6bed67f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/
-----------------------------------------------------------

(Updated Nov. 26, 2012, 9:44 p.m.)


Review request for cloudstack.


Changes
-------

- Removes trailing whitespace
- Removes tab characters
- Remerged NfsSecondaryStorageResource, SnapshotManagerImpl, TemplateManagerImpl, and GlobalLock to remove inadvertent changes caused by IDE reformatting -- the patch is now roughly half the size as previously submitted revisions
- Makes the s3xen plugin PEP-8 compliant


Description
-------

Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.

For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).


This addresses bug CLOUDSTACK-509.


Diffs (updated)
-----

  .gitignore 33f95c7 
  api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
  api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
  api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
  api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
  api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
  api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
  api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
  api/src/com/cloud/api/ApiConstants.java 78a3ded 
  api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
  api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
  api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
  api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
  api/src/com/cloud/resource/ResourceService.java 1065453 
  api/src/com/cloud/storage/S3.java PRE-CREATION 
  build/package.xml 09ed939 
  client/WEB-INF/classes/resources/messages.properties 626e44a 
  client/tomcatconf/commands.properties.in 149547e 
  core/pom.xml 15f0f7b 
  core/src/com/cloud/storage/S3VO.java PRE-CREATION 
  core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
  core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
  core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
  patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
  plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
  pom.xml 4a4276e 
  scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
  scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
  scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
  scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
  server/pom.xml e3308d8 
  server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
  server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
  server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
  server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
  server/src/com/cloud/configuration/Config.java 66ac276 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
  server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
  server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
  server/src/com/cloud/storage/StorageManagerImpl.java e252633 
  server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
  server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
  server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
  server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
  server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
  server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
  server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
  server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
  server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
  server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
  server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
  server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
  setup/db/create-schema.sql fff084e 
  setup/db/db/schema-40to410.sql PRE-CREATION 
  tools/apidoc/gen_toc.py eeaf2a2 
  tools/marvin/marvin/cloudstackConnection.py c805213 
  tools/marvin/marvin/deployDataCenter.py bdf08cc 
  ui/dictionary.jsp b80e296 
  ui/scripts/cloudStack.js de3bd73 
  ui/scripts/sharedFunctions.js b6b3ef8 
  ui/scripts/system.js 9e3932f 
  ui/scripts/templates.js 74511f0 
  utils/pom.xml 6bed67f 
  utils/src/com/cloud/utils/DateUtil.java be1627d 
  utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
  utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
  utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 

Diff: https://reviews.apache.org/r/8123/diff/


Testing
-------

I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:

   * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
   * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
   * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.


Screenshots
-----------

S3 Configuration Form
  https://reviews.apache.org/r/8123/s/13/
S3 Enable Menu on the Zone Tab
  https://reviews.apache.org/r/8123/s/14/


Thanks,

John Burwell


RE: Review Request: S3-backed Secondary Storage

Posted by Edison Su <Ed...@citrix.com>.

> -----Original Message-----
> From: John Burwell [mailto:noreply@reviews.apache.org] On Behalf Of John
> Burwell
> Sent: Monday, November 26, 2012 2:54 PM
> To: cloudstack; Rohit Yadav; John Burwell
> Subject: Re: Review Request: S3-backed Secondary Storage
> 
> 
> 
> > On Nov. 21, 2012, 4:32 a.m., Rohit Yadav wrote:
> > > Read the coding conventions and checkout the diff so you'll know.
> > >
> > > Watch out for the red lines and extra indents.
> > > For example, fix for following files with tabwidth=4 with spaces, no
> > > tabs; fix trailing spaces;
> > >
> > > api/src/com/cloud/agent/api/BackupSnapshotCommand.java
> > > api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java
> > > api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java
> > > api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java
> > > api/src/com/cloud/agent/api/to/S3TO.java
> > > core/src/com/cloud/storage/S3VO.java
> > > core/src/com/cloud/storage/SnapshotVO.java
> > > core/src/com/cloud/storage/VMTemplateS3VO.java
> > >
> core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java
> > > (lots of tab excess indents; 1tab>4spaces, trailing spaces)
> > > server/src/com/cloud/api/ApiDBUtils.java
> > > server/src/com/cloud/api/doc/ApiXmlDocWriter.java
> > > server/src/com/cloud/configuration/ConfigurationManagerImpl.java
> > > server/src/com/cloud/storage/StorageManagerImpl.java
> > > server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java
> > > server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java
> > > server/src/com/cloud/storage/s3/S3Manager.java
> > > server/src/com/cloud/storage/s3/S3ManagerImpl.java
> > > server/src/com/cloud/storage/snapshot/SnapshotManager.java
> > > server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
> > > server/src/com/cloud/template/S3SyncTask.java
> > > server/src/com/cloud/template/TemplateManagerImpl.java
> > > utils/src/com/cloud/utils/S3Utils.java
> > > utils/src/com/cloud/utils/StringUtils.java
> > >
> > > Not sure about js, though they look alright:
> > > ui/scripts/system.js
> > >
> > > For python code, it should be pep8 compliant (for example all under
> > > 80 char width etc.), as per coding conventions; to check that I uses
> > > pep8; (for ex. tools/cli/codemonkey/*.py is pep8 compliant) pip
> > > install pep8
> > > pep8 <filename> will tell you what to fix; for example:
> > > pep8 scripts/vm/hypervisor/xenserver/s3xen.py
> > >
> > > Try to fix it until you get no error/output from pep8.
> > > Unfortunately, we don't have anything like pep8 for Java.
> > >
> > > Code reviews:
> > >
> > > For tools/marvin/marvin/cloudstackConnection.py:
> > > Why was the function moved: make_request_with_auth? Port 8096 is
> the
> > > default integration port, that is added to configuration when we do
> deploydb with mvn, If we replace this;
> > >         if port == 8096:
> > > by;
> > >         if self.apiKey == None and self.securityKey == None:
> > >
> > > If still fails, as I can feed incorrect api and secret keys, and if port is 8096, I
> would still want non auth api calls? May be we can do this;
> > >         if port == 8096 or (self.apiKey == None and self.securityKey ==
> None):
> > > Lastly, why remove the logging code, we need that to see the response,
> to see what was returned.
> > >
> > > Great work, lastly one suggestion on submitting patches:
> > > Since, this is a  big patch you can split it and submit multiple patches. This
> would make it easier to review, commit and revert if necessary.
> > > And, use git patch mode, using git add -p. This would ask you what parts
> of the changes, even withing a file to add or stage for commit.
> > > To fix the current patch (there may be other better ways), you can
> > > do this; assuming on your branch it was one patch and is on HEAD git
> > > format-patch -o patches HEAD~1 (backup your patch) git rebase -i
> > > HEAD~2; NOTE: this deletes the current commit from vim that deletes
> > > the commit from history, make sure you've a backup patch
> > >
> > > git apply <patch file> (this won't commit, but stage the changes)
> > > git add -p (add what is needed for a commit, say group changes by
> > > java/python/sql/js code or by server, api etc.) git commit -s repeat
> > > git add -p say you generated 10 patches; git format-patch -o patches
> > > HEAD~10 (email them on ML, review board fails for multiple patches,
> > > there is one way though by adding them as parent-child one after the
> > > other)
> > >
> > >
> 
> I have pushed a new version of the patch that corrects the tab and
> whitespace issues, as well as, PEP-8 compliance for the s3xen plugin.
> 
> Unfortunately, this patch can't reasonably be split into smaller pieces.  The S3
> functionality, like Swift, cuts a wide path through the code base.  The
> enhancements added to utility classes (e.g. StringUtils, GlobalLock, etc) are


I want to add s3 as an example how to integrate storage into cloudstack storage subsystem I proposed in (https://cwiki.apache.org/confluence/display/CLOUDSTACK/Storage+subsystem+2.0), so hopefully, we don't need to change so many code, all over the place, to add a new storage.


> directly used by the S3 implementation.  There are roughly 10-20 lines of
> code (e.g. 1 line in cloudstackConnection.py and 3 lines in ApiXmlDocWriter)
> across the entire patch that are independent.  However, given the size of the
> work and the prospect of future submissions, it seems counter-productive to
> attempt to split them out from a 250k patch.
> 
> I made the change to the unauthenticated behavior of
> cloudstackConnection.py precisely because Marvin does not connect to the
> management server via 8096 in our integration environment.  I modified the
> change to match your recommendations, and narrowed the scope of the
> change to a single line in most recent revision of the patch,
> 
> 
> - John
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/#review13659
> -----------------------------------------------------------
> 
> 
> On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/8123/
> > -----------------------------------------------------------
> >
> > (Updated Nov. 26, 2012, 9:44 p.m.)
> >
> >
> > Review request for cloudstack.
> >
> >
> > Description
> > -------
> >
> > Backs NFS-based secondary storage with an S3-compatible object store.
> Periodically, a reaper thread synchronizes templates and ISOs stored on a
> NFS secondary storage mount with a configured S3 object store. It also
> pushes snapshots to the object store when they are created and downloads
> them in other zones on-demand. In addition to permitting the use of
> commodity or IaaS storage solutions for static assets, it provides a means of
> automatically synchronizing template and ISO assets across multiple zones.
> >
> > For more information about the design of the patch, please see the design
> document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-
> backed+Secondary+Storage).
> >
> >
> > This addresses bug CLOUDSTACK-509.
> >
> >
> > Diffs
> > -----
> >
> >   .gitignore 33f95c7
> >   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d
> >   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java
> 3fa8c2b
> >   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-
> CREATION
> >   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java
> PRE-CREATION
> >
> api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorag
> eCommand.java PRE-CREATION
> >
> api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageC
> ommand.java PRE-CREATION
> >   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION
> >   api/src/com/cloud/api/ApiConstants.java 78a3ded
> >   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8
> >   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION
> >   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION
> >   api/src/com/cloud/api/response/S3Response.java PRE-CREATION
> >   api/src/com/cloud/resource/ResourceService.java 1065453
> >   api/src/com/cloud/storage/S3.java PRE-CREATION
> >   build/package.xml 09ed939
> >   client/WEB-INF/classes/resources/messages.properties 626e44a
> >   client/tomcatconf/commands.properties.in 149547e
> >   core/pom.xml 15f0f7b
> >   core/src/com/cloud/storage/S3VO.java PRE-CREATION
> >   core/src/com/cloud/storage/SnapshotVO.java 08dfafa
> >   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION
> >
> core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java
> 155210d
> >   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0
> >
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixReso
> urceBase.java d2db85c
> >   pom.xml 4a4276e
> >   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION
> >   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414
> >   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740
> >   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109
> >   server/pom.xml e3308d8
> >   server/src/com/cloud/api/ApiDBUtils.java 3b5f634
> >   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f
> >   server/src/com/cloud/api/ApiResponseHelper.java ebe8415
> >   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a
> >   server/src/com/cloud/configuration/Config.java 66ac276
> >   server/src/com/cloud/configuration/ConfigurationManagerImpl.java
> ef940e8
> >   server/src/com/cloud/configuration/DefaultComponentLibrary.java
> ef61044
> >   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b
> >   server/src/com/cloud/storage/StorageManagerImpl.java e252633
> >   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION
> >   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION
> >   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913
> >   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8
> >   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-
> CREATION
> >   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-
> CREATION
> >   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION
> >   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION
> >   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e
> >   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
> 32e37e6
> >   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION
> >   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2
> >   setup/db/create-schema.sql fff084e
> >   setup/db/db/schema-40to410.sql PRE-CREATION
> >   tools/apidoc/gen_toc.py eeaf2a2
> >   tools/marvin/marvin/cloudstackConnection.py c805213
> >   tools/marvin/marvin/deployDataCenter.py bdf08cc
> >   ui/dictionary.jsp b80e296
> >   ui/scripts/cloudStack.js de3bd73
> >   ui/scripts/sharedFunctions.js b6b3ef8
> >   ui/scripts/system.js 9e3932f
> >   ui/scripts/templates.js 74511f0
> >   utils/pom.xml 6bed67f
> >   utils/src/com/cloud/utils/DateUtil.java be1627d
> >   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION
> >   utils/src/com/cloud/utils/StringUtils.java 0f0ef05
> >   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943
> >
> > Diff: https://reviews.apache.org/r/8123/diff/
> >
> >
> > Testing
> > -------
> >
> > I am submitting patch to begin the feedback process while we complete
> integration testing.  I have verified that it does not interfere with normal
> CloudStack operations when S3-backed Secondary Storage is disabled (the
> default setting) .  I have successfully tested operation of single zone
> template and ISO scenarios on devcloud described in the design document.  I
> am currently working through some issues in our multi-zone test
> environment to complete all scenarios described.  The following are the
> known deficiencies of the current implementation which I plan to correct in a
> subsequent patch:
> >
> >    * Cross zone garbage collection: When a global asset is deleted from one
> zone's secondary storage, it is not deleted from the secondary storage of
> other zones which have downloaded it from the object store
> >    * S3 Configuration Update: The API only supports adding an object store
> configuration.  Users should be able to edit the access key, secret key,
> connection timeout. max error retries, and socket timeout.
> >    * Multi-threaded Uploads: Permit the upload of multiple assets to the
> object store simultaneously to decrease the propagation latency across all
> zones.
> >
> >
> > Screenshots
> > -----------
> >
> > S3 Configuration Form
> >   https://reviews.apache.org/r/8123/s/13/
> > S3 Enable Menu on the Zone Tab
> >   https://reviews.apache.org/r/8123/s/14/
> >
> >
> > Thanks,
> >
> > John Burwell
> >
> >


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.

> On Nov. 21, 2012, 4:32 a.m., Rohit Yadav wrote:
> > Read the coding conventions and checkout the diff so you'll know.
> > 
> > Watch out for the red lines and extra indents.
> > For example, fix for following files with tabwidth=4 with spaces, no tabs; fix trailing spaces;
> > 
> > api/src/com/cloud/agent/api/BackupSnapshotCommand.java
> > api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java
> > api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java
> > api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java
> > api/src/com/cloud/agent/api/to/S3TO.java
> > core/src/com/cloud/storage/S3VO.java
> > core/src/com/cloud/storage/SnapshotVO.java
> > core/src/com/cloud/storage/VMTemplateS3VO.java
> > core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java (lots of tab excess indents; 1tab>4spaces, trailing spaces)
> > server/src/com/cloud/api/ApiDBUtils.java
> > server/src/com/cloud/api/doc/ApiXmlDocWriter.java
> > server/src/com/cloud/configuration/ConfigurationManagerImpl.java
> > server/src/com/cloud/storage/StorageManagerImpl.java
> > server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java
> > server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java
> > server/src/com/cloud/storage/s3/S3Manager.java
> > server/src/com/cloud/storage/s3/S3ManagerImpl.java
> > server/src/com/cloud/storage/snapshot/SnapshotManager.java
> > server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
> > server/src/com/cloud/template/S3SyncTask.java
> > server/src/com/cloud/template/TemplateManagerImpl.java
> > utils/src/com/cloud/utils/S3Utils.java
> > utils/src/com/cloud/utils/StringUtils.java
> > 
> > Not sure about js, though they look alright:
> > ui/scripts/system.js
> > 
> > For python code, it should be pep8 compliant (for example all under 80 char width etc.), as per coding conventions;
> > to check that I uses pep8; (for ex. tools/cli/codemonkey/*.py is pep8 compliant)
> > pip install pep8
> > pep8 <filename> will tell you what to fix; for example:
> > pep8 scripts/vm/hypervisor/xenserver/s3xen.py
> > 
> > Try to fix it until you get no error/output from pep8.
> > Unfortunately, we don't have anything like pep8 for Java.
> > 
> > Code reviews:
> > 
> > For tools/marvin/marvin/cloudstackConnection.py:
> > Why was the function moved: make_request_with_auth? Port 8096 is the default integration port, that is added to configuration when we do deploydb with mvn, 
> > If we replace this;
> >         if port == 8096:
> > by;
> >         if self.apiKey == None and self.securityKey == None:
> > 
> > If still fails, as I can feed incorrect api and secret keys, and if port is 8096, I would still want non auth api calls? May be we can do this;
> >         if port == 8096 or (self.apiKey == None and self.securityKey == None):
> > Lastly, why remove the logging code, we need that to see the response, to see what was returned.
> > 
> > Great work, lastly one suggestion on submitting patches:
> > Since, this is a  big patch you can split it and submit multiple patches. This would make it easier to review, commit and revert if necessary.
> > And, use git patch mode, using git add -p. This would ask you what parts of the changes, even withing a file to add or stage for commit.
> > To fix the current patch (there may be other better ways), you can do this; assuming on your branch it was one patch and is on HEAD
> > git format-patch -o patches HEAD~1 (backup your patch)
> > git rebase -i HEAD~2; NOTE: this deletes the current commit from vim that deletes the commit from history, make sure you've a backup patch
> > 
> > git apply <patch file> (this won't commit, but stage the changes)
> > git add -p (add what is needed for a commit, say group changes by java/python/sql/js code or by server, api etc.)
> > git commit -s
> > repeat git add -p
> > say you generated 10 patches;
> > git format-patch -o patches HEAD~10 (email them on ML, review board fails for multiple patches, there is one way though by adding them as parent-child one after the other)
> > 
> >

I have pushed a new version of the patch that corrects the tab and whitespace issues, as well as, PEP-8 compliance for the s3xen plugin.

Unfortunately, this patch can't reasonably be split into smaller pieces.  The S3 functionality, like Swift, cuts a wide path through the code base.  The enhancements added to utility classes (e.g. StringUtils, GlobalLock, etc) are directly used by the S3 implementation.  There are roughly 10-20 lines of code (e.g. 1 line in cloudstackConnection.py and 3 lines in ApiXmlDocWriter) across the entire patch that are independent.  However, given the size of the work and the prospect of future submissions, it seems counter-productive to attempt to split them out from a 250k patch.

I made the change to the unauthenticated behavior of cloudstackConnection.py precisely because Marvin does not connect to the management server via 8096 in our integration environment.  I modified the change to match your recommendations, and narrowed the scope of the change to a single line in most recent revision of the patch,


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13659
-----------------------------------------------------------


On Nov. 26, 2012, 9:44 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 9:44 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 33f95c7 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 4a4276e 
>   scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml e3308d8 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py c805213 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 6bed67f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by Rohit Yadav <ro...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13659
-----------------------------------------------------------


Read the coding conventions and checkout the diff so you'll know.

Watch out for the red lines and extra indents.
For example, fix for following files with tabwidth=4 with spaces, no tabs; fix trailing spaces;

api/src/com/cloud/agent/api/BackupSnapshotCommand.java
api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java
api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java
api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java
api/src/com/cloud/agent/api/to/S3TO.java
core/src/com/cloud/storage/S3VO.java
core/src/com/cloud/storage/SnapshotVO.java
core/src/com/cloud/storage/VMTemplateS3VO.java
core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java (lots of tab excess indents; 1tab>4spaces, trailing spaces)
server/src/com/cloud/api/ApiDBUtils.java
server/src/com/cloud/api/doc/ApiXmlDocWriter.java
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
server/src/com/cloud/storage/StorageManagerImpl.java
server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java
server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java
server/src/com/cloud/storage/s3/S3Manager.java
server/src/com/cloud/storage/s3/S3ManagerImpl.java
server/src/com/cloud/storage/snapshot/SnapshotManager.java
server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
server/src/com/cloud/template/S3SyncTask.java
server/src/com/cloud/template/TemplateManagerImpl.java
utils/src/com/cloud/utils/S3Utils.java
utils/src/com/cloud/utils/StringUtils.java

Not sure about js, though they look alright:
ui/scripts/system.js

For python code, it should be pep8 compliant (for example all under 80 char width etc.), as per coding conventions;
to check that I uses pep8; (for ex. tools/cli/codemonkey/*.py is pep8 compliant)
pip install pep8
pep8 <filename> will tell you what to fix; for example:
pep8 scripts/vm/hypervisor/xenserver/s3xen.py

Try to fix it until you get no error/output from pep8.
Unfortunately, we don't have anything like pep8 for Java.

Code reviews:

For tools/marvin/marvin/cloudstackConnection.py:
Why was the function moved: make_request_with_auth? Port 8096 is the default integration port, that is added to configuration when we do deploydb with mvn, 
If we replace this;
        if port == 8096:
by;
        if self.apiKey == None and self.securityKey == None:

If still fails, as I can feed incorrect api and secret keys, and if port is 8096, I would still want non auth api calls? May be we can do this;
        if port == 8096 or (self.apiKey == None and self.securityKey == None):
Lastly, why remove the logging code, we need that to see the response, to see what was returned.

Great work, lastly one suggestion on submitting patches:
Since, this is a  big patch you can split it and submit multiple patches. This would make it easier to review, commit and revert if necessary.
And, use git patch mode, using git add -p. This would ask you what parts of the changes, even withing a file to add or stage for commit.
To fix the current patch (there may be other better ways), you can do this; assuming on your branch it was one patch and is on HEAD
git format-patch -o patches HEAD~1 (backup your patch)
git rebase -i HEAD~2; NOTE: this deletes the current commit from vim that deletes the commit from history, make sure you've a backup patch

git apply <patch file> (this won't commit, but stage the changes)
git add -p (add what is needed for a commit, say group changes by java/python/sql/js code or by server, api etc.)
git commit -s
repeat git add -p
say you generated 10 patches;
git format-patch -o patches HEAD~10 (email them on ML, review board fails for multiple patches, there is one way though by adding them as parent-child one after the other)



- Rohit Yadav


On Nov. 19, 2012, 5:16 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2012, 5:16 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 4661ea2 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   console-proxy/scripts/_run.sh e408378 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 08f61f5 
>   scripts/vm/hypervisor/xenserver/s3xen.py PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml cd4fbbe 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/servlet/ConsoleProxyPasswordBasedEncryptor.java 2638c8b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py bd8a5b2 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 2e4e74f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/
-----------------------------------------------------------

(Updated Nov. 19, 2012, 5:16 p.m.)


Review request for cloudstack.


Changes
-------

Adds missing license headers to api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java  and 
api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java 


Description
-------

Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.

For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).


This addresses bug CLOUDSTACK-509.


Diffs (updated)
-----

  .gitignore 4661ea2 
  api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
  api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
  api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
  api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
  api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
  api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
  api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
  api/src/com/cloud/api/ApiConstants.java 78a3ded 
  api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
  api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
  api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
  api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
  api/src/com/cloud/resource/ResourceService.java 1065453 
  api/src/com/cloud/storage/S3.java PRE-CREATION 
  build/package.xml 09ed939 
  client/WEB-INF/classes/resources/messages.properties 626e44a 
  client/tomcatconf/commands.properties.in 149547e 
  console-proxy/scripts/_run.sh e408378 
  core/pom.xml 15f0f7b 
  core/src/com/cloud/storage/S3VO.java PRE-CREATION 
  core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
  core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
  core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
  patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
  plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
  pom.xml 08f61f5 
  scripts/vm/hypervisor/xenserver/s3xen.py PRE-CREATION 
  scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
  scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
  scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
  server/pom.xml cd4fbbe 
  server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
  server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
  server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
  server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
  server/src/com/cloud/configuration/Config.java 66ac276 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
  server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
  server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
  server/src/com/cloud/servlet/ConsoleProxyPasswordBasedEncryptor.java 2638c8b 
  server/src/com/cloud/storage/StorageManagerImpl.java e252633 
  server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
  server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
  server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
  server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
  server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
  server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
  server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
  server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
  server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
  server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
  server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
  server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
  setup/db/create-schema.sql fff084e 
  setup/db/db/schema-40to410.sql PRE-CREATION 
  tools/apidoc/gen_toc.py eeaf2a2 
  tools/marvin/marvin/cloudstackConnection.py bd8a5b2 
  tools/marvin/marvin/deployDataCenter.py bdf08cc 
  ui/dictionary.jsp b80e296 
  ui/scripts/cloudStack.js de3bd73 
  ui/scripts/sharedFunctions.js b6b3ef8 
  ui/scripts/system.js 9e3932f 
  ui/scripts/templates.js 74511f0 
  utils/pom.xml 2e4e74f 
  utils/src/com/cloud/utils/DateUtil.java be1627d 
  utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
  utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
  utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 

Diff: https://reviews.apache.org/r/8123/diff/


Testing
-------

I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:

   * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
   * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
   * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.


Screenshots
-----------

S3 Configuration Form
  https://reviews.apache.org/r/8123/s/13/
S3 Enable Menu on the Zone Tab
  https://reviews.apache.org/r/8123/s/14/


Thanks,

John Burwell


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.

> On Nov. 19, 2012, 1:51 p.m., David Nalley wrote:
> > utils/pom.xml, line 106
> > <https://reviews.apache.org/r/8123/diff/1/?file=217024#file217024line106>
> >
> >     How is this dependency licensed?

The Amazon AWS SDK for Java is licensed under the Apache 2.0 license.  (http://aws.amazon.com/sdkforjava/faqs/).  I have also added a section to the design document regarding licensing (


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13564
-----------------------------------------------------------


On Nov. 19, 2012, 5:16 p.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2012, 5:16 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 4661ea2 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   console-proxy/scripts/_run.sh e408378 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 08f61f5 
>   scripts/vm/hypervisor/xenserver/s3xen.py PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml cd4fbbe 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/servlet/ConsoleProxyPasswordBasedEncryptor.java 2638c8b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py bd8a5b2 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 2e4e74f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by David Nalley <da...@gnsa.us>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/#review13564
-----------------------------------------------------------



api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java
<https://reviews.apache.org/r/8123/#comment29118>

    Need an ASF license header here



api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java
<https://reviews.apache.org/r/8123/#comment29119>

    Need an ASF license header here



utils/pom.xml
<https://reviews.apache.org/r/8123/#comment29120>

    How is this dependency licensed? 


- David Nalley


On Nov. 19, 2012, 6:37 a.m., John Burwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8123/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2012, 6:37 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.
> 
> For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).
> 
> 
> This addresses bug CLOUDSTACK-509.
> 
> 
> Diffs
> -----
> 
>   .gitignore 9daca15 
>   api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
>   api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
>   api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
>   api/src/com/cloud/api/ApiConstants.java 78a3ded 
>   api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
>   api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
>   api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
>   api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
>   api/src/com/cloud/resource/ResourceService.java 1065453 
>   api/src/com/cloud/storage/S3.java PRE-CREATION 
>   build/package.xml 09ed939 
>   client/WEB-INF/classes/resources/messages.properties 626e44a 
>   client/tomcatconf/commands.properties.in 149547e 
>   console-proxy/scripts/_run.sh e408378 
>   core/pom.xml 15f0f7b 
>   core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 2dc7950 
>   core/src/com/cloud/storage/S3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
>   core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
>   patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
>   pom.xml 08f61f5 
>   scripts/vm/hypervisor/xenserver/s3xen.py PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
>   server/pom.xml cd4fbbe 
>   server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
>   server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
>   server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
>   server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
>   server/src/com/cloud/configuration/Config.java 66ac276 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
>   server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
>   server/src/com/cloud/servlet/ConsoleProxyPasswordBasedEncryptor.java 2638c8b 
>   server/src/com/cloud/storage/StorageManagerImpl.java e252633 
>   server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
>   server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
>   server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
>   server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
>   server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
>   server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
>   server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
>   server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
>   setup/db/create-schema.sql fff084e 
>   setup/db/db/schema-40to410.sql PRE-CREATION 
>   tools/apidoc/gen_toc.py eeaf2a2 
>   tools/marvin/marvin/cloudstackConnection.py bd8a5b2 
>   tools/marvin/marvin/deployDataCenter.py bdf08cc 
>   ui/dictionary.jsp b80e296 
>   ui/scripts/cloudStack.js de3bd73 
>   ui/scripts/sharedFunctions.js b6b3ef8 
>   ui/scripts/system.js 9e3932f 
>   ui/scripts/templates.js 74511f0 
>   utils/pom.xml 2e4e74f 
>   utils/src/com/cloud/utils/DateUtil.java be1627d 
>   utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
>   utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
>   utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
>   utils/src/com/cloud/utils/nio/NioConnection.java 50e6a88 
> 
> Diff: https://reviews.apache.org/r/8123/diff/
> 
> 
> Testing
> -------
> 
> I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:
> 
>    * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
>    * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
>    * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.
> 
> 
> Screenshots
> -----------
> 
> S3 Configuration Form
>   https://reviews.apache.org/r/8123/s/13/
> S3 Enable Menu on the Zone Tab
>   https://reviews.apache.org/r/8123/s/14/
> 
> 
> Thanks,
> 
> John Burwell
> 
>


Re: Review Request: S3-backed Secondary Storage

Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8123/
-----------------------------------------------------------

(Updated Nov. 19, 2012, 6:37 a.m.)


Review request for cloudstack.


Description
-------

Backs NFS-based secondary storage with an S3-compatible object store. Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS secondary storage mount with a configured S3 object store. It also pushes snapshots to the object store when they are created and downloads them in other zones on-demand. In addition to permitting the use of commodity or IaaS storage solutions for static assets, it provides a means of automatically synchronizing template and ISO assets across multiple zones.

For more information about the design of the patch, please see the design document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed+Secondary+Storage).


This addresses bug CLOUDSTACK-509.


Diffs
-----

  .gitignore 9daca15 
  api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d 
  api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b 
  api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATION 
  api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREATION 
  api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCommand.java PRE-CREATION 
  api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageCommand.java PRE-CREATION 
  api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION 
  api/src/com/cloud/api/ApiConstants.java 78a3ded 
  api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 
  api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION 
  api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION 
  api/src/com/cloud/api/response/S3Response.java PRE-CREATION 
  api/src/com/cloud/resource/ResourceService.java 1065453 
  api/src/com/cloud/storage/S3.java PRE-CREATION 
  build/package.xml 09ed939 
  client/WEB-INF/classes/resources/messages.properties 626e44a 
  client/tomcatconf/commands.properties.in 149547e 
  console-proxy/scripts/_run.sh e408378 
  core/pom.xml 15f0f7b 
  core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 2dc7950 
  core/src/com/cloud/storage/S3VO.java PRE-CREATION 
  core/src/com/cloud/storage/SnapshotVO.java 08dfafa 
  core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION 
  core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 155210d 
  patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 
  plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java d2db85c 
  pom.xml 08f61f5 
  scripts/vm/hypervisor/xenserver/s3xen.py PRE-CREATION 
  scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 
  scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 
  scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 
  server/pom.xml cd4fbbe 
  server/src/com/cloud/api/ApiDBUtils.java 3b5f634 
  server/src/com/cloud/api/ApiDispatcher.java dfe4a1f 
  server/src/com/cloud/api/ApiResponseHelper.java ebe8415 
  server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a 
  server/src/com/cloud/configuration/Config.java 66ac276 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e8 
  server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 
  server/src/com/cloud/resource/ResourceManagerImpl.java ced601b 
  server/src/com/cloud/servlet/ConsoleProxyPasswordBasedEncryptor.java 2638c8b 
  server/src/com/cloud/storage/StorageManagerImpl.java e252633 
  server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION 
  server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION 
  server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 
  server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 
  server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION 
  server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION 
  server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION 
  server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION 
  server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e 
  server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 
  server/src/com/cloud/template/S3SyncTask.java PRE-CREATION 
  server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 
  setup/db/create-schema.sql fff084e 
  setup/db/db/schema-40to410.sql PRE-CREATION 
  tools/apidoc/gen_toc.py eeaf2a2 
  tools/marvin/marvin/cloudstackConnection.py bd8a5b2 
  tools/marvin/marvin/deployDataCenter.py bdf08cc 
  ui/dictionary.jsp b80e296 
  ui/scripts/cloudStack.js de3bd73 
  ui/scripts/sharedFunctions.js b6b3ef8 
  ui/scripts/system.js 9e3932f 
  ui/scripts/templates.js 74511f0 
  utils/pom.xml 2e4e74f 
  utils/src/com/cloud/utils/DateUtil.java be1627d 
  utils/src/com/cloud/utils/S3Utils.java PRE-CREATION 
  utils/src/com/cloud/utils/StringUtils.java 0f0ef05 
  utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 
  utils/src/com/cloud/utils/nio/NioConnection.java 50e6a88 

Diff: https://reviews.apache.org/r/8123/diff/


Testing
-------

I am submitting patch to begin the feedback process while we complete integration testing.  I have verified that it does not interfere with normal CloudStack operations when S3-backed Secondary Storage is disabled (the default setting) .  I have successfully tested operation of single zone template and ISO scenarios on devcloud described in the design document.  I am currently working through some issues in our multi-zone test environment to complete all scenarios described.  The following are the known deficiencies of the current implementation which I plan to correct in a subsequent patch:

   * Cross zone garbage collection: When a global asset is deleted from one zone's secondary storage, it is not deleted from the secondary storage of other zones which have downloaded it from the object store
   * S3 Configuration Update: The API only supports adding an object store configuration.  Users should be able to edit the access key, secret key, connection timeout. max error retries, and socket timeout.
   * Multi-threaded Uploads: Permit the upload of multiple assets to the object store simultaneously to decrease the propagation latency across all zones.


Screenshots
-----------

S3 Configuration Form
  https://reviews.apache.org/r/8123/s/13/
S3 Enable Menu on the Zone Tab
  https://reviews.apache.org/r/8123/s/14/


Thanks,

John Burwell