diff --git a/apis/v1alpha1/gameserver_types.go b/apis/v1alpha1/gameserver_types.go index ce6f2b8f..20dcb85e 100644 --- a/apis/v1alpha1/gameserver_types.go +++ b/apis/v1alpha1/gameserver_types.go @@ -28,6 +28,7 @@ const ( GameServerUpdatePriorityKey = "game.kruise.io/gs-update-priority" GameServerDeletePriorityKey = "game.kruise.io/gs-delete-priority" GameServerDeletingKey = "game.kruise.io/gs-deleting" + GameServerUpdatingContainersKey = "game.kruise.io/gs-updating-containers" GameServerNetworkType = "game.kruise.io/network-type" GameServerNetworkConf = "game.kruise.io/network-conf" GameServerNetworkDisabled = "game.kruise.io/network-disabled" diff --git a/config/crd/bases/game.kruise.io_gameservers.yaml b/config/crd/bases/game.kruise.io_gameservers.yaml index 388b0b87..652e1c6a 100644 --- a/config/crd/bases/game.kruise.io_gameservers.yaml +++ b/config/crd/bases/game.kruise.io_gameservers.yaml @@ -71,7 +71,7 @@ spec: image: description: |- Image indicates the image of the container to update. - When Image updated, pod.spec.containers[*].image will be updated immediately. + When Image is updated, pod.spec.containers[*].image will be updated immediately. type: string name: description: Name indicates the name of the container to update. @@ -79,8 +79,8 @@ spec: resources: description: |- Resources indicates the resources of the container to update. - When Resources updated, pod.spec.containers[*].Resources will be not updated immediately, - which will be updated when pod recreate. + When Resources are updated, pod.spec.containers[*].Resources will not be updated immediately, + which will be updated when the pod is recreated. properties: claims: description: |- @@ -176,7 +176,7 @@ spec: type: string message: description: Human-readable message indicating details about - last transition. + the last transition. type: string reason: description: Unique, one-word, CamelCase reason for the condition's @@ -1672,8 +1672,8 @@ spec: name: type: string result: - description: Result indicate the probe message returned by the - script + description: Result indicates the probe message returned by + the script type: string status: type: string diff --git a/config/crd/bases/game.kruise.io_gameserversets.yaml b/config/crd/bases/game.kruise.io_gameserversets.yaml index 05afc9d8..650f9adb 100644 --- a/config/crd/bases/game.kruise.io_gameserversets.yaml +++ b/config/crd/bases/game.kruise.io_gameserversets.yaml @@ -21,7 +21,7 @@ spec: jsonPath: .spec.replicas name: DESIRED type: integer - - description: The number of currently all GameServers. + - description: The total number of current GameServers. jsonPath: .status.currentReplicas name: CURRENT type: integer @@ -794,7 +794,7 @@ spec: image: description: |- Image indicates the image of the container to update. - When Image updated, pod.spec.containers[*].image will be updated immediately. + When Image is updated, pod.spec.containers[*].image will be updated immediately. type: string name: description: Name indicates the name of the container @@ -803,8 +803,8 @@ spec: resources: description: |- Resources indicates the resources of the container to update. - When Resources updated, pod.spec.containers[*].Resources will be not updated immediately, - which will be updated when pod recreate. + When Resources are updated, pod.spec.containers[*].Resources will not be updated immediately, + which will be updated when the pod is recreated. properties: claims: description: |- @@ -882,8 +882,8 @@ spec: 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. + Result indicates the probe message returned by the script. + When Result is defined, it would exec action only when the corresponding Result is actually returned. type: string state: type: boolean @@ -957,7 +957,7 @@ spec: description: |- UnorderedUpdate contains strategies for non-ordered update. If it is not nil, pods will be updated with non-ordered sequence. - Noted that UnorderedUpdate can only be allowed to work with Parallel podManagementPolicy + Note that UnorderedUpdate can only be allowed to work with Parallel podManagementPolicy UnorderedUpdate *kruiseV1beta1.UnorderedUpdateStrategy `json:"unorderedUpdate,omitempty"` InPlaceUpdateStrategy contains strategies for in-place update. properties: diff --git a/pkg/controllers/gameserver/gameserver_controller.go b/pkg/controllers/gameserver/gameserver_controller.go index 18a5847e..5b047b3e 100644 --- a/pkg/controllers/gameserver/gameserver_controller.go +++ b/pkg/controllers/gameserver/gameserver_controller.go @@ -359,7 +359,7 @@ func (r *GameServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) tracing.AttrGameServerSetName(gss.GetName()), tracing.AttrGameServerName(gs.GetName()), )) - err = gsm.SyncGsToPod(ctx) + err = gsm.SyncGsToPod(ctx, gss) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, "SyncGsToPod failed") diff --git a/pkg/controllers/gameserver/gameserver_manager.go b/pkg/controllers/gameserver/gameserver_manager.go index 1ca24c32..246cdefd 100644 --- a/pkg/controllers/gameserver/gameserver_manager.go +++ b/pkg/controllers/gameserver/gameserver_manager.go @@ -60,7 +60,7 @@ 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. - SyncGsToPod(context.Context) error + SyncGsToPod(context.Context, *gameKruiseV1alpha1.GameServerSet) error // SyncPodToGs compares the GameServer with pod, and update the GameServer. SyncPodToGs(context.Context, *gameKruiseV1alpha1.GameServerSet) error // WaitOrNot compare the current game server network status to decide whether to re-queue. @@ -92,7 +92,7 @@ func syncMetadataFromGss(gss *gameKruiseV1alpha1.GameServerSet) metav1.ObjectMet } } -func (manager GameServerManager) SyncGsToPod(ctx context.Context) error { +func (manager GameServerManager) SyncGsToPod(ctx context.Context, gss *gameKruiseV1alpha1.GameServerSet) error { pod := manager.pod gs := manager.gameServer podLabels := pod.GetLabels() @@ -102,8 +102,8 @@ func (manager GameServerManager) SyncGsToPod(ctx context.Context) error { podGsState := podLabels[gameKruiseV1alpha1.GameServerStateKey] podNetworkDisabled := podLabels[gameKruiseV1alpha1.GameServerNetworkDisabled] - newLabels := make(map[string]string) - newAnnotations := make(map[string]string) + newLabels := make(map[string]interface{}) + newAnnotations := make(map[string]interface{}) // tolerate nil pointers in spec priorities var gsDpStr, gsUpStr string if gs.Spec.DeletionPriority != nil { @@ -215,6 +215,27 @@ func (manager GameServerManager) SyncGsToPod(ctx context.Context) error { } } + // sync GameServerUpdatingContainersKey annotation: when Pod enters the PreUpdate state, + // compute the diff container names and write them to the annotation; during the Updating + // state, keep the annotation as-is so that hooks can consume it; in any other state, + // remove the annotation to clean up after the update is complete. + switch gsState { + case gameKruiseV1alpha1.PreUpdate: + if gss != nil { + diffNames := util.GetDiffContainerNames(pod, gss) + updatingValue := strings.Join(diffNames, ",") + if pod.GetAnnotations()[gameKruiseV1alpha1.GameServerUpdatingContainersKey] != updatingValue { + newAnnotations[gameKruiseV1alpha1.GameServerUpdatingContainersKey] = updatingValue + } + } + case gameKruiseV1alpha1.Updating: + // keep the existing annotation unchanged so that hooks can read it + default: + if _, exists := pod.GetAnnotations()[gameKruiseV1alpha1.GameServerUpdatingContainersKey]; exists { + newAnnotations[gameKruiseV1alpha1.GameServerUpdatingContainersKey] = nil + } + } + // sync annotations from gs to pod for gsKey, gsValue := range gs.GetAnnotations() { if util.IsHasPrefixGsSyncToPod(gsKey) { @@ -255,7 +276,7 @@ func (manager GameServerManager) SyncGsToPod(ctx context.Context) error { traceparent := tracing.GenerateTraceparent(spanContext) if traceparent != "" { if len(newAnnotations) == 0 { - newAnnotations = make(map[string]string) + newAnnotations = make(map[string]interface{}) } newAnnotations[telemetryfields.AnnotationTraceparent] = traceparent } @@ -263,7 +284,7 @@ func (manager GameServerManager) SyncGsToPod(ctx context.Context) error { patchPod := make(map[string]interface{}) if len(newLabels) != 0 || len(newAnnotations) != 0 { - patchPod["metadata"] = map[string]map[string]string{"labels": newLabels, "annotations": newAnnotations} + patchPod["metadata"] = map[string]map[string]interface{}{"labels": newLabels, "annotations": newAnnotations} } if containers != nil { patchPod["spec"] = map[string]interface{}{"containers": containers} diff --git a/pkg/controllers/gameserver/gameserver_manager_test.go b/pkg/controllers/gameserver/gameserver_manager_test.go index d7bcaa0f..8f318ee1 100644 --- a/pkg/controllers/gameserver/gameserver_manager_test.go +++ b/pkg/controllers/gameserver/gameserver_manager_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/go-logr/logr/testr" + kruisePub "github.com/openkruise/kruise-api/apps/pub" kruiseV1alpha1 "github.com/openkruise/kruise-api/apps/v1alpha1" kruiseV1beta1 "github.com/openkruise/kruise-api/apps/v1beta1" gameKruiseV1alpha1 "github.com/openkruise/kruise-game/apis/v1alpha1" @@ -802,7 +803,7 @@ func TestSyncGsToPod(t *testing.T) { logger: testr.New(t), } - if err := manager.SyncGsToPod(context.TODO()); err != nil { + if err := manager.SyncGsToPod(context.TODO(), nil); err != nil { t.Error(err) } @@ -839,6 +840,210 @@ func TestSyncGsToPod(t *testing.T) { } } +func TestUpdatingContainersAnnotation(t *testing.T) { + tests := []struct { + name string + gs *gameKruiseV1alpha1.GameServer + pod *corev1.Pod + gss *gameKruiseV1alpha1.GameServerSet + expectPodAnnotation string + expectPodAnnotationExist bool + }{ + { + name: "PreUpdate state sets updating-containers on pod", + gs: &gameKruiseV1alpha1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "xxx-0", + Labels: map[string]string{ + gameKruiseV1alpha1.GameServerOwnerGssKey: "xxx", + }, + }, + Spec: gameKruiseV1alpha1.GameServerSpec{}, + }, + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "xxx-0", + Labels: map[string]string{ + kruisePub.LifecycleStateKey: string(kruisePub.LifecycleStatePreparingUpdate), + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionTrue}, + }, + ContainerStatuses: []corev1.ContainerStatus{ + {Name: "app", Image: "v1.0"}, + {Name: "sidecar", Image: "v1.0"}, + }, + }, + }, + gss: &gameKruiseV1alpha1.GameServerSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "xxx", + }, + Spec: gameKruiseV1alpha1.GameServerSetSpec{ + GameServerTemplate: gameKruiseV1alpha1.GameServerTemplate{ + PodTemplateSpec: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "app", Image: "v2.0"}, + {Name: "sidecar", Image: "v1.0"}, + }, + }, + }, + }, + }, + }, + expectPodAnnotation: "app", + expectPodAnnotationExist: true, + }, + { + name: "Updating state keeps existing annotation unchanged", + gs: &gameKruiseV1alpha1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "xxx-0", + Labels: map[string]string{ + gameKruiseV1alpha1.GameServerOwnerGssKey: "xxx", + }, + }, + Spec: gameKruiseV1alpha1.GameServerSpec{}, + }, + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "xxx-0", + Labels: map[string]string{ + kruisePub.LifecycleStateKey: string(kruisePub.LifecycleStateUpdating), + }, + Annotations: map[string]string{ + gameKruiseV1alpha1.GameServerUpdatingContainersKey: "app", + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionTrue}, + }, + ContainerStatuses: []corev1.ContainerStatus{ + {Name: "app", Image: "v2.0"}, + {Name: "sidecar", Image: "v1.0"}, + }, + }, + }, + gss: &gameKruiseV1alpha1.GameServerSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "xxx", + }, + Spec: gameKruiseV1alpha1.GameServerSetSpec{ + GameServerTemplate: gameKruiseV1alpha1.GameServerTemplate{ + PodTemplateSpec: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "app", Image: "v2.0"}, + {Name: "sidecar", Image: "v1.0"}, + }, + }, + }, + }, + }, + }, + expectPodAnnotation: "app", + expectPodAnnotationExist: true, + }, + { + name: "non-PreUpdate state removes updating-containers from pod", + gs: &gameKruiseV1alpha1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "xxx-0", + Labels: map[string]string{ + gameKruiseV1alpha1.GameServerOwnerGssKey: "xxx", + }, + }, + Spec: gameKruiseV1alpha1.GameServerSpec{}, + }, + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "xxx-0", + Annotations: map[string]string{ + gameKruiseV1alpha1.GameServerUpdatingContainersKey: "app", + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionTrue}, + }, + ContainerStatuses: []corev1.ContainerStatus{ + {Name: "app", Image: "v2.0"}, + {Name: "sidecar", Image: "v1.0"}, + }, + }, + }, + gss: &gameKruiseV1alpha1.GameServerSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "xxx", + }, + Spec: gameKruiseV1alpha1.GameServerSetSpec{ + GameServerTemplate: gameKruiseV1alpha1.GameServerTemplate{ + PodTemplateSpec: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "app", Image: "v2.0"}, + {Name: "sidecar", Image: "v1.0"}, + }, + }, + }, + }, + }, + }, + expectPodAnnotation: "", + expectPodAnnotationExist: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + objs := []client.Object{test.gs, test.pod} + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + manager := &GameServerManager{ + client: c, + gameServer: test.gs, + pod: test.pod, + logger: testr.New(t), + } + + if err := manager.SyncGsToPod(context.TODO(), test.gss); err != nil { + t.Fatal(err) + } + + pod := &corev1.Pod{} + if err := c.Get(context.TODO(), types.NamespacedName{ + Namespace: test.pod.Namespace, + Name: test.pod.Name, + }, pod); err != nil { + t.Fatal(err) + } + + val, exists := pod.Annotations[gameKruiseV1alpha1.GameServerUpdatingContainersKey] + if exists != test.expectPodAnnotationExist { + t.Errorf("expect annotation exists=%v, got exists=%v", test.expectPodAnnotationExist, exists) + } + if val != test.expectPodAnnotation { + t.Errorf("expect annotation value=%q, got %q", test.expectPodAnnotation, val) + } + }) + } +} + func TestSyncNetworkStatus(t *testing.T) { fakeTime := metav1.Now() portInternal := intstr.FromInt(80) diff --git a/pkg/util/pod.go b/pkg/util/pod.go index b450f662..3d3b62ee 100644 --- a/pkg/util/pod.go +++ b/pkg/util/pod.go @@ -35,7 +35,10 @@ func GetPodConditionFromList(conditions []corev1.PodCondition, conditionType cor return -1, nil } -func IsContainersPreInplaceUpdating(pod *corev1.Pod, gss *gameKruiseV1alpha1.GameServerSet, containerNames []string) bool { +// GetDiffContainerNames returns the names of containers whose current image +// (from pod.Status.ContainerStatuses) differs from the desired image +// (from gss.Spec.GameServerTemplate.Spec.Containers). +func GetDiffContainerNames(pod *corev1.Pod, gss *gameKruiseV1alpha1.GameServerSet) []string { var diffNames []string for _, actual := range pod.Status.ContainerStatuses { for _, expect := range gss.Spec.GameServerTemplate.Spec.Containers { @@ -44,6 +47,11 @@ func IsContainersPreInplaceUpdating(pod *corev1.Pod, gss *gameKruiseV1alpha1.Gam } } } + return diffNames +} + +func IsContainersPreInplaceUpdating(pod *corev1.Pod, gss *gameKruiseV1alpha1.GameServerSet, containerNames []string) bool { + diffNames := GetDiffContainerNames(pod, gss) for _, containerName := range containerNames { if IsStringInList(containerName, diffNames) { return true diff --git a/pkg/util/pod_test.go b/pkg/util/pod_test.go index 393c1233..520627f0 100644 --- a/pkg/util/pod_test.go +++ b/pkg/util/pod_test.go @@ -17,10 +17,11 @@ limitations under the License. package util import ( - gameKruiseV1alpha1 "github.com/openkruise/kruise-game/apis/v1alpha1" - corev1 "k8s.io/api/core/v1" "reflect" "testing" + + gameKruiseV1alpha1 "github.com/openkruise/kruise-game/apis/v1alpha1" + corev1 "k8s.io/api/core/v1" ) func TestGetPodConditionFromList(t *testing.T) { @@ -192,3 +193,97 @@ func TestIsContainersPreInplaceUpdating(t *testing.T) { } } } + +func TestGetDiffContainerNames(t *testing.T) { + tests := []struct { + pod *corev1.Pod + gss *gameKruiseV1alpha1.GameServerSet + expected []string + }{ + // case 0: one container differs + { + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + {Name: "name_A", Image: "v1.0"}, + {Name: "name_B", Image: "v1.0"}, + }, + }, + }, + gss: &gameKruiseV1alpha1.GameServerSet{ + Spec: gameKruiseV1alpha1.GameServerSetSpec{ + GameServerTemplate: gameKruiseV1alpha1.GameServerTemplate{ + PodTemplateSpec: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "name_A", Image: "v1.0"}, + {Name: "name_B", Image: "v2.0"}, + }, + }, + }, + }, + }, + }, + expected: []string{"name_B"}, + }, + // case 1: no containers differ + { + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + {Name: "name_A", Image: "v1.0"}, + {Name: "name_B", Image: "v1.0"}, + }, + }, + }, + gss: &gameKruiseV1alpha1.GameServerSet{ + Spec: gameKruiseV1alpha1.GameServerSetSpec{ + GameServerTemplate: gameKruiseV1alpha1.GameServerTemplate{ + PodTemplateSpec: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "name_A", Image: "v1.0"}, + {Name: "name_B", Image: "v1.0"}, + }, + }, + }, + }, + }, + }, + expected: nil, + }, + // case 2: both containers differ + { + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + {Name: "name_A", Image: "v1.0"}, + {Name: "name_B", Image: "v1.0"}, + }, + }, + }, + gss: &gameKruiseV1alpha1.GameServerSet{ + Spec: gameKruiseV1alpha1.GameServerSetSpec{ + GameServerTemplate: gameKruiseV1alpha1.GameServerTemplate{ + PodTemplateSpec: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "name_A", Image: "v2.0"}, + {Name: "name_B", Image: "v2.0"}, + }, + }, + }, + }, + }, + }, + expected: []string{"name_A", "name_B"}, + }, + } + + for i, test := range tests { + actual := GetDiffContainerNames(test.pod, test.gss) + if !reflect.DeepEqual(actual, test.expected) { + t.Errorf("case %d: expect GetDiffContainerNames is %v but actually got %v", i, test.expected, actual) + } + } +}