OnDependentUpdateFunc for Job will panic when enable volcano scheduler

[D0m021ng]

OnDependentUpdateFunc for TF Job will panic when enable volcano, because of job controller inject watching for job related podGroup.

OnDependentUpdateFunc

var logger *logrus.Entry
if _, ok := newObj.(*corev1.Pod); ok {
	logger = commonutil.LoggerForPod(newObj.(*corev1.Pod), jc.Controller.GetAPIGroupVersionKind().Kind)
}

if _, ok := newObj.(*corev1.Service); ok {
	logger = commonutil.LoggerForService(newObj.(*corev1.Service), jc.Controller.GetAPIGroupVersionKind().Kind)
}

...

// If it has a controller ref, that's all that matters.
if newControllerRef != nil {
	job := resolveControllerRef(jc, newObj.GetNamespace(), newControllerRef)
	if job == nil {
		return false
	}
	logger.Debugf("pod/service has a controller ref: %v, %v", newObj, oldObj)
	return true
}

logger might be null when newObj is v1beta1.PodGroup

TF Job controller

	// skip watching podgroup if podgroup is not installed
	_, err = mgr.GetRESTMapper().RESTMapping(schema.GroupKind{Group: v1beta1.SchemeGroupVersion.Group, Kind: "PodGroup"},
		v1beta1.SchemeGroupVersion.Version)
	if err == nil {
		// inject watching for job related podgroup
		if err = c.Watch(&source.Kind{Type: &v1beta1.PodGroup{}}, &handler.EnqueueRequestForOwner{
			IsController: true,
			OwnerType:    &kubeflowv1.TFJob{},
		}, predicate.Funcs{
			CreateFunc: util.OnDependentCreateFunc(r.Expectations),
			UpdateFunc: util.OnDependentUpdateFunc(&r.JobController),
			DeleteFunc: util.OnDependentDeleteFunc(r.Expectations),
		}); err != nil {
			return err
		}
	}

panic

[Crazybean-lwb]

not only tf, pytorch、mxnet... met the same problem :
#1679
I can only delete logger to avoid this problem

[johnugeorge]

which version of training operator are you using?

[D0m021ng]

k8s version: 1.20.15
training-operator version: v1-74655a (commit id 74655a177a7d886d6e2d9c86c800271d4d44687b)

[johnugeorge]

Seems like, bug got introduced as part of #1666

/cc @ggaaooppeenngg

[ggaaooppeenngg]

I only tested it with MPIJob. I made a temporary fix for this issue. @D0m021ng @johnugeorge

[johnugeorge]

@ggaaooppeenngg How did it work for MPIJob? Can you add a test case as well?

[ggaaooppeenngg]

@johnugeorge Only MPIJob controller uses OnDependentXXXFuncGeneric which initializes the generic logger at the beginning. I am trying to commit to a test for it.

[johnugeorge]

@johnugeorge Only MPIJob controller uses OnDependentXXXFuncGeneric which initializes the generic logger at the beginning. I am trying to commit to a test for it.

I see that pod watch uses OnDependentXXXFunc

training-operator/pkg/controller.v1/mpi/mpijob_controller.go

Line 200 in 74655a1

CreateFunc: util.OnDependentCreateFunc(jc.Expectations),

[ggaaooppeenngg]

@johnugeorge It is for the pod. For other objects, it uses the generic function.

training-operator/pkg/controller.v1/mpi/mpijob_controller.go

Line 207 in 74655a1

// Create generic predicates