Thoughts on a InstancesV2 interface?

[Andrew Kim]

Hi SIG Cloud Provider,

I noticed two recent PRs (90894 & 91319) that adds a new method to the Instances interface as part of the cloud provider API: `InstancesMetadataByProviderID`. The reason for the change is to reduce the # of cloud API requests required per node initialization, so `InstancesMetadataByProviderID` encapsulates existing logic from `NodeAddressesByProviderID`, `InstanceTypeByProviderID`, `InstanceID`, etc but in a single API call.

Overall I am in favor of this change, the cloud controllers hitting API quota limits on cloud providers is a common problem that comes up and this would definitely help. However, it does make me wonder if `InstancesMetadataByProviderID` should should be part of a new InstancesV2 interface as it effectively deprecates all of the existing methods in the current Instances interface. With a v2 interface, we can branch out some of the logic in the cloud controllers based on whether a provider supports v2 or not. We can fallback to the existing code paths if they don't. This would make the proposed changes in the PR less disruptive for providers to adopt. Or if folks are okay to bite the bullet and adopt the proposed changes (which includes deprecation of existing methods) that would be fine to.

Thoughts?

- Andrew Sy Kim

[yassine TIJANI]

Hi,

I think this brings an interesting discussion around the guarantees we are offering around cloud interface. IMHO there might be two cases here:

- If we can deprecate the existing functions (and remove them later), then it might make sense to do it on the same instance interface.

- If our deprecation policy is against removal then a instance v2 interface makes sense

Yassine Tijani

—

Github: @yastij

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-cloud-provider" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-cloud...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-cloud-provider/CABc050FfSEeDJ2SbQuAGBcZLoH3scD5_cdnaSg9q0vFwo2aEdw%40mail.gmail.com.

[woj...@google.com]

 I guess I agree with Yassine - it's a matter of deprecation policy for this interface, which (I might be missing it), I don't think we really stated clear.

 One clarification that I would like to add - this change can be thought as "packaging" question. Technically, we are not introducing any new logic, we're just packaging the existing one differently.

As a result, removing the existing methods, won't give us much (the maintainence overhead for them shouldn't be visible comparing to thew new ones) and we can absolutately wait couple releases before removing them.

So I think that re-using the existing interface is a better option (and we may simply say that we have 1 year deprecation and only remove them in an year from now).

W dniu wtorek, 26 maja 2020 17:10:54 UTC+2 użytkownik yassine TIJANI napisał:

Hi,

I think this brings an interesting discussion around the guarantees we are offering around cloud interface. IMHO there might be two cases here:

- If we can deprecate the existing functions (and remove them later), then it might make sense to do it on the same instance interface.

- If our deprecation policy is against removal then a instance v2 interface makes sense

Yassine Tijani

—

Github: @yastij

Le mar. 26 mai 2020 à 17:01, Andrew Kim <kim.a...@gmail.com> a écrit :

Hi SIG Cloud Provider,

I noticed two recent PRs (90894 & 91319) that adds a new method to the Instances interface as part of the cloud provider API: `InstancesMetadataByProviderID`. The reason for the change is to reduce the # of cloud API requests required per node initialization, so `InstancesMetadataByProviderID` encapsulates existing logic from `NodeAddressesByProviderID`, `InstanceTypeByProviderID`, `InstanceID`, etc but in a single API call.

Overall I am in favor of this change, the cloud controllers hitting API quota limits on cloud providers is a common problem that comes up and this would definitely help. However, it does make me wonder if `InstancesMetadataByProviderID` should should be part of a new InstancesV2 interface as it effectively deprecates all of the existing methods in the current Instances interface. With a v2 interface, we can branch out some of the logic in the cloud controllers based on whether a provider supports v2 or not. We can fallback to the existing code paths if they don't. This would make the proposed changes in the PR less disruptive for providers to adopt. Or if folks are okay to bite the bullet and adopt the proposed changes (which includes deprecation of existing methods) that would be fine to.

Thoughts?

- Andrew Sy Kim

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-cloud-provider" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-cloud-provider+unsub...@googlegroups.com.

[Andrew Kim]

Based on the changes here https://github.com/kubernetes/kubernetes/pull/91319, I wouldn't say we are "deprecating" those methods because we are removing the calls to those methods in the controllers. That is not any different from just removing the methods entirely. "Deprecating" would be allowing providers to opt into the existing behavior for some amount of time, which a v2 interface would allow them to do.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-cloud...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-cloud-provider/CABc050FfSEeDJ2SbQuAGBcZLoH3scD5_cdnaSg9q0vFwo2aEdw%40mail.gmail.com.

--

You received this message because you are subscribed to the Google Groups "kubernetes-sig-cloud-provider" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-cloud...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-cloud-provider/b63d980b-853c-417f-9446-3aae0917eba6%40googlegroups.com.

[Andrew Kim]

I am not opposed to the current changes if providers are willing to bite the bullet and replace their existing implementations. Maybe the changes here are less impactful than I think?

However, I do think the current Instances interface carries a lot of baggage from when kubelet was the only consumer of that interface. I think a clean slate wouldn't hurt and it wouldn't be difficult to implement either.

I'll add this to tomorrow's agenda for further discussion :)

- Andrew Sy Kim