/cc @andrewsykim /sig api /sig cloud-provider

/sig network cloud-provider apimachinery

/assign @thockin

@etune for viz

I don't know that adding a field is the right thing here. Some implementations will have a single "backend" resource and others will have multiple.

I've been suggesting that folks consider provider-specific annotations to carry this data. Would that be enough for you?

Did you mean to ping @erictune?

I don't know that adding a field is the right thing here. Some implementations will have a single "backend" resource and others will have multiple.

Can you clarify this a little bit? Do you mean that some providers can have multiple types of LBs so a single provider ID format is not sufficient to cover those types or did you mean there are implementations that create multiple LBs so more than 1 provider ID is associated with a Service?

To add more to the issue description, this is in relation to #69293. Here we're talking about setting a (unique) provider ID for the LB associated with a Service with type LoadBalancer (equivalent to node.Spec.ProviderID for nodes). By doing this we don't need to rely on cryptic loadbalancer names or worry about naming conflicts on the cloud provider. I'm not aware of any cloud provider that does not have unique IDs attached to a LB resource.

I think ideally we want this pattern for all providers, but not sure if requiring it from all providers warrants an addition to the core API. Maybe an annotation that is required to be set by all cloud providers is sufficient. Even if we went with annotations we would still need to ensure it is backwards compatible similar to v1 API going forward. On the other hand, adding a field lets us add our own validations from apiserver which like node.Spec.ProviderID is nice because we can pre-define some of the behaviours around LBs that we might want. What do you think?

yes, I meant @erictune because he was doing some thinking in this area

What I meant is that there's no requirement than an external load-balancer be represented as a single resource. E.g. in GCP it's actually 3 resources (FwdRule, HealthCheck, TargetPool). An Ingress is more like 5 resources. Which backend thing would this carry, and what about the rest? Comparatively, I can define as many gcp-specific annotations as I need and nobody needs to care except people who WANT to dig into backends, in which case they need it all.

I think it's a good idea to make this linkage more explicit. I just don't know that a new field is the right model.

Maybe an annotation that is required to be set by all cloud providers is sufficient. Even if we went with annotations we would still need to ensure it is backwards compatible similar to v1 API going forward

What are you going to do with this information generically? What validation can you actually do? Better LB names is a great thing (I have wanted it for years) but that logic exists within provider-specific-space only, so why try to generalize it?

What I meant is that there's no requirement than an external load-balancer be represented as a single resource. E.g. in GCP it's actually 3 resources (FwdRule, HealthCheck, TargetPool). An Ingress is more like 5 resources. Which backend thing would this carry, and what about the rest? Comparatively, I can define as many gcp-specific annotations as I need and nobody needs to care except people who WANT to dig into backends, in which case they need it all.

This is not something I have considered yet 🤔 which definitely changes my perspective on this problem.

What are you going to do with this information generically? What validation can you actually do? Better LB names is a great thing (I have wanted it for years) but that logic exists within provider-specific-space only, so why try to generalize it?

This makes sense. I'm 👍 for annotations given there are some consistent best practices shared amongst providers. Will wait for @erictune and @aoxn to comment.

Annotation seems OK.

For annotation, Since CloudProvider implementation(aws, alibaba etc.) does not allowed to modify service object directly, this would require CloudProvider framework doing the provider id annotation writing back on service object.

And method signature might need to modified to contain the required providerid info. Something like below.
`
func (c *Cloud) EnsureLoadBalancer(clusterName string, service *v1.Service, nodes []*v1.Node) (*v1.LoadBalancerStatus, providerid, error) {

`

@andrewsykim @thockin

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

/lifecycle frozen
/remove-lifecycle stale
/remove-lifecycle rotten

I have re-titled this. The question is whether this should just be a documented "best practice" or whether we should actually go through all of the existing providers and try to make this happen? I'd love to see it done.

Thanks Tim, added this to the SIG CP backlog kubernetes/cloud-provider#27

cc: @MrHohn may be relevant to garbage collection issues

Thanks for adding me --- this could be more useful when pairing with service finalizer (kubernetes/enhancements#980).

@andrewsykim what do we need here?

@bowei Where are we with this?

I think someone has to drive a KEP here to add the field and work through the details.

I think someone has to drive a KEP here to add the field and work through the details.

My interpretation of the comment thread is that instead of adding a field, we should define best practices for how to use annotations to track underlying resources (#70159 (comment)) and then go through the existing cloud providers and get them transitioned over to using the best practices (#70159 (comment)).

This is more specific than #9106, but largely duplicates it.

Now that we have CRDs we should probably encourage a pattern like:

• Create service
• Create $cloudLB CR(s) with finalizers
• annotate Service with info about CRs
• cloudLB controller actuates

This is also probably something to implement in external cloud providers.

What might be on us is to document the approach with finalizers, and to promote that.

This one has gotten quite old, bringing this one up for our next meeting so we can review it.

Potentially worth a spike-level related discussion: what is Kubernetes' common architectural pattern for a cloud provider integration to be able to store unstructured / provider-dependent status data?

Reason I mention it is that Karpenter, a cluster autoscaler, has faced a similar question recently.

We don't have a pattern.

Options:

1. Store a stringly-typed annotation
2. Store a same-named CR
3. Store a CR and name it in an annotation

Option 2 is the one I see most. We do not have a mechanism for managing 2 resources toghether, nor a standard way to denote "linked" objects.

we have .spec.loadBalancerClass https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-class

/assign @robscott