Operator 0.1.0 #2

jiangzho · 2024-04-02T23:02:54Z

What changes were proposed in this pull request?

This PR proposes Spark Operator v1.0.0-alpha, which supports operating Spark Applications on k8s.

Please also refer README & docs for design details.

For quick start & test the operator with sample application, use getting started guide at spark-operator-docs/getting_started.md

Why are the changes needed?

Spark on k8s Operator is a project that helps to effectively manage Spark application lifecycle on k8s in addition to native integration. For more details, please also refer Spark Operator SPIP doc.

Does this PR introduce any user-facing change?

No changes introduced in Spark API.

Spark Operator extends k8s API for SparkApplications.

How was this patch tested?

Unit test coverage is being actively added into this repository.

Getting started guide provides an example to start and test this project locally or on a remote cluster.

Operator e2e tests would be added incrementally with ci integration.

Was this patch authored or co-authored using generative AI tooling?

no

mridulm · 2024-04-02T23:29:29Z

Thanks for the contribution !

A 19k diff patch is unfortunately not going to be easy to review , or result in good quality reviews.
Can you please split this up into more manageable chunks to aid with it ?

We can keep this PR as reference to help drive the actual PR reviews

jiangzho · 2024-04-02T23:56:15Z

Thanks for quick turn around @mridulm !

I understand the concern regarding its size. This commit is result of effort to break down the original scope as much as possible. Given that this is a new project, this PR includes several fundamental modules that are essential to make the operator functional. I'd also like to highlight that ~7k lines are auto-generated CRD yaml, which can be bypassed during reviewing its POJO (1k lines). There are ~3k lines comprises of documentations and examples which may require less scrutiny. I appreciate your understanding and open to the discussing how we can streamline the review process together.

zwangsheng · 2024-04-03T08:40:16Z

build-tools/helm/spark-kubernetes-operator/templates/_helpers.tpl

+{{/*Licensed to the Apache Software Foundation (ASF) under one*/}}
+{{/*or more contributor license agreements.  See the NOTICE file*/}}
+{{/*distributed with this work for additional information*/}}
+{{/*regarding copyright ownership.  The ASF licenses this file*/}}
+{{/*to you under the Apache License, Version 2.0 (the*/}}
+{{/*"License"); you may not use this file except in compliance*/}}
+{{/*with the License.  You may obtain a copy of the License at*/}}
+
+{{/*  http://www.apache.org/licenses/LICENSE-2.0*/}}
+
+{{/*Unless required by applicable law or agreed to in writing,*/}}
+{{/*software distributed under the License is distributed on an*/}}
+{{/*"AS IS" BASIS, 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.*/}}


Suggested change

{{/*Licensed to the Apache Software Foundation (ASF) under one*/}}

{{/*or more contributor license agreements. See the NOTICE file*/}}

{{/*distributed with this work for additional information*/}}

{{/*regarding copyright ownership. The ASF licenses this file*/}}

{{/*to you under the Apache License, Version 2.0 (the*/}}

{{/*"License"); you may not use this file except in compliance*/}}

{{/*with the License. You may obtain a copy of the License at*/}}

{{/* http://www.apache.org/licenses/LICENSE-2.0*/}}

{{/*Unless required by applicable law or agreed to in writing,*/}}

{{/*software distributed under the License is distributed on an*/}}

{{/*"AS IS" BASIS, 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.*/}}

{{/*

Licensed to the Apache Software Foundation (ASF) under one or more

contributor license agreements. See the NOTICE file distributed with

this work for additional information regarding copyright ownership.

The ASF licenses this file to You under the Apache License, Version 2.0

(the "License"); you may not use this file except in compliance with

the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software

distributed under the License is distributed on an "AS IS" BASIS,

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.

*/}}

Maybe we can get the file to pass the license check by writing it this way.

BTW seems CI run license check with apache/skywalking-eyes, we should move excludes to .github/.licenserc.yaml

Co-authored-by: Qi Tan <qi.tan.jade@gmail.com>

dongjoon-hyun · 2024-04-04T15:20:24Z

.github/workflows/build_and_test.yml

+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        java-version: [ 11, 17 ]


We need to support Java 21 to be consistent with SPARK-43831 .

updated workflow for java version - can you please re-approve the workflow ?

.gitignore

README.md

dongjoon-hyun · 2024-04-04T15:23:28Z

build-tools/helm/spark-kubernetes-operator/.helmignore

+.bzrignore
+.hg/
+.hgignore
+.svn/


Do we need .svn and .hg?

We do not - it's a blanket ignore. Lemme remove this unused. Thanks for the catch

dongjoon-hyun · 2024-04-04T15:25:55Z

build-tools/helm/spark-kubernetes-operator/crds/sparkapplications.org.apache.spark-v1.yml

+                        - v3_3_2
+                        - v3_3_1
+                        - v3_3_0
+                        - v3_2_0


Please follow Apache Spark community release policy by removing all EOL versions.

removed old versions except 3.4.1 - once spark-docker releases 3.4.2 I'll replace that asap.

dongjoon-hyun · 2024-04-04T15:26:44Z

build-tools/helm/spark-kubernetes-operator/crds/sparkapplications.org.apache.spark-v1.yml

@@ -0,0 +1,6947 @@
+# Generated by Fabric8 CRDGenerator, manual edits might get overwritten!


I'm wondering if we can avoid having this by generating this during the compile time.

Yes, this can be achieved. It's added for

reference

packed & used by the helm chart

But this is not mandatory. We already have a gradle task to automatically generate this at compile time.

If it's more preferred to not have the generated yaml checked in, I can remove it when implementing the chart publishing to helm reporistory.

if it's easy(without much effort to set up external building tools, especially OS coupled) to generate the constant code, I think we can remove it, just like we did for antlr, grpc in the spark main repo

Should be straightforward - removed this 39a74e1

docs at d448092

config/checkstyle/checkstyle-suppressions.xml

e2e-tests/spark-apps/spark_3_4_1/pyspark-example.yaml

dongjoon-hyun · 2024-04-04T15:30:51Z

e2e-tests/spark-apps/spark_3_4_1/spark-pi_scala_2.12.yaml

+apiVersion: org.apache.spark/v1alpha1
+kind: SparkApplication
+metadata:
+  name: spark-pi-341-212


FYI, this is not the latest even in Spark 3.4.x line because we have 3.4.2 already. Let's use 3.5.1.

Ack - the e2e-tests dir aims to include examples for all currently supported Spark versions. I started this effort with all available versions with official image . I'll replace the examples to remove old versions to avoid confusing people.

gradle.properties

spark-operator-api/src/main/java/org/apache/spark/kubernetes/operator/Constants.java

spark-operator-api/src/main/java/org/apache/spark/kubernetes/operator/SparkApplication.java

spark-operator-api/src/main/java/org/apache/spark/kubernetes/operator/spec/BaseSpec.java

dongjoon-hyun · 2024-04-04T15:39:39Z

spark-operator-api/src/main/java/org/apache/spark/kubernetes/operator/spec/JDKVersion.java

+
+public enum JDKVersion {
+    Java11,
+    Java17


Please add Java 21.

spark-operator-api/src/main/java/org/apache/spark/kubernetes/operator/spec/SparkVersion.java

spark-operator-docs/getting_started.md

dongjoon-hyun · 2024-04-04T15:44:40Z

spark-operator-docs/getting_started.md

+      restartPolicy: NEVER
+  deploymentMode: CLUSTER_MODE
+  driverArgs: []
+  jars: local:///opt/spark/examples/jars/spark-examples_2.12-3.4.1.jar


Please use the latest one, 3.5.2, because this is getting_started.md.

thanks for catching this ! will update to 3.5.2 as soon as it's available

spark-operator-docs/metrics_logging.md

dongjoon-hyun · 2024-04-04T15:47:16Z

spark-operator-docs/metrics_logging.md

+| kubernetes.client.<ResourceName>.<Method>                 | Meter      | Tracking the rates of HTTP request for a combination of one Kubernetes resource and one http method                      |
+| kubernetes.client.<NamespaceName>.<ResourceName>.<Method> | Meter      | Tracking the rates of HTTP request for a combination of one namespace-scoped Kubernetes resource and one http method     |
+
+## Forward Metrics to Prometheus


Thank you for providing this section.

spark-operator-docs/metrics_logging.md

spark-operator-docs/operator_probes.md

spark-operator-docs/spark_application.md

.gitattributes

dongjoon-hyun · 2024-04-16T16:35:11Z

Yes, Apache Iceberg community didn't include it. I'd prefer to exclude that jar, @aaruna and @pan3793 .

https://github.com/apache/iceberg/tree/main/gradle/wrapper

This is a breakdown from #2 : set up [gradle](https://gradle.org/) as the build-tool for operator Closes #4 from jiangzho/gradle. Authored-by: zhou-jiang <zhou_jiang@apple.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

dongjoon-hyun · 2024-04-18T16:36:06Z

Please rebase this PR, @jiangzho .

jiangzho · 2024-04-19T00:33:56Z

Rebase pick failed due to schedule change - attempted a non fast forward merge commit. I may do a final squash to make the history clear

dongjoon-hyun · 2024-04-19T19:29:40Z

.github/workflows/build_and_test.yml

+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        java-version: [ 11, 17, 21 ]


Please remove Java 11 and make a spin-off PR for this build_and_test.yml, @jiangzho .

dongjoon-hyun · 2024-04-19T19:30:17Z

.gitignore

+
+# Packages #
+############
+*.7z


For my understanding, where this came from?

dongjoon-hyun · 2024-04-19T19:30:26Z

.gitignore

+*.dmg
+*.gz
+*.iso
+*.rar


dongjoon-hyun · 2024-04-19T19:31:05Z

.gitignore

+
+# Logs and databases #
+######################
+*.log


This looks too broad.

dongjoon-hyun · 2024-04-19T19:33:14Z

build-tools/helm/spark-kubernetes-operator/values.yaml

+    additionalContainers: { }
+    # additionalContainers:
+    #  - name: ""
+    #    image: ""


Please remove 80 ~ 82 because we have line 79.

dongjoon-hyun · 2024-04-19T19:34:12Z

build-tools/helm/spark-kubernetes-operator/values.yaml

+    watchGivenNamespacesOnly: false
+    data:
+  #    - "spark-demo"
+  #    - "spark-demo"


May I ask why we have the same comment twice?

dongjoon-hyun · 2024-04-19T19:35:14Z

build.gradle

+  }
+}
+
+assert JavaVersion.current().isJava11Compatible(): "Java 11 or newer is required"


Please drop Java 11.

dongjoon-hyun · 2024-04-19T19:38:09Z

config/checkstyle/checkstyle.xml

+
+ -->
+
+<module name="Checker">


Please make a spin-off PR for Static Analysis-related stuff, @jiangzho .

### What changes were proposed in this pull request? This is a breakdown PR from #2 - setting up common build Java tasks and corresponding plugins. ### Why are the changes needed? This PR includes checkstyle, pmd, spotbugs. Also includes jacoco for coverage analysis, spotless for formatting. These tasks can help to enhance the quality of future Java contributions. They can also be referred in CI tasks for automation. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Tested manually. ### Was this patch authored or co-authored using generative AI tooling? no Closes #6 from jiangzho/builder_task. Authored-by: zhou-jiang <zhou_jiang@apple.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

dongjoon-hyun · 2024-04-23T06:01:18Z

Hi, All.

For the record, the following basic build and test PRs are merged. We are moving to API part.

To @jiangzho , please make a PR for spark-operator-api part first.

Since Apache Spark 4.0.0-preview will start very soon, please use the matched dependency of master branch instead of Apache Spark 3.5.1 dependency. For example, K8s client library should be 6.12.1 instead of 6.7.1.

dongjoon-hyun · 2024-04-26T21:23:17Z

Java API module is merged.

[SPARK-47950] Add Java API Module for Spark Operator #8

To @jiangzho , please make a PR for spark-operator and what you think it's tightly-related.

dongjoon-hyun · 2024-05-03T20:31:37Z

Hi, All.

For the record,

We has been working on submission module and will merge with Apache Spark 4.0.0-preview as the first version. Version-agnostic submission will be achieved later when we add Spark 3.5.x additionally.
- [SPARK-48017] Add Spark application submission worker for operator #10
From Spark repo side, we made the following improvements to guarantee Spark and Spark K8s Operator collaborate in a robust and seamlessly in the future.

Thank you for driving this, @jiangzho .

### What changes were proposed in this pull request? This is a breakdown PR of #2 - adding a submission worker implementation for SparkApplication. ### Why are the changes needed? Spark Operator needs a submission worker to convert its abstraction (the SparkApplication API) into k8s resource spec. This is a light-weight implementation based on native k8s integration. As of now, it's based off Spark 4.0.0-preview1 - but it's assumed to serve all Spark LTS versions. This is feasible because as it aims to cover only the spec generation, Spark core jars are still brought-in by application images. E2Es would set up with operator later to ensure that. Per SPIP doc, in future operator version(s) we may add more implementations for submission worker based on different Spark versions to achieve 100% version agnostic, at the cost of having multiple workers stand-by. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Added unit test coverage. ### Was this patch authored or co-authored using generative AI tooling? no Closes #10 from jiangzho/worker. Authored-by: zhou-jiang <zhou_jiang@apple.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

dongjoon-hyun · 2024-05-20T17:48:28Z

For the record, submission module is merged finally today with SPARK-48326 (TODO).

[SPARK-48017] Add Spark application submission worker for operator #10

From Spark repo side, @jiangzho also made the following improvements additionally for submission module.

Thank you again, @jiangzho . Shall we move forward to the next modules?

dongjoon-hyun · 2024-07-29T04:08:10Z

For the record, the following (including spark-operator module without SparkOperator.java) were merged so far.

Thank you, @jiangzho . Please move forward to SparkOperator.java with test coverage and other remaining stuff.

dongjoon-hyun · 2024-08-06T20:51:27Z

The following are merged too.

Now, we can move on to the remaining modules, @jiangzho .

dongjoon-hyun · 2024-08-09T19:34:33Z

Let me close this because this PR is obsolete as of now, @jiangzho . :)

jiangzho force-pushed the operator-v1-alpha branch from e347a16 to 7fc3ef4 Compare April 2, 2024 23:30

zwangsheng reviewed Apr 3, 2024

View reviewed changes

Operator 1.0.0-alpha

233c145

Co-authored-by: Qi Tan <qi.tan.jade@gmail.com>

jiangzho force-pushed the operator-v1-alpha branch from 7fc3ef4 to 233c145 Compare April 3, 2024 22:14