Test-infra: Plank should not always require github oauth token

[krzyzacy]

If plank is not interfacing with github, i.e. it can be gerrit, plank should still be able to start

/area prow
cc @amwat @cjwagner
/assign

[cjwagner]

I think separating the report package into its own deployment is the way to go here. Plank should never need a github token. It should just be responsible for prowjob lifecycle. The reporting mechanism should be a separate component so that we can provide either a github reporter or a gerrit reporter (or maybe allow both).

[stevekuznetsov]

I think separating the report package into its own deployment is the way to go here

[phone]

[krzyzacy]

yeah, I guess I'll make the token optional for now (or else we won't have a working plank without github), and I'll get to a better design

[kargakis]

I think separating the report package into its own deployment is the way to go here. Plank should never need a github token. It should just be responsible for prowjob lifecycle. The reporting mechanism should be a separate component so that we can provide either a github reporter or a gerrit reporter (or maybe allow both).

Note that this used to be the case with crier but then its functionality was moved into plank presumably to reduce the moving components. cc @spxtr

[krzyzacy]

Probably because we want to run cross or kubeadm after bazel build?

[BenTheElder]

Note that this used to be the case with crier but then its functionality was moved into plank presumably to reduce the moving components. cc @spxtr

everything old is new again :-)

[spxtr]

The reason I split out the reporting logic into crier was because I needed to prevent individual line jobs from racing when writing the test results table.

[cjwagner]

@spxtr Was the rationale for combining crier into plank just to reduce the moving components though or was there some other, more important reason that reporting should be done from plank instead of a separate component?

[stevekuznetsov]

I noted on the DD in GDrive that we could have the entire reporting stack in one controller -- all it needs to do is watch events on ProwJobs and update GitHub status every time the ProwJob transitions states

[spxtr]

@spxtr Was the rationale for combining crier into plank just to reduce the moving components though or was there some other, more important reason that reporting should be done from plank instead of a separate component?

See #3017. There's no hard reason, but it's an extra binary, an extra deployment, extra network calls, and so on. It was roughly -500 lines to remove crier.

[krzyzacy]

I'll close this, follow up work will be tracked at https://github.com/kubernetes/test-infra/issues/7741