Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposed feature: Plugable revision deployers #4152

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/apis/serving/v1beta1/revision_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,8 @@ func (rs *RevisionSpec) SetDefaults(ctx context.Context) {
for i := range vms {
vms[i].ReadOnly = true
}

if rs.Deployer.Name == "" {
rs.Deployer.Name = "KnativeServing"
}
}
21 changes: 21 additions & 0 deletions pkg/apis/serving/v1beta1/revision_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/knative/pkg/kmeta"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"encoding/json"
)

// +genclient
Expand Down Expand Up @@ -127,6 +128,12 @@ type RevisionSpec struct {
// be provided.
// +optional
TimeoutSeconds *int64 `json:"timeoutSeconds,omitempty"`

// The deployer to handle this revision, used to disable Knatives deploying
// of revisions and injection of sidecars, and allow a third party controller
// to do it instead.
// +optional
Deployer Deployer `json:"deployer,omitempty"`
}

const (
Expand Down Expand Up @@ -168,3 +175,17 @@ type RevisionList struct {

Items []Revision `json:"items"`
}

// A deployer, to allow a third party controller to deploy revisions.
type Deployer struct {

// The name of the deployer. If no deployer is specified, or if the deployer
// is KnativeServing, then KnativeServing will deploy it.
Name string `json:"name"`

// Deployer specific configuration, if needed. The structure is specific to
// the deployer specified, and it's up to the deployer to do any necessary
// validation, updating the status if validation fails.
// +optional
Config json.RawMessage `json:"config,omitempty"`
}
46 changes: 28 additions & 18 deletions pkg/reconciler/revision/reconcile_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,34 +43,44 @@ const (
)

func (c *Reconciler) reconcileDeployment(ctx context.Context, rev *v1alpha1.Revision) error {

ns := rev.Namespace
deploymentName := resourcenames.Deployment(rev)
logger := logging.FromContext(ctx).With(zap.String(logkey.Deployment, deploymentName))

deployment, err := c.deploymentLister.Deployments(ns).Get(deploymentName)
if apierrs.IsNotFound(err) {
// Deployment does not exist. Create it.
rev.Status.MarkDeploying("Deploying")
deployment, err = c.createDeployment(ctx, rev)
if err != nil {
logger.Errorf("Error creating deployment %q: %v", deploymentName, err)

if rev.Spec.RevisionSpec.Deployer.Name == "KnativeServing" {
if apierrs.IsNotFound(err) {
// Deployment does not exist. Create it.
rev.Status.MarkDeploying("Deploying")
deployment, err = c.createDeployment(ctx, rev)
if err != nil {
logger.Errorf("Error creating deployment %q: %v", deploymentName, err)
return err
}
logger.Infof("Created deployment %q", deploymentName)
} else if err != nil {
logger.Errorf("Error reconciling deployment %q: %v", deploymentName, err)
return err
} else if !metav1.IsControlledBy(deployment, rev) {
// Surface an error in the revision's status, and return an error.
rev.Status.MarkResourceNotOwned("Deployment", deploymentName)
return fmt.Errorf("Revision: %q does not own Deployment: %q", rev.Name, deploymentName)
} else {
// The deployment exists, but make sure that it has the shape that we expect.
deployment, err = c.checkAndUpdateDeployment(ctx, rev, deployment)
if err != nil {
logger.Errorf("Error updating deployment %q: %v", deploymentName, err)
return err
}
}
logger.Infof("Created deployment %q", deploymentName)
} else if err != nil {
logger.Errorf("Error reconciling deployment %q: %v", deploymentName, err)
return err
} else if !metav1.IsControlledBy(deployment, rev) {
// Surface an error in the revision's status, and return an error.
rev.Status.MarkResourceNotOwned("Deployment", deploymentName)
return fmt.Errorf("Revision: %q does not own Deployment: %q", rev.Name, deploymentName)
} else {
// The deployment exists, but make sure that it has the shape that we expect.
deployment, err = c.checkAndUpdateDeployment(ctx, rev, deployment)
if err != nil {
logger.Errorf("Error updating deployment %q: %v", deploymentName, err)
return err
// There's an error (or it's not found), but we don't care because it's not ours to care about
return nil
}
// Otherwise, continue processing to update the status accordingly.
}

// If a container keeps crashing (no active pods in the deployment although we want some)
Expand Down