search

kubernetes / website

Kubernetes website and documentation repo:
https://kubernetes.io
Creative Commons Attribution 4.0 International
4.46k stars 14.37k forks source link

Register annotations for Azure integration #45718

Open sftim opened 6 months ago

sftim commented 6 months ago

This is a Feature Request

What would you like to be added Update Well-Known Labels, Annotations and Taints to include each of the annotations listed in https://cloud-provider-azure.sigs.k8s.io/topics/loadbalancer/#loadbalancer-annotations

Why is this needed We should register (and document) all of our official annotations, even the beta ones.

Comments /sig cloud-provider /language en

/lifecycle frozen /priority backlog it's required, but not urgent

sftim commented 6 months ago

/triage accepted

Ritikaa96 commented 6 months ago

A full list of all annotations are listed here There are about 30 annotations to be recorded and registered.

Ritikaa96 commented 6 months ago

I would like to try adding these annotations /assign

Ritikaa96 commented 3 months ago

Hi @sftim , just wanted to confirm is we have to create single PR for all 30 Annotations or we can break them in several PRs?

sftim commented 3 months ago

just wanted to confirm is we have to create single PR for all 30 Annotations or we can break them in several PRs?

Either way is fine!

Ritikaa96 commented 3 months ago

great I'll try to break the list into 2-3 PRs so that not many changes occur in one and previous review help the latest PR as well. Thanks @sftim

tengqm commented 3 months ago

For annotations that are not so qualified as "well-known" ones, there are two options in my mind:

sftim commented 3 months ago

These are official annotations. Switching the way we document annotations should be a separate issue from this one, which is about tracking the official Azure annotations.

@tengqm they are not dual-hosted; the code that uses them is part of Kubernetes.

tengqm commented 3 months ago

These are official annotations. Switching the way we document annotations should be a separate issue from this one, which is about tracking the official Azure annotations.

They are official to Azure doesn't mean they are official to Kubernetes, right? These annotations are already documented at https://cloud-provider-azure.sigs.k8s.io/topics/loadbalancer/#loadbalancer-annotations, along with many other Azure-specific contents, what we proposing here is to copy them over to k8s website?

@tengqm they are not dual-hosted; the code that uses them is part of Kubernetes.

Well ... it depends on how we define "dual-hosting". The existing annotations are well documented at the cloud-provider-azure website. I won't vote for copying them over and I see that as "dual-hosting" (my version of course).

sftim commented 3 months ago

We can hyperlink to https://cloud-provider-azure.sigs.k8s.io/topics/loadbalancer/#loadbalancer-annotations for details.

However, we should still register these annotations per the policy in https://kubernetes.io/docs/reference/labels-annotations-taints/

sftim commented 3 months ago

They are official to Azure doesn't mean they are official to Kubernetes, right?

No; they are official to Kubernetes because (for legacy reasons) they use a Kubernetes namespace. If we had a time machine we could go back and ask the Azure folks to use a different domain name in the annotation keys.

sftim commented 3 months ago

(We can still encourage the Azure provider folks to use a Microsoft domain name for the post-beta annotation and label keys, but that's again a separate issue to this one).

Ritikaa96 commented 3 months ago

If there is a second thought about registering these annotations or the related format, I'd like to know about it so that I can raise a PR that is still required.

tengqm commented 3 months ago

We can hyperlink to https://cloud-provider-azure.sigs.k8s.io/topics/loadbalancer/#loadbalancer-annotations for details.

However, we should still register these annotations per the policy in https://kubernetes.io/docs/reference/labels-annotations-taints/

Just checked the labels-annotations-taints/_index.md file, we already have AWS specific annotations there. Most of them are about LB Services. That file is now 2385 lines of markdown. Even if we want to add some pointers to the Azure-specific annotations, I'd still suggest to put them into a new file under labels-annotations-taints/. We may want to move AWS-specific, NPD-specific labels/annotations out of the _index.md file as well.

sftim commented 3 months ago

Eventually, I'd quite like [someone] to make it more like a searchable file where you type in some text and see the matching annotations. We have so many of them now that one file isn't obviously the right way to organise it all. I think @thockin had talked about moving the canonical registry into its own repo - perhaps with generator code. We haven't found the round tuits to make this happen.

But that's for another time.

tengqm commented 3 months ago

They are official to Azure doesn't mean they are official to Kubernetes, right?

No; they are official to Kubernetes because (for legacy reasons) they use a Kubernetes namespace. If we had a time machine we could go back and ask the Azure folks to use a different domain name in the annotation keys.

We cannot change history because we are not the Big Brother. The "kubernetes.io" namespace thing is not a rule strictly followed by cloud providers. For example, take a look at these:

These annotations serve the similar purposes, but they may and may not use "kubernetes.io" domain name. Given that we have so many cloud providers out there, I'm not inclined to document things for one or two providers while ignoring others.

sftim commented 3 months ago

I'm not inclined to document things for one or two providers while ignoring others

We should document all the in-project annotations; that would include the integration with Huawei. For the OpenStack integration, they have done the right thing so we don't need to register official annotations.

This issue is about Azure, right?

thockin commented 3 months ago

Annotations which are specific to one cloud-provider should use provider specific names. It boggles my mind that so many well-intentioned people get this wrong, but it IS somewhat messy because it is azure code that lives in a k8s org.

We cannot rip those out, probably.

Providers SHOULD be chided and SHOULD add better names that trigger the same meaning.

As for documentation, I agree we should document them and make it clear that they are provider-specific. If anyone wants to help get the "names DB" off the ground, let me know. It's not HARD, it's just tedious.

Ritikaa96 commented 3 months ago

Even if we want to add some pointers to the Azure-specific annotations, I'd still suggest to put them into a new file under labels-annotations-taints/. We may want to move AWS-specific, NPD-specific labels/annotations out of the _index.md file as well.

I agree , from reader's perspective , having a different file will serve us good and will also help organising these annotations in discussion as they are not few but 30 in number.

Ritikaa96 commented 3 months ago

Eventually, I'd quite like [someone] to make it more like a searchable file where you type in some text and see the matching annotations. We have so many of them now that one file isn't obviously the right way to organise it all. I think @thockin had talked about moving the canonical registry into its own repo - perhaps with generator code. We haven't found the round tuits to make this happen.

But that's for another time.

Do let me know about the new required changes and where to register the annotations now. I'll continue the work then.

Ritikaa96 commented 4 weeks ago

Hi Any update on this discussion? Just asking so we can start working on this one.

Eventually, I'd quite like [someone] to make it more like a searchable file where you type in some text and see the matching annotations. We have so many of them now that one file isn't obviously the right way to organise it all. I think @thockin had talked about moving the canonical registry into its own repo - perhaps with generator code. We haven't found the round tuits to make this happen.

But that's for another time.