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

Add some helm chart unit tests and fix spark service account render failure when extra annotations are specified #1967

Merged
merged 4 commits into from
Apr 15, 2024

Conversation

ChenYi015
Copy link
Contributor

@ChenYi015 ChenYi015 commented Apr 12, 2024

@vara-bonthu
Copy link
Contributor

@ChenYi015, could you provide further details on the execution process of these unit tests? Also, could you verify whether you've conducted tests on these modifications using the Helm chart?

@ChenYi015
Copy link
Contributor Author

@ChenYi015, could you provide further details on the execution process of these unit tests? Also, could you verify whether you've conducted tests on these modifications using the Helm chart?

Detailed information about how to run helm chart unit tests can be found here helm-unittest/helm-unittest. First, you need to install helm unit test plugin:

helm plugin install https://github.com/helm-unittest/helm-unittest.git

Then, run helm chart unitt tests as follows:

$ helm unittest charts/spark-operator-chart --strict

### Chart [ spark-operator ] charts/spark-operator-chart

 PASS  Test spark operator deployment   charts/spark-operator-chart/tests/deployment_test.yaml
 PASS  Test spark operator rbac charts/spark-operator-chart/tests/rbac_test.yaml
 PASS  Test spark operator service account      charts/spark-operator-chart/tests/serviceaccount_test.yaml
 PASS  Test spark rbac  charts/spark-operator-chart/tests/spark-rbac_test.yaml
 PASS  Test spark service account       charts/spark-operator-chart/tests/spark-serviceaccount_test.yaml
 PASS  Test spark operator webhook service      charts/spark-operator-chart/tests/webhook-service_test.yaml

Charts:      1 passed, 1 total
Test Suites: 6 passed, 6 total
Tests:       46 passed, 46 total
Snapshot:    0 passed, 0 total
Time:        150.450791ms

It will search for test files under charts/spark-operator-chart/tests directory with suffix _test.yaml and run the test suites.

@ChenYi015
Copy link
Contributor Author

I have conducted tests on the modifications with my own k8s cluster, and it works as expected.

@vara-bonthu
Copy link
Contributor

Thanks @ChenYi015 ! It would be nice to add this to our contributors.md doc

Copy link
Contributor

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

/approve

Signed-off-by: Yi Chen <github@chenyicn.net>
…e specified

Signed-off-by: Yi Chen <github@chenyicn.net>
@ChenYi015 ChenYi015 force-pushed the chart-unit-test branch 2 times, most recently from 5866e91 to 387e08d Compare April 14, 2024 08:13
@ChenYi015
Copy link
Contributor Author

@vara-bonthu I have updated the developer guide about how to setup git pre-commit hooks and developing with helm chart.

Copy link
Contributor

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

@ChenYi015 one more change required

@@ -72,3 +95,82 @@ make build-api-docs
```

Running the above command will update the file `docs/api-docs.md`.

## Develope with the Helm Chart
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: spelling "Develop"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Signed-off-by: Yi Chen <github@chenyicn.net>
Signed-off-by: Yi Chen <github@chenyicn.net>
Copy link
Contributor

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ChenYi015, vara-bonthu

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:

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

@google-oss-prow google-oss-prow bot merged commit 6ded3ac into kubeflow:master Apr 15, 2024
7 checks passed
AndrewChubatiuk pushed a commit to AndrewChubatiuk/spark-on-k8s-operator that referenced this pull request Apr 16, 2024
…ailure when extra annotations are specified (kubeflow#1967)

* Add helm unit tests

Signed-off-by: Yi Chen <github@chenyicn.net>

* Fix: failed to render spark service account when extra annotations are specified

Signed-off-by: Yi Chen <github@chenyicn.net>

* Update developer guide

Signed-off-by: Yi Chen <github@chenyicn.net>

* Bump helm chart version

Signed-off-by: Yi Chen <github@chenyicn.net>

---------

Signed-off-by: Yi Chen <github@chenyicn.net>
peter-mcclonski pushed a commit to TechnologyBrewery/spark-on-k8s-operator that referenced this pull request Apr 16, 2024
…ailure when extra annotations are specified (kubeflow#1967)

* Add helm unit tests

Signed-off-by: Yi Chen <github@chenyicn.net>

* Fix: failed to render spark service account when extra annotations are specified

Signed-off-by: Yi Chen <github@chenyicn.net>

* Update developer guide

Signed-off-by: Yi Chen <github@chenyicn.net>

* Bump helm chart version

Signed-off-by: Yi Chen <github@chenyicn.net>

---------

Signed-off-by: Yi Chen <github@chenyicn.net>
Signed-off-by: Peter McClonski <mcclonski_peter@bah.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

helm chart failed to render spark service account when extra annotations are specified
2 participants