No init containers #470

aaron-prindle · 2018-11-13T21:57:47Z

Fixes #9

Proposed Changes

Have Build Steps run as multiple "normal" containers in a Pod w/ entrypoint enforced linear execution vs. initContainers

This PR modifies the PodSpec generated by build to run Steps as normal containers and not initcontainers. As Build requires Steps to run in a linear fashion and normal containers would typically all run at the same time, entrypoint rewriting is used in order to enforce the run order of the containers. This should fix some of the issues described in #9 as normal container logs are not cleared as aggressively as initContainer logs. This change change will also allow Build to use sidecar containers in the future.

Known Issues (These need to be fixed):

12/19/2018 issue update:

As entrypoint is rewritten for these containers, containers relying on running Entrypoint and not Command will not work as expected. This can be resolved by fetching and storing the container entrypoint information directly as done here: Load entrypoint from remote tektoncd/pipeline#183
~~Support .dockercfg secrets (not just .dockerconfigjson) for entrypoint rewriting~~
~~Handle multiple registries (gcr.io, us.gcr.io, other registries, etc better for metadata server solution) (should prob just use the prefix from the image here)~~
~~Add a test, test-succeed that verifies that ko built, private images are pullable/runnable~~
~~Add a test, test-step-ordering that verifies that steps are run in the correct order~~
~~Update pod ResourceRequirements rewriting to use the max requirement from the set of containers~~
The entrypoint image is currently hosted in my own gcr.io repo. It needs to be uploaded to the main repository
The entrypoint image needs to be part of the release job (I am having trouble understanding how to do this...)
Currently secret waiting and service account waiting is blocking in the controller (maybe ok?)
Currently integration tests have a path that require network access (MakePod now calls out to registries, even on unit tests)
Add unit tests for all added methods (hopefully pull-knative-build-go-coverage will pass after!)
Get update vendored k8s to 1.11.3 google/go-containerregistry#328 merged so that this PR can rely on go-containerregistry and not my fork
~~Ensure that the correct registry is used when attempting to auth with secrets, see: https://github.com/knative/build/blob/master/test/docker-basic/0-secret.yaml~~

Edit: regarding the last bullet, it is my understanding that go-containerregistry uses the correct authentication from the image directly. I am marking this complete, please comment if I am misunderstanding.

Additional points:

Investigate better wait method than writing/reading files to shared volume between pods

knative-prow-robot · 2018-11-13T21:57:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aaron-prindle
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: mattmoor

If they are not already assigned, you can assign the PR to them by writing /assign @mattmoor in a comment when ready.

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

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

imjasonh · 2018-11-13T22:05:02Z

Super exciting to see this work happening, thanks for kicking it off. Would you mind talking through this at tomorrow's Build WG meeting?

Also, this needs test updates, and probably some kind of integration test that build step containers happen in order, since we used to just trust in init containers to do that.

aaron-prindle · 2018-11-14T00:00:32Z

Sure, I can go through the motivations and design of this change at the meeting tomorrow! I've updated the unit tests but am having some trouble with the integration test: TestBuildLowTimeout

bobcatfish

This is looking great @aaron-prindle ! I think the really important bit to review is probably in the image itself, to understand the logic that we're using to get the containers to wait before they execute. Before we merge this we'll need to take a look at it - maybe it can be added to this PR? (I think this PR was mostly to get feedback so sorry if I'm rushing you! :D I'm mostly very excited about maybe getting this done before tektoncd/pipeline#252)

bobcatfish · 2018-11-26T16:23:19Z

pkg/reconciler/build/resources/pod_test.go

+ shouldWaitForPrevStep, i, i+1)},
+ )
+ c.want.Containers[i].Args = []string{}
+ }


im guessing that writing this block of code made it easier to modify the expected test inputs and outputs without having to actually change the test cases - but i think for future readability and maintenance of these tests it'll be clearer to change the test cases themselves

bobcatfish · 2018-11-26T16:25:23Z

pkg/reconciler/build/resources/pod.go

@@ -277,12 +281,26 @@ func MakePod(build *v1alpha1.Build, kubeclient kubernetes.Interface) (*corev1.Po
 return nil, err
 }
 // Prepend the custom container to the steps, to be augmented later with env, volume mounts, etc.
+
+ // initContainers = append(initContainers, *cust)
+ // TODO(aaron-prindle) not if I correctly handled this piece....


looking at the Git and GCS cases, I don't think this block needs to change? I think this is just for a custom method of retrieving source data to operate on

bobcatfish · 2018-11-26T16:25:50Z

pkg/reconciler/build/resources/pod.go

+ JSONConfigEnvVar = "ENTRYPOINT_OPTIONS"
+ InitContainerName = "place-tools"
+ // TODO(aaron-prindle) change this to wherever is sensible
+ DefaultEntrypointImage = "gcr.io/aprindle-vm-test/entrypoint:latest"


i'm guessing this can get added to the existing release logic

imjasonh · 2018-11-26T17:18:36Z

I'm mostly very excited about maybe getting this done before tektoncd/pipeline#252)

I think I'd actually prefer to get the forklift done first, since after #464 it should be much easier. I agree though, whichever goes first, we should make sure the other work knows it's second, or we're going to waste effort.

I think this work is great, but I'd like to start with a design (even just an informal tech talk, possibly at the WG meeting Wednesday) to walk through how it works. And any PR that adds this behavior also needs e2e tests for step ordering, since we can't rely on k8s to order them anymore. For those reasons I think the forklift is going to be quicker.

bobcatfish · 2018-11-26T17:37:12Z

Sounds like a good plan to me @imjasonh ! Maybe we can let this PR be more of a POC in that case then.

vdemeester

Naive question : any reason to use an environment variable instead of /my/wrapper --flag1 --flag2 -- /original/entrypoint --and command ?

vdemeester · 2018-11-28T15:09:37Z

pkg/entrypoint/options.go

+
+// Options exposes the configuration necessary
+// for defining the process being watched and
+// where in GCS an upload will land.


GCS stands for Google Cloud Storage right ? We may want to be less specific than that here 👼

sorry this is fixed now! It is originally adapted from: https://github.com/kubernetes/test-infra/blob/master/prow/entrypoint/options.go#L35-L37

aaron-prindle · 2018-11-28T22:04:02Z

@vdemeester There shouldn't be any particular reason for using ENV vars vs flags. ENV vars are used here as it is based off of the entrypoint image in: https://github.com/kubernetes/test-infra/blob/master/prow/cmd/entrypoint/main.go

`test-infra` is updated to the latest version to include the required infrastructure. Bonuses: * documented the contents of the `hack` directory; * documented how to use the `release.sh` script;

aaron-prindle · 2018-12-14T01:26:57Z

Paired on this w/ @imjasonh today, got integration tests passing! Here were some other work in progress pieces that we discussed:

~~Add a test, test-succeed that verifies that ko built, private images are pullable/runnable~~
~~Add a test, test-step-ordering that verifies that steps are run in the correct order~~
Ensure that the correct registry is used when attempting to auth with secrets, see: https://github.com/knative/build/blob/master/test/docker-basic/0-secret.yaml

Edit: regarding the last point, it is my understanding that go-containerregistry uses the correct authentication from the image directly. I am marking this complete, please comment if I am misunderstanding.

Edit 2: I've put all these points up top

knative-metrics-robot · 2018-12-20T01:39:41Z

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

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/options.go	Do not exist	17.6%
pkg/entrypoint/run.go	Do not exist	0.0%
pkg/entrypoint/wrapper/options.go	Do not exist	71.4%
pkg/reconciler/build/resources/pod.go	88.4%	62.9%	-25.5

bobcatfish

So cool to see this moving along! 🎉

My main feedback is that I think we need to:

break up some of the longer functions into smaller pieces with unit test coverage (this will help address the -25.5 coverage on pod.go )
make sure we need all of the functionality in entrypoint and make sure entrypoint itself has good unit test coverage

A few more requests:

Can we add a README for the entrypoint binary?
In preparation for porting this to build-pipeline, could you include a markdown file that describes how the steps are implemented now? Something we could add to our deep dive docs that @shashwathi started
Could you squash the commits together into one that explains why we are switching away from init containers, and maybe gives a shout-out to https://github.com/kubernetes/test-infra/tree/master/prow/entrypoint which some of our entrypoint binary was based on?
Was this commit included on purpose? 7a0c771

bobcatfish · 2018-12-20T23:17:10Z

cmd/entrypoint/main.go

+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+


can you include in the comments here some docs on what this binary is and what it does? e.g. like https://github.com/knative/build-pipeline/blob/master/cmd/bash/main.go#L17

bobcatfish · 2018-12-20T23:22:21Z

pkg/entrypoint/options.go

+ ArtifactDir string `json:"artifact_dir,omitempty"`
+
+ *wrapper.Options
+}


this seems super similar to https://github.com/kubernetes/test-infra/blob/master/prow/entrypoint/options.go - we should at least give a shout-out to it probably.

also are we 100% sure we need all the same options? (e.g. we don't do automatic artifact uploading afaik, do we need ArtifactDir?)

bobcatfish · 2018-12-20T23:23:09Z

pkg/entrypoint/options.go

+ flags.StringVar(&o.PreRunFile, "prerun-file",
+ DefaultPreRunFile, "The prerun file to wait for.")
+ flags.StringVar(&o.PostRunFile, "postrun-file",
+ DefaultPostRunFile, "If postrun file to write.")


can you explain more about what "file to wait for" means here? (e.g. the process will wait for this file to exist before xyz)

bobcatfish · 2018-12-20T23:23:40Z

pkg/entrypoint/options/load.go

+ loader.Complete(fs.Args())
+
+ return nil
+}


can we add test coverage for these functions + files we're adding?

bobcatfish · 2018-12-20T23:24:27Z

pkg/entrypoint/run.go

+ }
+ }
+ return returnCode, commandErr
+}


id really like to see this function broken up a bit with unit test coverage

bobcatfish · 2018-12-20T23:25:39Z

pkg/entrypoint/run_test.go

+// }
+// if string(data) != expected {
+// t.Errorf("%s: expected contents: %q, got %q", name, expected, data)
+// }


was it intentional to comment this out? if so, let's delete it

this was unintentional, the tests are added back now

I don't see this in the latest commit.

it should be there now

bobcatfish · 2018-12-20T23:26:24Z

test/succeed/main.go

+)
+
+func main() {
+ fmt.Println("Welcome to succeed test!")


what is "succeed test"? can you add some docs to this main.go explaining what this binary is for?

bobcatfish · 2018-12-20T23:27:27Z

pkg/reconciler/build/resources/pod.go

@@ -491,3 +525,397 @@ func getFailureMessage(pod *corev1.Pod) string {
 // Lastly fall back on a generic error message.
 return "build failed for unspecified reasons."
 }
+
+// from build-pipeline


can this be broken up into multiple files? also i think this comment needs more details

bobcatfish · 2018-12-20T23:27:49Z

pkg/reconciler/build/resources/pod.go

+ TokenURI string `json:"token_uri"`
+ AuthProviderX509CertURL string `json:"auth_provider_x509_cert_url"`
+ ClientX509CertURL string `json:"client_x509_cert_url"`
+}


maybe add a docker package with this logic in it, with unit test coverage?

this code and all specific code related to docker has been removed thanks to k8schain!

bobcatfish · 2018-12-20T23:28:41Z

pkg/reconciler/build/resources/pod.go

+ return nil, fmt.Errorf("couldn't get config for image %s: %v", image, err)
+ }
+ cache.set(image, cfg.Config.Entrypoint)
+ return cfg.Config.Entrypoint, nil


i think this function needs to get broken up into a few functions (with unit test coverage)

jonjohnsonjr

Left some comments re: consuming ggcr. I haven't looked at the rest of the PR but figured some pointers would help.

cc @mattmoor we could probably make this more discoverable or easier to use if we added better examples to the READMEs

jonjohnsonjr · 2018-12-21T05:37:35Z

pkg/reconciler/build/resources/pod.go

+ c.mtx.Unlock()
+}
+
+type authToken struct {


Mentioned this before, but I think like 95% of the added code in this file could go away if you do something like:

kc, _ := k8schain.New(kubeclient, k8schain.Options{ ImagePullSecrets: secret.Name, }) img, _ := remote.Image(ref, remote.WithAuthFromKeychain(kc))

Obviously handle errors too, but what you're doing here is basically the point of k8schain.

https://github.com/google/go-containerregistry/blob/e0f538712eb0767ae6959531febe6041fa466602/pkg/authn/k8schain/k8schain.go#L46

Awesome, this is working for me now! Note: I have to use go-contaierregistry w/ this change: google/go-containerregistry#328 due to some k8s version import mismatch. More info in that PR

jonjohnsonjr · 2018-12-21T05:43:57Z

pkg/reconciler/build/resources/pod.go

+ ClientX509CertURL string `json:"client_x509_cert_url"`
+}
+
+func getGCRAuthorizationKey() ([]authToken, error) {


I think k8schain should be sufficient for you, but I already implemented this in a way that's consumable by ggcr.

Here's an implementation as a keychain:
https://github.com/google/go-containerregistry/blob/e0f538712eb0767ae6959531febe6041fa466602/pkg/v1/google/keychain.go

Here's an authn.Authenticator implementation that uses the same DefaultTokenSource, you can use it with the remote.WithAuth option:
https://github.com/google/go-containerregistry/blob/e0f538712eb0767ae6959531febe6041fa466602/pkg/v1/google/auth.go#L45

If you want to get fancy, you could combine your k8schain with the google.Keychain using authn.NewMultiKeychain:
https://github.com/google/go-containerregistry/blob/e0f538712eb0767ae6959531febe6041fa466602/pkg/authn/multikeychain.go

Something like:

// this will first try to authenticate using the k8schain, // then fall back to the google keychain, // then fall back to anonymous mkc := authn.NewMultiKeychain(kc, google.Keychain) img, _ := remote.Image(ref, remote.WithAuthFromKeychain(kc))

This is really awesome, this appears to be doing exactly what I wanted for authentication. Thanks for your help @jonjohnsonjr!!

imjasonh

I'd really love to see this change include a README describing how ordering is maintained without init containers. It's great having tests to prove it works, but anybody looking into it to fix bugs in the future is going to appreciate documentation to back it up.

imjasonh · 2018-11-13T22:02:45Z

pkg/reconciler/build/resources/pod.go

+ JSONConfigEnvVar = "ENTRYPOINT_OPTIONS"
+ InitContainerName = "place-tools"
+ // TODO(aaron-prindle) change this to wherever is sensible
+ DefaultEntrypointImage = "gcr.io/aprindle-vm-test/entrypoint:latest"


I think we should make this a flag, like *credsImage, etc.

imjasonh · 2018-11-13T22:03:25Z

pkg/reconciler/build/resources/pod.go

+ MountPoint = "/tools"
+ BinaryLocation = MountPoint + "/entrypoint"
+ JSONConfigEnvVar = "ENTRYPOINT_OPTIONS"
+ InitContainerName = "place-tools"


Do these need to be exported?

These are used in pod_test.go which is why they are exported

imjasonh · 2019-01-04T20:06:35Z

pkg/entrypoint/options_test.go

+ name string
+ input Options
+ expectedErr bool
+ }{


style nit: collapse {{ here (and }} at the end) to save vertical and horizontal space. Put },\n{ on one line like }, { for the same reason.

imjasonh · 2019-01-04T20:06:48Z

pkg/entrypoint/options_test.go

+ MarkerFile: "marker.txt",
+ },
+ },
+ expectedErr: false,


nit: This can be omitted since it's false by default.

imjasonh · 2019-01-04T20:07:07Z

pkg/entrypoint/options_test.go

+ }
+
+ for _, testCase := range testCases {
+ err := testCase.input.Validate()


nit: This can be an if err := ... block

imjasonh · 2019-01-04T20:11:35Z

pkg/entrypoint/run.go

+ done <- command.Wait()
+ }()
+ select {
+ case err := <-done:


ditto about case commandErr = <-done:

imjasonh · 2019-01-04T20:12:18Z

pkg/entrypoint/run_test.go

+// }
+// if string(data) != expected {
+// t.Errorf("%s: expected contents: %q, got %q", name, expected, data)
+// }


I don't see this in the latest commit.

imjasonh · 2019-01-04T20:12:43Z

pkg/entrypoint/wrapper/options_test.go

+ name string
+ input Options
+ expectedErr bool
+ }{


ditto about collapsing whitespace

the change should be in the commit here now

imjasonh · 2019-01-04T20:13:15Z

pkg/reconciler/build/validation_test.go

@@ -43,7 +43,7 @@ func TestValidateBuild(t *testing.T) {
 Template: &v1alpha1.TemplateInstantiationSpec{
 Arguments: []v1alpha1.ArgumentSpec{{
 Name: "foo",
- Value: "hello",
+ Value: "hello-world",


These changes don't seem necessary.

This is a WIP but these changes are unfortunately necessary for (until I change some pieces around) as these tests hit MakePod and MakePod now calls out to the internet to fetch image metadata. This is a temporary hack as "hello-world" is a docker image but this is obvi not great as now unit tests have an external image dependence and require internet access. I am working on fixing this.

imjasonh · 2019-01-04T20:13:30Z

test/serviceaccount/secret.yaml

@@ -15,6 +15,7 @@ apiVersion: v1
 kind: Secret
 metadata:
 name: test-readonly-credentials
+# type: kubernetes.io/dockerconfigjson


Why comment this out?

For testing originally, i had a setup that only worked with the dockerconfigjson format so I modified this secret to be of that type for my tests. Now it works for either type (dockercfg or dockerconfigjson), I will remove these comments.

knative-prow-robot · 2019-01-06T06:16:13Z

@aaron-prindle: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-knative-build-go-coverage	`2fa7aab`	link	`/test pull-knative-build-go-coverage`
pull-knative-build-build-tests	`2fa7aab`	link	`/test pull-knative-build-build-tests`
pull-knative-build-unit-tests	`2fa7aab`	link	`/test pull-knative-build-unit-tests`
pull-knative-build-integration-tests	`2fa7aab`	link	`/test pull-knative-build-integration-tests`

Full PR test history. Your PR dashboard.

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.

What is the problem being solved? This PR adds an entrypoint image which is capable of running another command with some pre-run/post-run hooks. The goal of this is to allow for the the entrypoint of a container in a Build Step to have it's entrypoint rewritten by mounting this entrypoint image into the container provided in a Step, rewriting the entrypoint, then running the user container w/ the additional pre-run/post-run hooks. The addition of this image enables the removal of init containers as per knative#521. This was originally part of knative#470 but is now split out. What future work remains to be done? This PR adds the entrypoint image and release step. The future work involved is modifying Build to stop using init container and use this entrypoint image as a way of making that possible.

bobcatfish · 2019-01-23T00:23:26Z

Note this is now getting broken up into separate PRs, starting with #522

evankanderson · 2019-02-20T00:45:20Z

It looks like #522 was closed without merging.

Is the removal of init containers dead?

imjasonh · 2019-02-20T01:14:02Z

Nope, just set back somewhat due to staffing, and moving the meat of the work to the Pipelines repo since that's where new development is happening. tektoncd/pipeline#448 is part of this work.

bobcatfish · 2019-02-21T01:51:21Z

@evankanderson tektoncd/pipeline#224 is probably the issue to watch if you're interested :D

knative-prow-robot requested review from mattmoor and shashwathi November 13, 2018 21:57

knative-prow-robot added the size/L label Nov 13, 2018

aaron-prindle force-pushed the no-init-containers branch from 80d210c to d7d8331 Compare November 13, 2018 23:25

aaron-prindle force-pushed the no-init-containers branch from d7d8331 to 6cc5a35 Compare November 14, 2018 00:06

bobcatfish reviewed Nov 26, 2018

View reviewed changes

aaron-prindle force-pushed the no-init-containers branch 5 times, most recently from 2b81f15 to 6d3f04d Compare November 27, 2018 23:36

knative-prow-robot added size/XXL and removed size/L labels Nov 27, 2018

aaron-prindle force-pushed the no-init-containers branch 7 times, most recently from 7d792b4 to 024a28f Compare November 28, 2018 01:49

vdemeester reviewed Nov 28, 2018

View reviewed changes

aaron-prindle force-pushed the no-init-containers branch 3 times, most recently from 7d7752a to 8cd9714 Compare November 28, 2018 19:00

aaron-prindle force-pushed the no-init-containers branch from e40f689 to 271a145 Compare December 11, 2018 02:32

Automate the creation of versioned releases (knative#448)

7a0c771

`test-infra` is updated to the latest version to include the required infrastructure. Bonuses: * documented the contents of the `hack` directory; * documented how to use the `release.sh` script;

aaron-prindle force-pushed the no-init-containers branch 4 times, most recently from fd7d566 to 4f56053 Compare December 13, 2018 23:47

This was referenced Dec 16, 2018

Explore moving away from init containers tektoncd/pipeline#224

Closed

Add script to tail logs for pipelinerun and taskruns tektoncd/pipeline#336

Merged

bobcatfish reviewed Dec 20, 2018

View reviewed changes

jonjohnsonjr reviewed Dec 21, 2018

View reviewed changes

aaron-prindle force-pushed the no-init-containers branch from b9e7928 to bb879ac Compare January 2, 2019 18:02

imjasonh suggested changes Jan 4, 2019

View reviewed changes

aaron-prindle force-pushed the no-init-containers branch from bb879ac to b13327b Compare January 6, 2019 04:56

aaron-prindle added 2 commits January 5, 2019 22:13

no init container vendor changes

c14bbca

no init container

2fa7aab

aaron-prindle force-pushed the no-init-containers branch from df2f486 to 2fa7aab Compare January 6, 2019 06:14

aaron-prindle mentioned this pull request Jan 7, 2019

Entrypoint img #522

Closed

bobcatfish mentioned this pull request Jan 10, 2019

TestPipelineRun/fan-in_and_fan-out is broken tektoncd/pipeline#375

Closed

imjasonh mentioned this pull request Jan 29, 2019

Add entrypoint overrider image tektoncd/pipeline#448

Merged

imjasonh closed this Mar 20, 2019

No init containers #470

No init containers #470

Conversation

aaron-prindle commented Nov 13, 2018 • edited Loading

Proposed Changes

knative-prow-robot commented Nov 13, 2018

imjasonh commented Nov 13, 2018

aaron-prindle commented Nov 14, 2018

bobcatfish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imjasonh commented Nov 26, 2018

bobcatfish commented Nov 26, 2018

vdemeester left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaron-prindle commented Nov 28, 2018 • edited Loading

aaron-prindle commented Dec 14, 2018 • edited Loading

knative-metrics-robot commented Dec 20, 2018

bobcatfish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaron-prindle Jan 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonjohnsonjr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imjasonh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knative-prow-robot commented Jan 6, 2019

bobcatfish commented Jan 23, 2019

evankanderson commented Feb 20, 2019

imjasonh commented Feb 20, 2019

bobcatfish commented Feb 21, 2019

aaron-prindle commented Nov 13, 2018 •

edited

Loading

aaron-prindle commented Nov 28, 2018 •

edited

Loading

aaron-prindle commented Dec 14, 2018 •

edited

Loading

aaron-prindle Jan 3, 2019 •

edited

Loading