Skip to content

Pipeline Implementation#1239

Merged
knative-prow-robot merged 31 commits into
knative:masterfrom
vaikas:pipeline
Jun 6, 2019
Merged

Pipeline Implementation#1239
knative-prow-robot merged 31 commits into
knative:masterfrom
vaikas:pipeline

Conversation

@vaikas
Copy link
Copy Markdown
Contributor

@vaikas vaikas commented May 15, 2019

Fixes #1067

Proposed Changes

  • Add messaging.knative.dev/pipelines for being able to define simple sequences

Release Note

Adds a pipeline (sequence of functions to execute in order) with an optional Reply to send results to.

@vaikas vaikas added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 15, 2019
@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label May 15, 2019
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 15, 2019
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vaikas-google

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 15, 2019
@vaikas vaikas removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 15, 2019
@knative-prow-robot knative-prow-robot added this to the v0.7.0 milestone May 15, 2019
Comment thread pkg/apis/messaging/v1alpha1/pipeline_lifecycle.go Outdated
Comment thread pkg/apis/messaging/v1alpha1/pipeline_types.go Outdated
Comment thread pkg/apis/messaging/v1alpha1/pipeline_types.go Outdated
Comment thread pkg/apis/messaging/v1alpha1/pipeline_types.go
Comment thread pkg/apis/messaging/v1alpha1/pipeline_types.go Outdated
Comment thread pkg/apis/messaging/v1alpha1/pipeline_types.go Outdated
@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 16, 2019
Comment thread pkg/apis/messaging/v1alpha1/pipeline_types.go
@lionelvillard
Copy link
Copy Markdown
Contributor

@vaikas-google If you want I can take an initial stab at the reconciler. LMK.

@knative-prow-robot knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 28, 2019
@vaikas
Copy link
Copy Markdown
Contributor Author

vaikas commented Jun 6, 2019

I wired a pipeline like this (using IMC (After my fix that got merged earlier today)), and the following pipeline worked, verified the data flew through the pipeline:
apiVersion: messaging.knative.dev/v1alpha1
kind: Pipeline
metadata:
name: vaikas-pipeline
namespace: foobar
spec:
channelTemplate:
apiVersion: messaging.knative.dev/v1alpha1
kind: InMemoryChannel
steps:

  • ref:
    apiVersion: serving.knative.dev/v1alpha1
    kind: Service
    name: first
  • ref:
    apiVersion: serving.knative.dev/v1alpha1
    kind: Service
    name: second
  • ref:
    apiVersion: serving.knative.dev/v1alpha1
    kind: Service
    name: third

@vaikas
Copy link
Copy Markdown
Contributor Author

vaikas commented Jun 6, 2019

If it looks good, I'll work on the validation / docs after this, but given the shift in time, unless there's something major that can't be fixed in a followup, I'd like to get this merged so that I don't have to keep rebasing :)
@lionelvillard @Harwayne

@n3wscott
Copy link
Copy Markdown
Contributor

n3wscott commented Jun 6, 2019

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2019
@Harwayne
Copy link
Copy Markdown
Contributor

Harwayne commented Jun 6, 2019

/hold

I want to review the changes.

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 6, 2019
if address == nil {
address := c.Status.AddressStatus.Address
if address != nil {
ps.ChannelStatuses[i].ReadyCondition = apis.Condition{Type: apis.ConditionReady, Status: corev1.ConditionTrue}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite accurate, a Channelable may be addressable but not ready.

For now, I'm OK with a TODO.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had a TODO there (and dropped it somewhere along the way), and I think we need to change Channelable a little bit, because it doesn't include anything useful besides this.

@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2019
Copy link
Copy Markdown
Contributor

@Harwayne Harwayne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/hold cancel
/lgtm

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 6, 2019
@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2019
@knative-metrics-robot
Copy link
Copy Markdown

The following is the coverage report on pkg/.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/messaging/v1alpha1/pipeline_defaults.go Do not exist 0.0%
pkg/apis/messaging/v1alpha1/pipeline_lifecycle.go Do not exist 98.1%
pkg/apis/messaging/v1alpha1/pipeline_types.go Do not exist 0.0%
pkg/apis/messaging/v1alpha1/pipeline_validation.go Do not exist 94.4%
pkg/reconciler/pipeline/pipeline.go Do not exist 71.7%

@knative-prow-robot knative-prow-robot merged commit 8023433 into knative:master Jun 6, 2019
@vaikas vaikas deleted the pipeline branch June 6, 2019 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Be able to define a simple pipeline

8 participants