> 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 > >
