sequence and parallel conversion webhook by lionelvillard · Pull Request #2611 · knative/eventing

lionelvillard · 2020-02-20T01:47:07Z

Proposed Changes

Add support for v1alpha1<->v1beta1 for sequences and parallels

Release Note

- 🎁 Add new feature
* Add support for Sequence and Parallel v1alpha1<->v1beta1

googlebot · 2020-02-20T15:44:05Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2020-02-20T20:40:08Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

lionelvillard · 2020-02-20T20:49:00Z

/assign @vaikas

vaikas

Thanks!! Just a thought about removing the schema section which has been giving us some trouble :( Also if you wouldn't mind v1beta1->v1alpha1->v1beta1 tests that would be awesome :)

vaikas · 2020-02-21T15:03:41Z

+                              type: string
+                              minLength: 1
+                            name:
+                              type: string


You'll probabaly beed the namespace here as well, or it won't necessarily roundtrip properly. We've had quite a few of these failures because things have drifted over time. Since you need to rebase anyways, maybe just get rid of the schemas. Look for example at inmemorychannel.yaml to see what it looks like without these. You need to add an overall validation section. Plan is to then autogenerate these and merge them rather than code them by hand.

ok done.

Still having an issue with the AdmissionController that cannot decode the new object. This happens just before defaulting. Not sure why yet

vaikas · 2020-02-21T15:06:30Z

+			},
+		},
+	}, {
+		name: "full configuration",


We started to also do tests where we start with v1beta1 -> v1alpha1 ->v1beta1, would you mind adding those tests as well? you can see for example pkg/apis/messaging/v1alpha1/channel_conversion_test.go.

vaikas · 2020-02-21T15:17:26Z

 	// for the namespace (or cluster, in case there are no defaults for the namespace).
 	// +optional
-	ChannelTemplate *eventingduckv1alpha1.ChannelTemplateSpec `json:"channelTemplate,omitempty"`
+	ChannelTemplate *messagingv1beta1.ChannelTemplateSpec `json:"channelTemplate,omitempty"`


lionelvillard · 2020-02-21T17:54:30Z

/hold

adding v1beta1->v1alpha1->v1beta1 tests

knative-metrics-robot · 2020-02-21T19:09:23Z

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

File	Old Coverage	New Coverage
pkg/apis/flows/v1alpha1/parallel_conversion.go	Do not exist	100.0%
pkg/apis/flows/v1alpha1/sequence_conversion.go	Do not exist	100.0%
pkg/apis/flows/v1beta1/parallel_conversion.go	Do not exist	100.0%
pkg/apis/flows/v1beta1/sequence_conversion.go	Do not exist	100.0%

vaikas · 2020-02-21T19:10:21Z

/lgtm
/approve

knative-prow-robot · 2020-02-21T19:10:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lionelvillard, vaikas

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:

~~OWNERS~~ [lionelvillard,vaikas]

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

lionelvillard · 2020-02-21T19:27:01Z

/hold cancel

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 20, 2020

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Feb 20, 2020

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 20, 2020

knative-prow-robot requested review from evankanderson and yolocs February 20, 2020 01:47

lionelvillard force-pushed the flows-v1alpha1-v1beta1 branch from b362108 to 15d5bcc Compare February 20, 2020 15:43

knative-prow-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 20, 2020

knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 20, 2020

googlebot added cla: no Indicates the PR's author has not signed the CLA. and removed cla: yes Indicates the PR's author has signed the CLA. labels Feb 20, 2020

knative-prow-robot added the area/test-and-release Test infrastructure, tests or release label Feb 20, 2020

mattmoor reviewed Feb 20, 2020

View reviewed changes

lionelvillard force-pushed the flows-v1alpha1-v1beta1 branch from 15d5bcc to 8c7102c Compare February 20, 2020 19:55

mattmoor reviewed Feb 20, 2020

View reviewed changes

lionelvillard force-pushed the flows-v1alpha1-v1beta1 branch from 8c7102c to 2c33f9f Compare February 20, 2020 20:40

googlebot removed the cla: no Indicates the PR's author has not signed the CLA. label Feb 20, 2020

knative-prow-robot removed the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 20, 2020

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Feb 20, 2020

knative-prow-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 20, 2020

lionelvillard changed the title ~~[WIP] sequence and parallel conversion webhook~~ sequence and parallel conversion webhook Feb 20, 2020

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 20, 2020

knative-prow-robot assigned vaikas Feb 20, 2020

knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 20, 2020

vaikas reviewed Feb 21, 2020

View reviewed changes

use v1alpha1 deliveryspec

cf91dc3

lionelvillard added 6 commits February 21, 2020 12:43

use v1beta1 DeliverySpec after all

037d599

[WIP] sequence and parallel conversion webhook

3e303c3

add v1beta1 schema

2e443b8

fix schema

9cf2c32

fix json deserialization

e65556f

remove parallel schema

fac97d5

lionelvillard force-pushed the flows-v1alpha1-v1beta1 branch from 831374a to fac97d5 Compare February 21, 2020 17:52

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 21, 2020

lionelvillard added 3 commits February 21, 2020 13:17

add v1beta1 rt tests

c3b4644

fix build and unit tests

145618b

fix e2e

14c52d4

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2020

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 21, 2020

knative-prow-robot merged commit dd11517 into knative:master Feb 21, 2020

This was referenced Jun 17, 2020

Implement v1 API for flows.* resources #3335

Closed

Implement V1 API for messaging.* resources. #3336

Closed

Implement V1 API for eventing.* resources. #3337

Closed

nachocano mentioned this pull request Jun 26, 2020

Move ContainerSource to v1beta1 #3426

Closed

nachocano mentioned this pull request Jul 8, 2020

Move SinkBinding to v1beta1 #3544

Closed

8 tasks

lionelvillard deleted the flows-v1alpha1-v1beta1 branch August 28, 2020 15:38

lionelvillard restored the flows-v1alpha1-v1beta1 branch September 3, 2020 15:42

lionelvillard deleted the flows-v1alpha1-v1beta1 branch September 3, 2020 15:58

Conversation

lionelvillard commented Feb 20, 2020

Proposed Changes

Uh oh!

googlebot commented Feb 20, 2020

Uh oh!

googlebot commented Feb 20, 2020

Uh oh!

lionelvillard commented Feb 20, 2020

Uh oh!

vaikas left a comment

Choose a reason for hiding this comment

Uh oh!

vaikas Feb 21, 2020

Choose a reason for hiding this comment

Uh oh!

lionelvillard Feb 21, 2020

Choose a reason for hiding this comment

Uh oh!

vaikas Feb 21, 2020

Choose a reason for hiding this comment

Uh oh!

vaikas Feb 21, 2020

Choose a reason for hiding this comment

Uh oh!

lionelvillard commented Feb 21, 2020

Uh oh!

knative-metrics-robot commented Feb 21, 2020

Uh oh!

vaikas commented Feb 21, 2020

Uh oh!

knative-prow-robot commented Feb 21, 2020

Uh oh!

lionelvillard commented Feb 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants