Calling Builder.For() multiple times loses previous apiTypes

[jim-minter]

At https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/builder/controller.go#L74 , For latches the apiType of the For() call.

If a controller-runtime consumer calls Builder.For(foo).For(bar), then foo will be silently lost in favour of bar.

This contrasts with Owns' behaviour, where Builder.Owns(foo).Owns(bar) will append both foo and bar to an internal slice.

Judging by the code comment on For (For defines the type of Object being *reconciled*), it may not be intended for For to be called multiple times on a Builder. Nonetheless, the current behaviour can be confusing.

As a workaround, a consumer who wants to watch multiple apiTypes can use Watches(foo).Watches(bar) instead.

IMO it would be nice to make the behaviour here clearer, either:

• by raising an error if For() is called and bldr.apiType is already set, or
• by making apiTypes a slice and allowing For() to be called multiple times, as Owns() or Watches() is.

[DirectXMan12]

You're never supposed to call For multiple times -- there should be one "root" type that you're reconciling generally. Saving an error and returning it on .Complete or .Build is a good idea though.

/good-first-issue
/kind feature
/priority important-longterm

[k8s-ci-robot]

@DirectXMan12:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

You're never supposed to call For multiple times -- there should be one "root" type that you're reconciling generally. Saving an error and returning it on .Complete or .Build is a good idea though.

/good-first-issue
/kind feature
/priority important-longterm

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.

[prafull01]

It looks simple enough. I can take it as my first issue for the controller-runtime.
/assign