enhance: service quality support patch labels & annotations (#159)

* enhance: service quality support patch labels & annotations

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

* remove ci markdownlint check

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

---------

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

.github/workflows/ci.yaml

@@ -49,19 +49,6 @@ jobs:
           args: --verbose --timeout=10m
           skip-pkg-cache: true
 
-  markdownlint-misspell-shellcheck:
-    runs-on: ubuntu-20.04
-    # this image is build from Dockerfile
-    # https://github.com/pouchcontainer/pouchlinter/blob/master/Dockerfile
-    container: pouchcontainer/pouchlinter:v0.1.2
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v3
-      - name: Run misspell
-        run: find  ./* -name  "*"  | grep -v vendor | xargs misspell -error
-      - name: Run shellcheck
-        run: find ./ -name "*.sh" | grep -v vendor | xargs shellcheck
-
   unit-tests:
     runs-on: ubuntu-20.04
     steps:

apis/v1alpha1/gameserver_types.go

@@ -109,6 +109,8 @@ type ServiceQualityAction struct {
 	// When Result is defined, it would exec action only when the according Result is actually returns.
 	Result         string `json:"result,omitempty"`
 	GameServerSpec `json:",inline"`
+	Annotations    map[string]string `json:"annotations,omitempty"`
+	Labels         map[string]string `json:"labels,omitempty"`
 }
 
 // GameServerStatus defines the observed state of GameServer

apis/v1alpha1/zz_generated.deepcopy.go

@@ -567,6 +567,20 @@ func (in *ServiceQuality) DeepCopy() *ServiceQuality {
 func (in *ServiceQualityAction) DeepCopyInto(out *ServiceQualityAction) {
 	*out = *in
 	in.GameServerSpec.DeepCopyInto(&out.GameServerSpec)
+	if in.Annotations != nil {
+		in, out := &in.Annotations, &out.Annotations
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
+	if in.Labels != nil {
+		in, out := &in.Labels, &out.Labels
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceQualityAction.

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

@@ -582,6 +582,10 @@ spec:
                     serviceQualityAction:
                       items:
                         properties:
+                          annotations:
+                            additionalProperties:
+                              type: string
+                            type: object
                           containers:
                             description: Containers can be used to make the corresponding
                               GameServer container fields different from the fields
@@ -637,6 +641,10 @@ spec:
                             - type: integer
                             - type: string
                             x-kubernetes-int-or-string: true
+                          labels:
+                            additionalProperties:
+                              type: string
+                            type: object
                           networkDisabled:
                             type: boolean
                           opsState:

pkg/controllers/gameserver/gameserver_manager.go

@@ -235,6 +235,10 @@ func (manager GameServerManager) SyncGsToPod() error {
 func (manager GameServerManager) SyncPodToGs(gss *gameKruiseV1alpha1.GameServerSet) error {
 	gs := manager.gameServer
 	pod := manager.pod
+	oldGsSpec := gs.Spec.DeepCopy()
+	oldGsLabels := gs.GetLabels()
+	oldGsAnnotations := gs.GetAnnotations()
+	oldGsStatus := *gs.Status.DeepCopy()
 
 	// sync DeletePriority/UpdatePriority/State
 	podLabels := pod.GetLabels()
@@ -243,14 +247,18 @@ func (manager GameServerManager) SyncPodToGs(gss *gameKruiseV1alpha1.GameServerS
 	podGsState := gameKruiseV1alpha1.GameServerState(podLabels[gameKruiseV1alpha1.GameServerStateKey])
 
 	// sync Service Qualities
-	spec, sqConditions := syncServiceQualities(gss.Spec.ServiceQualities, pod.Status.Conditions, gs.Status.ServiceQualitiesCondition)
+	sqConditions := syncServiceQualities(gss.Spec.ServiceQualities, pod.Status.Conditions, gs)
 
-	if isNeedToSyncMetadata(gss, gs) || !reflect.DeepEqual(spec, gs.Spec) {
+	// sync Metadata from Gss
-		// sync metadata
+	if isNeedToSyncMetadata(gss, gs) {
 		gsMetadata := syncMetadataFromGss(gss)
+		gs.SetLabels(util.MergeMapString(gs.GetLabels(), gsMetadata.GetLabels()))
+		gs.SetAnnotations(util.MergeMapString(gs.GetAnnotations(), gsMetadata.GetAnnotations()))
+	}
 
+	if !reflect.DeepEqual(oldGsSpec, gs.Spec) || !reflect.DeepEqual(oldGsLabels, gs.GetLabels()) || !reflect.DeepEqual(oldGsAnnotations, gs.GetAnnotations()) {
 		// patch gs spec & metadata
-		patchSpec := map[string]interface{}{"spec": spec, "metadata": gsMetadata}
+		patchSpec := map[string]interface{}{"spec": gs.Spec, "metadata": map[string]interface{}{"labels": gs.GetLabels(), "annotations": gs.GetAnnotations()}}
 		jsonPatchSpec, err := json.Marshal(patchSpec)
 		if err != nil {
 			return err
@@ -270,7 +278,6 @@ func (manager GameServerManager) SyncPodToGs(gss *gameKruiseV1alpha1.GameServerS
 	}
 
 	// patch gs status
-	oldStatus := *gs.Status.DeepCopy()
 	newStatus := gameKruiseV1alpha1.GameServerStatus{
 		PodStatus:                 pod.Status,
 		CurrentState:              podGsState,
@@ -279,10 +286,10 @@ func (manager GameServerManager) SyncPodToGs(gss *gameKruiseV1alpha1.GameServerS
 		DeletionPriority:          &podDeletePriority,
 		ServiceQualitiesCondition: sqConditions,
 		NetworkStatus:             manager.syncNetworkStatus(),
-		LastTransitionTime:        oldStatus.LastTransitionTime,
+		LastTransitionTime:        oldGsStatus.LastTransitionTime,
 		Conditions:                conditions,
 	}
-	if !reflect.DeepEqual(oldStatus, newStatus) {
+	if !reflect.DeepEqual(oldGsStatus, newStatus) {
 		newStatus.LastTransitionTime = metav1.Now()
 		patchStatus := map[string]interface{}{"status": newStatus}
 		jsonPatchStatus, err := json.Marshal(patchStatus)
@@ -355,11 +362,10 @@ func desiredNetworkState(disabled bool) gameKruiseV1alpha1.NetworkState {
 	return gameKruiseV1alpha1.NetworkReady
 }
 
-func syncServiceQualities(serviceQualities []gameKruiseV1alpha1.ServiceQuality, podConditions []corev1.PodCondition, sqConditions []gameKruiseV1alpha1.ServiceQualityCondition) (gameKruiseV1alpha1.GameServerSpec, []gameKruiseV1alpha1.ServiceQualityCondition) {
+func syncServiceQualities(serviceQualities []gameKruiseV1alpha1.ServiceQuality, podConditions []corev1.PodCondition, gs *gameKruiseV1alpha1.GameServer) []gameKruiseV1alpha1.ServiceQualityCondition {
-	var spec gameKruiseV1alpha1.GameServerSpec
 	var newGsConditions []gameKruiseV1alpha1.ServiceQualityCondition
 	sqConditionsMap := make(map[string]gameKruiseV1alpha1.ServiceQualityCondition)
-	for _, sqc := range sqConditions {
+	for _, sqc := range gs.Status.ServiceQualitiesCondition {
 		sqConditionsMap[sqc.Name] = sqc
 	}
 	timeNow := metav1.Now()
@@ -380,10 +386,12 @@ func syncServiceQualities(serviceQualities []gameKruiseV1alpha1.ServiceQuality,
 				for _, action := range sq.ServiceQualityAction {
 					state, err := strconv.ParseBool(string(podCondition.Status))
 					if err == nil && state == action.State && (action.Result == "" || podConditionMessage == action.Result) {
-						spec.DeletionPriority = action.DeletionPriority
+						gs.Spec.DeletionPriority = action.DeletionPriority
-						spec.UpdatePriority = action.UpdatePriority
+						gs.Spec.UpdatePriority = action.UpdatePriority
-						spec.OpsState = action.OpsState
+						gs.Spec.OpsState = action.OpsState
-						spec.NetworkDisabled = action.NetworkDisabled
+						gs.Spec.NetworkDisabled = action.NetworkDisabled
+						gs.SetLabels(util.MergeMapString(gs.GetLabels(), action.Labels))
+						gs.SetAnnotations(util.MergeMapString(gs.GetAnnotations(), action.Annotations))
 						lastActionTransitionTime = timeNow
 					}
 				}
@@ -401,7 +409,7 @@ func syncServiceQualities(serviceQualities []gameKruiseV1alpha1.ServiceQuality,
 		}
 		newGsConditions = append(newGsConditions, newSqCondition)
 	}
-	return spec, newGsConditions
+	return newGsConditions
 }
 
 func (manager GameServerManager) syncPodContainers(gsContainers []gameKruiseV1alpha1.GameServerContainer, podContainers []corev1.Container) []corev1.Container {

pkg/controllers/gameserver/gameserver_manager_test.go

@@ -39,8 +39,10 @@ func TestSyncServiceQualities(t *testing.T) {
 	tests := []struct {
 		serviceQualities []gameKruiseV1alpha1.ServiceQuality
 		podConditions    []corev1.PodCondition
-		sqConditions     []gameKruiseV1alpha1.ServiceQualityCondition
+		gs               *gameKruiseV1alpha1.GameServer
 		spec             gameKruiseV1alpha1.GameServerSpec
+		labels           map[string]string
+		annotations      map[string]string
 		newSqConditions  []gameKruiseV1alpha1.ServiceQualityCondition
 	}{
 		//case 0
@@ -77,7 +79,12 @@ func TestSyncServiceQualities(t *testing.T) {
 					LastProbeTime: fakeProbeTime,
 				},
 			},
-			sqConditions: nil,
+			gs: &gameKruiseV1alpha1.GameServer{
+				Spec: gameKruiseV1alpha1.GameServerSpec{},
+				Status: gameKruiseV1alpha1.GameServerStatus{
+					ServiceQualitiesCondition: nil,
+				},
+			},
 			spec: gameKruiseV1alpha1.GameServerSpec{
 				UpdatePriority: &up,
 			},
@@ -124,12 +131,17 @@ func TestSyncServiceQualities(t *testing.T) {
 					LastProbeTime: fakeProbeTime,
 				},
 			},
-			sqConditions: []gameKruiseV1alpha1.ServiceQualityCondition{
+			gs: &gameKruiseV1alpha1.GameServer{
-				{
+				Spec: gameKruiseV1alpha1.GameServerSpec{},
-					Name:                     "healthy",
+				Status: gameKruiseV1alpha1.GameServerStatus{
-					Status:                   string(corev1.ConditionFalse),
+					ServiceQualitiesCondition: []gameKruiseV1alpha1.ServiceQualityCondition{
-					LastProbeTime:            fakeProbeTime,
+						{
-					LastActionTransitionTime: fakeActionTime,
+							Name:                     "healthy",
+							Status:                   string(corev1.ConditionFalse),
+							LastProbeTime:            fakeProbeTime,
+							LastActionTransitionTime: fakeActionTime,
+						},
+					},
 				},
 			},
 			spec: gameKruiseV1alpha1.GameServerSpec{},
@@ -171,8 +183,13 @@ func TestSyncServiceQualities(t *testing.T) {
 					LastProbeTime: fakeProbeTime,
 				},
 			},
-			sqConditions: nil,
+			gs: &gameKruiseV1alpha1.GameServer{
-			spec:         gameKruiseV1alpha1.GameServerSpec{},
+				Spec: gameKruiseV1alpha1.GameServerSpec{},
+				Status: gameKruiseV1alpha1.GameServerStatus{
+					ServiceQualitiesCondition: nil,
+				},
+			},
+			spec: gameKruiseV1alpha1.GameServerSpec{},
 			newSqConditions: []gameKruiseV1alpha1.ServiceQualityCondition{
 				{
 					Name: "healthy",
@@ -207,8 +224,13 @@ func TestSyncServiceQualities(t *testing.T) {
 					LastProbeTime: fakeProbeTime,
 				},
 			},
-			sqConditions: nil,
+			gs: &gameKruiseV1alpha1.GameServer{
-			spec:         gameKruiseV1alpha1.GameServerSpec{},
+				Spec: gameKruiseV1alpha1.GameServerSpec{},
+				Status: gameKruiseV1alpha1.GameServerStatus{
+					ServiceQualitiesCondition: nil,
+				},
+			},
+			spec: gameKruiseV1alpha1.GameServerSpec{},
 			newSqConditions: []gameKruiseV1alpha1.ServiceQualityCondition{
 				{
 					Name:          "healthy",
@@ -229,6 +251,9 @@ func TestSyncServiceQualities(t *testing.T) {
 							GameServerSpec: gameKruiseV1alpha1.GameServerSpec{
 								UpdatePriority: &up,
 							},
+							Annotations: map[string]string{
+								"case-4": "new",
+							},
 						},
 						{
 							State: false,
@@ -251,17 +276,25 @@ func TestSyncServiceQualities(t *testing.T) {
 					LastProbeTime: fakeProbeTime,
 				},
 			},
-			sqConditions: []gameKruiseV1alpha1.ServiceQualityCondition{
+			gs: &gameKruiseV1alpha1.GameServer{
-				{
+				Spec: gameKruiseV1alpha1.GameServerSpec{},
-					Name:                     "healthy",
+				Status: gameKruiseV1alpha1.GameServerStatus{
-					Status:                   string(corev1.ConditionFalse),
+					ServiceQualitiesCondition: []gameKruiseV1alpha1.ServiceQualityCondition{
-					LastProbeTime:            fakeProbeTime,
+						{
-					LastActionTransitionTime: fakeActionTime,
+							Name:                     "healthy",
+							Status:                   string(corev1.ConditionFalse),
+							LastProbeTime:            fakeProbeTime,
+							LastActionTransitionTime: fakeActionTime,
+						},
+					},
 				},
 			},
 			spec: gameKruiseV1alpha1.GameServerSpec{
 				UpdatePriority: &up,
 			},
+			annotations: map[string]string{
+				"case-4": "new",
+			},
 			newSqConditions: []gameKruiseV1alpha1.ServiceQualityCondition{
 				{
 					Name:                     "healthy",
@@ -310,7 +343,12 @@ func TestSyncServiceQualities(t *testing.T) {
 					LastProbeTime: fakeProbeTime,
 				},
 			},
-			sqConditions: nil,
+			gs: &gameKruiseV1alpha1.GameServer{
+				Spec: gameKruiseV1alpha1.GameServerSpec{},
+				Status: gameKruiseV1alpha1.GameServerStatus{
+					ServiceQualitiesCondition: nil,
+				},
+			},
 			spec: gameKruiseV1alpha1.GameServerSpec{
 				OpsState: "B",
 			},
@@ -363,13 +401,18 @@ func TestSyncServiceQualities(t *testing.T) {
 					LastProbeTime: fakeProbeTime,
 				},
 			},
-			sqConditions: []gameKruiseV1alpha1.ServiceQualityCondition{
+			gs: &gameKruiseV1alpha1.GameServer{
-				{
+				Spec: gameKruiseV1alpha1.GameServerSpec{},
-					Name:                     "multi-return",
+				Status: gameKruiseV1alpha1.GameServerStatus{
-					Result:                   "B",
+					ServiceQualitiesCondition: []gameKruiseV1alpha1.ServiceQualityCondition{
-					Status:                   string(corev1.ConditionTrue),
+						{
-					LastProbeTime:            fakeProbeTime,
+							Name:                     "multi-return",
-					LastActionTransitionTime: fakeActionTime,
+							Result:                   "B",
+							Status:                   string(corev1.ConditionTrue),
+							LastProbeTime:            fakeProbeTime,
+							LastActionTransitionTime: fakeActionTime,
+						},
+					},
 				},
 			},
 			spec: gameKruiseV1alpha1.GameServerSpec{
@@ -388,11 +431,17 @@ func TestSyncServiceQualities(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		actualSpec, actualNewSqConditions := syncServiceQualities(test.serviceQualities, test.podConditions, test.sqConditions)
+		actualNewSqConditions := syncServiceQualities(test.serviceQualities, test.podConditions, test.gs)
 		expectSpec := test.spec
 		expectNewSqConditions := test.newSqConditions
-		if !reflect.DeepEqual(actualSpec, expectSpec) {
+		if !reflect.DeepEqual(test.gs.Spec, expectSpec) {
-			t.Errorf("case %d: expect spec %v but got %v", i, expectSpec, actualSpec)
+			t.Errorf("case %d: expect spec %v but got %v", i, expectSpec, test.gs.Spec)
+		}
+		if !reflect.DeepEqual(test.gs.GetLabels(), test.labels) {
+			t.Errorf("case %d: expect labels %v but got %v", i, test.labels, test.gs.GetLabels())
+		}
+		if !reflect.DeepEqual(test.gs.GetAnnotations(), test.annotations) {
+			t.Errorf("case %d: expect annotations %v but got %v", i, test.annotations, test.gs.GetAnnotations())
 		}
 		if len(actualNewSqConditions) != len(expectNewSqConditions) {
 			t.Errorf("case %d: expect sq conditions len %v but got %v", i, len(expectNewSqConditions), len(actualNewSqConditions))

pkg/util/map.go

@@ -0,0 +1,18 @@
+package util
+
+func MergeMapString(map1, map2 map[string]string) map[string]string {
+	if map1 == nil && map2 == nil {
+		return nil
+	}
+	mergedMap := make(map[string]string)
+
+	for key, value := range map1 {
+		mergedMap[key] = value
+	}
+
+	for key, value := range map2 {
+		mergedMap[key] = value
+	}
+
+	return mergedMap
+}

pkg/util/map_test.go

@@ -0,0 +1,67 @@
+package util
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestMergeMapString(t *testing.T) {
+	tests := []struct {
+		a      map[string]string
+		b      map[string]string
+		result map[string]string
+	}{
+		{
+			a: map[string]string{
+				"foo-A": "bar",
+			},
+			b: map[string]string{
+				"foo-B": "bar",
+			},
+			result: map[string]string{
+				"foo-A": "bar",
+				"foo-B": "bar",
+			},
+		},
+		{
+			a: map[string]string{
+				"foo-A": "bar",
+			},
+			b: map[string]string{
+				"foo-A": "barB",
+			},
+			result: map[string]string{
+				"foo-A": "barB",
+			},
+		},
+		{
+			a: map[string]string{},
+			b: map[string]string{
+				"foo-A": "barB",
+			},
+			result: map[string]string{
+				"foo-A": "barB",
+			},
+		},
+		{
+			a: map[string]string{
+				"foo-A": "bar",
+			},
+			b: map[string]string{},
+			result: map[string]string{
+				"foo-A": "bar",
+			},
+		},
+		{
+			result: nil,
+		},
+	}
+
+	for _, test := range tests {
+		expect := test.result
+		actual := MergeMapString(test.a, test.b)
+		if !reflect.DeepEqual(expect, actual) {
+			t.Errorf("expect %v but got %v", expect, actual)
+		}
+	}
+}