Update container source documentation with `template` field #1491

owenthereal · 2019-06-19T20:45:27Z

Proposed Changes

knative/eventing#1321 introduces PodTemplateSpec to ContainerSourceSpec. This feature will be out in 0.7.x. Updating documentation to reflect the template field.

knative-prow-robot · 2019-06-19T20:45:42Z

Hi @jingweno. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

owenthereal · 2019-06-19T20:46:52Z

/kind eventing

n3wscott · 2019-06-19T22:45:49Z

/lgtm
/ok-to-test

owenthereal · 2019-06-19T23:37:50Z

@n3wscott I'm unsure why the tests failed. Any pointers?

vaikas · 2019-06-20T08:39:27Z

/test pull-knative-docs-integration-tests

vaikas · 2019-06-20T08:39:35Z

/test pull-knative-docs-integration-tests

samodell · 2019-06-20T17:35:36Z

Thanks, @jingweno !
/approve

samodell · 2019-06-20T17:36:02Z

/test pull-knative-docs-integration-tests

owenthereal · 2019-06-20T21:26:14Z

According to the build log, the e2e tests failed at https://github.com/knative/docs/blob/master/test/e2e/sampleapptestbase.go#L111 or https://github.com/knative/docs/blob/master/test/e2e/sampleapptestbase.go#L114 due to tests running too long: Test killed with quit: ran too long (10m0s). I have no idea why docker build or docker push would fail with a doc change. Prob. there were some auth issues when pushing the images?

samodell · 2019-06-20T22:10:21Z

Yeah, it's definitely an issue with the tests. Hey @chaodaiG , could you take a look at this failure?

chaodaiG · 2019-06-20T23:13:32Z

Master branch looks fine to me, and these change shouldn't break test
/test pull-knative-docs-integration-tests

chizhg · 2019-06-20T23:23:04Z

/hold
@jingweno knative/eventing#1395 also needs your attention.
Can you update https://github.com/knative/docs/blob/master/docs/eventing/broker-trigger.md in this PR as well after knative/eventing-contrib#469 is in?

chaodaiG · 2019-06-20T23:45:20Z

/retest

…

On Thu, Jun 20, 2019, 4:42 PM Knative Prow Robot ***@***.***> wrote: @jingweno <https://github.com/jingweno>: The following test *failed*, say /retest to rerun them all: Test name Commit Details Rerun command pull-knative-docs-integration-tests e22ed7b <e22ed7b> link <https://gubernator.knative.dev/build/knative-prow/pr-logs/pull/knative_docs/1491/pull-knative-docs-integration-tests/1141846708116787201/> /test pull-knative-docs-integration-tests Full PR test history <https://gubernator.knative.dev/pr/knative_docs/1491>. Your PR dashboard <https://gubernator.knative.dev/pr/jingweno>. Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/guide/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. I understand the commands that are listed here <https://go.k8s.io/bot-commands>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1491?email_source=notifications&email_token=AKXNDYMPMQEIMSABQS4QFNLP3QIXHA5CNFSM4HZNGJCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYHAYXI#issuecomment-504237149>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKXNDYPNXWTR2H4SSHVYZJLP3QIXHANCNFSM4HZNGJCA> .

owenthereal · 2019-06-21T17:19:37Z

@Fredy-Z:

Can you update https://github.com/knative/docs/blob/master/docs/eventing/broker-trigger.md in this PR as well after knative/eventing-contrib#469 is in?

I think the broker/trigger doc fix is better to be in another PR because this one is about updating the doc for the new template field of container source. And I saw that you already have a PR to fix the broker/trigger doc: #1496

chizhg · 2019-06-21T19:05:35Z

@Fredy-Z:

Can you update https://github.com/knative/docs/blob/master/docs/eventing/broker-trigger.md in this PR as well after knative/eventing-contrib#469 is in?

I think the broker/trigger doc fix is better to be in another PR because this one is about updating the doc for the new template field of container source. And I saw that you already have a PR to fix the broker/trigger doc: #1496

My PR is to update docs of release-0.6 branch.
The master branch also needs to be updated, and since this broker/trigger doc uses ContainerSource as a demo (see Knative Source part of https://github.com/knative/docs/blob/master/docs/eventing/broker-trigger.md), it also needs to be updated by using the new template field of ContainerSource.

chizhg · 2019-06-21T22:48:13Z

/hold cancel

chizhg · 2019-06-21T23:15:31Z

/hold

chizhg · 2019-06-21T23:18:00Z

docs/eventing/samples/container-source/README.md

+  template:
+    spec:
+      containers:
+        - image: <heartbeats_image_uri>


Seems container name is missed here, this yaml template will result in an error.

@Fredy-Z Good catch. Fixed.

chizhg · 2019-06-21T23:18:07Z

docs/eventing/samples/container-source/heartbeats-source.yaml

+      containers:
+        # This corresponds to a heartbeats image uri you build and publish,
+        # e.g. gcr.io/[gcloud-project]/github.com/knative/eventing-contrib/cmd/heartbeats
+        - image: index.docker.io/jingweno/heartbeats-6410d29c99b6ec4e0dd2fb3e6e31829e


knative/eventing#1321 introduces `PodTemplateSpec` to `ContainerSourceSpec`. This feature will be out in 0.7.x. Update documentation to reflect the `template` field.

chizhg · 2019-06-25T17:38:59Z

/hold cancel
/lgtm

owenthereal · 2019-06-25T17:39:58Z

@Fredy-Z:

My PR is to update docs of release-0.6 branch.
The master branch also needs to be updated, and since this broker/trigger doc uses ContainerSource as a demo (see Knative Source part of https://github.com/knative/docs/blob/master/docs/eventing/broker-trigger.md), it also needs to be updated by using the new template field of ContainerSource.

Gotcha. I saw that you also fixed master: #1504. I should have addressed all feedback in this PR.

samodell · 2019-06-25T17:42:36Z

/approve

knative-prow-robot · 2019-06-25T17:42:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingweno, samodell

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [samodell]

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

…1491) knative/eventing#1321 introduces `PodTemplateSpec` to `ContainerSourceSpec`. This feature will be out in 0.7.x. Update documentation to reflect the `template` field.

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 19, 2019

knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 19, 2019

knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 19, 2019

knative-prow-robot requested review from RichieEscarez and samodell June 19, 2019 20:45

knative-prow-robot added the kind/eventing label Jun 19, 2019

knative-prow-robot assigned n3wscott Jun 19, 2019

knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. lgtm Indicates that a PR is ready to be merged. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 19, 2019

knative-prow-robot added the approved label Jun 20, 2019

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 20, 2019

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 21, 2019

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 21, 2019

chizhg suggested changes Jun 21, 2019

View reviewed changes

owenthereal force-pushed the container_source_template branch from e22ed7b to 9831069 Compare June 25, 2019 17:36

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2019

Update container source documentation with template field

30f4119

knative/eventing#1321 introduces `PodTemplateSpec` to `ContainerSourceSpec`. This feature will be out in 0.7.x. Update documentation to reflect the `template` field.

owenthereal force-pushed the container_source_template branch from 9831069 to 30f4119 Compare June 25, 2019 17:37

knative-prow-robot assigned chizhg Jun 25, 2019

knative-prow-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 25, 2019

knative-prow-robot merged commit ccfa98c into knative:master Jun 25, 2019

Update container source documentation with template field #1491

Update container source documentation with template field #1491

Uh oh!

Conversation

owenthereal commented Jun 19, 2019

Proposed Changes

Uh oh!

knative-prow-robot commented Jun 19, 2019

Uh oh!

owenthereal commented Jun 19, 2019

Uh oh!

n3wscott commented Jun 19, 2019

Uh oh!

owenthereal commented Jun 19, 2019

Uh oh!

vaikas commented Jun 20, 2019

Uh oh!

vaikas commented Jun 20, 2019

Uh oh!

samodell commented Jun 20, 2019

Uh oh!

samodell commented Jun 20, 2019

Uh oh!

owenthereal commented Jun 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samodell commented Jun 20, 2019

Uh oh!

chaodaiG commented Jun 20, 2019

Uh oh!

chizhg commented Jun 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chaodaiG commented Jun 20, 2019 via email

Uh oh!

owenthereal commented Jun 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chizhg commented Jun 21, 2019

Uh oh!

chizhg commented Jun 21, 2019

Uh oh!

chizhg commented Jun 21, 2019

Uh oh!

chizhg Jun 21, 2019

Choose a reason for hiding this comment

Uh oh!

owenthereal Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

chizhg Jun 21, 2019

Choose a reason for hiding this comment

Uh oh!

owenthereal Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

chizhg commented Jun 25, 2019

Uh oh!

owenthereal commented Jun 25, 2019

Uh oh!

samodell commented Jun 25, 2019

Uh oh!

knative-prow-robot commented Jun 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Update container source documentation with `template` field #1491

Update container source documentation with `template` field #1491

owenthereal commented Jun 20, 2019 •

edited

Loading

chizhg commented Jun 20, 2019 •

edited

Loading

owenthereal commented Jun 21, 2019 •

edited

Loading