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

Fix for #868 #876

Merged
merged 1 commit into from
Apr 22, 2020
Merged

Fix for #868 #876

merged 1 commit into from
Apr 22, 2020

Conversation

liyinan926
Copy link
Collaborator

Fixes #868.

func addVolumeMount(pod *corev1.Pod, mount corev1.VolumeMount) *patchOperation {
i := findContainer(pod)
if i < 0 {
glog.Warningf("not able to add VolumeMount %s as Spark container was not found in pod %s", mount.Name, pod.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think others who might depend on podTemplates may find it useful to have the container name said out in the warning-log, which is "executor" for Spark 2 and "spark-kubernetes-executor" for Spark 3. It may help to debug problems of this kind. Verbosity in case of error is always useful. :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't parse the sparkVersion due to the lack of validation on the field and a proper parser. This is why in findContainer we match executor container with one of the two possible container names. Because of that, it's no easy way of knowing which container is missing.

@liyinan926 liyinan926 merged commit 92a6611 into kubeflow:master Apr 22, 2020
akhurana001 pushed a commit to lyft/spark-on-k8s-operator that referenced this pull request Oct 29, 2020
jbhalodia-slack pushed a commit to jbhalodia-slack/spark-operator that referenced this pull request Oct 4, 2024
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.

Linear search does not account for the case when the container is not found
2 participants