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)