enhance: ServiceQuality supports multiple results returned by a single probe (#117)

Signed-off-by: ChrisLiu <chrisliu1995@163.com>

apis/v1alpha1/gameserver_types.go

@@ -77,15 +77,20 @@ type ServiceQuality struct {
 }
 
 type ServiceQualityCondition struct {
-	Name                     string      `json:"name"`
+	Name   string `json:"name"`
-	Status                   string      `json:"status,omitempty"`
+	Status string `json:"status,omitempty"`
+	// Result indicate the probe message returned by the script
+	Result                   string      `json:"result,omitempty"`
 	LastProbeTime            metav1.Time `json:"lastProbeTime,omitempty"`
 	LastTransitionTime       metav1.Time `json:"lastTransitionTime,omitempty"`
 	LastActionTransitionTime metav1.Time `json:"lastActionTransitionTime,omitempty"`
 }
 
 type ServiceQualityAction struct {
-	State          bool `json:"state"`
+	State bool `json:"state"`
+	// Result indicate the probe message returned by the script.
+	// When Result is defined, it would exec action only when the according Result is actually returns.
+	Result         string `json:"result,omitempty"`
 	GameServerSpec `json:",inline"`
 }
 

config/crd/bases/game.kruise.io_gameservers.yaml

@@ -855,6 +855,10 @@ spec:
                       type: string
                     name:
                       type: string
+                    result:
+                      description: Result indicate the probe message returned by the
+                        script
+                      type: string
                     status:
                       type: string
                   required:

config/crd/bases/game.kruise.io_gameserversets.yaml

@@ -544,6 +544,11 @@ spec:
                             type: boolean
                           opsState:
                             type: string
+                          result:
+                            description: Result indicate the probe message returned
+                              by the script. When Result is defined, it would exec
+                              action only when the according Result is actually returns.
+                            type: string
                           state:
                             type: boolean
                           updatePriority:

pkg/controllers/gameserver/gameserver_manager.go

@@ -33,6 +33,7 @@ import (
 	"reflect"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"strconv"
+	"strings"
 	"time"
 )
 
@@ -316,25 +317,29 @@ func syncServiceQualities(serviceQualities []gameKruiseV1alpha1.ServiceQuality,
 	for _, sqc := range sqConditions {
 		sqConditionsMap[sqc.Name] = sqc
 	}
+	timeNow := metav1.Now()
 	for _, sq := range serviceQualities {
 		var newSqCondition gameKruiseV1alpha1.ServiceQualityCondition
 		newSqCondition.Name = sq.Name
 		index, podCondition := util.GetPodConditionFromList(podConditions, corev1.PodConditionType(util.AddPrefixGameKruise(sq.Name)))
 		if index != -1 {
+			podConditionMessage := strings.ReplaceAll(podCondition.Message, "|", "")
+			podConditionMessage = strings.ReplaceAll(podConditionMessage, "\n", "")
 			newSqCondition.Status = string(podCondition.Status)
+			newSqCondition.Result = podConditionMessage
 			newSqCondition.LastProbeTime = podCondition.LastProbeTime
 			var lastActionTransitionTime metav1.Time
 			sqCondition, exist := sqConditionsMap[sq.Name]
-			if !exist || (sqCondition.Status != string(podCondition.Status) && (sqCondition.LastActionTransitionTime.IsZero() || !sq.Permanent)) {
+			if !exist || ((sqCondition.Status != string(podCondition.Status) || (sqCondition.Result != podConditionMessage)) && (sqCondition.LastActionTransitionTime.IsZero() || !sq.Permanent)) {
 				// exec action
 				for _, action := range sq.ServiceQualityAction {
 					state, err := strconv.ParseBool(string(podCondition.Status))
-					if err == nil && state == action.State {
+					if err == nil && state == action.State && (action.Result == "" || podConditionMessage == action.Result) {
 						spec.DeletionPriority = action.DeletionPriority
 						spec.UpdatePriority = action.UpdatePriority
 						spec.OpsState = action.OpsState
 						spec.NetworkDisabled = action.NetworkDisabled
-						lastActionTransitionTime = metav1.Now()
+						lastActionTransitionTime = timeNow
 					}
 				}
 			} else {
@@ -342,7 +347,13 @@ func syncServiceQualities(serviceQualities []gameKruiseV1alpha1.ServiceQuality,
 			}
 			newSqCondition.LastActionTransitionTime = lastActionTransitionTime
 		}
-		newSqCondition.LastTransitionTime = metav1.Now()
+
+		// Set LastTransitionTime, which depends on which value, the LastActionTransitionTime or LastProbeTime, is closer to the current time.
+		if timeNow.Sub(newSqCondition.LastActionTransitionTime.Time) < timeNow.Sub(newSqCondition.LastProbeTime.Time) {
+			newSqCondition.LastTransitionTime = newSqCondition.LastActionTransitionTime
+		} else {
+			newSqCondition.LastTransitionTime = newSqCondition.LastProbeTime
+		}
 		newGsConditions = append(newGsConditions, newSqCondition)
 	}
 	return spec, newGsConditions

pkg/controllers/gameserver/gameserver_manager_test.go

@@ -42,6 +42,7 @@ func TestSyncServiceQualities(t *testing.T) {
 		spec             gameKruiseV1alpha1.GameServerSpec
 		newSqConditions  []gameKruiseV1alpha1.ServiceQualityCondition
 	}{
+		//case 0
 		{
 			serviceQualities: []gameKruiseV1alpha1.ServiceQuality{
 				{
@@ -88,6 +89,7 @@ func TestSyncServiceQualities(t *testing.T) {
 				},
 			},
 		},
+		// case 1
 		{
 			serviceQualities: []gameKruiseV1alpha1.ServiceQuality{
 				{
@@ -139,6 +141,7 @@ func TestSyncServiceQualities(t *testing.T) {
 				},
 			},
 		},
+		// case 2
 		{
 			serviceQualities: []gameKruiseV1alpha1.ServiceQuality{
 				{
@@ -175,6 +178,7 @@ func TestSyncServiceQualities(t *testing.T) {
 				},
 			},
 		},
+		// case 3
 		{
 			serviceQualities: []gameKruiseV1alpha1.ServiceQuality{
 				{
@@ -212,6 +216,7 @@ func TestSyncServiceQualities(t *testing.T) {
 				},
 			},
 		},
+		// case 4
 		{
 			serviceQualities: []gameKruiseV1alpha1.ServiceQuality{
 				{
@@ -265,17 +270,131 @@ func TestSyncServiceQualities(t *testing.T) {
 				},
 			},
 		},
+		// case 5
+		{
+			serviceQualities: []gameKruiseV1alpha1.ServiceQuality{
+				{
+					Name:      "multi-return",
+					Permanent: false,
+					ServiceQualityAction: []gameKruiseV1alpha1.ServiceQualityAction{
+						{
+							State:  true,
+							Result: "A",
+							GameServerSpec: gameKruiseV1alpha1.GameServerSpec{
+								OpsState: "A",
+							},
+						},
+						{
+							State:  true,
+							Result: "B",
+							GameServerSpec: gameKruiseV1alpha1.GameServerSpec{
+								OpsState: "B",
+							},
+						},
+						{
+							State:  true,
+							Result: "C",
+							GameServerSpec: gameKruiseV1alpha1.GameServerSpec{
+								OpsState: "C",
+							},
+						},
+					},
+				},
+			},
+			podConditions: []corev1.PodCondition{
+				{
+					Type:          "game.kruise.io/multi-return",
+					Status:        corev1.ConditionTrue,
+					Message:       "B",
+					LastProbeTime: fakeProbeTime,
+				},
+			},
+			sqConditions: nil,
+			spec: gameKruiseV1alpha1.GameServerSpec{
+				OpsState: "B",
+			},
+			newSqConditions: []gameKruiseV1alpha1.ServiceQualityCondition{
+				{
+					Name:                     "multi-return",
+					Result:                   "B",
+					Status:                   string(corev1.ConditionTrue),
+					LastProbeTime:            fakeProbeTime,
+					LastActionTransitionTime: fakeActionTime,
+				},
+			},
+		},
+		// case 6
+		{
+			serviceQualities: []gameKruiseV1alpha1.ServiceQuality{
+				{
+					Name:      "multi-return",
+					Permanent: false,
+					ServiceQualityAction: []gameKruiseV1alpha1.ServiceQualityAction{
+						{
+							State:  true,
+							Result: "A",
+							GameServerSpec: gameKruiseV1alpha1.GameServerSpec{
+								OpsState: "A",
+							},
+						},
+						{
+							State:  true,
+							Result: "B",
+							GameServerSpec: gameKruiseV1alpha1.GameServerSpec{
+								OpsState: "B",
+							},
+						},
+						{
+							State:  true,
+							Result: "C",
+							GameServerSpec: gameKruiseV1alpha1.GameServerSpec{
+								OpsState: "C",
+							},
+						},
+					},
+				},
+			},
+			podConditions: []corev1.PodCondition{
+				{
+					Type:          "game.kruise.io/multi-return",
+					Status:        corev1.ConditionTrue,
+					Message:       "A",
+					LastProbeTime: fakeProbeTime,
+				},
+			},
+			sqConditions: []gameKruiseV1alpha1.ServiceQualityCondition{
+				{
+					Name:                     "multi-return",
+					Result:                   "B",
+					Status:                   string(corev1.ConditionTrue),
+					LastProbeTime:            fakeProbeTime,
+					LastActionTransitionTime: fakeActionTime,
+				},
+			},
+			spec: gameKruiseV1alpha1.GameServerSpec{
+				OpsState: "A",
+			},
+			newSqConditions: []gameKruiseV1alpha1.ServiceQualityCondition{
+				{
+					Name:                     "multi-return",
+					Result:                   "A",
+					Status:                   string(corev1.ConditionTrue),
+					LastProbeTime:            fakeProbeTime,
+					LastActionTransitionTime: fakeActionTime,
+				},
+			},
+		},
 	}
 
-	for _, test := range tests {
+	for i, test := range tests {
 		actualSpec, actualNewSqConditions := syncServiceQualities(test.serviceQualities, test.podConditions, test.sqConditions)
 		expectSpec := test.spec
 		expectNewSqConditions := test.newSqConditions
 		if !reflect.DeepEqual(actualSpec, expectSpec) {
-			t.Errorf("expect spec %v but got %v", expectSpec, actualSpec)
+			t.Errorf("case %d: expect spec %v but got %v", i, expectSpec, actualSpec)
 		}
 		if len(actualNewSqConditions) != len(expectNewSqConditions) {
-			t.Errorf("expect sq conditions len %v but got %v", len(expectNewSqConditions), len(actualNewSqConditions))
+			t.Errorf("case %d: expect sq conditions len %v but got %v", i, len(expectNewSqConditions), len(actualNewSqConditions))
 		}
 		for _, expectNewSqCondition := range expectNewSqConditions {
 			exist := false
@@ -283,19 +402,19 @@ func TestSyncServiceQualities(t *testing.T) {
 				if actualNewSqCondition.Name == expectNewSqCondition.Name {
 					exist = true
 					if actualNewSqCondition.Status != expectNewSqCondition.Status {
-						t.Errorf("expect sq condition status %v but got %v", expectNewSqCondition.Status, actualNewSqCondition.Status)
+						t.Errorf("case %d: expect sq condition status %v but got %v", i, expectNewSqCondition.Status, actualNewSqCondition.Status)
 					}
 					if actualNewSqCondition.LastProbeTime != expectNewSqCondition.LastProbeTime {
-						t.Errorf("expect sq condition LastProbeTime %v but got %v", expectNewSqCondition.LastProbeTime, actualNewSqCondition.LastProbeTime)
+						t.Errorf("case %d: expect sq condition LastProbeTime %v but got %v", i, expectNewSqCondition.LastProbeTime, actualNewSqCondition.LastProbeTime)
 					}
 					if actualNewSqCondition.LastActionTransitionTime.IsZero() != expectNewSqCondition.LastActionTransitionTime.IsZero() {
-						t.Errorf("expect sq condition LastActionTransitionTime IsZero %v but got %v", expectNewSqCondition.LastActionTransitionTime.IsZero(), actualNewSqCondition.LastActionTransitionTime.IsZero())
+						t.Errorf("case %d: expect sq condition LastActionTransitionTime IsZero %v but got %v", i, expectNewSqCondition.LastActionTransitionTime.IsZero(), actualNewSqCondition.LastActionTransitionTime.IsZero())
 					}
 					break
 				}
 			}
 			if !exist {
-				t.Errorf("expect sq condition %s exist, but actually not", expectNewSqCondition.Name)
+				t.Errorf("case %d: expect sq condition %s exist, but actually not", i, expectNewSqCondition.Name)
 			}
 		}
 	}