Skip to content

[WIP] trigger and broker to have a description via 'kubectl explain'#3832

Closed
nlopezgi wants to merge 2 commits into
knative:masterfrom
nlopezgi:issue-3054
Closed

[WIP] trigger and broker to have a description via 'kubectl explain'#3832
nlopezgi wants to merge 2 commits into
knative:masterfrom
nlopezgi:issue-3054

Conversation

@nlopezgi
Copy link
Copy Markdown
Contributor

@nlopezgi nlopezgi commented Aug 13, 2020

Part of #3054

Proposed Changes

Sample output of kubectl explain
Trigger:

>kubectl explain trigger
KIND:     Trigger
VERSION:  eventing.knative.dev/v1

DESCRIPTION:
     Trigger represents a request to have events delivered to a consumer from a
     Broker's event pool.

FIELDS:
   apiVersion	<string>
     APIVersion defines the versioned schema of this representation of an
     object. Servers should convert recognized schemas to the latest internal
     value, and may reject unrecognized values. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources

   kind	<string>
     Kind is a string value representing the REST resource this object
     represents. Servers may infer this from the endpoint the client submits
     requests to. Cannot be updated. In CamelCase. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds

   metadata	<Object>
     Standard object's metadata. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata

   spec	<Object>
     Spec defines the desired state of the Trigger.

   status	<map[string]>
     Status represents the current state of the Trigger. This data may be out of
     date.

Broker:

kubectl explain Broker
KIND:     Broker
VERSION:  eventing.knative.dev/v1

DESCRIPTION:
     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 request delivery of events from a Broker's pool to a specific
     URL or Addressable endpoint.

FIELDS:
   apiVersion	<string>
     APIVersion defines the versioned schema of this representation of an
     object. Servers should convert recognized schemas to the latest internal
     value, and may reject unrecognized values. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources

   kind	<string>
     Kind is a string value representing the REST resource this object
     represents. Servers may infer this from the endpoint the client submits
     requests to. Cannot be updated. In CamelCase. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds

   metadata	<Object>
     Standard object's metadata. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata

   spec	<Object>
     Spec defines the desired state of the Broker.

   status	<map[string]>
     Status represents the current state of the Broker. This data may be out of
     date.

Content was partially generated using https://github.com/Harwayne/knative-gcp/tree/reflective-schema/hack/schema
fyi @lionelvillard

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Aug 13, 2020
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 13, 2020
@nlopezgi
Copy link
Copy Markdown
Contributor Author

nlopezgi commented Aug 13, 2020

Used @Harwayne's generator (https://github.com/Harwayne/knative-gcp/tree/reflective-schema/hack/schema) to create some of the content, but modified manually files as the output from his tool is not exactly in the right format. My feeling is that getting a tool such as that one to work perfectly for all cases is not worth the effort/payback.

Comment thread config/core/resources/broker.yaml Outdated
schema:
openAPIV3Schema:
type: object
description: "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 request delivery of events from a Broker's pool to a specific URL or Addressable endpoint."
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.

nit: I see receiver/consumer/subscriber used in the descriptions. would prefer to pick one.

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.

Used Subscriber (as its part of the API) everywhere possible. Updated source docs (in *_types.go)

type: object
properties:
backoffDelay:
description: 'BackoffDelay is the delay before retrying. More
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.

nit: is there any convention in using single or double quotes in description?

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.

decided to use single quotes as its can be applied homogeneously (i.e., double quotes are not good in some cases)

@lionelvillard
Copy link
Copy Markdown
Contributor

/approve

Letting @liu-cong LGTM it after addressing the comments

@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 13, 2020
@knative-prow-robot
Copy link
Copy Markdown
Contributor

@nlopezgi: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-knative-eventing-conformance-tests 27bfded link /test pull-knative-eventing-conformance-tests
pull-knative-eventing-integration-tests 27bfded link /test pull-knative-eventing-integration-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@knative-test-reporter-robot
Copy link
Copy Markdown

The following jobs failed:

Test name Triggers Retries
pull-knative-eventing-conformance-tests 0/3
pull-knative-eventing-integration-tests 0/3

Failed non-flaky tests preventing automatic retry of pull-knative-eventing-integration-tests:

test/e2e.TestDefaultBrokerWithManyTriggers/test_default_broker_with_many_attribute_triggers_using_v1beta1_trigger
test/e2e.TestDefaultBrokerWithManyTriggers/test_default_broker_with_many_deprecated_triggers
test/e2e.TestDefaultBrokerWithManyTriggers/test_default_broker_with_many_attribute_and_extension_triggers
test/e2e.TestDefaultBrokerWithManyTriggers/test_default_broker_with_many_attribute_triggers
test/e2e.TestDefaultBrokerWithManyTriggers
test/e2e.TestPingSourceV1Beta1EventTypes
test/e2e.TestApiServerSourceV1Alpha2EventTypes
test/e2e.TestPingSourceV1Alpha2EventTypes

and 64 more.

@nlopezgi nlopezgi changed the title trigger and broker to have a description via 'kubectl explain' [WIP] trigger and broker to have a description via 'kubectl explain' Aug 14, 2020
@knative-prow-robot 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 Aug 14, 2020
@nlopezgi
Copy link
Copy Markdown
Contributor Author

marking as WIP until I figure out how to fix the testing issues

This was referenced Aug 14, 2020
@nlopezgi
Copy link
Copy Markdown
Contributor Author

superseded by the two PRs mentioned above

@nlopezgi nlopezgi closed this Aug 14, 2020
@nlopezgi nlopezgi deleted the issue-3054 branch August 14, 2020 19:19
@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 14, 2020
@knative-prow-robot
Copy link
Copy Markdown
Contributor

@nlopezgi: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants