fix: replicaset KubeGroupNameAnnotation handling

[hajnalmt]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Fixing a bug in the podgroup controller, where the user provided the podgroup in an annotation but we created the podgroup anyway disregarding it.

I added a unit test that covers the test case both for statefulsets and replicasets and added some minor enhancements regarding the logging in some other tests.

The failed unit test case output if the codes are not present:
For the replicaset:

go test -v -run TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists  ./pkg/controllers/podgroup/
=== RUN   TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists
=== RUN   TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists/Replicaset_shall_not_create_new_podgroup
I1210 09:23:17.198663 1426032 pg_controller.go:168] pod test/rs-pod-with-pg-annotation has created podgroup
I1210 09:23:17.199015 1426032 pg_controller_handler.go:103] Try to create podgroup for pod test/rs-pod-with-pg-annotation
I1210 09:23:17.199104 1426032 pg_controller_handler.go:348] PodGroup <test/podgroup-rs-uid-123> created for Pod <test/rs-pod-with-pg-annotation>
E1210 09:23:17.199111 1426032 pg_controller_handler.go:210] normal pod test/rs-pod-with-pg-annotation annotations scheduling.k8s.io/group-name value is not podgroup-rs-uid-123, but existing-pg
I1210 09:23:17.199140 1426032 pg_controller_test.go:1427] PodGroups found: [existing-pg podgroup-rs-uid-123]
    pg_controller_test.go:1429:
                Error Trace:    /home/uih20178/go/src/volcano.sh/volcano/pkg/controllers/podgroup/pg_controller_test.go:1429
                Error:          Not equal:
                                expected: 1
                                actual  : 2
                Test:           TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists/Replicaset_shall_not_create_new_podgroup
                Messages:       Expected 1 PodGroup, found 2: [existing-pg podgroup-rs-uid-123]
--- FAIL: TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists (0.00s)
    --- FAIL: TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists/Replicaset_shall_not_create_new_podgroup (0.00s)
FAIL
FAIL    volcano.sh/volcano/pkg/controllers/podgroup     0.029s
FAIL

For statefulset:

go test -v -run TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists  ./pkg/controllers/podgroup/
=== RUN   TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists
=== RUN   TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists/Replicaset_shall_not_create_new_podgroup
I1210 09:43:41.274330 1462029 pg_controller.go:168] pod test/rs-pod-with-pg-annotation has created podgroup
I1210 09:43:41.274801 1462029 pg_controller_handler.go:103] Try to create podgroup for pod test/rs-pod-with-pg-annotation
I1210 09:43:41.275008 1462029 pg_controller_handler.go:110] Pod test/rs-pod-with-pg-annotation is already associated with a podgroup existing-pg
=== RUN   TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists/StatefulSet_shall_not_create_new_podgroup
I1210 09:43:41.275234 1462029 pg_controller_handler.go:169] Try to create or update podgroup for pod test/sts-pod-with-pg-annotation when statefulset add or update
I1210 09:43:41.275263 1462029 pg_controller_handler.go:350] PodGroup <test/podgroup-sts-uid-123> created for Pod <test/sts-pod-with-pg-annotation>
E1210 09:43:41.275268 1462029 pg_controller_handler.go:212] normal pod test/sts-pod-with-pg-annotation annotations scheduling.k8s.io/group-name value is not podgroup-sts-uid-123, but existing-pg
I1210 09:43:41.275430 1462029 pg_controller_test.go:1491] "PodGroup list after StatefulSet update" pgList=[{"metadata":{"name":"existing-pg","namespace":"test"},"spec":{"minMember":1},"status":{}},{"metadata":{"name":"podgroup-sts-uid-123","namespace":"test","ownerReferences":[{"apiVersion":"apps/v1","kind":"StatefulSet","name":"sts1","uid":"sts-uid-123","controller":true}]},"spec":{"minMember":1,"minResources":{"count/pods":"1","pods":"1"}},"status":{"phase":"Pending"}}]
    pg_controller_test.go:1496:
                Error Trace:    /home/uih20178/go/src/volcano.sh/volcano/pkg/controllers/podgroup/pg_controller_test.go:1496
                Error:          Not equal:
                                expected: 1
                                actual  : 2
                Test:           TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists/StatefulSet_shall_not_create_new_podgroup
                Messages:       Expected 1 PodGroup, found 2: [existing-pg podgroup-sts-uid-123]
--- FAIL: TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists (0.00s)
    --- PASS: TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists/Replicaset_shall_not_create_new_podgroup (0.00s)
    --- FAIL: TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists/StatefulSet_shall_not_create_new_podgroup (0.00s)
FAIL
FAIL    volcano.sh/volcano/pkg/controllers/podgroup     0.026s
FAIL

Which issue(s) this PR fixes:

Fixes #4784

Does this PR introduce a user-facing change?

NONE

[gemini-code-assist]

Summary of Changes

Hello @hajnalmt, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a bug in the podgroup controller that led to the creation of unnecessary duplicate podgroups when pods were already annotated with an existing podgroup name. The fix introduces a check to prevent this redundant creation and is accompanied by new unit tests to ensure the correct handling of pre-annotated pods for both ReplicaSets and StatefulSets. Additionally, it includes a minor code style update and improved logging in some test cases.

Highlights

• Bug Fix: Corrects an issue in the podgroup controller where it would erroneously create a new podgroup even if a pod already specified one via the scheduling.k8s.io/group-name annotation.
• Duplicate PodGroup Prevention: Ensures that the controller respects existing KubeGroupNameAnnotationKey annotations on pods, preventing the creation of redundant podgroups.
• New Unit Tests: Introduces TestNoPodGroupCreatedForWhenKubeGroupNameAnnotationExists to validate this behavior for both ReplicaSets and StatefulSets, confirming that no new podgroups are created when the annotation is present.
• Code Style Improvement: Updates the worker loop in pg_controller.go to use the more idiomatic for range int(pg.workers) syntax.
• Enhanced Test Logging: Improves error messages in existing unit tests for better debugging output.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request fixes a bug where the podgroup controller would create a new podgroup for a pod even if one was already specified via an annotation. The fix is correctly implemented for the ReplicaSet handler, and new unit tests are added to verify the behavior for both ReplicaSets and StatefulSets. The changes are generally good, but I have a couple of suggestions to improve code compatibility and test robustness.

[hzxuzhonghu]

LGTM Give others a chance to have a look

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• pkg/controllers/OWNERS [hzxuzhonghu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[JesseStutler]

/lgtm
Thanks for your work, mate, you help a lot! @hajnalmt 😄

[hajnalmt]

Thank you for the review! You are superb guys ☺️

[hajnalmt]

/cherrypick release-1.13

[volcano-sh-bot]

@hajnalmt: new pull request created: #4826

Details

In response to this:

/cherrypick release-1.13

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[hajnalmt]

/cherrypick release-1.12

[volcano-sh-bot]

@hajnalmt: #4792 failed to apply on top of branch "release-1.12":

Applying: fix: podgroup controller replicaset annotation fix
Using index info to reconstruct a base tree...
M	pkg/controllers/podgroup/pg_controller_handler.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/controllers/podgroup/pg_controller_handler.go
CONFLICT (content): Merge conflict in pkg/controllers/podgroup/pg_controller_handler.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix: podgroup controller replicaset annotation fix
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick release-1.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[volcano-sh-bot]

@hajnalmt: new issue created for failed cherrypick: #4827

Details

In response to this:

/cherrypick release-1.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.