From af700f06cc4793deaa102d9f24f86ea3d4dff5cf Mon Sep 17 00:00:00 2001 From: ChrisLiu Date: Mon, 18 Dec 2023 20:23:19 +0800 Subject: [PATCH] enhance: ServiceQuality supports multiple results returned by a single probe Signed-off-by: ChrisLiu --- apis/v1alpha1/gameserver_types.go | 11 +- .../crd/bases/game.kruise.io_gameservers.yaml | 4 + .../bases/game.kruise.io_gameserversets.yaml | 5 + .../gameserver/gameserver_manager.go | 19 ++- .../gameserver/gameserver_manager_test.go | 133 +++++++++++++++++- 5 files changed, 158 insertions(+), 14 deletions(-) diff --git a/apis/v1alpha1/gameserver_types.go b/apis/v1alpha1/gameserver_types.go index bc29fd06..b9c93b18 100644 --- a/apis/v1alpha1/gameserver_types.go +++ b/apis/v1alpha1/gameserver_types.go @@ -77,15 +77,20 @@ type ServiceQuality struct { } type ServiceQualityCondition struct { - Name string `json:"name"` - Status string `json:"status,omitempty"` + Name string `json:"name"` + 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"` } diff --git a/config/crd/bases/game.kruise.io_gameservers.yaml b/config/crd/bases/game.kruise.io_gameservers.yaml index 590167ee..00902561 100644 --- a/config/crd/bases/game.kruise.io_gameservers.yaml +++ b/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: diff --git a/config/crd/bases/game.kruise.io_gameserversets.yaml b/config/crd/bases/game.kruise.io_gameserversets.yaml index 12d1cce5..60ae5be9 100644 --- a/config/crd/bases/game.kruise.io_gameserversets.yaml +++ b/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: diff --git a/pkg/controllers/gameserver/gameserver_manager.go b/pkg/controllers/gameserver/gameserver_manager.go index d49cf590..3f9f25d8 100644 --- a/pkg/controllers/gameserver/gameserver_manager.go +++ b/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 diff --git a/pkg/controllers/gameserver/gameserver_manager_test.go b/pkg/controllers/gameserver/gameserver_manager_test.go index 2c23f347..f62a2e7a 100644 --- a/pkg/controllers/gameserver/gameserver_manager_test.go +++ b/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) } } }