Ensure kaniko e2e test pushes image to registry

[tanner-bruce]

Adds github.com/google/go-containerregistry as a dependency for
accessing container registries

Adds KANIKO_SECRET_CONFIG_FILE for adding a service account to the
kaniko task when running e2e test locally.

Fixes #150

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

[tanner-bruce]

I signed it!

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

[googlebot]

CLAs look good, thanks!

[bobcatfish]

Super excited to see this @tanner-bruce ! I'd like to give this a thorough review, you should see it shortly!

In the meantime, I can see you added secrets for running tests locally, I assume so that you can push to your registry? I have two follow up requests/questions:

1. Can you update the commit message to describe a bit about why this was needed? (I ran into something similar but I worked around it by using a GKE specific setting :S anyway very curious about what behavior you were seeing, and future readers of the commit could benefit too)
2. In Support service account authorization #151 we want to add support for specifying a ServiceAccount to use for each Run, do you think the change in this PR will still be needed once we have that?

Thanks again for this contribution!!!

[bobcatfish]

Added my feedback! Thanks again for this @tanner-bruce :D

[bobcatfish]

/ok-to-test

whoa i didnt even know we needed that, i should probably add something to CONTRIBUTING.md 🤔

[tanner-bruce]

Everything should be addressed now @bobcatfish !

[bobcatfish]

A couple minor comments, one major comment re. I'm shocked that this works hahaha, but I think this is almost ready to go!

/meow space

[bobcatfish]

I have 2 more requests:

1. Can we move all the logic around getting the init logs into a separate function as well? My ideal would be that this part of the test looked something like this:

	// The Build created by the TaskRun will have the same name
	b, err := c.BuildClient.Get(kanikoTaskRunName, metav1.GetOptions{})
	if err != nil {
		t.Errorf("Expected there to be a Build with the same name as TaskRun %s but got error: %s", kanikoTaskRunName, err)
	}
        podLogs := getPodLogs(b...)
	if !strings.Contains(podLogs, kanikoBuildOutput) {
		t.Fatalf("Expected output %s from pod %s but got %s", kanikoBuildOutput, podName, string(bs))
	}
        initLogs := getInitLogs(b...)
	if checkForDigest(initLogs,....) {
		t.Fatalf("Expected latest digest to be __ but was __ ".....)
	}

2. It is crazy that this is working!!! XD I'm running it repeatedly locally and it's succeeding but this goes against everything we've been seeing in Add log verification to integration tests #119 and Create solution for making logs available when running Tasks #143 - @tejal29 and I looked at it and if we switch this back to the hello world task, the init container logs are always gone when we look at them (we get an error like Unable to retrieve container logs for docker://6489e33a7432f735953b542d910ecd6ab7033e13d075fa13b16ad3f3babcb052), so it seems like the fact that this Build is going more is making it stick around longer, but we don't know how long!! I really don't want to have flakes in our tests so I think we have 2 options:

   1. Commit the test as is, if we start seeing flakes, make the check optional
   2. Log a warning if we can't get the initLogs but don't fail the test <-- the downside here is that i'm pretty sure our test scripts will swallow any test output and we'd never see this, unless it happened locally

I'm actually leaning toward (i), since adding this is an improvement on what we had before (and once we finish #143 we can swap out the part that grabs the logs here), but maybe @tejal29 @aaron-prindle @imjasonh @dlorenc have a different opinion

[tejal29]

Now that i think, i like 1. Or else, we might ignore the warning and its easier to not fix it.
Its easy to make the check optional later.

[tanner-bruce]

1. For sure, I should have done this in the first place. I took a slightly different approach but I think you'll like it.

2. I agree, I was worried about flakiness when I started working on it. One thought I had that could be investigated is that GetLogs allows following the logs of a container. I think with that, as soon as the pod is created we could grab a handle to all of the containers log streams and build up the logs as they run. There could be a small race condition here but I think it would still be less than what could happen with the current behaviour.

With how I handled 1 we could easily test out my idea on 2

[bobcatfish]

I think with that, as soon as the pod is created we could grab a handle to all of the containers log streams and build up the logs as they run.

oooh that sounds like an interesting approach, i like it! I'm gonna lgtm this PR and put a hold on it, if you want to make this change as part of this PR feel free, otherwise feel free to remove the hold and we can go ahead with this, and address any flakes as they happen

[knative-prow-robot]

@bobcatfish:

In response to this:

A couple minor comments, one major comment re. I'm shocked that this works hahaha, but I think this is almost ready to go!

/meow space

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.

[bobcatfish]

I think with that, as soon as the pod is created we could grab a handle to all of the containers log streams and build up the logs as they run.

oooh that sounds like an interesting approach, i like it! I'm gonna lgtm this PR and put a hold on it, if you want to make this change as part of this PR feel free, otherwise feel free to remove the hold and we can go ahead with this, and address any flakes as they happen

[bobcatfish]

Looks great!!!! Thanks for iterating on this also :D

/lgtm

If you want to make any other changes go ahead, otherwise comment with /hold cancel on its own line to merge as is:
/hold

[tanner-bruce]

No problem!

/hold cancel

[bobcatfish]

whoops, forgot:
/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bobcatfish, tanner-bruce

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 [bobcatfish]

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