Fix clusterInfoSnapshot may be nil it will caused painc

[mathlsj]

Signed-off-by: mathlsj mathlsj@outlook.com

What type of PR is this?
Fix clusterInfoSnapshot may be nil it will caused painc

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

[codecov-commenter]

Codecov Report

Merging #2645 (5a9003d) into master (05be79f) will decrease coverage by 0.11%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #2645      +/-   ##
==========================================
- Coverage   23.81%   23.70%   -0.12%     
==========================================
  Files         184      184              
  Lines       18502    18505       +3     
==========================================
- Hits         4407     4387      -20     
- Misses      13749    13770      +21     
- Partials      346      348       +2     

Flag	Coverage Δ
unittests	23.70% <0.00%> (-0.12%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/scheduler/core/generic_scheduler.go	0.00% <0.00%> (ø)
pkg/util/serviceaccount.go	27.77% <0.00%> (-18.06%)	⬇️
pkg/search/proxy/store/store.go	65.26% <0.00%> (-3.16%)	⬇️
pkg/search/proxy/store/util.go	52.63% <0.00%> (-2.34%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[jwcesign]

/lgtm

[karmada-bot]

@jwcesign: changing LGTM is restricted to collaborators

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[RainbowMango]

Given the snapshot just holds a cluster slice, and in most cases, it wouldn't empty, so I suggest to get rid of the point here:

diff --git a/pkg/scheduler/cache/cache.go b/pkg/scheduler/cache/cache.go
index ecd8b335..c221e781 100644
--- a/pkg/scheduler/cache/cache.go
+++ b/pkg/scheduler/cache/cache.go
@@ -15,7 +15,7 @@ type Cache interface {
        UpdateCluster(cluster *clusterv1alpha1.Cluster)
        DeleteCluster(cluster *clusterv1alpha1.Cluster)
        // Snapshot returns a snapshot of the current clusters info
-       Snapshot() *Snapshot
+       Snapshot() Snapshot
 }

 type schedulerCache struct {
@@ -42,13 +42,15 @@ func (c *schedulerCache) DeleteCluster(cluster *clusterv1alpha1.Cluster) {
 }

 // TODO: need optimization, only clone when necessary
-func (c *schedulerCache) Snapshot() *Snapshot {
+func (c *schedulerCache) Snapshot() Snapshot {
+       out := NewEmptySnapshot()
+
        clusters, err := c.clusterLister.List(labels.Everything())
        if err != nil {
                klog.Errorf("Failed to list clusters: %v", err)
-               return nil
+               return out
        }
-       out := NewEmptySnapshot()
+
        out.clusterInfoList = make([]*framework.ClusterInfo, 0, len(clusters))
        for _, cluster := range clusters {
                cloned := cluster.DeepCopy()
diff --git a/pkg/scheduler/cache/snapshot.go b/pkg/scheduler/cache/snapshot.go
index 7b9c3b84..2bc5e319 100644
--- a/pkg/scheduler/cache/snapshot.go
+++ b/pkg/scheduler/cache/snapshot.go
@@ -15,8 +15,8 @@ type Snapshot struct {
 }

 // NewEmptySnapshot initializes a Snapshot struct and returns it.
-func NewEmptySnapshot() *Snapshot {
-       return &Snapshot{}
+func NewEmptySnapshot() Snapshot {
+       return Snapshot{}
 }
 
 // NumOfClusters returns the number of clusters.

[mathlsj]

Given the snapshot just holds a cluster slice, and in most cases, it wouldn't empty, so I suggest to get rid of the point here:

diff --git a/pkg/scheduler/cache/cache.go b/pkg/scheduler/cache/cache.go
index ecd8b335..c221e781 100644
--- a/pkg/scheduler/cache/cache.go
+++ b/pkg/scheduler/cache/cache.go
@@ -15,7 +15,7 @@ type Cache interface {
        UpdateCluster(cluster *clusterv1alpha1.Cluster)
        DeleteCluster(cluster *clusterv1alpha1.Cluster)
        // Snapshot returns a snapshot of the current clusters info
-       Snapshot() *Snapshot
+       Snapshot() Snapshot
 }

 type schedulerCache struct {
@@ -42,13 +42,15 @@ func (c *schedulerCache) DeleteCluster(cluster *clusterv1alpha1.Cluster) {
 }

 // TODO: need optimization, only clone when necessary
-func (c *schedulerCache) Snapshot() *Snapshot {
+func (c *schedulerCache) Snapshot() Snapshot {
+       out := NewEmptySnapshot()
+
        clusters, err := c.clusterLister.List(labels.Everything())
        if err != nil {
                klog.Errorf("Failed to list clusters: %v", err)
-               return nil
+               return out
        }
-       out := NewEmptySnapshot()
+
        out.clusterInfoList = make([]*framework.ClusterInfo, 0, len(clusters))
        for _, cluster := range clusters {
                cloned := cluster.DeepCopy()
diff --git a/pkg/scheduler/cache/snapshot.go b/pkg/scheduler/cache/snapshot.go
index 7b9c3b84..2bc5e319 100644
--- a/pkg/scheduler/cache/snapshot.go
+++ b/pkg/scheduler/cache/snapshot.go
@@ -15,8 +15,8 @@ type Snapshot struct {
 }

 // NewEmptySnapshot initializes a Snapshot struct and returns it.
-func NewEmptySnapshot() *Snapshot {
-       return &Snapshot{}
+func NewEmptySnapshot() Snapshot {
+       return Snapshot{}
 }
 
 // NumOfClusters returns the number of clusters.

@RainbowMango Update

[RainbowMango]

/lgtm
/approve

[karmada-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment