We can use the same ids & names, but we don't have to use the same ids if we use names, which is a little easier because names are user readable but ids are not, so we don't need to memorize/check all the ids when we add new regions in multiple regions, which can be confusing.
Thanks Alex Ough On Tue, Jun 24, 2014 at 8:33 PM, Alena Prokharchyk < alena.prokharc...@citrix.com> wrote: > Aren’t we supposed to sync the regions across the multiple regions Dbs? > Because that’s what region FS states: > > > https://cwiki.apache.org/confluence/display/CLOUDSTACK/AWS-Style+Regions+Functional+Spec, > “Adding 2nd region” paragraph, bullet #4: > > 1. Install a 2nd CS instance. > > 2. While installing database set region_id using -r option in > cloud-setup-databases script (Make sure *database_key* is same across all > regions). > > *cloud-setup-databases cloud:**<**dbpassword**>**@localhost > --deploy-as=root:**<**password**>** -e **<**encryption_type**>** -m **<* > *management_server_key**>** -k **<**database_key**> **-r <region_id>* > > 3. Start mgmt server > > 4. *Using addRegion API, add region 1 to region 2 and also region 2 to > region 1.* > > > I assume that we expect the admin to add the region with the same name > and the same id to ALL regions Dbs (both id and name should be passed to > createRegion call). So they are all in sync. Isn’t it the requirement? If > so, we should rely on the fact that all regions Dbs will have the same set > of regions having the same ids and names cross regions. > > > Thanks, > > Alena. > From: Alex Ough <alex.o...@sungardas.com> > Date: Tuesday, June 24, 2014 at 5:17 PM > To: Alena Prokharchyk <alena.prokharc...@citrix.com> > Cc: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>, Kishan > Kavala <kishan.kav...@citrix.com>, Murali Reddy <murali.re...@citrix.com>, > Ram Ganesh <ram.gan...@citrix.com>, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> > > Subject: Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > What I'm trying to say is that when we pass the ids of regions, the > receivers do not know what the originated region is and the id of each > region is not same across all the regions. > > Thanks > Alex Ough > > > On Tue, Jun 24, 2014 at 7:35 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > >> Alex, thank you for summarizing. >> >> I still don’t see why id can’t be unique across regions as you can >> control the id assignment – id is required when createRegion call is made. >> And that’s how the region should be represented in other region’s Dbs – by >> its id that is unique across the regions. Kishan/Murali, please confirm. >> >> Thank you, >> Alena. >> >> From: Alex Ough <alex.o...@sungardas.com> >> Date: Tuesday, June 24, 2014 at 4:22 PM >> To: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org> >> Cc: Alena Prokharchyk <alena.prokharc...@citrix.com>, Kishan Kavala < >> kishan.kav...@citrix.com>, Murali Reddy <murali.re...@citrix.com>, Ram >> Ganesh <ram.gan...@citrix.com>, Animesh Chaturvedi < >> animesh.chaturv...@citrix.com> >> >> Subject: Re: Review Request 20099: Domain-Account-User Sync Up Among >> Multiple Regions (Core Changes) >> >> All, >> >> There is one open question in this topic, which is to figure out which >> value is appropriate to pass the region object, id or name? >> During this implementation, we decided to add the information of regions >> where user/account/domain objects have been originally >> created/modified/removed. >> But the ids of regions are not same across the regions and currently the >> regions do not have uuids(they will not be same either if we add them to >> regions), so I'd like to use names. >> >> Please let me know what you think. >> Thanks >> Alex Ough >> >> >> On Tue, Jun 24, 2014 at 7:05 PM, Animesh Chaturvedi < >> animesh.chaturv...@citrix.com> wrote: >> >>> Let’s have the discussion on dev mailing list >>> >>> >>> >>> Thanks >>> >>> Animesh >>> >>> >>> >>> *From:* Alena Prokharchyk >>> *Sent:* Tuesday, June 24, 2014 3:06 PM >>> *To:* Alex Ough; Kishan Kavala; Murali Reddy >>> *Cc:* Animesh Chaturvedi; Ram Ganesh >>> >>> *Subject:* Re: Review Request 20099: Domain-Account-User Sync Up Among >>> Multiple Regions (Core Changes) >>> >>> >>> >>> Adding Kishan to the thread as he was the one who implemented the region >>> feature originally. >>> >>> >>> >>> Kishan, in a situation when there are 2 regions in the system, we expect >>> “region” table to be populated with the same id/name in both Dbs for both >>> regions, right? So my question is – what uniquely identifies the region in >>> CS system in cross region setup – id/name? >>> >>> >>> >>> That unique identifier should be the value that is passed to the calls >>> you modify, Alex. WE can’t just pass some random name to the call without >>> making any further verification. >>> >>> >>> >>> Kishan/Murali, please help to verify this part of Alex’s fix as it >>> should really be someone with an expertise in Regions. I’ve reviewed the >>> rest of the feature, just this one item is open. See my latest comment to >>> the https://reviews.apache.org/r/17790/diff/?page=1#0 as well as refer >>> to this email thread for the context. >>> >>> >>> >>> -Alena. >>> >>> >>> >>> *From: *Alena Prokharchyk <alena.prokharc...@citrix.com> >>> *Date: *Tuesday, June 24, 2014 at 2:54 PM >>> *To: *Alex Ough <alex.o...@sungardas.com> >>> *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among >>> Multiple Regions (Core Changes) >>> >>> >>> >>> That what would everybody assume 100% just by looking at the parameter >>> description and parameter – that you refer to region UUID : "Region where >>> this account is created.”/ORIGINATEDREGIONUUID >>> >>> In CS the UUID has a special meaning. It has to have the UUID format, >>> and its randomly generated value that is stored in the DB along with the >>> actual db id. I can see that regionVO lacks UUID field. Looks like existing >>> RegionVO object lacks this filed unlike other CS objects (uservm, etc). I >>> will follow up with Murali on that. >>> >>> >>> >>> Alex, so originatedRegionUUID refers to the region name, correct?. Why >>> don’t use the region id instead? That’s what we do when refer to CS objects >>> – we always refer to them by id which is unique. Which is true even for the >>> region: >>> >>> >>> >>> mysql> show create table region; >>> >>> >>> >>> UNIQUE KEY `id` (`id`), >>> >>> UNIQUE KEY `name` (`name`) >>> >>> >>> >>> >>> >>> That’s what you do when you manipulate the region itself >>> (delete/updateRegion) - refer to the region by its id. And this field is >>> returned to you when you call listRegions API: >>> >>> >>> >>> http://localhost:8096/?command=listRegions >>> >>> <region> >>> >>> <id>1</id> >>> >>> <name>Local</name> >>> >>> <endpoint>http://localhost:8080/client/</endpoint> >>> >>> <gslbserviceenabled>true</gslbserviceenabled> >>> >>> <portableipserviceenabled>false</portableipserviceenabled> >>> >>> </region> >>> >>> >>> >>> >>> >>> Please correct if I miss something. >>> >>> -Alena. >>> >>> >>> >>> *From: *Alex Ough <alex.o...@sungardas.com> >>> *Date: *Tuesday, June 24, 2014 at 2:33 PM >>> *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> >>> *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among >>> Multiple Regions (Core Changes) >>> >>> >>> >>> Thanks for the clarification, but here is a thing. >>> >>> >>> >>> I'm passing names as the values of originatedRegionUuids because the >>> uuids are randomly generated and the same regions do NOT have the same >>> uuidss. >>> >>> So I'd like to change the parameter types into String. >>> >>> Let me know if you think otherwise. >>> >>> >>> >>> Thanks >>> >>> Alex Ough >>> >>> >>> >>> On Tue, Jun 24, 2014 at 5:17 PM, Alena Prokharchyk < >>> alena.prokharc...@citrix.com> wrote: >>> >>> Alex, >>> >>> >>> >>> take a look at ParamProcessWorker class, and how API parameters are >>> being dispatched/verified. >>> >>> >>> >>> >>> >>> 1) public void processParameters(final BaseCmd cmd, final Map params) >>> method >>> >>> >>> >>> First of all, EntityType parameter should be defined in the @Parameter >>> annotation for the originatedRegionID field. This parameter is used by >>> paramProcessWorker to make "if entity exists" validation >>> >>> >>> >>> >>> >>> 2) Check another method in the same class: >>> >>> >>> >>> private void setFieldValue(final Field field, final BaseCmd cmdObj, >>> final Object paramObj, final Parameter annotation) throws >>> IllegalArgumentException, ParseException { >>> >>> >>> >>> Thats the method responsible for dispatching/setting the field values. >>> Here is the snippet of the code for the case when UUID is defined: >>> >>> >>> >>> case UUID: >>> >>> if (paramObj.toString().isEmpty()) >>> >>> break; >>> >>> final Long internalId = >>> translateUuidToInternalId(paramObj.toString(), annotation); >>> >>> field.set(cmdObj, internalId); >>> >>> break; >>> >>> >>> >>> it always transforms the UUID to Long id, not string. And at the end, it >>> will be internal DB UUID, not the UUID. If you need the UUID, you have to >>> get it by a) retrieving the object from the DB by id b) Getting its UUID >>> property. >>> >>> >>> >>> If you leave it as a String, you will hit IllegalArgumentException at >>> "field.set(cmdObj, internalId);" line. >>> >>> >>> >>> >>> >>> Hope it answers your questions, and let me know if anything else needs >>> to be clarified. >>> >>> >>> >>> -alena. >>> >>> >>> >>> *From: *Alex Ough <alex.o...@sungardas.com> >>> *Date: *Tuesday, June 24, 2014 at 1:57 PM >>> >>> >>> *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> >>> *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among >>> Multiple Regions (Core Changes) >>> >>> >>> >>> Why do you want to change UUID to 'Long'? >>> >>> Can you just correct what I fixed? >>> >>> >>> >>> On Tue, Jun 24, 2014 at 4:21 PM, Alena Prokharchyk < >>> alena.prokharc...@citrix.com> wrote: >>> >>> You need to put: >>> >>> >>> >>> * the entityType parameter to the annotation. >>> >>> - Change the type to Long as I’ve already mentioned. Check how other >>> commands handle the parameters (networkId, vpcId, etc) >>> >>> —Alena. >>> >>> >>> >>> *From: *Alex Ough <alex.o...@sungardas.com> >>> *Date: *Tuesday, June 24, 2014 at 12:47 PM >>> >>> >>> *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> >>> *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among >>> Multiple Regions (Core Changes) >>> >>> >>> >>> Will this change work? >>> >>> >>> >>> @Parameter(name = ApiConstants.ORIGINATED_REGION_ID, type = >>> CommandType.UUID, description = "Region UUID where this account is >>> created.", since = "4.5") >>> >>> private String originatedRegionUUID; >>> >>> >>> >>> >>> >>> On Tue, Jun 24, 2014 at 3:25 PM, Alex Ough <alex.o...@sungardas.com> >>> wrote: >>> >>> Alena, >>> >>> >>> >>> This is what really frustrates me, but can you give the final items >>> instead of keeping adding more items whenever I post a review, please? >>> >>> Can you gurantee that this is the only item you want me to fix? >>> >>> >>> >>> On Tue, Jun 24, 2014 at 3:04 PM, Alena Prokharchyk < >>> alena.prokharc...@citrix.com> wrote: >>> >>> Alex, as a part of the fix, also change the param name to be regionId >>> (there should be a value in apiconstants already) as the parameter really >>> reflects CS region object, and we usually refer to those as networkID, >>> vpcID (not uuid) although uuid are passed in. Check if the rest of the api >>> changes you've done, respect this rule. Sorry, out of the office now and >>> cant check myself if there are any. >>> >>> -alena >>> >>> >>> > On Jun 24, 2014, at 11:12 AM, "Alena Prokharchyk" < >>> alena.prokharc...@citrix.com> wrote: >>> > >>> > >>> > ----------------------------------------------------------- >>> >>> > This is an automatically generated e-mail. To reply, visit: >>> >>> > https://reviews.apache.org/r/20099/#review46557 >>> > ----------------------------------------------------------- >>> >>> > >>> > >>> > Alex, one small thing. >>> > >>> > Just noticed that in the API commands you pass regionUUID as a string. >>> You should pass it as a type of UUID and specify the entityType parameter >>> in @Parameter so the entity validation is done correctly. Example: >>> > >>> > @Parameter(name=ApiConstants.ZONE_ID, type=CommandType.UUID, >>> entityType = ZoneResponse.class, >>> > required=true, description="the Zone ID for the network") >>> > private Long zoneId; >>> > >>> > That is the rule when passing id/uuid of the first class CS object to >>> the API call >>> > >>> > Then be aware of the fact that the APIDispatcher will transform UUID >>> to the actual DB id, and that would be the Id that you pass to the services >>> call. If what you need is UUID, not the actual id, to be saved in the >>> callContext, you have to transform it explicitly. >>> > >>> > - Alena Prokharchyk >>> > >>> > >>> >>> >> On June 24, 2014, 3:54 p.m., Alex Ough wrote: >>> >> >>> >> ----------------------------------------------------------- >>> >>> >> This is an automatically generated e-mail. To reply, visit: >>> >> https://reviews.apache.org/r/20099/ >>> >>> >> ----------------------------------------------------------- >>> >> >>> >> (Updated June 24, 2014, 3:54 p.m.) >>> >> >>> >> >>> >> Review request for cloudstack. >>> >> >>> >> >>> >> Repository: cloudstack-git >>> >> >>> >> >>> >> Description >>> >> ------- >>> >>> >> >>> >> This is the review request for the core changes related with #17790 >>> that has only the new plugin codes. >>> >> >>> >> >>> >>> >> Diffs >>> >> ----- >>> >> >>> >> api/src/com/cloud/event/EventTypes.java 0fa3cd5 >>> >>> >> api/src/com/cloud/user/AccountService.java eac8a76 >>> >> api/src/com/cloud/user/DomainService.java 4c1f93d >>> >> api/src/org/apache/cloudstack/api/ApiConstants.java adda5f4 >>> >> api/src/org/apache/cloudstack/api/BaseCmd.java ac9a208 >>> >> >>> >>> api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java >>> 50d67d9 >>> >> >>> >>> api/src/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java >>> 5754ec5 >>> >> >>> >>> api/src/org/apache/cloudstack/api/command/admin/account/DisableAccountCmd.java >>> 3e5e1d3 >>> >> >>> >>> api/src/org/apache/cloudstack/api/command/admin/account/EnableAccountCmd.java >>> f30c985 >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/account/LockAccountCmd.java >>> 3c185e4 >>> >> >>> >>> api/src/org/apache/cloudstack/api/command/admin/account/UpdateAccountCmd.java >>> a7ce74a >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/domain/CreateDomainCmd.java >>> 312c9ee >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/domain/DeleteDomainCmd.java >>> a6d2b0b >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/domain/UpdateDomainCmd.java >>> 409a84d >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/region/AddRegionCmd.java >>> f6743ba >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/region/UpdateRegionCmd.java >>> b08cbbb >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java >>> 8f223ac >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/user/DeleteUserCmd.java >>> 08ba521 >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/user/DisableUserCmd.java >>> c6e09ef >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/user/EnableUserCmd.java >>> d69eccf >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/user/LockUserCmd.java >>> 69623d0 >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/user/RegisterCmd.java >>> 2090d21 >>> >> >>> api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java >>> f21e264 >>> >> api/src/org/apache/cloudstack/api/response/RegionResponse.java >>> 6c74fa6 >>> >> api/src/org/apache/cloudstack/region/Region.java df64e44 >>> >> api/src/org/apache/cloudstack/region/RegionService.java afefcc7 >>> >> api/test/org/apache/cloudstack/api/command/test/RegionCmdTest.java >>> 10c3d85 >>> >> client/pom.xml 29fef4f >>> >> >>> >>> engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml >>> 2ef0d20 >>> >> engine/schema/src/com/cloud/user/AccountVO.java 0f5a044 >>> >> engine/schema/src/org/apache/cloudstack/region/RegionVO.java 608bd2b >>> >> >>> >>> plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java >>> 4136b5c >>> >> plugins/pom.xml b5e6a61 >>> >> >>> >>> plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java >>> b753952 >>> >> >>> >>> plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java >>> 6f7be90 >>> >>> >> server/src/com/cloud/api/ApiResponseHelper.java f1f0d2c >>> >> server/src/com/cloud/api/dispatch/ParamProcessWorker.java 1592b93 >>> >> server/src/com/cloud/event/ActionEventUtils.java 2b3cfea >>> >> server/src/com/cloud/projects/ProjectManagerImpl.java d10c059 >>> >> server/src/com/cloud/user/AccountManager.java 194c5d2 >>> >> server/src/com/cloud/user/AccountManagerImpl.java 7a889f1 >>> >> server/src/com/cloud/user/DomainManager.java f72b18a >>> >> server/src/com/cloud/user/DomainManagerImpl.java fbbe0c2 >>> >> server/src/org/apache/cloudstack/region/RegionManager.java 6f25481 >>> >> server/src/org/apache/cloudstack/region/RegionManagerImpl.java >>> 8910714 >>> >> server/src/org/apache/cloudstack/region/RegionServiceImpl.java >>> 98cf500 >>> >> server/test/com/cloud/user/AccountManagerImplTest.java 176cf1d >>> >> server/test/com/cloud/user/MockAccountManagerImpl.java 746fa1b >>> >> server/test/com/cloud/user/MockDomainManagerImpl.java 7dddefb >>> >> server/test/org/apache/cloudstack/region/RegionManagerTest.java >>> d7bc537 >>> >> setup/db/db/schema-440to450.sql ee419a2 >>> >> ui/scripts/regions.js 368c1bf >>> >> >>> >> Diff: https://reviews.apache.org/r/20099/diff/ >>> >> >>> >> >>> >> Testing >>> >> ------- >>> >> >>> >>> >> 1. Successfully tested real time synchronization as soon as resources >>> are created/deleted/modified in one region. >>> >> 2. Successfully tested full scans to synchronize resources that were >>> missed during real time synchronization because of any reasons like network >>> connection issues. >>> >> 3. The tests were done manually and also automatically by randomly >>> generating changes each region. >>> >> >>> >> >>> >>> >> Thanks, >>> >> >>> >> Alex Ough >>> > >>> >>> >>> >>> >>> >>> >>> >>> >>> >> >> >
