Skip to content

Commit

Permalink
fix: the statistics of ippool's status.AllocatedIPCount are inaccurate
Browse files Browse the repository at this point in the history
Signed-off-by: tao.yang <tao.yang@daocloud.io>
Signed-off-by: ty-dc <tao.yang@daocloud.io>
  • Loading branch information
ty-dc committed Aug 23, 2024
1 parent 802f450 commit d2cedea
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 5 deletions.
30 changes: 25 additions & 5 deletions pkg/gcmanager/scanAll_IPPool.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ import (
"sync"
"time"

"go.uber.org/zap"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/tools/cache"

"github.com/spidernet-io/spiderpool/pkg/constant"
spiderpoolv2beta1 "github.com/spidernet-io/spiderpool/pkg/k8s/apis/spiderpool.spidernet.io/v2beta1"
"github.com/spidernet-io/spiderpool/pkg/logutils"
"github.com/spidernet-io/spiderpool/pkg/podmanager"
"github.com/spidernet-io/spiderpool/pkg/types"
"github.com/spidernet-io/spiderpool/pkg/utils/convert"
"go.uber.org/zap"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/tools/cache"
"k8s.io/utils/ptr"
)

// monitorGCSignal will monitor signal from CLI, DefaultGCInterval
Expand Down Expand Up @@ -105,8 +105,15 @@ func (s *SpiderGC) executeScanAll(ctx context.Context) {
logger.Sugar().Errorf("failed to parse IPPool '%v' status AllocatedIPs, error: %v", pool, err)
continue
}

flagNeedUpdateIPPoolAllocatedIPCount := false
tempAllocatedIPCount := 0
if pool.Status.AllocatedIPCount != nil {
if *pool.Status.AllocatedIPCount > *pool.Status.TotalIPCount {
flagNeedUpdateIPPoolAllocatedIPCount = true

Check failure on line 112 in pkg/gcmanager/scanAll_IPPool.go

View workflow job for this annotation

GitHub Actions / lint-golang

ineffectual assignment to flagNeedUpdateIPPoolAllocatedIPCount (ineffassign)

Check failure on line 112 in pkg/gcmanager/scanAll_IPPool.go

View workflow job for this annotation

GitHub Actions / lint-golang

ineffectual assignment to flagNeedUpdateIPPoolAllocatedIPCount (ineffassign)
}
}
for poolIP, poolIPAllocation := range poolAllocatedIPs {
tempAllocatedIPCount++
podNS, podName, err := cache.SplitMetaNamespaceKey(poolIPAllocation.NamespacedName)
if err != nil {
logger.Error(err.Error())
Expand Down Expand Up @@ -352,6 +359,7 @@ func (s *SpiderGC) executeScanAll(ctx context.Context) {

GCIP:
if flagGCIPPoolIP {
tempAllocatedIPCount--
err = s.ippoolMgr.ReleaseIP(ctx, pool.Name, []types.IPAndUID{{
IP: poolIP,
UID: poolIPAllocation.PodUID},
Expand All @@ -371,6 +379,18 @@ func (s *SpiderGC) executeScanAll(ctx context.Context) {
}
}
}

if *pool.Status.AllocatedIPCount != int64(tempAllocatedIPCount) {
flagNeedUpdateIPPoolAllocatedIPCount = true
} else {
flagNeedUpdateIPPoolAllocatedIPCount = false
}
if flagNeedUpdateIPPoolAllocatedIPCount {
err = s.ippoolMgr.UpdateAllocatedIPCount(ctx, pool.Name, ptr.To(int64(tempAllocatedIPCount)))
if err != nil {
logger.Sugar().Errorf("failed to update IPPool '%v' status AllocatedIPCount, error: %v", pool.Name, err)
}
}
}
}

Expand Down
22 changes: 22 additions & 0 deletions pkg/ippoolmanager/ippool_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type IPPoolManager interface {
ReleaseIP(ctx context.Context, poolName string, ipAndUIDs []types.IPAndUID) error
UpdateAllocatedIPs(ctx context.Context, poolName, namespacedName string, ipAndCIDs []types.IPAndUID) error
ParseWildcardPoolNameList(ctx context.Context, PoolNames []string, ipVersion types.IPVersion) (newPoolNames []string, hasWildcard bool, err error)
UpdateAllocatedIPCount(ctx context.Context, poolName string, allocatedIPCount *int64) error
}

type ipPoolManager struct {
Expand Down Expand Up @@ -397,3 +398,24 @@ func (im *ipPoolManager) ParseWildcardPoolNameList(ctx context.Context, poolName

return poolNamesArr, false, nil
}

func (im *ipPoolManager) UpdateAllocatedIPCount(ctx context.Context, poolName string, allocatedIPCount *int64) error {
logger := logutils.FromContext(ctx)

Check warning on line 403 in pkg/ippoolmanager/ippool_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/ippoolmanager/ippool_manager.go#L402-L403

Added lines #L402 - L403 were not covered by tests

ipPool, err := im.GetIPPoolByName(ctx, poolName, constant.IgnoreCache)
if err != nil {
return err

Check warning on line 407 in pkg/ippoolmanager/ippool_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/ippoolmanager/ippool_manager.go#L405-L407

Added lines #L405 - L407 were not covered by tests
}

ipPool.Status.AllocatedIPCount = allocatedIPCount
resourceVersion := ipPool.ResourceVersion
if err := im.client.Status().Update(ctx, ipPool); err != nil {
if apierrors.IsConflict(err) {
metric.IpamAllocationUpdateIPPoolConflictCounts.Add(ctx, 1)
logger.With(zap.String("IPPool-ResourceVersion", resourceVersion)).Warn("An conflict occurred when updating the status of IPPool")

Check warning on line 415 in pkg/ippoolmanager/ippool_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/ippoolmanager/ippool_manager.go#L410-L415

Added lines #L410 - L415 were not covered by tests
}
return err

Check warning on line 417 in pkg/ippoolmanager/ippool_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/ippoolmanager/ippool_manager.go#L417

Added line #L417 was not covered by tests
}

return nil

Check warning on line 420 in pkg/ippoolmanager/ippool_manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/ippoolmanager/ippool_manager.go#L420

Added line #L420 was not covered by tests
}
17 changes: 17 additions & 0 deletions test/e2e/reclaim/reclaim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,23 @@ var _ = Describe("test ip with reclaim ip case", Label("reclaim"), func() {
// Delete Pod
Expect(frame.DeletePod(podName, namespace)).To(Succeed(), "Failed to delete pod %v/%v\n", namespace, podName)
GinkgoWriter.Printf("succeed to delete pod %v/%v\n", namespace, podName)

// Check whether the dirty IP data is recovered successfully and whether the AllocatedIPCount decreases and meets expectations?
Eventually(func() error {
if frame.Info.IpV4Enabled {
if err := common.CheckIppoolSanity(frame, v4poolName); err != nil {
return err
}
GinkgoWriter.Printf("successfully checked sanity of spiderpool %v \n", v4poolName)
}
if frame.Info.IpV6Enabled {
if err := common.CheckIppoolSanity(frame, v6poolName); err != nil {
return err
}
GinkgoWriter.Printf("successfully checked sanity of spiderpool %v \n", v6poolName)
}
return nil
}).WithTimeout(time.Minute).WithPolling(time.Second * 10).Should(BeNil())
},
Entry("a dirty IP record (pod name is wrong or containerID is wrong) in the IPPool should be auto clean by Spiderpool", Serial, Label("G00005", "G00007")),
)
Expand Down

0 comments on commit d2cedea

Please sign in to comment.