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.

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

Jump to bottom

Add cluster command to show cluster and peer statuses. #2256

Merged
merged 1 commit into from
May 18, 2020
Merged

Add cluster command to show cluster and peer statuses. #2256

merged 1 commit into from
May 18, 2020

Conversation

ffahri
Copy link
Contributor

@ffahri ffahri commented May 13, 2020 •
edited by simonpasquier
Loading

Closes #2192
This pr implements amtool cluster show command

Simple output

$ amtool --alertmanager.url http://localhost:9094 cluster show
Cluster Status:  ready

Extended output

$ amtool --alertmanager.url http://localhost:9094 cluster show -o extended
Cluster Status:  ready

Address            Name
192.30.10.111:9094  01E4R8TX0WTNW1JYH3MB0NGHA1
192.30.42.5:9094    01E3S5C0DTS3RMJZ71RH8LPD69
192.30.42.140:9094  01E3S5BA4TX4KR030ZXXKEA6WC

Json output

$amtool --alertmanager.url http://localhost:9094 cluster show -o json
{"name":"01E3S5BY4GX4KR034ZXXREA6WC","peers":[{"address":"192.30.42.140:9094","name":"01E3S5BA4TX4KR030ZXXKEA6WC"},{"address":"192.30.42.5:9094","name":"001E3S5C0DTS3RMJZ71RH8LPD69"},{"address":"192.30.10.111:9094","name":"01E4R8TX0WTNW1JYH3MB0NGHA1"}],"status":"ready"}

simonpasquier
simonpasquier reviewed May 14, 2020
View reviewed changes
Copy link
Member

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

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

Thanks for this! A few comments but this is in the right direction.

cli/cluster.go Outdated Show resolved Hide resolved
cli/cluster.go Outdated Show resolved Hide resolved
cli/format/format_extended.go Outdated
@@ -89,6 +89,23 @@ func (formatter *ExtendedFormatter) FormatConfig(status *models.AlertmanagerStat
return nil
}

// FormatClusterStatus formats the cluster status with peers into a readable string
Copy link
Member

Choose a reason for hiding this comment

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

(nit) missing dot at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added 🎉

cli/format/format_extended.go Outdated
// FormatClusterStatus formats the cluster status with peers into a readable string
func (formatter *ExtendedFormatter) FormatClusterStatus(status *models.ClusterStatus) error {
w := tabwriter.NewWriter(formatter.writer, 0, 0, 2, ' ', 0)
fmt.Fprintf(w, "Cluster Status:\t%s\n\n", *status.Status)
Copy link
Member

Choose a reason for hiding this comment

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

I would add the node's name (e.g. status.Name).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the node name into the simple format as well

cli/format/format_simple.go Outdated
@@ -75,6 +75,12 @@ func (formatter *SimpleFormatter) FormatConfig(status *models.AlertmanagerStatus
return nil
}

func (formatter *SimpleFormatter) FormatClusterStatus(status *models.ClusterStatus) error {
w := tabwriter.NewWriter(formatter.writer, 0, 0, 2, ' ', 0)
fmt.Fprintf(w, "Cluster Status:\t%s\n", *status.Status)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

cli/format/sort.go Outdated
func (s ByAddress) Len() int { return len(s) }
func (s ByAddress) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
func (s ByAddress) Less(i, j int) bool {
return bytes.Compare([]byte(*s[i].Address), []byte(*s[j].Address)) < 0
Copy link
Member

Choose a reason for hiding this comment

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

not sure why you cast string to bytes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I double-checked this logic and that was wrong. I need to collect an IP addresses from host:ip format. Then using net.IP to comparing using bytes will be much correct

Copy link
Contributor Author

@ffahri ffahri May 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Here is the new way

func (s ByAddress) Less(i, j int) bool {
	ip1, port1, _ := net.SplitHostPort(*s[i].Address)
	ip2, port2, _ := net.SplitHostPort(*s[j].Address)
	if ip1 == ip2 {
		return port1 < port2
	} else {
		return bytes.Compare(net.ParseIP(ip1), net.ParseIP(ip2)) < 0
	}
}

Im not sure about the ignoring errors what do you think about this implementation

This way it can also sort same-ip different ports

@ffahri ffahri force-pushed the add-amtool-cluster-show-command branch from 8b18362 to cc0d7d6 Compare May 14, 2020 17:07
@ffahri ffahri requested a review from simonpasquier May 14, 2020 17:07
@ffahri ffahri force-pushed the add-amtool-cluster-show-command branch from cc0d7d6 to 242c5d6 Compare May 14, 2020 19:57
simonpasquier
simonpasquier reviewed May 15, 2020
View reviewed changes
Copy link
Member

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

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

one last nit

@ffahri
Copy link
Contributor Author

ffahri commented May 15, 2020

Which part is this nit @simonpasquier 🤔

simonpasquier
simonpasquier reviewed May 15, 2020
View reviewed changes
cli/format/sort.go Outdated Show resolved Hide resolved
@simonpasquier
Copy link
Member

hmm GitHub discarded my previous comment...

@ffahri
Add cluster command to show cluster and peer statuses.
a6adf4d 
                                                      Signed-off-by: Fahri Yardımcı <f.yardimci06@gmail.com>

Signed-off-by: Fahri Yardımcı <f.yardimci06@gmail.com>
@ffahri ffahri force-pushed the add-amtool-cluster-show-command branch from b8dbe09 to a6adf4d Compare May 15, 2020 16:21
@simonpasquier
Copy link
Member

Thanks!

@simonpasquier simonpasquier merged commit 12db463 into prometheus:master May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonpasquier simonpasquier simonpasquier left review comments

Assignees
No one assigned
Labels
kind/enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add command to amtool to view the cluster status
2 participants
@ffahri @simonpasquier