search

radanalyticsio / spark-operator

Operator for managing the Spark clusters on Kubernetes and OpenShift.
Apache License 2.0
157 stars 61 forks source link

Ignore onModify if only the status block has changed #283

Closed tmckayus closed 4 years ago

tmckayus commented 4 years ago

Description

add overloads in SparkApplication and SparkHistoryServer for onModify and test whether the object has actually changed before calling the super method.

Types of changes

:bug: Bug fix (non-breaking change which fixes an issue)

tmckayus commented 4 years ago

@elmiko @jkremser ptal

This is to fix inifinite loops in SparkApplication and SparkHistoryServer caused by updates to the status block. They did not overload onModify from the super class, and so every status write caused "delete/add" to be done.

The tracking of existing objects is based on the RunningClusters code for the SparkCluster type, here we just have a local map so we can compare objects. The comparison works because the status blocks are not actually part of the formal types generated from the json, they are inserted directly into the CRDs in the AbstractOperator and so they exist outside of the internal SparkApplication and SparkHistoryServer classes.

tmckayus commented 4 years ago

this is kind of urgent, since the last build spark-operator image (1.0.5) has this bug for SparkApplication and SparkHistoryServer (SparkCluster is okay)/

todo[bot] commented 4 years ago

This comparison works to rule out a change in status because

https://github.com/radanalyticsio/spark-operator/blob/05c53fdc874ec722f714793854c7a1f1bd6f0e64/src/main/java/io/radanalytics/operator/app/AppOperator.java#L70-L75

This comment was generated by todo based on a TODO comment in 05c53fdc874ec722f714793854c7a1f1bd6f0e64 in #283. cc @tmckayus.
todo[bot] commented 4 years ago

This comparison works to rule out a change in status because

https://github.com/radanalyticsio/spark-operator/blob/05c53fdc874ec722f714793854c7a1f1bd6f0e64/src/main/java/io/radanalytics/operator/historyServer/HistoryServerOperator.java#L87-L92

This comment was generated by todo based on a TODO comment in 05c53fdc874ec722f714793854c7a1f1bd6f0e64 in #283. cc @tmckayus.