Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
huikang committed Jan 10, 2025
1 parent 8d36db2 commit 646e476
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 62 deletions.
9 changes: 6 additions & 3 deletions gubernator.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,16 +577,19 @@ func (s *V1Instance) HealthCheck(ctx context.Context, r *HealthCheckReq) (health
}

health = &HealthCheckResp{
PeerCount: int32(len(localPeers) + len(regionPeers)),
Status: Healthy,
PeerCount: int32(len(localPeers) + len(regionPeers)),
Status: Healthy,
AdvertiseAddress: ownPeerAddress,
}

if len(errs) != 0 {
health.Status = UnHealthy
health.Message = strings.Join(errs, "|")
}

health.OwnPeer = ownPeerAddress
if health.AdvertiseAddress == "" {
health.Message = strings.Join(errs, "this instance is not found in the peer list")
}

span.SetAttributes(
attribute.Int64("health.peerCount", int64(health.PeerCount)),
Expand Down
101 changes: 55 additions & 46 deletions gubernator.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 12 additions & 11 deletions gubernator.proto
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,16 @@ message HealthCheckResp {
string message = 2;
// The number of peers we know about
int32 peer_count = 3;
// advertise_address is the address advertised to other peers and
// is verified as owned by this instance. If advertise_address is
// empty, this indicates the instance is unable find it's advertised
// address in the list of peers provided to SetPeers(). This is likely
// due to an incorrect GUBER_ADVERTISE_ADDRESS, or the discovery method
// used to call SetPeers() is missing our instance address in the peer
// list provided.
//
// If advertise_address is empty, this gubernator instance is considered
// unhealthy.
string advertise_address = 4;

// advertise_address is the address advertised to other peers and
// is verified as owned by this instance. If advertise_address is
// empty, this indicates the instance is unable to find it's advertised
// address in the list of peers provided to SetPeers(). This is likely
// due to an incorrect GUBER_ADVERTISE_ADDRESS, or the discovery method
// used to call SetPeers() is missing our instance address in the peer
// list provided.
//
// If advertise_address is empty, this gubernator instance is considered
// unhealthy.
string advertise_address = 4;
}
4 changes: 2 additions & 2 deletions tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func TestHTTPSClientAuth(t *testing.T) {
defer resp.Body.Close()
b, err := io.ReadAll(resp.Body)
require.NoError(t, err)
assert.Equal(t, `{"status":"healthy","message":"","peer_count":1,"own_peer":"127.0.0.1:9695"}`, strings.ReplaceAll(string(b), " ", ""))
assert.Equal(t, `{"status":"healthy","message":"","peer_count":1,"advertise_address":"127.0.0.1:9695"}`, strings.ReplaceAll(string(b), " ", ""))

// Verify we get an error when we try to access existing HTTPListenAddress without cert
//nolint:bodyclose // Expect error, no body to close.
Expand All @@ -346,7 +346,7 @@ func TestHTTPSClientAuth(t *testing.T) {
defer resp2.Body.Close()
b, err = io.ReadAll(resp2.Body)
require.NoError(t, err)
assert.Equal(t, `{"status":"healthy","message":"","peer_count":1,"own_peer":"127.0.0.1:9695"}`, strings.ReplaceAll(string(b), " ", ""))
assert.Equal(t, `{"status":"healthy","message":"","peer_count":1,"advertise_address":"127.0.0.1:9695"}`, strings.ReplaceAll(string(b), " ", ""))

}

Expand Down

0 comments on commit 646e476

Please sign in to comment.