Check if workerGroupSpec is not empty before accessing it

[ChristianZaccaria]

Issue link

Jira: https://issues.redhat.com/browse/RHOAIENG-6614

What changes have been made

• The failing Ray Tests were caused by an index out of range error in the webhook logic. The workerGroupSpec was attempted to be accessed when it doesn't exist.
• Check is now made to verify it's not empty before accessing it.

Verification steps

0. Have all components deployed including this version of the CFO and Kueue.
1. Deploy KubeRay-Operator in the same namespace as referenced in the DSCI default-dsci
2. In the KubeRay repo, run the previously failing test: go test -timeout 30m -v ./test/e2e/rayjob_lightweight_test.go ./test/e2e/support.go
3. Test should pass.

Checks

• [ ] I've made sure the tests are passing.
• Testing Strategy
  • [ ] Unit tests
  • [ ] Manual tests
  • [ ] Testing is not required for this change

[sutaakar]

@ChristianZaccaria Can you please provide a unit test verifying the changes?

[ChristianZaccaria]

@sutaakar I added unit tests covering these changes. Please let me know if there are other test cases I should be covering here.

Also, I am thinking of adding a RayCluster template in the test support file and perhaps append items to it in the tests to reduce the amount of code in the test file. Is this a good approach or what is the best practices on this? Thank you! :)

[sutaakar]

@ChristianZaccaria Creating one valid RayCluster CR and then modify it is a possible approach, may increase test readability.

[ChristianZaccaria]

@sutaakar latest changes include one positive test for each "main" function, and several negative tests to cover the full functionality of the raycluster_webhook.go file.

[sutaakar]

@ChristianZaccaria Can you also adjust test-unit Makefile target to run the new unit test (currently it doesn't run anything). That way the unit test will be executed for PRs.

Otherwise this PR looks great, good job.

[ChristianZaccaria]

@sutaakar I made the last few changes, tests are now running as part of this PR check in the Unit tests workflow. Thank you!

[ChristianZaccaria]

I made a PR to move the core functions used here to the codeflare-common repository. Should we merge that and then make the changes here or what is the appropriate approach? Thanks @sutaakar

[sutaakar]

/lgtm

I made a https://github.com/project-codeflare/codeflare-common/pull/48 used here to the codeflare-common repository. Should we merge that and then make the changes here or what is the appropriate approach?

That is up to you, can be merged now and refactored later or it can wait.

[sutaakar]

/lgtm

[sutaakar]

/approve

[openshift-ci[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sutaakar

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/project-codeflare/codeflare-operator/blob/main/OWNERS)~~ [sutaakar] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment