Skip to content

Commit

Permalink
m/n/c/consensus: handle empty etcd member Name
Browse files Browse the repository at this point in the history
When an etcd member has not been started yet, the member.Name field is
the empty string. In this case, we need to extract the node id from
PeerURLs instead.

Change-Id: I41aa39423bd4c7888467d65eb2a3f96e7d02e617
Reviewed-on: https://review.monogon.dev/c/monogon/+/3385
Tested-by: Jenkins CI
Reviewed-by: Serge Bazanski <serge@monogon.tech>
  • Loading branch information
jscissr committed Sep 9, 2024
1 parent 93d2e6c commit 442cf68
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 2 deletions.
1 change: 1 addition & 0 deletions metropolis/node/core/consensus/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ go_library(
"//osbase/logtree/unraw",
"//osbase/pki",
"//osbase/supervisor",
"@io_etcd_go_etcd_api_v3//etcdserverpb",
"@io_etcd_go_etcd_client_v3//:client",
"@io_etcd_go_etcd_server_v3//embed",
],
Expand Down
25 changes: 25 additions & 0 deletions metropolis/node/core/consensus/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ import (
"net/url"
"time"

"go.etcd.io/etcd/api/v3/etcdserverpb"
clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/server/v3/embed"

Expand Down Expand Up @@ -143,6 +144,26 @@ func pkiPeerCertificate(pubkey ed25519.PublicKey, extraNames []string) x509.Cert
}
}

// GetEtcdMemberNodeId returns the node ID of an etcd member. It works even for
// members which have not started, where member.Name is empty.
func GetEtcdMemberNodeId(member *etcdserverpb.Member) string {
if member.Name != "" {
return member.Name
}
if len(member.PeerURLs) == 0 {
return ""
}
u, err := url.Parse(member.PeerURLs[0])
if err != nil {
return ""
}
nodeId, _, err := net.SplitHostPort(u.Host)
if err != nil {
return ""
}
return nodeId
}

// Service is the etcd cluster member service. See package-level documentation
// for more information.
type Service struct {
Expand Down Expand Up @@ -497,6 +518,10 @@ func (s *Service) autopromoter(ctx context.Context) error {
if !member.IsLearner {
continue
}
if member.Name == "" {
// If the name is empty, the member has not started.
continue
}
// Always call PromoteMember since the metadata necessary to decide if we should
// is private. Luckily etcd already does consistency checks internally and will
// refuse to promote nodes that aren't connected or are still behind on
Expand Down
2 changes: 1 addition & 1 deletion metropolis/node/core/consensus/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (s *Status) AddNode(ctx context.Context, pk ed25519.PublicKey, opts ...*Add
var existingNodes []ExistingNode
var newExists bool
for _, m := range members.Members {
if m.Name == nodeID {
if GetEtcdMemberNodeId(m) == nodeID {
newExists = true
}
if m.IsLearner {
Expand Down
3 changes: 2 additions & 1 deletion metropolis/node/core/curator/impl_leader_curator.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
tpb "google.golang.org/protobuf/types/known/timestamppb"

common "source.monogon.dev/metropolis/node"
"source.monogon.dev/metropolis/node/core/consensus"
ipb "source.monogon.dev/metropolis/node/core/curator/proto/api"
"source.monogon.dev/metropolis/node/core/identity"
"source.monogon.dev/metropolis/node/core/rpc"
Expand Down Expand Up @@ -616,7 +617,7 @@ func (l *leaderCurator) GetConsensusStatus(ctx context.Context, _ *ipb.GetConsen
}
for _, member := range members.Members {
st := ipb.GetConsensusStatusResponse_EtcdMember{
Id: member.Name,
Id: consensus.GetEtcdMemberNodeId(member),
Status: ipb.GetConsensusStatusResponse_EtcdMember_STATUS_FULL,
}
if member.IsLearner {
Expand Down

0 comments on commit 442cf68

Please sign in to comment.