Skip to content

refactor: refactor the domain / cli commands for active-active's cluster-attr #7299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 7, 2025
Merged

refactor: refactor the domain / cli commands for active-active's cluster-attr #7299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
90ce3f8
Cli
davidporter-id-au Oct 3, 2025
0ece1eb
feedback
davidporter-id-au Oct 6, 2025
fdca827
fixed some more tests
davidporter-id-au Oct 6, 2025
a29b3ce
missing flag
davidporter-id-au Oct 6, 2025
651ed9a
fix test
davidporter-id-au Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions common/types/shared.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1710,6 +1710,13 @@ type DescribeDomainResponse struct {
FailoverInfo *FailoverInfo `json:"failoverInfo,omitempty"`
}

func (v *DomainReplicationConfiguration) GetActiveClusters() (o *ActiveClusters) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not new, it's just shuffled from below due to my removing and then re-adding it. Happy to put it back in it's previous spot if necessary

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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
71 changes: 71 additions & 0 deletions common/types/shared_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
133 changes: 67 additions & 66 deletions tools/cli/domain_commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"errors"
"fmt"
"os"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -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) {
Expand All @@ -136,17 +127,9 @@ func (d *domainCLIImpl) RegisterDomain(c *cli.Context) error {

activeClusterName := ""
if c.IsSet(FlagActiveClusterName) {
if isActiveActiveDomain {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.. am not actually sure what the behaviour here should be

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) {
Expand All @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()),
}
}
Expand Down Expand Up @@ -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<attribute>[a-zA-Z0-9_]+).(?P<scope>[a-zA-Z0-9_]+):(?P<name>[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}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might check duplication and return an error here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I think my impl is a bit loose. let me add a bit more validation

}

return activeClustersByRegion, nil
return out, nil
}
Loading