PIP: Async pipeline executions (EPIC)

[kispaljr]

PIP: Async pipeline executions

PIP stands for "Porch Improvement Proposal", I tried to model it after KEPs

Summary

In this PIP we propose changes aiming to limit the time it takes for porch server API calls to return with a result (success or failure). This PIP specifically focuses on avoiding the execution of the kpt package pipeline during API call handling.

Motivation

Currently the porch-server performs pipeline executions (package rendering) as part of Create and Update PackageRevision and PackageRevisionResources API calls. This makes these API calls to take a potentially very long time and that causes multiple problems. The most important among those is that the apiserver runtime we use has a built-in time limit for all API calls that is hardcoded to 34 seconds. This means that for long running pipelines (e.g. for deployment package validations) these API calls will fail with a timeout, even if the pipeline execution would have been successful otherwise.

Goals

Make sure that no API call handler in the porch apiserver will wait for the execution of a render operation.
If a render is needed (e.g. after a "Create PackageRevision" or an "Update PackageRevisionResources" call),
the it should be scheduled to run asynchronously and the API call must return ASAP without waiting for the results.

Non-goals

Git fetch operations performed during the processing of some API calls also make those API calls to sometimes take a long time to finish. Fixing that is out-of-scope of this PIP, but should be addressed in a separate PIP.

Proposal

When a PackageRevision or a PackageRevisionResources object is modified, thus a render operation is needed for that package revision:

• if a render operation is not scheduled or ongoing for the revision, then schedule a render operation
• if a render operation is scheduled, but hasn't started to execute yet, then do nothing and let the scheduled render to run
• if is ongoing for that revision, then cancel the ongoing Render operation and schedule a new one. 

When the render operation has finally finished:

• if render was successful: try an optimistic Update() on the corresponding PackageRevisionResources object to save the render results into the package revision contents.
  • If the Update is successful record the results in the Kptfile status, in the "RenderFinished" and "Rendered" conditions
  • If the Update fails, because the contents of the package has changed, do not retry, but ensure that a new render is scheduled for the new contents (if it hasn't been already)
• if render failed: record the results in the Kptfile status, in the "RenderFinished" and "Rendered" conditions

The new code must ensure that a PackageRevision with a scheduled or ongoing Render operation cannot be Proposed or Approved. Please note that an earlier PR already prevents package revisions with a failed render to be proposed or approved.

While a render operation is scheduled or ongoing, a new "RenderFinished" condition of the Kptfile is set to "False", and the "Rendered" condition (introduced in #194) is removed from the conditions list. Otherwise the "RenderFinished" condition is set to "True" and the "Rendered" condition contains the results of the last render operation, the same way as in the current implementation.

Please note that this proposal changes how our clients use porch. If this is implemented Create and Update package revisions will return without running the pipeline, and it is up to the client to regularly check the status conditions to observe if the render has finished, and with what results if it is important for him/her. This behavior is similar to how most Kubernetes resources are working.

Detailed Design

The following analysis assumes that both PR nephio-project/nephio#252: Ensure task list is single element, introduce upgrade task and PR nephio-project/nephio#194: Allow creating/updating draft package revisions with a failing pipeline has already been merged.

New RenderScheduler struct

A new struct type name RenderScheduler should be added. The names of the fields and methods below are only mentioned for reference, they can be changed during implementation.

It should contain the following fields:

• workQueue: a buffered channel of type repository.PackageRevision that acts as a work queue. When the render of a PackageRevision is scheduled it is put into the channel, and a set a of worker go routines are continuously reading the PackageRevisions from the channel and executing a render operation on them.

• scheduled: set of UIDs of scheduled PackageRevisions. It should always contain the UIDs of PackageRevisions that have been already added to the channel buffer, but haven't been read out of it. This has to be maintained during adding to and reading out from the work queue.

• ongoing: set of UIDs of PackageRevisions that has a currently running render operation. This has to be maintained in the background go routines processing render operation requests from the work queue.

It should have the following methods:

• ScheduleRender(pr repository.PackageRevision) error that implements the business logic detailed above:

  • if pr is in ongoing, then cancel the ongoing Render operation and add pr to the work queue.
  • if pr is in scheduled, but not in ongoing, then do nothing and let the scheduled render to run
  • if pr is not in scheduled, nor in ongoing, then add it to the work queue.
    This should also set the RenderFinished condition of the Kptfile to "False", and remove the Rendered condition.
    If a new render is scheduled an event should be emitted stating the a render started for "this-and-that" PackageRevision.

• RenderExecutionStatus(pr repository.PackageRevision) RenderExecutionStatus that should return an enum of the following possible values: None|Scheduled|Ongoing

• StartProcessing(n int) error: start n background go routines that will continuously read PackageRevisions from the work queue and call process(pr) on them

• StopProcessing() cancels all background go routines and all ongoing render operations

• process(pr repository.PackageRevision) is a private method called by the background go routines. It should implement the same business logic as renderPackageMutation.apply() for executing the render operation. It should also implement the followings:

  • when the render has finished, emit an event with the results stating either:

    • Render finished, but a concurent Render is ongoing, so dropping the results
    • Render succeeded
    • Render failed with the following error: ...

  • when the render has finished it should be checked whether another render is scheduled or ongoing for the same PackageRevision. If it is, then simply return, without recording the results.

  • if the render operation was successful: try an optimistic Update() API call on the corresponding PackageRevisionResources object to save the render results into the package revision contents.

    • If the Update is successful record the results in the Kptfile status, in the "RenderFinished" and "Rendered" conditions.
    • If the Update fails, because the contents of the package has changed, do not retry, but call ScheduleRender() on the PR to ensure that a new render is scheduled for the new contents (if it hasn't been already)

  • if the render operation failed: record the results in the Kptfile status, in the "RenderFinished" and "Rendered" conditions by calling an Update() on the PackageRevision. Same as above, if the Update fails, because the contents of the package has changed, do not retry, but call ScheduleRender() on the PR to ensure that a new render is scheduled for the new contents

Further code changes

The porch server code should also be modified at least at the following points:

• in /pkg/engine/engine.go:
  add a new field to cadEngine of type RenderScheduler. Initialize it in NewCaDEngine(), that includes calling the StartProcessing() method

• in /pkg/engine/engine.go, UpdatePackageRevision():
  if pkgrev.Spec.Lifecycle is changed to Proposed or Approved, then check if RenderScheduler.RenderExecutionStatus(pkgrev) returns with None, otherwise reject proposal/approval until the render operation has finished.

• in /pkg/task/generictaskhandler.go: wherever th.renderMutation(obj.Namespace).apply(ctx, resources) is called, replace it with a call to RenderScheduler.ScheduleRender()

• remove the renderPackageMutation task from the codebase, since it is no longer necessary

• when the porch apiserver (re)starts it should rebuild the workqueue based on the "RenderFinished" conditions of draft PackageRevisions

[nagygergo]

Not a comprehensive review, but what happens with the workqueue if the porch-server pod restarts between the point where the response was generated and the execution of the kpt pipeline?

[nagygergo]

if the render operation was successful: try an optimistic Update() API call on the corresponding PackageRevisionResources object to save the render results into the package revision contents.

Optimistic in this case means that if the user has executed an update for the packageRevisionResources, then the user's changes take priority, and the render pipeline's results should be discarded?

[kispaljr]

Not a comprehensive review, but what happens with the workqueue if the porch-server pod restarts between the point where the response was generated and the execution of the kpt pipeline?

good point. On porch-server restart we have to rebuild the work queue from the PkgRev statuses. I will add this to the PIP

[kispaljr]

Optimistic in this case means that if the user has executed an update for the packageRevisionResources, then the user's changes take priority, and the render pipeline's results should be discarded?

My wording was sloppy :) I just wanted refer to the optimistic locking nature of K8s Updates. What I wanted to say is to try to send an Update with a resourceVersion that is coming from the Get() that was called in the beginning of the render operation. If the PkgRev has been changed since it will be automatically rejected with a Conflict error. And yes I propose that instead of retrying the Update, just make sure that a render operation is re-scheduled on the new contents.

[kispaljr]

For the record the PIP was updated by:

• workqueue should be rebuilt on porch server (re)start (suggestion from @nagygergo)
• emitting relevant Events at critical points to make troubleshooting easier (suggestion from @mozesl-nokia)
• splitting render status into two separate conditions