apache / incubator-uniffle

Uniffle is a high performance, general purpose Remote Shuffle Service.
https://uniffle.apache.org/
Apache License 2.0
388 stars 149 forks source link

[MINOR] feat(operator): Add support to pass annotations to coordinator nodeport/headless service #2187

Closed shlomitubul closed 1 month ago

shlomitubul commented 1 month ago

What changes were proposed in this pull request?

Add support to pass a list of annotations per coordinator service (node port/headless).

Why are the changes needed?

Some tools use service annotations to create DNS records and currently is not supported.

Does this PR introduce any user-facing change?

Introduce 2 new optional fields in CRD:

  1. headlessServiceAnnotations
  2. nodePortServiceAnnotations

How was this patch tested?

UT + manually

github-actions[bot] commented 1 month ago

Test Results

 2 926 files  +21   2 926 suites  +21   5h 57m 56s :stopwatch: + 10m 3s  1 043 tests + 2   1 041 :white_check_mark: + 3   2 :zzz: ±0  0 :x:  - 1  13 033 runs  +80  13 003 :white_check_mark: +81  30 :zzz: ±0  0 :x:  - 1 

Results for commit f40980b5. ± Comparison against base commit a08c2272.

:recycle: This comment has been updated with latest results.

jerqi commented 1 month ago

Could you add a UT in coordinator_test.go?

shlomitubul commented 1 month ago

Could you add a UT in coordinator_test.go?

done

jerqi commented 1 month ago

Could you fix the UT?

shlomitubul commented 1 month ago

Could you fix the UT?

UT seems to pass. Build failure might be related to uncommitted auto-generated files, so I added those files (I didn't understand why they were needed in the first place since they were created in part of the CI build anyway?).

jerqi commented 1 month ago

Could you fix the UT?

UT seems to pass. Build failure might be related to uncommitted auto-generated files, so I added those files (I didn't understand why they were needed in the first place since they were created in part of the CI build anyway?).

Thanks for your contribution. The build script will format the code and check whether there exists code not committed.

advancedxy commented 1 month ago

late +1 from my side as well.