Flatten ProvisionerReference

[scothis]

ProvisionerReference contains a single required field. We can simplify
the object model by moving the ObjectRef for a provisioner up one level.

Release Note

NONE

/assign @n3wscott

[evankanderson]

Question: would we prefer an ObjectReference or a ProvisionerReference (like so):

type ProvisionerReference struct {
  Name string `json: "name"`
  ClusterName string `json: "clusterName"`
}

[scothis]

@evankanderson good question. The main difference will be in how we handle upgrades for ClusterChannelProvisioner from v1alpha1 to v1alpha2. I don't have a strong opinion here.

[evankanderson]

I think the idea is that a name, Kind and api group (e.g. eventing.knative.dev, without the version) is sufficient to reference an object, and that v1alpha1 and v1alpha2 would reference the same object with a different representation.

In particular, CRDs today do not permit multiple apiVersions for the same Kind and api group that have different schema.

[vaikas]

My preference would be to try to use ObjectReference (as that's the normal way to do refs in k8s) unless there's a very good reason not to. Seems like if there's a new version of the provisioner, that might be important to distinguish. Perhaps we could still use the ObjectReference but only use certain fields of it if we want to be able to handle multiple versions of the referenced object?

[vaikas]

Oh, and the version is necessary to reference an object. Unless we have some logic to see what's available from the server and then try each of them for example, so it's not only for different representation of the object.

[n3wscott]

+1 on ObjectReference because it plugs into lots of other helpful tooling we have that would have to be coded around to get the same integration if we just name/clustername.

/lgtm
/hold
need to fix the conflict.

[scothis]

@n3wscott fixed the conflict, PTAL

[vaikas]

what is kind in this context? Isn't it just ref.Kind?

[scothis]

@vaikas-google We haven't updated ClusterProvisioner to ClusterChannelProvisioner yet, the kind should go away with that PR.

As currently implement the only value passed in is "Channel" which the shouldReconcile method then checks to make sure it is indeed channel.

[scothis]

PRed in #562

[vaikas]

we could just change L111 to pass in ref.Kind and ignore the kind but it's busy work with that PR :)

[vaikas]

/lgtm
/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: scothis, vaikas-google

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:

• OWNERS [scothis,vaikas-google]

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

[vaikas]

@n3wscott care to remove the hold, or do you have remaining concerns?

[scothis]

Updated to resolve new conflicts

[scothis]

Updated to resolve new conflicts (again)

[vaikas]

/lgtm

[scothis]

/hold cancel

@n3wscott if you have further concerns I can follow up in a new PR