From ed716b588412d3335fa2437b7f9ade0fd51cac72 Mon Sep 17 00:00:00 2001 From: ChrisLiu Date: Thu, 23 Mar 2023 20:54:13 +0800 Subject: [PATCH] fix: avoid gs status sync delay when network failure Signed-off-by: ChrisLiu --- cloudprovider/utils/network_manager.go | 8 ++-- .../gameserver/gameserver_controller.go | 4 +- .../gameserver/gameserver_manager.go | 35 ++++++++---------- .../gameserver/gameserver_manager_test.go | 37 ++++++++++++++----- 4 files changed, 49 insertions(+), 35 deletions(-) diff --git a/cloudprovider/utils/network_manager.go b/cloudprovider/utils/network_manager.go index e4300cb1..783914ee 100644 --- a/cloudprovider/utils/network_manager.go +++ b/cloudprovider/utils/network_manager.go @@ -47,11 +47,11 @@ func (nm *NetworkManager) SetNetworkState(disabled bool) error { } // Initial annotations if necessary - if patchPod.Annotations == nil { - patchPod.Annotations = make(map[string]string) + if patchPod.Labels == nil { + patchPod.Labels = make(map[string]string) } - patchPod.Annotations[v1alpha1.GameServerNetworkDisabled] = strconv.FormatBool(disabled) + patchPod.Labels[v1alpha1.GameServerNetworkDisabled] = strconv.FormatBool(disabled) patch := client.MergeFrom(patchPod) return nm.client.Patch(context.Background(), nm.pod, patch, nil) } @@ -126,7 +126,7 @@ func NewNetworkManager(pod *corev1.Pod, client client.Client) *NetworkManager { } var networkDisabled bool - if networkDisabledStr, ok := pod.Annotations[v1alpha1.GameServerNetworkDisabled]; ok { + if networkDisabledStr, ok := pod.Labels[v1alpha1.GameServerNetworkDisabled]; ok { networkDisabled, err = strconv.ParseBool(networkDisabledStr) if err != nil { log.Warningf("Pod %s has invalid network disabled option, err: %s", pod.Name, err.Error()) diff --git a/pkg/controllers/gameserver/gameserver_controller.go b/pkg/controllers/gameserver/gameserver_controller.go index ed9f4c9b..25b33e16 100644 --- a/pkg/controllers/gameserver/gameserver_controller.go +++ b/pkg/controllers/gameserver/gameserver_controller.go @@ -202,8 +202,8 @@ func (r *GameServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) return reconcile.Result{}, err } - podUpdated, err := gsm.SyncGsToPod() - if err != nil || podUpdated { + err = gsm.SyncGsToPod() + if err != nil { return reconcile.Result{RequeueAfter: 3 * time.Second}, err } diff --git a/pkg/controllers/gameserver/gameserver_manager.go b/pkg/controllers/gameserver/gameserver_manager.go index 43ec4952..3d17c56c 100644 --- a/pkg/controllers/gameserver/gameserver_manager.go +++ b/pkg/controllers/gameserver/gameserver_manager.go @@ -48,9 +48,8 @@ const ( type Control interface { // SyncGsToPod compares the pod with GameServer, and decide whether to update the pod based on the results. - // When the fields of the pod is different from that of GameServer, - // pod will be updated and true is returned; otherwise, the pod won't be updated and false is returned. - SyncGsToPod() (bool, error) + // When the fields of the pod is different from that of GameServer, pod will be updated. + SyncGsToPod() error // SyncPodToGs compares the GameServer with pod, and update the GameServer. SyncPodToGs(*gameKruiseV1alpha1.GameServerSet) error // WaitOrNot compare the current game server network status to decide whether to re-queue. @@ -64,7 +63,7 @@ type GameServerManager struct { eventRecorder record.EventRecorder } -func (manager GameServerManager) SyncGsToPod() (bool, error) { +func (manager GameServerManager) SyncGsToPod() error { pod := manager.pod gs := manager.gameServer podLabels := pod.GetLabels() @@ -72,8 +71,8 @@ func (manager GameServerManager) SyncGsToPod() (bool, error) { podUpdatePriority := podLabels[gameKruiseV1alpha1.GameServerUpdatePriorityKey] podGsOpsState := podLabels[gameKruiseV1alpha1.GameServerOpsStateKey] podGsState := podLabels[gameKruiseV1alpha1.GameServerStateKey] + podNetworkDisabled := podLabels[gameKruiseV1alpha1.GameServerNetworkDisabled] - updated := false newLabels := make(map[string]string) newAnnotations := make(map[string]string) if gs.Spec.DeletionPriority.String() != podDeletePriority { @@ -81,14 +80,12 @@ func (manager GameServerManager) SyncGsToPod() (bool, error) { if podDeletePriority != "" { manager.eventRecorder.Eventf(gs, corev1.EventTypeNormal, StateReason, "DeletionPriority turn from %s to %s ", podDeletePriority, gs.Spec.DeletionPriority.String()) } - updated = true } if gs.Spec.UpdatePriority.String() != podUpdatePriority { newLabels[gameKruiseV1alpha1.GameServerUpdatePriorityKey] = gs.Spec.UpdatePriority.String() if podUpdatePriority != "" { manager.eventRecorder.Eventf(gs, corev1.EventTypeNormal, StateReason, "UpdatePriority turn from %s to %s ", podUpdatePriority, gs.Spec.UpdatePriority.String()) } - updated = true } if string(gs.Spec.OpsState) != podGsOpsState { newLabels[gameKruiseV1alpha1.GameServerOpsStateKey] = string(gs.Spec.OpsState) @@ -99,7 +96,12 @@ func (manager GameServerManager) SyncGsToPod() (bool, error) { } manager.eventRecorder.Eventf(gs, eventType, StateReason, "OpsState turn from %s to %s ", podGsOpsState, string(gs.Spec.OpsState)) } - updated = true + } + if podNetworkDisabled != strconv.FormatBool(gs.Spec.NetworkDisabled) { + newLabels[gameKruiseV1alpha1.GameServerNetworkDisabled] = strconv.FormatBool(gs.Spec.NetworkDisabled) + if podNetworkDisabled != "" { + manager.eventRecorder.Eventf(gs, corev1.EventTypeNormal, StateReason, "NetworkDisabled turn from %s to %s ", podNetworkDisabled, strconv.FormatBool(gs.Spec.NetworkDisabled)) + } } var gsState gameKruiseV1alpha1.GameServerState @@ -142,36 +144,29 @@ func (manager GameServerManager) SyncGsToPod() (bool, error) { } manager.eventRecorder.Eventf(gs, eventType, StateReason, "State turn from %s to %s ", podGsState, string(gsState)) } - updated = true } if gsState == gameKruiseV1alpha1.Ready && pod.Annotations[gameKruiseV1alpha1.GameServerNetworkType] != "" { - if pod.Annotations[gameKruiseV1alpha1.GameServerNetworkDisabled] != strconv.FormatBool(gs.Spec.NetworkDisabled) { - newAnnotations[gameKruiseV1alpha1.GameServerNetworkDisabled] = strconv.FormatBool(gs.Spec.NetworkDisabled) - updated = true - } - oldTime, err := time.Parse(TimeFormat, pod.Annotations[gameKruiseV1alpha1.GameServerNetworkTriggerTime]) - if (err == nil && time.Since(oldTime) > NetworkIntervalTime) || (pod.Annotations[gameKruiseV1alpha1.GameServerNetworkTriggerTime] == "") { + if (err == nil && time.Since(oldTime) > NetworkIntervalTime && time.Since(gs.Status.NetworkStatus.LastTransitionTime.Time) < NetworkTotalWaitTime) || (pod.Annotations[gameKruiseV1alpha1.GameServerNetworkTriggerTime] == "") { newAnnotations[gameKruiseV1alpha1.GameServerNetworkTriggerTime] = time.Now().Format(TimeFormat) - updated = true } } - if updated { + if len(newLabels) != 0 || len(newAnnotations) != 0 { patchPod := map[string]interface{}{"metadata": map[string]map[string]string{"labels": newLabels, "annotations": newAnnotations}} patchPodBytes, err := json.Marshal(patchPod) if err != nil { - return updated, err + return err } err = manager.client.Patch(context.TODO(), pod, client.RawPatch(types.StrategicMergePatchType, patchPodBytes)) if err != nil && !errors.IsNotFound(err) { klog.Errorf("failed to patch Pod %s in %s,because of %s.", pod.GetName(), pod.GetNamespace(), err.Error()) - return updated, err + return err } } - return updated, nil + return nil } func (manager GameServerManager) SyncPodToGs(gss *gameKruiseV1alpha1.GameServerSet) error { diff --git a/pkg/controllers/gameserver/gameserver_manager_test.go b/pkg/controllers/gameserver/gameserver_manager_test.go index 76ec2dcb..878b9051 100644 --- a/pkg/controllers/gameserver/gameserver_manager_test.go +++ b/pkg/controllers/gameserver/gameserver_manager_test.go @@ -1,18 +1,21 @@ package gameserver import ( + "context" kruiseV1alpha1 "github.com/openkruise/kruise-api/apps/v1alpha1" kruiseV1beta1 "github.com/openkruise/kruise-api/apps/v1beta1" gameKruiseV1alpha1 "github.com/openkruise/kruise-game/apis/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "reflect" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "strconv" "testing" ) @@ -302,9 +305,8 @@ func TestSyncGsToPod(t *testing.T) { up := intstr.FromInt(20) dp := intstr.FromInt(10) tests := []struct { - gs *gameKruiseV1alpha1.GameServer - pod *corev1.Pod - isUpdate bool + gs *gameKruiseV1alpha1.GameServer + pod *corev1.Pod }{ { gs: &gameKruiseV1alpha1.GameServer{ @@ -330,7 +332,6 @@ func TestSyncGsToPod(t *testing.T) { Phase: corev1.PodPending, }, }, - isUpdate: true, }, { @@ -366,7 +367,6 @@ func TestSyncGsToPod(t *testing.T) { Phase: corev1.PodPending, }, }, - isUpdate: false, }, } @@ -379,13 +379,32 @@ func TestSyncGsToPod(t *testing.T) { pod: test.pod, } - isUpdate, err := manager.SyncGsToPod() - if err != nil { + if err := manager.SyncGsToPod(); err != nil { t.Error(err) } - if isUpdate != test.isUpdate { - t.Errorf("expect isUpdate is %v ,but actually is %v", test.isUpdate, isUpdate) + pod := &corev1.Pod{} + if err := manager.client.Get(context.TODO(), types.NamespacedName{ + Namespace: test.gs.Namespace, + Name: test.gs.Name, + }, pod); err != nil { + t.Error(err) + } + + if pod.Labels[gameKruiseV1alpha1.GameServerOpsStateKey] != string(test.gs.Spec.OpsState) { + t.Errorf("expect opsState is %s ,but actually is %s", string(test.gs.Spec.OpsState), pod.Labels[gameKruiseV1alpha1.GameServerOpsStateKey]) + } + + if pod.Labels[gameKruiseV1alpha1.GameServerUpdatePriorityKey] != test.gs.Spec.UpdatePriority.String() { + t.Errorf("expect UpdatePriority is %s ,but actually is %s", test.gs.Spec.UpdatePriority.String(), pod.Labels[gameKruiseV1alpha1.GameServerUpdatePriorityKey]) + } + + if pod.Labels[gameKruiseV1alpha1.GameServerDeletePriorityKey] != test.gs.Spec.DeletionPriority.String() { + t.Errorf("expect DeletionPriority is %s ,but actually is %s", test.gs.Spec.DeletionPriority.String(), pod.Labels[gameKruiseV1alpha1.GameServerDeletePriorityKey]) + } + + if pod.Labels[gameKruiseV1alpha1.GameServerNetworkDisabled] != strconv.FormatBool(test.gs.Spec.NetworkDisabled) { + t.Errorf("expect NetworkDisabled is %s ,but actually is %s", strconv.FormatBool(test.gs.Spec.NetworkDisabled), pod.Labels[gameKruiseV1alpha1.GameServerNetworkDisabled]) } } }