Merge pull request #69268 from Lion-Wei/fix-graceful-termination

pkg/proxy/ipvs/graceful_termination.go

@@ -82,9 +82,6 @@ func (q *graceTerminateRSList) remove(rs *listItem) bool {
 }
 
 func (q *graceTerminateRSList) flushList(handler func(rsToDelete *listItem) (bool, error)) bool {
-	q.lock.Lock()
-	defer q.lock.Unlock()
-
 	success := true
 	for name, rs := range q.list {
 		deleted, err := handler(rs)
@@ -105,8 +102,8 @@ func (q *graceTerminateRSList) exist(uniqueRS string) (*listItem, bool) {
 	q.lock.Lock()
 	defer q.lock.Unlock()
 
-	if _, ok := q.list[uniqueRS]; ok {
-		return nil, false
+	if rs, ok := q.list[uniqueRS]; ok {
+		return rs, true
 	}
 	return nil, false
 }
@@ -144,7 +141,7 @@ func (m *GracefulTerminationManager) GracefulDeleteRS(vs *utilipvs.VirtualServer
 	}
 	deleted, err := m.deleteRsFunc(ele)
 	if err != nil {
-		glog.Errorf("Delete rs %q err: %v", err)
+		glog.Errorf("Delete rs %q err: %v", ele.String(), err)
 	}
 	if deleted {
 		return nil
@@ -178,7 +175,7 @@ func (m *GracefulTerminationManager) deleteRsFunc(rsToDelete *listItem) (bool, e
 			return true, nil
 		}
 	}
-	return false, fmt.Errorf("Failed to delete rs %q, can't find the real server", rsToDelete.String())
+	return true, fmt.Errorf("Failed to delete rs %q, can't find the real server", rsToDelete.String())
 }
 
 func (m *GracefulTerminationManager) tryDeleteRs() {

pkg/proxy/ipvs/proxier.go

@@ -1533,7 +1533,7 @@ func (proxier *Proxier) syncEndpoint(svcPortName proxy.ServicePortName, onlyNode
 		}
 
 		if curEndpoints.Has(ep) {
-			// check if newEndpoint is in gracefulDelete list, is true, delete this ep immediately
+			// check if newEndpoint is in gracefulDelete list, if true, delete this ep immediately
 			uniqueRS := GetUniqueRSName(vs, newDest)
 			if !proxier.gracefuldeleteManager.InTerminationList(uniqueRS) {
 				continue
@@ -1591,10 +1591,14 @@ func (proxier *Proxier) cleanLegacyService(activeServices map[string]bool, curre
 			// This service was not processed in the latest sync loop so before deleting it,
 			// make sure it does not fall within an excluded CIDR range.
 			okayToDelete := true
-			rsList, err := proxier.ipvs.GetRealServers(svc)
-			if len(rsList) != 0 && err == nil {
-				glog.V(5).Infof("Will not delete VS: %v, cause it have RS: %v", svc, rsList)
-				okayToDelete = false
+			rsList, _ := proxier.ipvs.GetRealServers(svc)
+			for _, rs := range rsList {
+				uniqueRS := GetUniqueRSName(svc, rs)
+				// if there are in terminating real server in this service, then handle it later
+				if proxier.gracefuldeleteManager.InTerminationList(uniqueRS) {
+					okayToDelete = false
+					break
+				}
 			}
 			for _, excludedCIDR := range proxier.excludeCIDRs {
 				// Any validation of this CIDR already should have occurred.

pkg/util/ipvs/ipvs.go

@@ -108,6 +108,5 @@ func (rs *RealServer) String() string {
 // We don't use struct == since it doesn't work because of slice.
 func (rs *RealServer) Equal(other *RealServer) bool {
 	return rs.Address.Equal(other.Address) &&
-		rs.Port == other.Port &&
-		rs.Weight == other.Weight
+		rs.Port == other.Port
 }

pkg/util/ipvs/ipvs_test.go

@@ -249,12 +249,10 @@ func TestRealServerEqual(t *testing.T) {
 			rsA: &RealServer{
 				Address: net.ParseIP("10.20.30.40"),
 				Port:    80,
-				Weight:  1,
 			},
 			rsB: &RealServer{
 				Address: net.ParseIP("10.20.30.41"),
 				Port:    80,
-				Weight:  1,
 			},
 			equal:  false,
 			reason: "IPv4 address not equal",
@@ -263,12 +261,10 @@ func TestRealServerEqual(t *testing.T) {
 			rsA: &RealServer{
 				Address: net.ParseIP("2012::beef"),
 				Port:    80,
-				Weight:  1,
 			},
 			rsB: &RealServer{
 				Address: net.ParseIP("2017::beef"),
 				Port:    80,
-				Weight:  1,
 			},
 			equal:  false,
 			reason: "IPv6 address not equal",
@@ -277,40 +273,22 @@ func TestRealServerEqual(t *testing.T) {
 			rsA: &RealServer{
 				Address: net.ParseIP("2012::beef"),
 				Port:    80,
-				Weight:  1,
 			},
 			rsB: &RealServer{
 				Address: net.ParseIP("2012::beef"),
 				Port:    8080,
-				Weight:  1,
 			},
 			equal:  false,
 			reason: "Port not equal",
 		},
 		{
 			rsA: &RealServer{
-				Address: net.ParseIP("10.20.30.40"),
-				Port:    8080,
-				Weight:  1,
-			},
-			rsB: &RealServer{
-				Address: net.ParseIP("10.20.30.40"),
-				Port:    8080,
-				Weight:  10,
-			},
-			equal:  false,
-			reason: "Weight not equal",
-		},
-		{
-			rsA: &RealServer{
 				Address: net.ParseIP("1.2.3.4"),
 				Port:    3080,
-				Weight:  10,
 			},
 			rsB: &RealServer{
 				Address: net.ParseIP("1.2.3.4"),
 				Port:    3080,
-				Weight:  10,
 			},
 			equal:  true,
 			reason: "All fields equal",
@@ -319,12 +297,10 @@ func TestRealServerEqual(t *testing.T) {
 			rsA: &RealServer{
 				Address: net.ParseIP("2012::beef"),
 				Port:    3080,
-				Weight:  10,
 			},
 			rsB: &RealServer{
 				Address: net.ParseIP("2012::beef"),
 				Port:    3080,
-				Weight:  10,
 			},
 			equal:  true,
 			reason: "All fields equal",