Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration Test Framework Prototype #605

Merged
merged 15 commits into from
Oct 27, 2022

Conversation

kevin85421
Copy link
Member

@kevin85421 kevin85421 commented Sep 29, 2022

Why are these changes needed?

Currently, existing tests for operators only test for the default configuration. However, from the last 6 weeks (the design doc is created on Sep. 26, 2022), 11 out of 34 commits are used to fix bugs caused by configurations. Hence, this project decided to focus on configuration. This PR is a prototype that can reproduce every YAML-related configuration bug (in the 11 reported config bugs) by adding a simple JsonPatch.

if __name__ == '__main__':
    template_name = 'config/ray-cluster.mini.yaml.template'
    namespace = 'default'
    with open(template_name) as base_yaml:
        baseCR = yaml.load(base_yaml, Loader=yaml.FullLoader)

    patch_list = [
        jsonpatch.JsonPatch([{'op': 'replace', 'path': '/spec/headGroupSpec/template/spec/containers/0/name', 'value': 'ray-head-1'}]),
        jsonpatch.JsonPatch([{'op': 'replace', 'path': '/spec/headGroupSpec/template/spec/containers/0/name', 'value': 'ray-head-2'}]),
        # Reproduce #612
        jsonpatch.JsonPatch([{'op': 'replace', 'path': '/spec/headGroupSpec/replicas', 'value': 2}]),
        # Reproduce #587
        jsonpatch.JsonPatch([
            {'op': 'replace', 'path': '/spec/workerGroupSpecs/0/replicas', 'value': 2},
            {'op': 'add', 'path': '/spec/workerGroupSpecs/0/template/metadata/name', 'value': 'haha'}
            ]),
        # Reproduce #585
        jsonpatch.JsonPatch([{'op': 'add', 'path': '/spec/headGroupSpec/rayStartParams/object-manager-port', 'value': '12345'}]),
        # Reproduce #572 #530
        jsonpatch.JsonPatch([{'op': 'add', 'path': '/spec/headGroupSpec/template/metadata/labels/app.kubernetes.io~1name', 'value': 'ray'}]),
        # Reproduce #529
        jsonpatch.JsonPatch([
            {'op': 'replace', 'path': '/spec/headGroupSpec/template/spec/containers/0/resources/requests/memory', 'value': '256Mi'},
            {'op': 'replace', 'path': '/spec/headGroupSpec/template/spec/containers/0/resources/limits/memory', 'value': '512Mi'}
        ])
    ]

    rs = RuleSet([HeadPodNameRule(), EasyJobRule(), HeadSvcRule()])
    mut = Mutator(baseCR, patch_list)
    images = ['rayproject/ray:2.0.0', 'kuberay/operator:v0.3.0', 'kuberay/apiserver:v0.3.0']

    test_cases = unittest.TestSuite()
    for new_cr in mut.mutate():
        addEvent = RayClusterAddCREvent(new_cr, [rs], 90, namespace)
        test_cases.addTest(GeneralTestCase('runTest', images, addEvent))
    runner=unittest.TextTestRunner()
    runner.run(test_cases)

Related issue number

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

@kevin85421 kevin85421 changed the title Config test prototype (DO NOT MERGE) Configuration Test Framework Prototype Sep 29, 2022
@DmitriGekhtman
Copy link
Collaborator

Notes --
We should be able to use this framework to test deployment using configs other than CRs
(for example, Helm charts).

At some point, we may also wish to support testing configuration for multiple deployed objects.
For example, we may wish to test operator configuration changes.
Or we may wish to test deploying a RayCluster and a RayJob which carries a reference to that RayCluster.

@kevin85421
Copy link
Member Author

[Discussion]: Which one is better, unittest or pytest?

@DmitriGekhtman
Copy link
Collaborator

[Discussion]: Which one is better, unittest or pytest?

Or both.
The Ray CI uses pytest to run tests written using unittest.
For example:
https://github.com/ray-project/ray/blob/3e7c207f02e7368e1245e2cfafd27cb0bf179ff7/python/ray/tests/test_autoscaler_yaml.py#L99
https://github.com/ray-project/ray/blob/3e7c207f02e7368e1245e2cfafd27cb0bf179ff7/python/ray/tests/test_autoscaler_yaml.py#L667

@simon-mo
Do you have thoughts on pytest vs unittest?

@simon-mo
Copy link
Collaborator

simon-mo commented Oct 4, 2022

Pytest fixture helps modularize setup and tear down code. Generally considered more powerful. So most of the Ray codebase uses it except autoscaler and tune. Pytest is compatible with unittest anyway. In the end it's up to your taste.

@kevin85421
Copy link
Member Author

kevin85421 commented Oct 5, 2022

TODO:

  1. Refactor
def replace_patch(path, value):
    return jsonpatch(...)
  1. Add some simple config tests in KubeRay CI

tests/framework/prototype.py Outdated Show resolved Hide resolved
tests/framework/prototype.py Outdated Show resolved Hide resolved
@kevin85421 kevin85421 force-pushed the config-test-prototype branch from 29576c6 to 4406a4d Compare October 21, 2022 18:46
@kevin85421 kevin85421 changed the title (DO NOT MERGE) Configuration Test Framework Prototype Configuration Test Framework Prototype Oct 21, 2022
Copy link
Collaborator

@DmitriGekhtman DmitriGekhtman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more suggestions/questions to see if we can simplify things a bit.

tests/framework/prototype.py Show resolved Hide resolved
tests/framework/prototype.py Show resolved Hide resolved
tests/framework/prototype.py Outdated Show resolved Hide resolved
tests/framework/prototype.py Show resolved Hide resolved
Copy link
Collaborator

@DmitriGekhtman DmitriGekhtman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're at a good place for this prototype!
Let's get this running in the KubeRay CI in a follow-up.
After that, we can write more tests using this framework.
Later, we can think about running the tests in the Ray buildkite CI, if it becomes necessary due to compute limitations.

@DmitriGekhtman DmitriGekhtman merged commit 8282e6b into ray-project:master Oct 27, 2022
DmitriGekhtman pushed a commit that referenced this pull request Nov 7, 2022
…e ones (#678)

Use #605 to test sample RayCluster YAMLs.

Signed-off-by: Kai-Hsun Chen <kaihsun@apache.org>
kevin85421 pushed a commit that referenced this pull request Nov 19, 2022
… one (#731)

Use #605 to test sample RayService YAML.
The test includes the following two checking rules.

1. It is able to [submit a simple job](https://github.com/jasoonn/kuberay/blob/69efe590970f9d8a4fe0fff66300049f008c6ae9/tests/framework/prototype.py#L188-L197) to the created raycluster.

2. It is able to use [curl in the pod](https://github.com/jasoonn/kuberay/blob/69efe590970f9d8a4fe0fff66300049f008c6ae9/tests/framework/prototype.py#L199-L216) to access the service.
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
Currently, existing tests for operators only test for the default configuration. However, from the last 6 weeks (the design doc is created on Sep. 26, 2022), 11 out of 34 commits are used to fix bugs caused by configurations. Hence, this project decided to focus on configuration. This PR is a prototype that can reproduce every YAML-related configuration bug (in the 11 reported config bugs) by adding a simple JsonPatch.
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
…e ones (ray-project#678)

Use ray-project#605 to test sample RayCluster YAMLs.

Signed-off-by: Kai-Hsun Chen <kaihsun@apache.org>
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
… one (ray-project#731)

Use ray-project#605 to test sample RayService YAML.
The test includes the following two checking rules.

1. It is able to [submit a simple job](https://github.com/jasoonn/kuberay/blob/69efe590970f9d8a4fe0fff66300049f008c6ae9/tests/framework/prototype.py#L188-L197) to the created raycluster.

2. It is able to use [curl in the pod](https://github.com/jasoonn/kuberay/blob/69efe590970f9d8a4fe0fff66300049f008c6ae9/tests/framework/prototype.py#L199-L216) to access the service.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants