Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Oct 24, 2022

What changes were proposed in this pull request?

This PR aims to support zsh in K8s entrypoint.sh.

Why are the changes needed?

zsh doesn't support readarray.

% readarray
zsh: command not found: readarray

Instead, zsh supports the following way.

% SPARK_EXECUTOR_JAVA_OPTS=("${(@f)$(< /tmp/java_opts.txt)}")
% echo $SPARK_EXECUTOR_JAVA_OPTS[@]
-Djava.net.preferIPv6Addresses=false -XX:+IgnoreUnrecognizedVMOptions --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.lang.reflect=ALL-UNNAMED --add-opens=java.base/java.io=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.nio=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.base/sun.nio.ch=ALL-UNNAMED --add-opens=java.base/sun.nio.cs=ALL-UNNAMED --add-opens=java.base/sun.security.action=ALL-UNNAMED --add-opens=java.base/sun.util.calendar=ALL-UNNAMED --add-opens=java.security.jgss/sun.security.krb5=ALL-UNNAMED -Dspark.driver.blockManager.port=7079 -Dspark.driver.port=7078

Does this PR introduce any user-facing change?

No. This PR only introduces additional behavior when there is no readarray command.

How was this patch tested?

Pass the K8s CIs.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-40904][K8S] Support zsh in K8s entrypoint.sh [SPARK-40904][K8S] Support zsh in K8s entrypoint.sh Oct 25, 2022
@dongjoon-hyun
Copy link
Member Author

All tests passed.

Could you review this, please, @viirya ?

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

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

Looks good. How about just leave one line (non-readarray case)?

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Oct 25, 2022

Thank you, @viirya . The else statement's command is only supported by zsh~

@dongjoon-hyun
Copy link
Member Author

Merged to master for Apache Spark 3.4.0.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-40904 branch October 25, 2022 00:25
Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM2

@dongjoon-hyun
Copy link
Member Author

Thank you, @HyukjinKwon !

SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
### What changes were proposed in this pull request?

This PR aims to support `zsh` in K8s `entrypoint.sh`.

### Why are the changes needed?

`zsh` doesn't support `readarray`.
```
% readarray
zsh: command not found: readarray
```

Instead, `zsh` supports the following way.
```
% SPARK_EXECUTOR_JAVA_OPTS=("${(f)$(< /tmp/java_opts.txt)}")
% echo $SPARK_EXECUTOR_JAVA_OPTS[]
-Djava.net.preferIPv6Addresses=false -XX:+IgnoreUnrecognizedVMOptions --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.lang.reflect=ALL-UNNAMED --add-opens=java.base/java.io=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.nio=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.base/sun.nio.ch=ALL-UNNAMED --add-opens=java.base/sun.nio.cs=ALL-UNNAMED --add-opens=java.base/sun.security.action=ALL-UNNAMED --add-opens=java.base/sun.util.calendar=ALL-UNNAMED --add-opens=java.security.jgss/sun.security.krb5=ALL-UNNAMED -Dspark.driver.blockManager.port=7079 -Dspark.driver.port=7078
```

### Does this PR introduce _any_ user-facing change?

No. This PR only introduces additional behavior when there is no `readarray` command.

### How was this patch tested?

Pass the K8s CIs.

Closes apache#38380 from dongjoon-hyun/SPARK-40904.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
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.

3 participants