fix: don't route member-ID discovery onto a still-learner self (#26)

discoverMemberID built its endpoint list from voter peers and then
*unconditionally* appended the member's own client URL as a "self
last" fallback. The comment claimed the learner rejection "only fires
when we route past a voter to a learner", but clientv3's balancer
does not honor endpoint order — it round-robins. So when the member
being discovered is itself a freshly-added learner and a voter peer
is present, the balancer can land MemberList on our own etcd, which
returns "rpc not supported for learner". With a 5s context budget
that wedges discovery: MemberID never gets populated, MemberReady
never flips true, the cluster controller's allMembersReady gate never
opens, the scale-up stalls past progressDeadline, and the next member
is never added.

Observed on dev4 during the cert-manager TLS smoke: a 3-replica
cluster stuck at 2 members (one voter + one perpetual learner) with
the operator logging MemberList against the learner's own endpoint.
Not TLS-specific — any multi-member scale-up where a learner's pod
restarts before promotion (re-triggering discovery) can hit it.

Fix: make self a true fallback — append it only when no voter peer is
available (single-node bootstrap, or no other voter Ready yet),
mirroring memberEndpoints' voter-or-fallback shape. When a voter peer
exists, discovery dials only voters and resolves the learner's ID
from their MemberList by name / peer URL.

Tests: TestDiscoverMemberID_ExcludesSelfWhenVoterAvailable reproduces
the wedge (target IS the learner, one voter peer) and asserts self is
excluded — it fails on the old code with the exact two-endpoint list.
TestDiscoverMemberID_FallsBackToSelfWhenNoVoter pins the single-node
fallback the tightening must preserve.

Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

Author: lllamnyp

controllers/etcdmember_controller.go

@@ -883,11 +883,16 @@ func (r *EtcdMemberReconciler) discoverMemberID(ctx context.Context, member *lll
 		}
 		endpoints = append(endpoints, clientURL(scheme, m.Name, member.Spec.ClusterName, member.Namespace))
 	}
-	// Self last — used during single-node bootstrap when there are no
-	// other peers, or when no other peer is yet Ready. Etcd handles
-	// MemberList on a single-member-voter cluster fine; the learner
-	// rejection only fires when we route past a voter to a learner.
-	endpoints = append(endpoints, clientURL(scheme, member.Name, member.Spec.ClusterName, member.Namespace))
+	// Self is a *fallback*, not an always-on endpoint: dial our own etcd
+	// only when no voter peer is available (single-node bootstrap, or no
+	// other voter Ready yet). When this member is itself a still-learner
+	// being discovered, appending self alongside a voter lets clientv3's
+	// balancer round-robin MemberList onto our own etcd, which rejects it
+	// with "rpc not supported for learner" — wedging discovery even though
+	// a voter was in the list. Mirrors memberEndpoints' voter-or-fallback.
+	if len(endpoints) == 0 {
+		endpoints = append(endpoints, clientURL(scheme, member.Name, member.Spec.ClusterName, member.Namespace))
+	}
 
 	tlsCfg, err := r.memberTLSConfig(ctx, member)
 	if err != nil {

controllers/etcdmember_controller_test.go

@@ -961,6 +961,113 @@ func TestDiscoverMemberID_ExcludesNonVoterPeers(t *testing.T) {
 	}
 }
 
+// TestDiscoverMemberID_ExcludesSelfWhenVoterAvailable reproduces the dev4
+// wedge directly: the member whose ID we're discovering is ITSELF a freshly
+// added learner (Pod up, no MemberID, IsVoter=false). discoverMemberID must
+// not append our own client URL to the endpoint list while a voter peer is
+// reachable. Appending self lets clientv3's balancer round-robin MemberList
+// onto our own learner etcd, which returns "rpc not supported for learner",
+// stalling discovery past the progress deadline — exactly what kept the
+// third member from ever being added during the cert-manager TLS smoke.
+//
+// The earlier _ExcludesNonVoterPeers / _FallsBackToPeers tests miss this
+// because their target is a distinct member from the voter/learner peers,
+// so self being appended is never exercised against a learner.
+func TestDiscoverMemberID_ExcludesSelfWhenVoterAvailable(t *testing.T) {
+	ctx := context.Background()
+	now := metav1.Now()
+
+	cluster := &lll.EtcdCluster{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}}
+	voter := &lll.EtcdMember{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-seed", Namespace: "ns", Labels: memberLabels("test", "test-seed")},
+		Spec:       lll.EtcdMemberSpec{ClusterName: "test"},
+		Status: lll.EtcdMemberStatus{
+			PodName: "test-seed", MemberID: "0000000000000001",
+			IsVoter:    true,
+			Conditions: []metav1.Condition{{Type: lll.MemberReady, Status: metav1.ConditionTrue, Reason: "PodReady", LastTransitionTime: now}},
+		},
+	}
+	// Target is the just-joined learner discovering its own ID.
+	target := &lll.EtcdMember{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-learner", Namespace: "ns", Labels: memberLabels("test", "test-learner")},
+		Spec:       lll.EtcdMemberSpec{ClusterName: "test"},
+		Status: lll.EtcdMemberStatus{
+			PodName:    "test-learner", // Pod up, but no MemberID and IsVoter=false.
+			Conditions: []metav1.Condition{{Type: lll.MemberReady, Status: metav1.ConditionFalse, Reason: "DiscoveringMemberID", LastTransitionTime: now}},
+		},
+	}
+	c, _ := newTestClient(t, cluster, voter, target)
+
+	const wantID uint64 = 0xfeedface
+	fe := newFakeEtcd(0xdead,
+		&etcdserverpb.Member{ID: 0x1, Name: "test-seed", PeerURLs: []string{peerURL("http", "test-seed", "test", "ns")}},
+		&etcdserverpb.Member{ID: wantID, Name: "test-learner", PeerURLs: []string{peerURL("http", "test-learner", "test", "ns")}},
+	)
+	var captured []string
+	factory := func(_ context.Context, eps []string, _ *tls.Config) (EtcdClusterClient, error) {
+		captured = append([]string(nil), eps...)
+		return fe, nil
+	}
+	r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factory}
+
+	id, err := r.discoverMemberID(ctx, target)
+	if err != nil {
+		t.Fatalf("discoverMemberID: %v", err)
+	}
+	if id != wantID {
+		t.Fatalf("id = %x, want %x", id, wantID)
+	}
+	selfURL := clientURL("http", "test-learner", "test", "ns")
+	for _, ep := range captured {
+		if ep == selfURL {
+			t.Fatalf("discoverMemberID must not dial the learner's own URL while a voter is available; got %v", captured)
+		}
+	}
+	if len(captured) != 1 || captured[0] != clientURL("http", "test-seed", "test", "ns") {
+		t.Fatalf("expected only the voter's URL; got %v", captured)
+	}
+}
+
+// TestDiscoverMemberID_FallsBackToSelfWhenNoVoter pins the fallback the
+// above tightening must preserve: with no voter peer available (single-node
+// bootstrap — the seed discovering its own ID), self is the only endpoint
+// we can dial, and etcd serves MemberList fine on a single-member voter.
+func TestDiscoverMemberID_FallsBackToSelfWhenNoVoter(t *testing.T) {
+	ctx := context.Background()
+
+	cluster := &lll.EtcdCluster{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}}
+	// Only the seed exists; it has no MemberID yet and no voter peers.
+	seed := &lll.EtcdMember{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-seed", Namespace: "ns", Labels: memberLabels("test", "test-seed")},
+		Spec:       lll.EtcdMemberSpec{ClusterName: "test"},
+		Status:     lll.EtcdMemberStatus{PodName: "test-seed"},
+	}
+	c, _ := newTestClient(t, cluster, seed)
+
+	const wantID uint64 = 0xabcdef
+	fe := newFakeEtcd(0xdead,
+		&etcdserverpb.Member{ID: wantID, Name: "test-seed", PeerURLs: []string{peerURL("http", "test-seed", "test", "ns")}},
+	)
+	var captured []string
+	factory := func(_ context.Context, eps []string, _ *tls.Config) (EtcdClusterClient, error) {
+		captured = append([]string(nil), eps...)
+		return fe, nil
+	}
+	r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factory}
+
+	id, err := r.discoverMemberID(ctx, seed)
+	if err != nil {
+		t.Fatalf("discoverMemberID: %v", err)
+	}
+	if id != wantID {
+		t.Fatalf("id = %x, want %x", id, wantID)
+	}
+	selfURL := clientURL("http", "test-seed", "test", "ns")
+	if len(captured) != 1 || captured[0] != selfURL {
+		t.Fatalf("expected self URL as sole fallback endpoint; got %v", captured)
+	}
+}
+
 // TestDiscoverMemberID_FallsBackToPeerURL covers blocker #2: in the window
 // between MemberAddAsLearner and etcd propagating the joiner's Name, the
 // only stable identifier we have is the peer URL. discoverMemberID must