Skip to content

Commit

Permalink
Fix create pod use old ip when update StatefulSet pool name with anno…
Browse files Browse the repository at this point in the history 
…tation
  • Loading branch information
@lou-lan
lou-lan committed Jun 26, 2024
1 parent cd571c1 commit 21368fd
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 14 deletions.
12 changes: 11 additions & 1 deletion pkg/gcmanager/pod_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package gcmanager
import (
"context"
"fmt"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"time"

appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -141,7 +142,16 @@ func (s *SpiderGC) buildPodEntry(oldPod, currentPod *corev1.Pod, deleted bool) (
// check StatefulSet pod, we will trace it if its controller StatefulSet object was deleted or decreased its replicas and the pod index was out of the replicas.
if s.gcConfig.EnableStatefulSet && ownerRef != nil &&
ownerRef.APIVersion == appsv1.SchemeGroupVersion.String() && ownerRef.Kind == constant.KindStatefulSet {
isValidStsPod, err := s.stsMgr.IsValidStatefulSetPod(ctx, currentPod.Namespace, currentPod.Name, ownerRef.Kind)

endpoint, err := s.wepMgr.GetEndpointByName(ctx, currentPod.Namespace, currentPod.Name, constant.IgnoreCache)
if err != nil {
if apierrors.IsNotFound(err) {
return nil, nil
}
logger.Sugar().Errorf("failed to get SpiderEndpoint: %v", err)
return nil, err
}
isValidStsPod, err := s.stsMgr.IsValidStatefulSetPod(ctx, currentPod.Namespace, currentPod.Name, ownerRef.Kind, endpoint)
if nil != err {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/gcmanager/scanAll_IPPool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (s *SpiderGC) executeScanAll(ctx context.Context) {
// case: The pod in IPPool's ip-allocationDetail is not exist in k8s
if apierrors.IsNotFound(err) {
wrappedLog := scanAllLogger.With(zap.String("gc-reason", "pod not found in k8s but still exists in IPPool allocation"))
endpoint, err := s.wepMgr.GetEndpointByName(ctx, podNS, podName, constant.UseCache)
endpoint, err := s.wepMgr.GetEndpointByName(ctx, podNS, podName, constant.IgnoreCache)
if nil != err {
// just continue if we meet other errors
if !apierrors.IsNotFound(err) {
Expand All @@ -134,7 +134,7 @@ func (s *SpiderGC) executeScanAll(ctx context.Context) {
}
} else {
if s.gcConfig.EnableStatefulSet && endpoint.Status.OwnerControllerType == constant.KindStatefulSet {
isValidStsPod, err := s.stsMgr.IsValidStatefulSetPod(ctx, podNS, podName, constant.KindStatefulSet)
isValidStsPod, err := s.stsMgr.IsValidStatefulSetPod(ctx, podNS, podName, constant.KindStatefulSet, endpoint)
if nil != err {
scanAllLogger.Sugar().Errorf("failed to check StatefulSet pod IP '%s' should be cleaned or not, error: %v", poolIP, err)
continue
Expand Down
2 changes: 1 addition & 1 deletion pkg/ipam/release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (i *ipam) releaseForAllNICs(ctx context.Context, uid, nic string, endpoint
// Check whether an StatefulSet needs to release its currently allocated IP addresses.
// It is discussed in https://github.com/spidernet-io/spiderpool/issues/1045
if i.config.EnableStatefulSet && endpoint.Status.OwnerControllerType == constant.KindStatefulSet {
isValidStatefulSetPod, err := i.stsManager.IsValidStatefulSetPod(ctx, endpoint.Namespace, endpoint.Name, endpoint.Status.OwnerControllerType)
isValidStatefulSetPod, err := i.stsManager.IsValidStatefulSetPod(ctx, endpoint.Namespace, endpoint.Name, endpoint.Status.OwnerControllerType, endpoint)
if nil != err {
return fmt.Errorf("failed to check pod '%s/%s' whether is a valid StatefulSet pod, error: %w", endpoint.Namespace, endpoint.Name, err)
}
Expand Down
87 changes: 85 additions & 2 deletions pkg/statefulsetmanager/sts_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ package statefulsetmanager

import (
"context"
"encoding/json"
"fmt"
spiderpoolv2beta1 "github.com/spidernet-io/spiderpool/pkg/k8s/apis/spiderpool.spidernet.io/v2beta1"
"github.com/spidernet-io/spiderpool/pkg/types"

appsv1 "k8s.io/api/apps/v1"
apitypes "k8s.io/apimachinery/pkg/types"
Expand All @@ -17,7 +20,7 @@ import (
type StatefulSetManager interface {
GetStatefulSetByName(ctx context.Context, namespace, name string, cached bool) (*appsv1.StatefulSet, error)
ListStatefulSets(ctx context.Context, cached bool, opts ...client.ListOption) (*appsv1.StatefulSetList, error)
IsValidStatefulSetPod(ctx context.Context, namespace, podName, podControllerType string) (bool, error)
IsValidStatefulSetPod(ctx context.Context, namespace, podName, podControllerType string, endpoint *spiderpoolv2beta1.SpiderEndpoint) (bool, error)
}

type statefulSetManager struct {
Expand Down Expand Up @@ -70,7 +73,7 @@ func (sm *statefulSetManager) ListStatefulSets(ctx context.Context, cached bool,
// IsValidStatefulSetPod only serves for StatefulSet pod, it will check the pod whether need to be cleaned up with the given params podNS, podName.
// Once the pod's controller StatefulSet was deleted, the pod's corresponding IPPool IP and Endpoint need to be cleaned up.
// Or the pod's controller StatefulSet decreased its replicas and the pod's index is out of replicas, it needs to be cleaned up too.
func (sm *statefulSetManager) IsValidStatefulSetPod(ctx context.Context, namespace, podName, podControllerType string) (bool, error) {
func (sm *statefulSetManager) IsValidStatefulSetPod(ctx context.Context, namespace, podName, podControllerType string, endpoint *spiderpoolv2beta1.SpiderEndpoint) (bool, error) {
if podControllerType != constant.KindStatefulSet {
return false, fmt.Errorf("pod '%s/%s' is controlled by '%s' instead of StatefulSet", namespace, podName, podControllerType)
}
Expand All @@ -85,6 +88,86 @@ func (sm *statefulSetManager) IsValidStatefulSetPod(ctx context.Context, namespa
return false, client.IgnoreNotFound(err)
}

// The pool name in the StatefulSet is inconsistent with the assigned pool name in the Endpoint.
if endpoint != nil {
if anno, ok := sts.Spec.Template.Annotations[constant.AnnoPodIPPool]; ok {
stsPoolMap := make(map[string]struct{})
var annoIPPoolValue types.AnnoPodIPPoolValue
err := json.Unmarshal([]byte(anno), &annoIPPoolValue)
if err != nil {
return false, fmt.Errorf("failed to unmarshal StatefulSet .spec.template pool annotation: %w", err)

Check warning on line 98 in pkg/statefulsetmanager/sts_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/statefulsetmanager/sts_manager.go#L98 

Added line #L98 was not covered by tests
}
if len(annoIPPoolValue.IPv4Pools) != 0 {
for _, val := range annoIPPoolValue.IPv4Pools {
stsPoolMap[val] = struct{}{}
}
}
if len(annoIPPoolValue.IPv6Pools) != 0 {
for _, val := range annoIPPoolValue.IPv6Pools {
stsPoolMap[val] = struct{}{}

Check warning on line 107 in pkg/statefulsetmanager/sts_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/statefulsetmanager/sts_manager.go#L106-L107 

Added lines #L106 - L107 were not covered by tests
}
}
if len(stsPoolMap) != 0 {
allocatedEndpointPool := make([]string, 0)
for _, ip := range endpoint.Status.Current.IPs {
if ip.IPv4Pool != nil && *ip.IPv4Pool != "" {
allocatedEndpointPool = append(allocatedEndpointPool, *ip.IPv4Pool)
}
if ip.IPv6Pool != nil && *ip.IPv6Pool != "" {
allocatedEndpointPool = append(allocatedEndpointPool, *ip.IPv6Pool)

Check warning on line 117 in pkg/statefulsetmanager/sts_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/statefulsetmanager/sts_manager.go#L117 

Added line #L117 was not covered by tests
}
}
if len(allocatedEndpointPool) == 0 {
return false, nil

Check warning on line 121 in pkg/statefulsetmanager/sts_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/statefulsetmanager/sts_manager.go#L121 

Added line #L121 was not covered by tests
}
for _, pool := range allocatedEndpointPool {
if _, ok := stsPoolMap[pool]; !ok {
return false, nil

Check warning on line 125 in pkg/statefulsetmanager/sts_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/statefulsetmanager/sts_manager.go#L125 

Added line #L125 was not covered by tests
}
}
}
} else if anno, ok := sts.Spec.Template.Annotations[constant.AnnoPodIPPools]; ok {
stsNICToPoolMap := make(map[string]map[string]struct{})
var annoIPPoolValue types.AnnoPodIPPoolsValue
err := json.Unmarshal([]byte(anno), &annoIPPoolValue)
if err != nil {
return false, fmt.Errorf("failed to unmarshal StatefulSet .spec.template pools annotation: %w", err)

Check warning on line 134 in pkg/statefulsetmanager/sts_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/statefulsetmanager/sts_manager.go#L134 

Added line #L134 was not covered by tests
}
for _, item := range annoIPPoolValue {
if _, ok := stsNICToPoolMap[item.NIC]; !ok {
stsNICToPoolMap[item.NIC] = make(map[string]struct{})
}
if len(item.IPv4Pools) != 0 {
for _, val := range item.IPv4Pools {
stsNICToPoolMap[item.NIC][val] = struct{}{}
}
}
if len(item.IPv6Pools) != 0 {
for _, val := range item.IPv6Pools {
stsNICToPoolMap[item.NIC][val] = struct{}{}
}
}
}
if len(stsNICToPoolMap) != 0 {
for _, ip := range endpoint.Status.Current.IPs {
if _, ok := stsNICToPoolMap[ip.NIC]; !ok {
return false, nil

Check warning on line 154 in pkg/statefulsetmanager/sts_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/statefulsetmanager/sts_manager.go#L154 

Added line #L154 was not covered by tests
}
if ip.IPv4Pool != nil && *ip.IPv4Pool != "" {
if _, ok := stsNICToPoolMap[ip.NIC][*ip.IPv4Pool]; !ok {
return false, nil

Check warning on line 158 in pkg/statefulsetmanager/sts_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/statefulsetmanager/sts_manager.go#L158 

Added line #L158 was not covered by tests
}
}
if ip.IPv6Pool != nil && *ip.IPv6Pool != "" {
if _, ok := stsNICToPoolMap[ip.NIC][*ip.IPv6Pool]; !ok {
return false, nil

Check warning on line 163 in pkg/statefulsetmanager/sts_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/statefulsetmanager/sts_manager.go#L163 

Added line #L163 was not covered by tests
}
}
}
}
}
}

// Ref: https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#start-ordinal
if sts.Spec.Ordinals != nil {
startIndex := int(sts.Spec.Ordinals.Start)
Expand Down
104 changes: 96 additions & 8 deletions pkg/statefulsetmanager/sts_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package statefulsetmanager_test
import (
"context"
"fmt"
spiderpool "github.com/spidernet-io/spiderpool/pkg/k8s/apis/spiderpool.spidernet.io/v2beta1"
"strconv"
"sync/atomic"

Expand Down Expand Up @@ -226,7 +227,7 @@ var _ = Describe("StatefulSetManager", Label("sts_manager_test"), func() {

Describe("IsValidStatefulSetPod", func() {
It("is not a Pod of StatefulSet", func() {
valid, err := stsManager.IsValidStatefulSetPod(ctx, namespace, "orphan-pod", constant.KindPod)
valid, err := stsManager.IsValidStatefulSetPod(ctx, namespace, "orphan-pod", constant.KindPod, nil)
Expect(err).To(HaveOccurred())
Expect(valid).To(BeFalse())
})
Expand All @@ -235,13 +236,13 @@ var _ = Describe("StatefulSetManager", Label("sts_manager_test"), func() {
patches := gomonkey.ApplyFuncReturn(strconv.ParseInt, int64(0), constant.ErrUnknown)
defer patches.Reset()

valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 0), constant.KindStatefulSet)
valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 0), constant.KindStatefulSet, nil)
Expect(err).NotTo(HaveOccurred())
Expect(valid).To(BeFalse())
})

It("is a valid Pod controlled by StatefulSet, but the StatefulSet no longer exists", func() {
valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 0), constant.KindStatefulSet)
valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 0), constant.KindStatefulSet, nil)
Expect(err).NotTo(HaveOccurred())
Expect(valid).To(BeFalse())
})
Expand All @@ -250,7 +251,7 @@ var _ = Describe("StatefulSetManager", Label("sts_manager_test"), func() {
patches := gomonkey.ApplyMethodReturn(fakeAPIReader, "Get", constant.ErrUnknown)
defer patches.Reset()

valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 0), constant.KindStatefulSet)
valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 0), constant.KindStatefulSet, nil)
Expect(err).To(HaveOccurred())
Expect(valid).To(BeFalse())
})
Expand All @@ -262,7 +263,7 @@ var _ = Describe("StatefulSetManager", Label("sts_manager_test"), func() {
err := tracker.Add(stsT)
Expect(err).NotTo(HaveOccurred())

valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, replicas), constant.KindStatefulSet)
valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, replicas), constant.KindStatefulSet, nil)
Expect(err).NotTo(HaveOccurred())
Expect(valid).To(BeFalse())
})
Expand All @@ -274,7 +275,7 @@ var _ = Describe("StatefulSetManager", Label("sts_manager_test"), func() {
err := tracker.Add(stsT)
Expect(err).NotTo(HaveOccurred())

valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, replicas-1), constant.KindStatefulSet)
valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, replicas-1), constant.KindStatefulSet, nil)
Expect(err).NotTo(HaveOccurred())
Expect(valid).To(BeTrue())
})
Expand All @@ -286,7 +287,7 @@ var _ = Describe("StatefulSetManager", Label("sts_manager_test"), func() {
err := tracker.Add(stsT)
Expect(err).NotTo(HaveOccurred())

valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 0), constant.KindStatefulSet)
valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 0), constant.KindStatefulSet, nil)
Expect(err).NotTo(HaveOccurred())
Expect(valid).To(BeFalse())
})
Expand All @@ -298,10 +299,97 @@ var _ = Describe("StatefulSetManager", Label("sts_manager_test"), func() {
err := tracker.Add(stsT)
Expect(err).NotTo(HaveOccurred())

valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 2), constant.KindStatefulSet)
valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 2), constant.KindStatefulSet, nil)
Expect(err).NotTo(HaveOccurred())
Expect(valid).To(BeTrue())
})

It("a valid StatefulSet with annotation pool", func() {
stsT.Spec.Replicas = ptr.To(int32(1))
stsT.Spec.Ordinals = &appsv1.StatefulSetOrdinals{Start: 2}

raw := `{"ipv4": ["master-10-6"]}`

stsT.Spec.Template.Annotations = map[string]string{
constant.AnnoPodIPPool: raw,
}

pool := "master-10-6"
endpoint := &spiderpool.SpiderEndpoint{
Status: spiderpool.WorkloadEndpointStatus{
Current: spiderpool.PodIPAllocation{
UID: "",
Node: "",
IPs: []spiderpool.IPAllocationDetail{
{
IPv4Pool: &pool,
},
},
},
},
}

err := tracker.Add(stsT)
Expect(err).NotTo(HaveOccurred())

valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 2), constant.KindStatefulSet, endpoint)
Expect(err).NotTo(HaveOccurred())
Expect(valid).To(BeTrue())
})
})

It("a valid StatefulSet with annotation pools", func() {
stsT.Spec.Replicas = ptr.To(int32(1))
stsT.Spec.Ordinals = &appsv1.StatefulSetOrdinals{Start: 2}

raw := `[{
"interface": "eth0",
"ipv4": ["demo-v4-ippool1"],
"ipv6": ["demo-v6-ippool1"],
"cleangateway": true
},{
"interface": "net1",
"ipv4": ["demo-v4-ippool2"],
"ipv6": ["demo-v6-ippool2"],
"cleangateway": false
}]`

stsT.Spec.Template.Annotations = map[string]string{
constant.AnnoPodIPPools: raw,
}

demov4ippool1 := "demo-v4-ippool1"
demov6ippool1 := "demo-v6-ippool1"
demov4ippool2 := "demo-v4-ippool2"
demov6ippool2 := "demo-v6-ippool2"

endpoint := &spiderpool.SpiderEndpoint{
Status: spiderpool.WorkloadEndpointStatus{
Current: spiderpool.PodIPAllocation{
UID: "",
Node: "",
IPs: []spiderpool.IPAllocationDetail{
{
IPv4Pool: &demov4ippool1,
IPv6Pool: &demov6ippool1,
NIC: "eth0",
},
{
IPv4Pool: &demov4ippool2,
IPv6Pool: &demov6ippool2,
NIC: "net1",
},
},
},
},
}

err := tracker.Add(stsT)
Expect(err).NotTo(HaveOccurred())

valid, err := stsManager.IsValidStatefulSetPod(ctx, stsT.Namespace, fmt.Sprintf("%s-%d", stsName, 2), constant.KindStatefulSet, endpoint)
Expect(err).NotTo(HaveOccurred())
Expect(valid).To(BeTrue())
})
})
})

0 comments on commit 21368fd

Please sign in to comment.