Resume Experiment from Volume

[andreyvelich]

Fixes: #1250.

This is implementation for resuming experiment from the Volume.

Steps:

1. I added new ResumePolicy=FromVolume which represents resuming experiment from the volume.
   @gaocegege @johnugeorge @sperlingxx what do you think about the name? Should we rename it to ResumePolicy=VolumeSource or think about better name, maybe rename LongRunning also?


2. I added ResumePolicy to Suggestion API to understand if controller needs to deploy volume for the Suggestion.
   Do we need to add some volume specification to Suggestion API also.
   For example: StorageClassName, PVC name ?

3. If ResumePolicy=VolumeSource, PVC is created. If StorageClassName=DefaultSuggestionStorageClass, PV with local cluster path is created also.

4. I didn’t attach Experiment labels to PVC and PV, like we did here: https://github.com/andreyvelich/katib/blob/issue-1250-resume-from-volume/pkg/controller.v1beta1/suggestion/composer/composer.go#L74 (it was done to connect service with deployment).
   Do we want to see these labels in the PVC/PV?

5. Currently, for the PV default cluster local volume path is: prefix + <suggestion name>-<suggestion namespace>. We need to add name and namespace to not conflict with other submitted Experiments in various namespaces.
   Prefix can be changed in katib-config later.

6. Currently, for suggestion container default path is /opt/katib/data. Also can be changed in katib-config.

7. PVC name = <suggestion name>

8. PV name = <suggestion name>-<suggestion namespace>

9. I added new validation for the webhook. That is needed to prevent submitting Experiment with the same name, if corresponding PV was not deleted after deleting Experiment.
   We can't Watch for the PV, like for the PVC. And suggestion controller can't be triggered and create PV again, once PV is deleted from the cluster.

10. I had to change suggestion status workflow to allow resuming experiment from the volume.
    This change doesn't affect current workflow for LongRunning Suggestion.
    For the ResumePolicy=VolumeSource:
    When User submits Experiment, Suggestion status:

Created (status = True) -> DeploymentReady (status = False) -> DeploymentReady (status = True) -> Running (status = True) -> (experiment is finished) -> Running (status = False) -> DeploymentReady (status = False) -> Succeeded (status = True) .

When User restarts Experiment, Suggestion status:

Running (status = True, reason="Experiment is restarting") -> (remove succeeded condition) -> DeploymentReady (status = True) -> Running (status = True, reason="SuggestionRunning")) -> (experiment is finished) -> Running (status = False) -> DeploymentReady (status = False) -> Succeeded (status = True).

This approach can avoid adding additional condition for Suggestion: Restarting.

What do you think @gaocegege @johnugeorge @sperlingxx ?

I still need to make some testing, but this PR is ready for review.
I will add unit test and e2e test in the future PR.

/assign @gaocegege @johnugeorge @sperlingxx

/cc @jlewi

[kubeflow-bot]

This change is 

[andreyvelich]

I tested example, everything was working.
I will remove WIP status.

Since we are using PV with local host path after deletion Suggestion PV and PVC, directory on cluster will be not deleted.
I am not sure that we can/should clean-up directory in host node's filesystem.

[sperlingxx]

Hi @andreyvelich , after a careful review, I am still confused about how we persist/retrieve suggestion instance to/from PV. Since in restartSuggestion function, we fetch suggestion instance via kubernetes client directly.

[andreyvelich]

@sperlingxx Thank you for the review.

When user submits Experiment, we deploy Suggestion service and deployment with attached PVC on it.
When Experiment is succeeded, we mark Suggestion succeeded and delete deployment and service:

katib/pkg/controller.v1beta1/suggestion/suggestion_controller.go

Lines 147 to 157 in 44b0ae6

	if instance.IsSucceeded() {
	err = r.deleteDeployment(instance)
	if err != nil {
	return reconcile.Result{}, err
	}
	err = r.deleteService(instance)
	if err != nil {
	return reconcile.Result{}, err
	}
	return reconcile.Result{}, nil
	}

.
We don't delete PVC and it saves information from the Suggestion deployment.
Ones user wants to restart Experiment, we change Suggestion status:

katib/pkg/controller.v1beta1/experiment/experiment_util.go

Line 163 in 44b0ae6

suggestion.MarkSuggestionStatusRunning(corev1.ConditionFalse, reason, msg)

.
Then it automatically runs ReconcileSuggestion

katib/pkg/controller.v1beta1/suggestion/suggestion_controller.go

Line 187 in 44b0ae6

func (r *ReconcileSuggestion) ReconcileSuggestion(instance *suggestionsv1beta1.Suggestion) error {

and it creates Deployment and Service that we have deleted before.
PV/PVC is not created again because we didn't delete it.

If user deletes Experiment, it automatically clean-up all Suggestion resources (deployment/service/pvc/pv) since we have owner reference for each object.

[johnugeorge]

why do we add restart message when suggestion is terminated?

[andreyvelich]

Just for additional information after Suggestion is succeeded for the user.
@johnugeorge You think it is not necessary?

[johnugeorge]

4. Why not?

[sperlingxx]

@andreyvelich Thanks for detailed explanation. So, we assume suggestion programs can recover from disk (or maybe they are stateless) in FromVolume mode ?

[andreyvelich]

@andreyvelich Thanks for detailed explanation. So, we assume suggestion programs can recover from disk (or maybe they are stateless) in FromVolume mode ?

Yes, later add functionality to set StorageClassName from the katib-config. To allow user set provisioner for the Suggestion volume.

[andreyvelich]

4. Why not?

@johnugeorge Not sure if it is needed for the Volume, since we have Owner Reference for the volume: https://github.com/andreyvelich/katib/blob/issue-1250-resume-from-volume/pkg/controller.v1beta1/suggestion/composer/composer.go#L288.

[andreyvelich]

I renamed PVC to -, PV to -- to be consistent with Suggestion's deployment and service.
I think user can easily understand which k8s resources related to Suggestion.
/cc @johnugeorge @gaocegege

[sperlingxx]

StorageClassName from

Cool!
/lgtm

[johnugeorge]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [johnugeorge]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[johnugeorge]

/lgtm