Skip to content

kubectl explain for Trigger #3845

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.

Sign up for GitHub

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

Merged
knative-prow-robot merged 6 commits into from
Aug 14, 2020
Merged

kubectl explain for Trigger #3845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
da2f61a
add content for triggar and broker to have a description via
Aug 13, 2020
27bfded
fix review nits
Aug 13, 2020
849f017
scope down changes
Aug 14, 2020
2dc87bb
address review comments
Aug 14, 2020
43afd7b
fix
Aug 14, 2020
ae0c5de
nit
Aug 14, 2020
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
27 changes: 17 additions & 10 deletions config/core/resources/trigger.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,22 @@ spec:
properties:
broker:
type: string
description: "Broker that this trigger receives events from. If not specified, will default to 'default'."
Copy link
Copy Markdown
Member

@pierDipi pierDipi Aug 14, 2020

Choose a reason for hiding this comment

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

" If not specified, will default to 'default'.", isn't it valid anymore?

Copy link
Copy Markdown
Contributor Author

@nlopezgi nlopezgi Aug 14, 2020

Choose a reason for hiding this comment

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

I removed this following changes from #3573 (which I forgot to mention this PR supersedes). Let me know if you think I should add it back.

Copy link
Copy Markdown
Member

@pierDipi pierDipi Aug 14, 2020

Choose a reason for hiding this comment

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

I think this defaulting behavior exists and we should mention it in the description. Also, I see that Ville there leaves this https://github.com/knative/eventing/pull/3573/files#diff-35c872630bcc973ea91f0d72122bb2f7R60 untouched, and removes that from the trigger_types.go, but I'm not sure why.
WDYT?

Copy link
Copy Markdown
Contributor

@lionelvillard lionelvillard Aug 14, 2020

Choose a reason for hiding this comment

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

the default behavior exists in v1beta1, not v1. I would add it just in the v1beta1 schema.

Copy link
Copy Markdown
Member

@pierDipi pierDipi Aug 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, only v1beta1.

Copy link
Copy Markdown
Contributor Author

@nlopezgi nlopezgi Aug 14, 2020

Choose a reason for hiding this comment

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

added it here for v1beta1 schema.

description: 'Broker that this trigger receives events from. If not specified, will default to ''default''.'
filter:
type: object
properties:
attributes:
type: object
description: "Map of CloudEvents attributes used for filtering events. If not specified, will default to all events"
description: 'Map of CloudEvents attributes used for filtering events. If not specified, will default to all events'
additionalProperties:
type: string
subscriber:
type: object
description: "the destination that should receive events."
description: 'the destination that should receive events.'
properties:
ref:
type: object
description: "a reference to a Kubernetes object from which to retrieve the target URI."
description: 'a reference to a Kubernetes object from which to retrieve the target URI.'
required:
- apiVersion
- kind
Expand All @@ -90,7 +90,7 @@ spec:
minLength: 1
uri:
type: string
description: "the target URI or, if ref is provided, a relative URI reference that will be combined with ref to produce a target URI."
description: 'the target URI or, if ref is provided, a relative URI reference that will be combined with ref to produce a target URI.'
status:
type: object
x-kubernetes-preserve-unknown-fields: true
Expand All @@ -101,31 +101,37 @@ spec:
schema:
openAPIV3Schema:
type: object
description: 'Trigger represents a request to have events delivered to a subscriber from a Broker''s event pool.'
properties:
spec:
description: 'Spec defines the desired state of the Trigger.'
required:
- subscriber
- broker
type: object
properties:
broker:
type: string
description: "Broker that this trigger receives events from."
description: 'Broker that this trigger receives events from.'
filter:
type: object
description: 'Filter is the filter to apply against all events from
the Broker. Only events that pass this filter will be sent to
the Subscriber. If not specified, will default to allowing all
events.'
properties:
attributes:
type: object
description: "Map of CloudEvents attributes used for filtering events. If not specified, will default to all events"
description: 'Map of CloudEvents attributes used for filtering events. If not specified, will default to all events'
additionalProperties:
type: string
subscriber:
type: object
description: "the destination that should receive events."
description: 'the destination that should receive events.'
properties:
ref:
type: object
description: "a reference to a Kubernetes object from which to retrieve the target URI."
description: 'a reference to a Kubernetes object from which to retrieve the target URI.'
required:
- apiVersion
- kind
Expand All @@ -145,8 +151,9 @@ spec:
minLength: 1
uri:
type: string
description: "the target URI or, if ref is provided, a relative URI reference that will be combined with ref to produce a target URI."
description: 'the target URI or, if ref is provided, a relative URI reference that will be combined with ref to produce a target URI.'
status:
description: 'Status represents the current state of the Trigger. This data may be out of date.'
type: object
x-kubernetes-preserve-unknown-fields: true
names:
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/eventing/v1/broker_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (

// Broker collects a pool of events that are consumable using Triggers. Brokers
// provide a well-known endpoint for event delivery that senders can use with
// minimal knowledge of the event routing strategy. Receivers use Triggers to
// minimal knowledge of the event routing strategy. Subscribers use Triggers to
// request delivery of events from a Broker's pool to a specific URL or
// Addressable endpoint.
type Broker struct {
Expand Down
5 changes: 2 additions & 3 deletions pkg/apis/eventing/v1/trigger_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const (
// +genreconciler
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// Trigger represents a request to have events delivered to a consumer from a
// Trigger represents a request to have events delivered to a subscriber from a
// Broker's event pool.
type Trigger struct {
metav1.TypeMeta `json:",inline"`
Expand Down Expand Up @@ -75,8 +75,7 @@ var (
)

type TriggerSpec struct {
// Broker is the broker that this trigger receives events from. If not specified, will default
// to 'default'.
Comment thread
nlopezgi marked this conversation as resolved.
// Broker is the broker that this trigger receives events from.
Broker string `json:"broker,omitempty"`

// Filter is the filter to apply against all events from the Broker. Only events that pass this
Expand Down