This is an automated email from the ASF dual-hosted git repository.

roryqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new 3aa03cf84 [#289] feat(operator): Support annotations (#1817)
3aa03cf84 is described below

commit 3aa03cf84f683096a587cb22b3cd8b3ca27f26a3
Author: roryqi <ror...@apache.org>
AuthorDate: Tue Jun 25 11:32:09 2024 +0800

    [#289] feat(operator): Support annotations (#1817)
    
    ### What changes were proposed in this pull request?
    
    Previous pull request is #467
    
    I address the comments and add the uts.
    
    ### Why are the changes needed?
    
    Fix: #289
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    UT and run the operator in the GKE.
---
 .../uniffle/v1alpha1/remoteshuffleservice_types.go |  5 ++++
 .../api/uniffle/v1alpha1/zz_generated.deepcopy.go  |  7 +++++
 .../uniffle.apache.org_remoteshuffleservices.yaml  | 14 +++++++++
 .../pkg/controller/sync/coordinator/coordinator.go |  7 +++++
 .../sync/coordinator/coordinator_test.go           | 26 +++++++++++++++--
 .../controller/sync/shuffleserver/shuffleserver.go | 29 +++++++++++++-----
 .../sync/shuffleserver/shuffleserver_test.go       | 34 ++++++++++++++++++++--
 7 files changed, 110 insertions(+), 12 deletions(-)

diff --git 
a/deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go 
b/deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go
index 978695343..2903f0932 100644
--- 
a/deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go
+++ 
b/deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go
@@ -309,6 +309,11 @@ type RSSPodSpec struct {
        // Affinity is a group of affinity scheduling rules.
        // +optional
        Affinity *corev1.Affinity `json:"affinity,omitempty"`
+
+       // Annotations is an unstructured key value map stored with a resource 
that may be
+       // set by external tools to store and retrieve arbitrary metadata.
+       // +optional
+       Annotations map[string]string `json:"annotations,omitempty"`
 }
 
 // MainContainer stores information of the main container of coordinators or 
shuffle servers,
diff --git 
a/deploy/kubernetes/operator/api/uniffle/v1alpha1/zz_generated.deepcopy.go 
b/deploy/kubernetes/operator/api/uniffle/v1alpha1/zz_generated.deepcopy.go
index 0c7389f29..c9e15d7a5 100644
--- a/deploy/kubernetes/operator/api/uniffle/v1alpha1/zz_generated.deepcopy.go
+++ b/deploy/kubernetes/operator/api/uniffle/v1alpha1/zz_generated.deepcopy.go
@@ -274,6 +274,13 @@ func (in *RSSPodSpec) DeepCopyInto(out *RSSPodSpec) {
                *out = new(v1.Affinity)
                (*in).DeepCopyInto(*out)
        }
+       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
+               }
+       }
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, 
creating a new RSSPodSpec.
diff --git 
a/deploy/kubernetes/operator/config/crd/bases/uniffle.apache.org_remoteshuffleservices.yaml
 
b/deploy/kubernetes/operator/config/crd/bases/uniffle.apache.org_remoteshuffleservices.yaml
index 9dca5b422..46e3e23b8 100644
--- 
a/deploy/kubernetes/operator/config/crd/bases/uniffle.apache.org_remoteshuffleservices.yaml
+++ 
b/deploy/kubernetes/operator/config/crd/bases/uniffle.apache.org_remoteshuffleservices.yaml
@@ -937,6 +937,13 @@ spec:
                             type: array
                         type: object
                     type: object
+                  annotations:
+                    additionalProperties:
+                      type: string
+                    description: Annotations is an unstructured key value map 
stored
+                      with a resource that may be set by external tools to 
store and
+                      retrieve arbitrary metadata.
+                    type: object
                   args:
                     description: Args represents args used by coordinators or 
shuffle
                       servers.
@@ -5899,6 +5906,13 @@ spec:
                             type: array
                         type: object
                     type: object
+                  annotations:
+                    additionalProperties:
+                      type: string
+                    description: Annotations is an unstructured key value map 
stored
+                      with a resource that may be set by external tools to 
store and
+                      retrieve arbitrary metadata.
+                    type: object
                   args:
                     description: Args represents args used by coordinators or 
shuffle
                       servers.
diff --git 
a/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go 
b/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go
index dad46ed5a..84b6c84a6 100644
--- a/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go
+++ b/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go
@@ -235,6 +235,13 @@ func GenerateDeploy(rss 
*unifflev1alpha1.RemoteShuffleService, index int) *appsv
                deploy.Spec.Template.Spec.RuntimeClassName = 
rss.Spec.Coordinator.RuntimeClassName
        }
 
+       // add custom annotations
+       annotations := map[string]string{}
+       for key, value := range rss.Spec.Coordinator.Annotations {
+               annotations[key] = value
+       }
+       deploy.Spec.Template.Annotations = annotations
+
        // add init containers, the main container and other containers.
        deploy.Spec.Template.Spec.InitContainers = 
util.GenerateInitContainers(rss.Spec.Coordinator.RSSPodSpec)
        containers := []corev1.Container{*generateMainContainer(rss)}
diff --git 
a/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator_test.go
 
b/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator_test.go
index d1464043f..22caf5fc4 100644
--- 
a/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator_test.go
+++ 
b/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator_test.go
@@ -133,6 +133,11 @@ var (
                        Name: "default-secret",
                },
        }
+
+       testAnnotations = map[string]string{
+               "key1": "value1",
+               "key2": "value2",
+       }
 )
 
 func buildRssWithLabels() *uniffleapi.RemoteShuffleService {
@@ -177,6 +182,12 @@ func withCustomImagePullSecrets(imagePullSecrets 
[]corev1.LocalObjectReference)
        return rss
 }
 
+func withCustomAnnotations(annotations map[string]string) 
*uniffleapi.RemoteShuffleService {
+       rss := utils.BuildRSSWithDefaultValue()
+       rss.Spec.Coordinator.Annotations = annotations
+       return rss
+}
+
 func buildRssWithCustomRPCPort() *uniffleapi.RemoteShuffleService {
        rss := utils.BuildRSSWithDefaultValue()
        rss.Spec.Coordinator.RPCPort = pointer.Int32(testRPCPort)
@@ -438,7 +449,6 @@ func TestGenerateDeploy(t *testing.T) {
                        rss:  withCustomAffinity(testAffinity),
                        IsValidDeploy: func(deploy *appsv1.Deployment, rss 
*uniffleapi.RemoteShuffleService) (bool, error) {
                                if deploy.Spec.Template.Spec.Affinity != nil {
-                                       deploy.Spec.Template.Spec.Affinity = 
rss.Spec.Coordinator.Affinity
                                        equal := 
reflect.DeepEqual(deploy.Spec.Template.Spec.Affinity, testAffinity)
                                        if equal {
                                                return true, nil
@@ -452,7 +462,6 @@ func TestGenerateDeploy(t *testing.T) {
                        rss:  withCustomImagePullSecrets(testImagePullSecrets),
                        IsValidDeploy: func(deploy *appsv1.Deployment, rss 
*uniffleapi.RemoteShuffleService) (bool, error) {
                                if deploy.Spec.Template.Spec.ImagePullSecrets 
!= nil {
-                                       
deploy.Spec.Template.Spec.ImagePullSecrets = rss.Spec.ImagePullSecrets
                                        equal := 
reflect.DeepEqual(deploy.Spec.Template.Spec.ImagePullSecrets, 
testImagePullSecrets)
                                        if equal {
                                                return true, nil
@@ -461,6 +470,19 @@ func TestGenerateDeploy(t *testing.T) {
                                return false, fmt.Errorf("generated deploy 
should include imagePullSecrets: %v", testImagePullSecrets)
                        },
                },
+               {
+                       name: "set custom annotations",
+                       rss:  withCustomAnnotations(testAnnotations),
+                       IsValidDeploy: func(deploy *appsv1.Deployment, rss 
*uniffleapi.RemoteShuffleService) (bool, error) {
+                               if deploy.Spec.Template.Annotations != nil {
+                                       equal := 
reflect.DeepEqual(deploy.Spec.Template.Annotations, testAnnotations)
+                                       if equal {
+                                               return true, nil
+                                       }
+                               }
+                               return false, fmt.Errorf("generated deploy 
should include annotations: %v", testAnnotations)
+                       },
+               },
        } {
                t.Run(tt.name, func(tc *testing.T) {
                        deploy := GenerateDeploy(tt.rss, 0)
diff --git 
a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go 
b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
index 867a40386..63fbc222d 100644
--- 
a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
+++ 
b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
@@ -173,14 +173,6 @@ func GenerateSts(kubeClient kubernetes.Interface, rss 
*unifflev1alpha1.RemoteShu
                        Template: corev1.PodTemplateSpec{
                                ObjectMeta: metav1.ObjectMeta{
                                        Labels: make(map[string]string),
-                                       Annotations: map[string]string{
-                                               constants.AnnotationRssName: 
rss.Name,
-                                               constants.AnnotationRssUID:  
string(rss.UID),
-                                               
constants.AnnotationMetricsServerPort: fmt.Sprintf("%v",
-                                                       
*rss.Spec.ShuffleServer.HTTPPort),
-                                               
constants.AnnotationShuffleServerPort: fmt.Sprintf("%v",
-                                                       
*rss.Spec.ShuffleServer.RPCPort),
-                                       },
                                },
                                Spec: podSpec,
                        },
@@ -209,6 +201,27 @@ func GenerateSts(kubeClient kubernetes.Interface, rss 
*unifflev1alpha1.RemoteShu
                })
        }
 
+       // add custom annotation, and default annotation used by rss
+       reservedAnnotations := map[string]string{
+               constants.AnnotationRssName: rss.Name,
+               constants.AnnotationRssUID:  string(rss.UID),
+               constants.AnnotationMetricsServerPort: fmt.Sprintf("%v",
+                       *rss.Spec.ShuffleServer.HTTPPort),
+               constants.AnnotationShuffleServerPort: fmt.Sprintf("%v",
+                       *rss.Spec.ShuffleServer.RPCPort),
+       }
+
+       annotations := map[string]string{}
+       for key, value := range rss.Spec.ShuffleServer.Annotations {
+               annotations[key] = value
+       }
+       // reserved annotations have higher preference.
+       for key, value := range reservedAnnotations {
+               annotations[key] = value
+       }
+
+       sts.Spec.Template.Annotations = annotations
+
        // add init containers, the main container and other containers.
        sts.Spec.Template.Spec.InitContainers = 
util.GenerateInitContainers(rss.Spec.ShuffleServer.RSSPodSpec)
        containers := []corev1.Container{*generateMainContainer(rss)}
diff --git 
a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go
 
b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go
index dc151c5df..401436880 100644
--- 
a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go
+++ 
b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go
@@ -160,6 +160,12 @@ var (
                        },
                },
        }
+
+       testAnnotations = map[string]string{
+               "key1":                      "value1",
+               "key2":                      "value2",
+               constants.AnnotationRssName: "override",
+       }
 )
 
 func buildRssWithLabels() *uniffleapi.RemoteShuffleService {
@@ -215,6 +221,12 @@ func withCustomAffinity(affinity *corev1.Affinity) 
*uniffleapi.RemoteShuffleServ
        return rss
 }
 
+func withCustomAnnotations(annotations map[string]string) 
*uniffleapi.RemoteShuffleService {
+       rss := utils.BuildRSSWithDefaultValue()
+       rss.Spec.ShuffleServer.Annotations = annotations
+       return rss
+}
+
 func withCustomImagePullSecrets(imagePullSecrets 
[]corev1.LocalObjectReference) *uniffleapi.RemoteShuffleService {
        rss := utils.BuildRSSWithDefaultValue()
        rss.Spec.ImagePullSecrets = imagePullSecrets
@@ -497,7 +509,6 @@ func TestGenerateSts(t *testing.T) {
                        rss:  withCustomAffinity(testAffinity),
                        IsValidSts: func(sts *appsv1.StatefulSet, rss 
*uniffleapi.RemoteShuffleService) (valid bool, err error) {
                                if sts.Spec.Template.Spec.Affinity != nil {
-                                       sts.Spec.Template.Spec.Affinity = 
rss.Spec.ShuffleServer.Affinity
                                        equal := 
reflect.DeepEqual(sts.Spec.Template.Spec.Affinity, testAffinity)
                                        if equal {
                                                return true, nil
@@ -511,7 +522,6 @@ func TestGenerateSts(t *testing.T) {
                        rss:  withCustomImagePullSecrets(testImagePullSecrets),
                        IsValidSts: func(deploy *appsv1.StatefulSet, rss 
*uniffleapi.RemoteShuffleService) (bool, error) {
                                if deploy.Spec.Template.Spec.ImagePullSecrets 
!= nil {
-                                       
deploy.Spec.Template.Spec.ImagePullSecrets = rss.Spec.ImagePullSecrets
                                        equal := 
reflect.DeepEqual(deploy.Spec.Template.Spec.ImagePullSecrets, 
testImagePullSecrets)
                                        if equal {
                                                return true, nil
@@ -535,6 +545,26 @@ func TestGenerateSts(t *testing.T) {
                                return false, fmt.Errorf("generated sts should 
include volumeClaimTemplates: %v", testImagePullSecrets)
                        },
                },
+               {
+                       name: "set custom annotations",
+                       rss:  withCustomAnnotations(testAnnotations),
+                       IsValidSts: func(deploy *appsv1.StatefulSet, rss 
*uniffleapi.RemoteShuffleService) (bool, error) {
+                               if deploy.Spec.Template.Annotations != nil {
+                                       for key, value := range testAnnotations 
{
+                                               equal := 
reflect.DeepEqual(deploy.Spec.Template.Annotations[key], value)
+                                               if key == 
constants.AnnotationRssName && equal {
+                                                       return false, 
fmt.Errorf("generated deploy shouldn't override reserved annotations: %v", key)
+                                               }
+                                               if key != 
constants.AnnotationRssName && !equal {
+                                                       return false, 
fmt.Errorf("generated deploy should include annotations: %v", key)
+                                               }
+                                       }
+                               } else {
+                                       return false, fmt.Errorf("generated 
deploy should include annotations: %v", testAnnotations)
+                               }
+                               return true, nil
+                       },
+               },
        } {
                t.Run(tt.name, func(tc *testing.T) {
                        sts := GenerateSts(nil, tt.rss)

Reply via email to