diff --git a/common/types/shared.go b/common/types/shared.go index 756f02ea666..2cb8cbb6f32 100644 --- a/common/types/shared.go +++ b/common/types/shared.go @@ -1710,6 +1710,13 @@ type DescribeDomainResponse struct { FailoverInfo *FailoverInfo `json:"failoverInfo,omitempty"` } +func (v *DomainReplicationConfiguration) GetActiveClusters() (o *ActiveClusters) { + if v != nil && v.ActiveClusters != nil { + return v.ActiveClusters + } + return +} + // GetDomainInfo is an internal getter (TBD...) func (v *DescribeDomainResponse) GetDomainInfo() (o *DomainInfo) { if v != nil && v.DomainInfo != nil { @@ -2253,6 +2260,20 @@ type DomainReplicationConfiguration struct { ActiveClusters *ActiveClusters `json:"activeClusters,omitempty"` } +func (v *DomainReplicationConfiguration) IsActiveActiveDomain() bool { + if v == nil || v.ActiveClusters == nil { + return false + } + if v.ActiveClusters.AttributeScopes != nil && len(v.ActiveClusters.AttributeScopes) > 0 { + return true + } + // todo (david.porter) remove this once we have fully migrated to AttributeScopes + if v.ActiveClusters.ActiveClustersByRegion != nil && len(v.ActiveClusters.ActiveClustersByRegion) > 0 { + return true + } + return false +} + // GetActiveClusterName is an internal getter (TBD...) func (v *DomainReplicationConfiguration) GetActiveClusterName() (o string) { if v != nil { @@ -2269,13 +2290,6 @@ func (v *DomainReplicationConfiguration) GetClusters() (o []*ClusterReplicationC return } -func (v *DomainReplicationConfiguration) GetActiveClusters() (o *ActiveClusters) { - if v != nil && v.ActiveClusters != nil { - return v.ActiveClusters - } - return -} - // ByteSize returns the approximate memory used in bytes func (v *DomainReplicationConfiguration) ByteSize() uint64 { if v == nil { diff --git a/common/types/shared_test.go b/common/types/shared_test.go index af22c29be70..eb5c074914a 100644 --- a/common/types/shared_test.go +++ b/common/types/shared_test.go @@ -120,6 +120,77 @@ func TestActiveClustersConfigDeepCopy(t *testing.T) { } } +func TestIsActiveActiveDomain(t *testing.T) { + tests := []struct { + name string + activeClusters *DomainReplicationConfiguration + want bool + }{ + { + name: "empty DomainReplicationConfiguration should return false", + activeClusters: &DomainReplicationConfiguration{}, + want: false, + }, + { + name: "nil receiver should return false", + activeClusters: nil, + want: false, + }, + { + name: "empty ActiveClusters should return false", + activeClusters: &DomainReplicationConfiguration{ActiveClusters: &ActiveClusters{}}, + want: false, + }, + { + name: "ActiveClusters with only old format populated should return true", + activeClusters: &DomainReplicationConfiguration{ + ActiveClusters: &ActiveClusters{ + ActiveClustersByRegion: map[string]ActiveClusterInfo{ + "us-east-1": {ActiveClusterName: "cluster1"}, + }, + }, + }, + want: true, + }, + { + name: "ActiveClusters with only new format populated should return true", + activeClusters: &DomainReplicationConfiguration{ + ActiveClusters: &ActiveClusters{ + AttributeScopes: map[string]ClusterAttributeScope{ + "region": {ClusterAttributes: map[string]ActiveClusterInfo{ + "us-east-1": {ActiveClusterName: "cluster1"}, + }}, + }, + }, + }, + want: true, + }, + { + name: "ActiveClusters with both formats populated should return true", + activeClusters: &DomainReplicationConfiguration{ + ActiveClusters: &ActiveClusters{ + ActiveClustersByRegion: map[string]ActiveClusterInfo{ + "us-east-1": {ActiveClusterName: "cluster1"}, + }, + AttributeScopes: map[string]ClusterAttributeScope{ + "region": {ClusterAttributes: map[string]ActiveClusterInfo{ + "us-east-1": {ActiveClusterName: "cluster1"}, + }}, + }, + }, + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.activeClusters.IsActiveActiveDomain() + assert.Equal(t, tt.want, got) + }) + } +} + // identicalByteArray returns true if a and b are the same slice, false otherwise. func identicalByteArray(a, b []byte) bool { return len(a) == len(b) && unsafe.SliceData(a) == unsafe.SliceData(b) diff --git a/tools/cli/domain_commands.go b/tools/cli/domain_commands.go index 141f2cf1d60..9186963da67 100644 --- a/tools/cli/domain_commands.go +++ b/tools/cli/domain_commands.go @@ -27,6 +27,7 @@ import ( "errors" "fmt" "os" + "regexp" "strconv" "strings" "time" @@ -112,16 +113,6 @@ func (d *domainCLIImpl) RegisterDomain(c *cli.Context) error { return commoncli.Problem(fmt.Sprintf("Option %s format is invalid.", FlagIsGlobalDomain), err) } } - isActiveActiveDomain := false - if c.IsSet(FlagIsActiveActiveDomain) { - isActiveActiveDomain, err = strconv.ParseBool(c.String(FlagIsActiveActiveDomain)) - if err != nil { - return commoncli.Problem(fmt.Sprintf("Option %s format is invalid.", FlagIsActiveActiveDomain), err) - } - - // Also set isGlobalDomain to true if it is active-active domain - isGlobalDomain = isActiveActiveDomain - } var domainData *flag.StringMap if c.IsSet(FlagDomainData) { @@ -136,17 +127,9 @@ func (d *domainCLIImpl) RegisterDomain(c *cli.Context) error { activeClusterName := "" if c.IsSet(FlagActiveClusterName) { - if isActiveActiveDomain { - return commoncli.Problem("Option --active_cluster is not supported for active-active domain. Use --active_clusters_by_region instead.", nil) - } activeClusterName = c.String(FlagActiveClusterName) } - activeClustersByRegion, err := parseActiveClustersByRegion(c, isActiveActiveDomain) - if err != nil { - return err - } - var clusters []*types.ClusterReplicationConfiguration if c.IsSet(FlagClusters) { for _, clusterStr := range c.StringSlice(FlagClusters) { @@ -165,6 +148,15 @@ func (d *domainCLIImpl) RegisterDomain(c *cli.Context) error { return fmt.Errorf("failed to parse %s flag: %w", FlagVisibilityArchivalStatus, err) } + var activeClusters *types.ActiveClusters + if c.IsSet(FlagActiveClusters) { + ac, err := parseActiveClustersByClusterAttribute(c.String(FlagActiveClusters)) + if err != nil { + return err + } + activeClusters = &ac + } + request := &types.RegisterDomainRequest{ Name: domainName, Description: description, @@ -173,7 +165,7 @@ func (d *domainCLIImpl) RegisterDomain(c *cli.Context) error { WorkflowExecutionRetentionPeriodInDays: int32(retentionDays), Clusters: clusters, ActiveClusterName: activeClusterName, - ActiveClustersByRegion: activeClustersByRegion, + ActiveClusters: activeClusters, SecurityToken: securityToken, HistoryArchivalStatus: has, HistoryArchivalURI: c.String(FlagHistoryArchivalURI), @@ -225,23 +217,16 @@ func (d *domainCLIImpl) UpdateDomain(c *cli.Context) error { ActiveClusterName: common.StringPtr(activeCluster), FailoverTimeoutInSeconds: failoverTimeout, } - } else if c.IsSet(FlagActiveClustersByRegion) { // active-active domain failover - activeClustersByRegion, err := parseActiveClustersByRegion(c, true) + } else if c.IsSet(FlagActiveClusters) { // active-active domain failover + activeClusters, err := parseActiveClustersByClusterAttribute(c.String(FlagActiveClusters)) if err != nil { return err } - acbr := make(map[string]types.ActiveClusterInfo) - for region, cluster := range activeClustersByRegion { - acbr[region] = types.ActiveClusterInfo{ - ActiveClusterName: cluster, - } - } - updateRequest = &types.UpdateDomainRequest{ Name: domainName, ActiveClusters: &types.ActiveClusters{ - ActiveClustersByRegion: acbr, + AttributeScopes: activeClusters.AttributeScopes, }, } } else { @@ -634,26 +619,27 @@ type ActiveClusterInfoRow struct { } type DomainRow struct { - Name string `header:"Name"` - UUID string `header:"UUID"` - Description string - OwnerEmail string - DomainData map[string]string `header:"Domain Data"` - Status types.DomainStatus `header:"Status"` - IsGlobal bool `header:"Is Global Domain"` - ActiveCluster string `header:"Active Cluster"` - Clusters []string `header:"Clusters"` - RetentionDays int32 `header:"Retention Days"` - EmitMetrics bool - HistoryArchivalStatus types.ArchivalStatus `header:"History Archival Status"` - HistoryArchivalURI string `header:"History Archival URI"` - VisibilityArchivalStatus types.ArchivalStatus `header:"Visibility Archival Status"` - VisibilityArchivalURI string `header:"Visibility Archival URI"` - BadBinaries []BadBinaryRow - FailoverInfo *FailoverInfoRow - LongRunningWorkFlowNum *int - IsActiveActiveDomain bool - ActiveClustersByRegion []ActiveClusterInfoRow + Name string `header:"Name"` + UUID string `header:"UUID"` + Description string + OwnerEmail string + DomainData map[string]string `header:"Domain Data"` + Status types.DomainStatus `header:"Status"` + IsGlobal bool `header:"Is Global Domain"` + ActiveCluster string `header:"Active Cluster"` + Clusters []string `header:"Clusters"` + RetentionDays int32 `header:"Retention Days"` + EmitMetrics bool + HistoryArchivalStatus types.ArchivalStatus `header:"History Archival Status"` + HistoryArchivalURI string `header:"History Archival URI"` + VisibilityArchivalStatus types.ArchivalStatus `header:"Visibility Archival Status"` + VisibilityArchivalURI string `header:"Visibility Archival URI"` + BadBinaries []BadBinaryRow + FailoverInfo *FailoverInfoRow + LongRunningWorkFlowNum *int + IsActiveActiveDomain bool + ActiveClustersByRegion []ActiveClusterInfoRow // todo (david.porter) remove this as it's not in use + ActiveClustersByClusterAttribute []ActiveClusterInfoRow } type DomainMigrationRow struct { @@ -697,7 +683,7 @@ func newDomainRow(domain *types.DescribeDomainResponse) DomainRow { VisibilityArchivalURI: domain.Configuration.GetVisibilityArchivalURI(), BadBinaries: newBadBinaryRows(domain.Configuration.BadBinaries), FailoverInfo: newFailoverInfoRow(domain.FailoverInfo), - IsActiveActiveDomain: domain.ReplicationConfiguration.GetActiveClusters() != nil, + IsActiveActiveDomain: domain.ReplicationConfiguration.IsActiveActiveDomain(), ActiveClustersByRegion: newActiveClustersByRegion(domain.ReplicationConfiguration.GetActiveClusters()), } } @@ -925,27 +911,42 @@ func clustersToStrings(clusters []*types.ClusterReplicationConfiguration) []stri return res } -func parseActiveClustersByRegion(c *cli.Context, isActiveActiveDomain bool) (map[string]string, error) { - var activeClustersByRegion map[string]string - if c.IsSet(FlagActiveClustersByRegion) { - if !isActiveActiveDomain { - return nil, commoncli.Problem("Option --active_clusters_by_region is only supported for active-active domain. Use --active_cluster instead.", nil) +func parseActiveClustersByClusterAttribute(clusters string) (types.ActiveClusters, error) { + split := regexp.MustCompile(`(?P[a-zA-Z0-9_]+).(?P[a-zA-Z0-9_]+):(?P[a-zA-Z0-9_]+)`) + matches := split.FindAllStringSubmatch(clusters, -1) + if len(matches) == 0 { + return types.ActiveClusters{}, fmt.Errorf("option %s format is invalid. Expected format is 'region.dca:dev2_dca,region.phx:dev2_phx'", FlagActiveClusters) + } + + out := types.ActiveClusters{ + AttributeScopes: map[string]types.ClusterAttributeScope{}, + } + + for _, match := range matches { + if len(match) != 4 { + return types.ActiveClusters{}, fmt.Errorf("option %s format is invalid. Expected format is 'region.dca:dev2_dca,region.phx:dev2_phx'", FlagActiveClusters) } + attribute := match[1] + scope := match[2] + name := match[3] - activeClustersByRegion = make(map[string]string) - for _, regionCluster := range c.StringSlice(FlagActiveClustersByRegion) { - splitted := strings.Split(regionCluster, ":") - if len(splitted) != 2 { - return nil, commoncli.Problem(fmt.Sprintf("Option --%s format is invalid. Expected format is 'region1:cluster1,region2:cluster2'", FlagActiveClustersByRegion), nil) + existing, ok := out.AttributeScopes[attribute] + if !ok { + out.AttributeScopes[attribute] = types.ClusterAttributeScope{ + ClusterAttributes: map[string]types.ActiveClusterInfo{ + scope: {ActiveClusterName: name}, + }, } - region, cluster := strings.TrimSpace(splitted[0]), strings.TrimSpace(splitted[1]) - activeClustersByRegion[region] = cluster + } else { + if _, ok := existing.ClusterAttributes[scope]; ok { + return types.ActiveClusters{}, fmt.Errorf("option active_clusters format is invalid. the key %q was duplicated. This can only map to a single active cluster", scope) + } + existing.ClusterAttributes[scope] = types.ActiveClusterInfo{ActiveClusterName: name} + out.AttributeScopes[attribute] = existing } - } - if isActiveActiveDomain && len(activeClustersByRegion) == 0 { - return nil, commoncli.Problem("Option --active_clusters_by_region is required for active-active domain.", nil) + out.AttributeScopes[attribute].ClusterAttributes[scope] = types.ActiveClusterInfo{ActiveClusterName: name} } - return activeClustersByRegion, nil + return out, nil } diff --git a/tools/cli/domain_commands_test.go b/tools/cli/domain_commands_test.go index 485540ca424..962f02ae725 100644 --- a/tools/cli/domain_commands_test.go +++ b/tools/cli/domain_commands_test.go @@ -24,7 +24,9 @@ package cli import ( "fmt" + "testing" + "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" "github.com/uber/cadence/common" @@ -59,30 +61,30 @@ func (s *cliAppSuite) TestDomainRegister() { }, { "active-active domain", - "cadence --do test-domain domain register --active_active_domain true --active_clusters_by_region region1:cluster1,region2:cluster2", + "cadence --do test-domain domain register --active_clusters region.region1:cluster1,region.region2:cluster2", "", func() { s.serverFrontendClient.EXPECT().RegisterDomain(gomock.Any(), &types.RegisterDomainRequest{ Name: "test-domain", WorkflowExecutionRetentionPeriodInDays: 3, IsGlobalDomain: true, - ActiveClustersByRegion: map[string]string{ - "region1": "cluster1", - "region2": "cluster2", + ActiveClusters: &types.ActiveClusters{ + AttributeScopes: map[string]types.ClusterAttributeScope{ + "region": { + ClusterAttributes: map[string]types.ActiveClusterInfo{ + "region1": {ActiveClusterName: "cluster1"}, + "region2": {ActiveClusterName: "cluster2"}, + }, + }, + }, }, }).Return(nil) }, }, { "active-active domain with invalid active clusters by region", - "cadence --do test-domain domain register --active_active_domain true --active_clusters_by_region region1=cluster1", - "Option --active_clusters_by_region format is invalid. Expected format is 'region1:cluster1,region2:cluster2'", - nil, - }, - { - "active-active domain with no active clusters by region", - "cadence --do test-domain domain register --active_active_domain true", - "Option --active_clusters_by_region is required for active-active domain.", + "cadence --do test-domain domain register --active_clusters region1=cluster1", + "option active_clusters format is invalid. Expected format is 'region.dca:dev2_dca,region.phx:dev2_phx", nil, }, { @@ -148,7 +150,7 @@ func (s *cliAppSuite) TestDomainRegister() { Name: "test-domain", WorkflowExecutionRetentionPeriodInDays: 3, IsGlobalDomain: true, - }).Return(&types.BadRequestError{"fake error"}) + }).Return(&types.BadRequestError{Message: "fake error"}) }, }, { @@ -293,15 +295,19 @@ func (s *cliAppSuite) TestDomainUpdate() { }, { "active-active domain failover", - "cadence --do test-domain domain update --active_clusters_by_region region1:c1,region2:c2", + "cadence --do test-domain domain update --active_clusters region.region1:c1,region.region2:c2", "", func() { s.serverFrontendClient.EXPECT().UpdateDomain(gomock.Any(), &types.UpdateDomainRequest{ Name: "test-domain", ActiveClusters: &types.ActiveClusters{ - ActiveClustersByRegion: map[string]types.ActiveClusterInfo{ - "region1": {ActiveClusterName: "c1"}, - "region2": {ActiveClusterName: "c2"}, + AttributeScopes: map[string]types.ClusterAttributeScope{ + "region": { + ClusterAttributes: map[string]types.ActiveClusterInfo{ + "region1": {ActiveClusterName: "c1"}, + "region2": {ActiveClusterName: "c2"}, + }, + }, }, }, }).Return(&types.UpdateDomainResponse{}, nil) @@ -388,3 +394,58 @@ func (s *cliAppSuite) TestListDomains() { }) } } + +func TestParseActiveClustersByClusterAttribute(t *testing.T) { + + testCases := map[string]struct { + clusters string + expected types.ActiveClusters + expectedError error + }{ + "valid active clusters by cluster attribute": { + clusters: "region.newyork:cluster0,region.manilla:cluster1", + expected: types.ActiveClusters{ + AttributeScopes: map[string]types.ClusterAttributeScope{ + "region": {ClusterAttributes: map[string]types.ActiveClusterInfo{ + "newyork": {ActiveClusterName: "cluster0"}, + "manilla": {ActiveClusterName: "cluster1"}, + }}, + }, + }, + }, + "valid active clusters by cluster attribute with multiple scopes": { + clusters: "region.newyork:cluster0,location.brussels:cluster2,region.madrid:cluster1", + expected: types.ActiveClusters{ + AttributeScopes: map[string]types.ClusterAttributeScope{ + "region": {ClusterAttributes: map[string]types.ActiveClusterInfo{ + "newyork": {ActiveClusterName: "cluster0"}, + "madrid": {ActiveClusterName: "cluster1"}, + }}, + "location": {ClusterAttributes: map[string]types.ActiveClusterInfo{ + "brussels": {ActiveClusterName: "cluster2"}, + }}, + }, + }, + }, + "duplicate keys consistutes an error in parsing and shouldn't be allowed": { + clusters: "region.newyork:cluster0,region.newyork:cluster1", + expectedError: fmt.Errorf(`option active_clusters format is invalid. the key "newyork" was duplicated. This can only map to a single active cluster`), + }, + "Some invalid input": { + clusters: "bad-data", + expectedError: fmt.Errorf("option active_clusters format is invalid. Expected format is 'region.dca:dev2_dca,region.phx:dev2_phx'"), + }, + "empty input": { + clusters: "", + expectedError: fmt.Errorf("option active_clusters format is invalid. Expected format is 'region.dca:dev2_dca,region.phx:dev2_phx'"), + }, + } + + for name, td := range testCases { + t.Run(name, func(t *testing.T) { + activeClusters, err := parseActiveClustersByClusterAttribute(td.clusters) + assert.Equal(t, td.expected, activeClusters) + assert.Equal(t, td.expectedError, err) + }) + } +} diff --git a/tools/cli/domain_utils.go b/tools/cli/domain_utils.go index c7556c76a0a..bfbfc374d02 100644 --- a/tools/cli/domain_utils.go +++ b/tools/cli/domain_utils.go @@ -73,9 +73,9 @@ var ( Usage: "Active cluster name", }, &cli.StringSliceFlag{ - Name: FlagActiveClustersByRegion, - Aliases: []string{"acbr"}, - Usage: "Active clusters by region in the format 'region1:cluster1,region2:cluster2'", + Name: FlagActiveClusters, + Aliases: []string{"acs"}, + Usage: "Active clusters by cluster attribute in the format '.: ie: region.manilla:cluster0,region.newyork:cluster1'", }, &cli.StringSliceFlag{ Name: FlagClusters, @@ -88,12 +88,6 @@ var ( Usage: "Flag to indicate whether domain is a global domain (active-passive domain). Default to true. Local domain is now legacy.", Value: "true", }, - &cli.StringFlag{ - Name: FlagIsActiveActiveDomain, - Aliases: []string{"aad"}, - Usage: "Flag to indicate whether domain is an active-active domain. Default to false.", - Value: "false", - }, &cli.GenericFlag{ Name: FlagDomainData, Aliases: []string{"dmd"}, @@ -149,9 +143,9 @@ var ( Usage: "Active cluster name", }, &cli.StringSliceFlag{ - Name: FlagActiveClustersByRegion, - Aliases: []string{"acbr"}, - Usage: "Active clusters by region in the format 'region1:cluster1,region2:cluster2'", + Name: FlagActiveClusters, + Aliases: []string{"acs"}, + Usage: "Active clusters by cluster attribute in the format '.: ie: region.manilla:cluster0,region.newyork:cluster1'", }, &cli.StringSliceFlag{ Name: FlagClusters, diff --git a/tools/cli/flags.go b/tools/cli/flags.go index 9fcfa26adbf..fd38bf24835 100644 --- a/tools/cli/flags.go +++ b/tools/cli/flags.go @@ -125,10 +125,9 @@ const ( FlagQueryConsistencyLevel = "query_consistency_level" FlagShowDetail = "show_detail" FlagActiveClusterName = "active_cluster" - FlagActiveClustersByRegion = "active_clusters_by_region" + FlagActiveClusters = "active_clusters" FlagClusters = "clusters" - FlagIsGlobalDomain = "global_domain" // active-passive domain - FlagIsActiveActiveDomain = "active_active_domain" // active-active domain + FlagIsGlobalDomain = "global_domain" // active-passive domain FlagDomainData = "domain_data" FlagEventID = "event_id" FlagActivityID = "activity_id"