Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Upgrade the spark-on-k8s-operator module to spark-operator-chart-1.1.15 #minor #232

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

akumor
Copy link
Contributor

@akumor akumor commented Jan 5, 2022

This PR upgrades the spark-on-k8s-operator plugin with the goal of fixing support for defining the podSecurityContext for a SparkApplication created by executing a Spark task.

TL;DR

This upgrades the module to the spark-operator-chart-1.1.15 release.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

I fixed the bug by upgrading the version of the spark-on-k8s-operator to the most recently tagged release spark-operator-chart-1.1.15 and updated the spark flyteplugin code to be compatible.

Tracking Issue

flyteorg/flyte#2025

Follow-up issue

NA

@welcome
Copy link

welcome bot commented Jan 5, 2022

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

@akumor akumor force-pushed the upgrade-spark-operator-chart-1.1.15 branch from 12e6bfe to 755efbf Compare January 5, 2022 23:43
@akumor akumor marked this pull request as ready for review January 5, 2022 23:49
@codecov
Copy link

codecov bot commented Jan 6, 2022

Codecov Report

Merging #232 (3a7f924) into master (6b72fd6) will increase coverage by 1.44%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #232      +/-   ##
==========================================
+ Coverage   62.50%   63.95%   +1.44%     
==========================================
  Files         142      142              
  Lines        8849     9365     +516     
==========================================
+ Hits         5531     5989     +458     
- Misses       2816     2852      +36     
- Partials      502      524      +22     
Flag Coverage Δ
unittests 63.56% <100.00%> (+1.45%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
go/tasks/plugins/k8s/spark/spark.go 78.77% <100.00%> (+0.23%) ⬆️
go/tasks/plugins/array/k8s/task.go 54.91% <0.00%> (-4.41%) ⬇️
go/tasks/plugins/array/awsbatch/executor.go 39.09% <0.00%> (-1.48%) ⬇️
...plugins/array/awsbatch/definition/job_def_cache.go 0.00% <0.00%> (ø)
go/tasks/plugins/k8s/pod/plugin.go 89.28% <0.00%> (+0.39%) ⬆️
go/tasks/plugins/k8s/sagemaker/plugin.go 76.66% <0.00%> (+1.22%) ⬆️
go/tasks/plugins/array/awsbatch/client.go 66.32% <0.00%> (+2.83%) ⬆️
go/tasks/plugins/webapi/athena/plugin.go 21.76% <0.00%> (+3.91%) ⬆️
go/tasks/plugins/array/awsbatch/transformer.go 83.75% <0.00%> (+4.88%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b72fd6...3a7f924. Read the comment docs.

Copy link
Contributor

@kumare3 kumare3 left a comment

Choose a reason for hiding this comment

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

@kumare3
Copy link
Contributor

kumare3 commented Jan 11, 2022

LGTM, @akumor weird that the coverage is broken

kumare3
kumare3 previously approved these changes Jan 11, 2022
kumare3
kumare3 previously approved these changes Feb 1, 2022
go.mod Outdated
Comment on lines 54 to 74
k8s.io/api => k8s.io/api v0.20.2
k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.20.2
k8s.io/apimachinery => k8s.io/apimachinery v0.20.2
k8s.io/apiserver => k8s.io/apiserver v0.20.2
k8s.io/cli-runtime => k8s.io/cli-runtime v0.20.2
k8s.io/client-go => k8s.io/client-go v0.20.2
k8s.io/cloud-provider => k8s.io/cloud-provider v0.20.2
k8s.io/cluster-bootstrap => k8s.io/cluster-bootstrap v0.20.2
k8s.io/code-generator => k8s.io/code-generator v0.20.2
k8s.io/component-base => k8s.io/component-base v0.20.2
k8s.io/cri-api => k8s.io/cri-api v0.20.2
k8s.io/csi-translation-lib => k8s.io/csi-translation-lib v0.20.2
k8s.io/kube-aggregator => k8s.io/kube-aggregator v0.20.2
k8s.io/kube-controller-manager => k8s.io/kube-controller-manager v0.20.2
k8s.io/kube-proxy => k8s.io/kube-proxy v0.20.2
k8s.io/kube-scheduler => k8s.io/kube-scheduler v0.20.2
k8s.io/kubectl => k8s.io/kubectl v0.20.2
k8s.io/kubelet => k8s.io/kubelet v0.20.2
k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.20.2
k8s.io/metrics => k8s.io/metrics v0.20.2
k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.20.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Per this report, we may have to bump the deps to v0.20.11 for all of these...

Copy link
Contributor Author

@akumor akumor Feb 2, 2022

Choose a reason for hiding this comment

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

Unable to display this organization
The organization does not exist, or you do not have permissions to access it.

I can't view that report, but upgrading to v0.20.11 doesn't sound like it will be a problem. I'll look into bumping that version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@EngHabu I pushed a new commit that changed the deps to v0.20.11 but it looks like snyk is still failing

@akumor akumor force-pushed the upgrade-spark-operator-chart-1.1.15 branch from 1a7784f to 3688404 Compare February 3, 2022 00:47
@akumor akumor force-pushed the upgrade-spark-operator-chart-1.1.15 branch from 3688404 to 3a7f924 Compare February 3, 2022 22:56
@EngHabu EngHabu closed this Mar 9, 2022
@EngHabu EngHabu reopened this Mar 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants