Skip to content
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.

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

Add admin failover for managed domains #3154

Merged
merged 7 commits into from
Mar 31, 2020
Merged

Add admin failover for managed domains #3154

merged 7 commits into from
Mar 31, 2020

Conversation

vancexu
Copy link
Contributor

@vancexu vancexu commented Mar 30, 2020

What changed?
Add admin CLI command to failover managed domains

Why?
Currently domain failover is by design managed by cadence user. Some users dislike to failover themselves because they don't know how or when to failover.
By adding this managed failover support, cadence now have the ability to failover users domain on their behalf.

How did you test it?
Unit test.
Local test.
Will do staging test before release.

Potential risks
No risk.

@vancexu vancexu requested review from yycptt, emrahs and a team March 30, 2020 21:51
@coveralls
Copy link

coveralls commented Mar 30, 2020

Coverage Status

Coverage increased (+2.5%) to 69.552% when pulling 71605ef on mangedfo into 66e4d0d on master.

tools/cli/domainCommands.go Outdated Show resolved Hide resolved
tools/cli/defs.go Outdated Show resolved Hide resolved
tools/cli/domainCommands.go Show resolved Hide resolved
Name: "failover",
Aliases: []string{"fo"},
Usage: "Failover domains with domain data IsManagedByCadence=true to target cluster",
Flags: []cli.Flag{
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this command should only be executed by cadence team. Should the command also takes in a security token and verify its content before actually execution?

Just an idea to explore. I understand this is not easy as the underlying domain failover command doesn't require security token.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. But security token on open sourced repo is merely support on server API.
Internally we will be protected by the auth feature.

@vancexu vancexu requested a review from yycptt March 31, 2020 01:11
@vancexu vancexu merged commit daa8998 into master Mar 31, 2020
@vancexu vancexu deleted the mangedfo branch April 28, 2020 17:15
emrahs pushed a commit that referenced this pull request Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants