From bd31bfc8e8155f1bfe3666d9a43b2d4cd537030f Mon Sep 17 00:00:00 2001 From: nikParasyr Date: Sat, 1 Nov 2025 14:57:42 +0100 Subject: [PATCH] Ensure pool member reach active state When (re)creating a pool member, wait until its provisioning_status is ACTIVE. This avoids scenarios were CAPO contrinues removing members before new ones are actually active --- pkg/clients/loadbalancer.go | 10 +++++ pkg/clients/mock/loadbalancer.go | 15 +++++++ .../services/loadbalancer/loadbalancer.go | 27 ++++++++++++- .../loadbalancer/loadbalancer_test.go | 39 ++++++++++++++++--- 4 files changed, 84 insertions(+), 7 deletions(-) diff --git a/pkg/clients/loadbalancer.go b/pkg/clients/loadbalancer.go index 1c35823967..3cf46350da 100644 --- a/pkg/clients/loadbalancer.go +++ b/pkg/clients/loadbalancer.go @@ -51,6 +51,7 @@ type LbClient interface { DeletePool(id string) error CreatePoolMember(poolID string, opts pools.CreateMemberOptsBuilder) (*pools.Member, error) ListPoolMember(poolID string, opts pools.ListMembersOptsBuilder) ([]pools.Member, error) + GetPoolMember(poolID string, lbMemberID string) (*pools.Member, error) DeletePoolMember(poolID string, lbMemberID string) error CreateMonitor(opts monitors.CreateOptsBuilder) (*monitors.Monitor, error) ListMonitors(opts monitors.ListOptsBuilder) ([]monitors.Monitor, error) @@ -213,6 +214,15 @@ func (l lbClient) ListPoolMember(poolID string, opts pools.ListMembersOptsBuilde return pools.ExtractMembers(allPages) } +func (l lbClient) GetPoolMember(poolID string, lbMemberID string) (*pools.Member, error) { + mc := metrics.NewMetricPrometheusContext("loadbalancer_member", "get") + member, err := pools.GetMember(context.TODO(), l.serviceClient, poolID, lbMemberID).Extract() + if mc.ObserveRequest(err) != nil { + return nil, fmt.Errorf("error getting lbmember: %s", err) + } + return member, nil +} + func (l lbClient) DeletePoolMember(poolID string, lbMemberID string) error { mc := metrics.NewMetricPrometheusContext("loadbalancer_member", "delete") err := pools.DeleteMember(context.TODO(), l.serviceClient, poolID, lbMemberID).ExtractErr() diff --git a/pkg/clients/mock/loadbalancer.go b/pkg/clients/mock/loadbalancer.go index 59024521f8..586f788437 100644 --- a/pkg/clients/mock/loadbalancer.go +++ b/pkg/clients/mock/loadbalancer.go @@ -251,6 +251,21 @@ func (mr *MockLbClientMockRecorder) GetPool(id any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPool", reflect.TypeOf((*MockLbClient)(nil).GetPool), id) } +// GetPoolMember mocks base method. +func (m *MockLbClient) GetPoolMember(poolID, lbMemberID string) (*pools.Member, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPoolMember", poolID, lbMemberID) + ret0, _ := ret[0].(*pools.Member) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetPoolMember indicates an expected call of GetPoolMember. +func (mr *MockLbClientMockRecorder) GetPoolMember(poolID, lbMemberID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPoolMember", reflect.TypeOf((*MockLbClient)(nil).GetPoolMember), poolID, lbMemberID) +} + // ListListeners mocks base method. func (m *MockLbClient) ListListeners(opts listeners.ListOptsBuilder) ([]listeners.Listener, error) { m.ctrl.T.Helper() diff --git a/pkg/cloud/services/loadbalancer/loadbalancer.go b/pkg/cloud/services/loadbalancer/loadbalancer.go index 5c88c979b7..394a9ed0e0 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer.go @@ -52,6 +52,7 @@ const ( const ( loadBalancerProvisioningStatusActive = "ACTIVE" loadBalancerProvisioningStatusPendingDelete = "PENDING_DELETE" + poolMemberProvisioningStatusActive = "ACTIVE" ) // Default values for Monitor, sync with `kubebuilder:default` annotations on APIServerLoadBalancerMonitor object. @@ -718,7 +719,12 @@ func (s *Service) ReconcileLoadBalancerMember(openStackCluster *infrav1.OpenStac return err } - if _, err := s.loadbalancerClient.CreatePoolMember(pool.ID, lbMemberOpts); err != nil { + member, err := s.loadbalancerClient.CreatePoolMember(pool.ID, lbMemberOpts) + if err != nil { + return err + } + + if _, err := s.waitForPoolMemberActive(pool.ID, member.ID); err != nil { return err } @@ -932,6 +938,25 @@ func (s *Service) waitForLoadBalancerActive(id string) (*loadbalancers.LoadBalan return lb, nil } +// Possible Pool Member states are documented here: https://docs.openstack.org/api-ref/load-balancer/v2/#prov-status +func (s *Service) waitForPoolMemberActive(poolID, memberID string) (*pools.Member, error) { + var member *pools.Member + + s.scope.Logger().Info("Waiting for pool member", "pool_id", poolID, "member_id", memberID, "targetStatus", "ACTIVE") + err := wait.ExponentialBackoff(backoff, func() (bool, error) { + var err error + member, err = s.loadbalancerClient.GetPoolMember(poolID, memberID) + if err != nil { + return false, err + } + return member.ProvisioningStatus == poolMemberProvisioningStatusActive, nil + }) + if err != nil { + return nil, err + } + return member, nil +} + func (s *Service) waitForListener(id, target string) error { s.scope.Logger().Info("Waiting for load balancer listener", "id", id, "targetStatus", target) return wait.ExponentialBackoff(backoff, func() (bool, error) { diff --git a/pkg/cloud/services/loadbalancer/loadbalancer_test.go b/pkg/cloud/services/loadbalancer/loadbalancer_test.go index a6a04d481f..9a2ec0d61e 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer_test.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer_test.go @@ -1052,8 +1052,19 @@ func Test_ReconcileLoadBalancerMember(t *testing.T) { ).DoAndReturn(func(_ string, got pools.CreateMemberOpts) (*pools.Member, error) { // SubnetID must be empty here g.Expect(got.SubnetID).To(Equal("")) - return &pools.Member{ID: "member-2"}, nil + return &pools.Member{ID: memberID}, nil }) + + pendingMember := pools.Member{ + ID: memberID, + Name: poolMemberName, + ProvisioningStatus: "PENDING_CREATE", + } + m.GetPoolMember(poolID, memberID).Return(&pendingMember, nil) + + activeMember := pendingMember + activeMember.ProvisioningStatus = "ACTIVE" + m.GetPoolMember(poolID, memberID).Return(&activeMember, nil) }, wantError: nil, }, @@ -1092,8 +1103,10 @@ func Test_ReconcileLoadBalancerMember(t *testing.T) { } m.ListPools(pools.ListOpts{Name: pool.Name}).Return([]pools.Pool{pool}, nil) + poolMemberName := fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName) + member := pools.Member{ - Name: fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName), + Name: poolMemberName, Address: wrongMemberIP, ID: memberID, } @@ -1107,8 +1120,15 @@ func Test_ReconcileLoadBalancerMember(t *testing.T) { ).DoAndReturn(func(_ string, got pools.CreateMemberOpts) (*pools.Member, error) { // SubnetID must be empty here g.Expect(got.SubnetID).To(Equal("")) - return &pools.Member{ID: "member-2"}, nil + return &pools.Member{ID: memberID}, nil }) + + activeMember := pools.Member{ + ID: memberID, + Name: poolMemberName, + ProvisioningStatus: "ACTIVE", + } + m.GetPoolMember(poolID, memberID).Return(&activeMember, nil).AnyTimes() }, wantError: nil, }, @@ -1133,8 +1153,8 @@ func Test_ReconcileLoadBalancerMember(t *testing.T) { } m.ListPools(pools.ListOpts{Name: pool.Name}).Return([]pools.Pool{pool}, nil) - memberName := fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName) - m.ListPoolMember(poolID, pools.ListMembersOpts{Name: memberName}).Return([]pools.Member{}, nil) + poolMemberName := fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName) + m.ListPoolMember(poolID, pools.ListMembersOpts{Name: poolMemberName}).Return([]pools.Member{}, nil) // Expect CreatePoolMember; capture opts to assert SubnetID is set m.CreatePoolMember( @@ -1148,8 +1168,15 @@ func Test_ReconcileLoadBalancerMember(t *testing.T) { g.Expect(got.SubnetID).To(Equal(subnetID)) // Tags should pass through g.Expect(got.Tags).To(ConsistOf("k8s", "clusterapi")) - return &pools.Member{ID: "member-1", Address: memberIP, ProtocolPort: port}, nil + return &pools.Member{ID: memberID, Address: memberIP, ProtocolPort: port}, nil }) + + activeMember := pools.Member{ + ID: memberID, + Name: poolMemberName, + ProvisioningStatus: "ACTIVE", + } + m.GetPoolMember(poolID, memberID).Return(&activeMember, nil).AnyTimes() }, wantError: nil, },