-
Notifications
You must be signed in to change notification settings - Fork 50
[SPARK-48398] Add Helm Chart #44
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
Conversation
| @@ -0,0 +1,25 @@ | |||
| ################################################################################ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this because this repository doesn't use this style for ASF license.
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| ################################################################################ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto.
| @@ -0,0 +1,148 @@ | |||
| ################################################################################ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto.
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| ################################################################################ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto.
| # https://kubernetes.io/docs/concepts/scheduling-eviction/topology-spread-constraints/ | ||
| topologySpreadConstraints: [ ] | ||
| operatorContainer: | ||
| jvmArgs: "-XX:+UseG1GC -Xms3G -Xmx3G -Dfile.encoding=UTF8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conflicts with memory 2Gi.
| port: 19091 | ||
| livenessProbe: | ||
| periodSeconds: 10 | ||
| initialDelaySeconds: 30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason for this 30s? Could you add a comment why the default value of livenessProbe is insufficient? Or, need to be overridden?
| periodSeconds: 10 | ||
| initialDelaySeconds: 30 | ||
| startupProbe: | ||
| failureThreshold: 30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the same way, why the default value, 3s, is insufficient for this?
dev/.rat-excludes
Outdated
| .*gradle | ||
| .*json | ||
| .helmignore | ||
| .*yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you want to exclude here? This PR adds .yaml file instead of yml, doesn't it?
In addition, AFAIK, you added the license header already. Does this mean the added header (of this file) is incompatible with Apache RAT check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added K8s integration test pipeline for you. Please revise this PR to run this helm chart on Minikube. nvm. Let's merge with the manual test and do this seperately.
|
I revised this PR, @jiangzho and @viirya .
|
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM. Merged to main
What changes were proposed in this pull request?
This PR proposes Spark Kubernetes Operator Helm chart, which describes deployment manifest for the operator.
Why are the changes needed?
Helm chart helps users to install, upgrade and uninstall Spark Operator in Kubernetes cluster. It provides a cloud native way of managing Operator in place. This chart helps us to complete the end-to-end flow for the operator app.
Does this PR introduce any user-facing change?
No (not yet released)
How was this patch tested?
Start minikube
Start miniKube and make it access locally-built image
Build Spark Operator Locally
Install the Spark Operator
Verify the Installation
helm test spark-kubernetes-operatorHere's a sample spark app yaml
Create a file named
spark-pi.yamland runThe operator should be able to operate given app from start to complete.
You can delete job with the following.
Uninstallation
To remove the installed resources from your cluster, reset environment to the defaults and
shutdown the cluster:
Was this patch authored or co-authored using generative AI tooling?
No