Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Availability Zone ID topology labels #300

Closed
rifelpet opened this issue Jan 26, 2022 · 33 comments · Fixed by #855
Closed

Support Availability Zone ID topology labels #300

rifelpet opened this issue Jan 26, 2022 · 33 comments · Fixed by #855
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@rifelpet
Copy link
Member

rifelpet commented Jan 26, 2022

What would you like to be added:
Currently AWS CCM populates the topology.kubernetes.io/region and topology.kubernetes.io/zone node labels with the node's region and AZ name respectively:

    topology.kubernetes.io/region: us-east-1
    topology.kubernetes.io/zone: us-east-1a

It would be useful to include a topology label for AZ IDs. topology.kubernetes.io/zone-id or something along those lines. Only the original two labels are standard in the k/k API so we'll need to decide on a new label to use.

    topology.k8s.aws/zone-id: use1-az1

Why is this needed:

For inter-cluster communication that spans AWS accounts, ensuring zonal isolation becomes challenging when the accounts do not have the same AZ mappings. By including AZ ID information in topologies we can more easily ensure that workloads are scheduled to consistent AZ IDs and communicate with other workloads in the same AZ ID regardless of their AWS account or the account's AZ mapping.

/kind feature

@olemarkus
Copy link
Member

Yes please! We have the exact same need.

@olemarkus
Copy link
Member

Actually, I wonder if not the zone label could change to using zone IDs. I think the transition should be fairly painless as the new CCM leader would just patch all existing nodes.

@rifelpet
Copy link
Member Author

rifelpet commented Jan 26, 2022

If we reuse the zone topology label then there may be strange behavior of pod topology spread constraints during the upgrade process when nodes have a mismatch of zone IDs and zone names.

Its common for zonal deployments to use nodeAffinity on specific zone label values, so updating the label value in place would break their scheduling unless they were updated prior to the CCM upgrade:

    nodeAffinity:
      requiredDuringSchedulingIgnoredDuringExecution:
        nodeSelectorTerms:
        - matchExpressions:
          - key: topology.kubernetes.io/zone
            operator: In
            values:
            - us-east-1a
            - use1-az1

@olemarkus
Copy link
Member

Spread constraints is only evaluated during scheduling, so this shouldn't be affected.

But nodeAffinity on a given zone label can cause problems, I suppose.

@rifelpet
Copy link
Member Author

rifelpet commented Feb 4, 2022

An alternative AWS-specific label could be topology.k8s.aws/zone-id

@rifelpet
Copy link
Member Author

sig-cloud-provider agreed to use an AWS-specific label and/or the ability to configure CCM to use the zone ID in the topology.kubernetes.io/zone label

@andrewsykim
Copy link
Member

configure CCM to use the zone ID in the topology.kubernetes.io/zone label

My only caution to using zone-id in topology.kubernetes.io/zone is that it could negatively impact portability of deployments across different AWS accounts.

@olemarkus
Copy link
Member

E.g EBS CSI drivers use topology keys and changing the convention on a live cluster may cause problems with PVs. So I think the safer route is to add new labels for now. I'd really like zone to use zone-ids, but it seems like too many things may break over it.

@sftim
Copy link

sftim commented Feb 23, 2022

Reposting from aws/containers-roadmap#1638 (comment)

topology.kubernetes.io/zone-id is not registered as a Kubernetes label key. To report an AWS-specific detail, I recommend minting one or two new label keys. For example:

  • kubernetes.amazonaws.example/availability-zone: us-east-1a
  • kubernetes.amazonaws.example/availability-zone-id: use1-az1

@sftim
Copy link

sftim commented Feb 23, 2022

topology.k8s.aws/zone-id - suggested in #300 (comment) - is also fine as a label key; I hadn't seen that before.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 24, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 23, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.

@dims
Copy link
Member

dims commented May 11, 2023

/reopen

@k8s-ci-robot k8s-ci-robot reopened this May 11, 2023
@k8s-ci-robot
Copy link
Contributor

@dims: Reopened this issue.

In response to this:

/reopen

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.

@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If cloud-provider-aws contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label May 11, 2023
@josh-ferrell
Copy link

/assign

@sftim
Copy link

sftim commented May 30, 2023

/remove-lifecycle rotten

still seems relevant and referenced

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label May 30, 2023
@josh-ferrell
Copy link

josh-ferrell commented May 30, 2023

An alternative AWS-specific label could be topology.k8s.aws/zone-id

I'm in favor of this approach and possibly later adding the ability to configure CCM to populate topology.kubernetes.io/zone differently. I'll start looking into the first idea.

@josh-ferrell josh-ferrell removed their assignment Jun 5, 2023
@josh-ferrell
Copy link

/assign

@JoelSpeed
Copy link
Contributor

Is there any parallel on other cloud providers? Do we know if Azure/GCP do the similar zone remapping that AWS does?

@rifelpet
Copy link
Member Author

I asked in a sig-cloud-provider meeting around when I opened this issue and was told there is no equivalent concept in GCP and Azure.

@sayap
Copy link

sayap commented Jun 16, 2023

OCI does it:

To help balance capacity in data centers, Oracle Cloud Infrastructure randomizes availability domains by tenancy . For example, the availability domain labeled PHX-AD-1 for tenancyA might be a different data center than the one labeled PHX-AD-1 for tenancyB.

(https://docs.oracle.com/en-us/iaas/Content/General/Concepts/regions.htm)

@JoelSpeed
Copy link
Contributor

So if we want to implement this feature, it might make sense to have it consistent across clouds that support it, to do that, we need to know that they expose a way for the CCM to set it. Do we know if oracle has an API similar to AWS?

What happens if the zone-id isn't needed for a platform, lets take Azure for example, if they don't do this, then zone-id isn't needed, we could set it to zone and duplicate the data, or omit it?

If omitted, then manifest examples might not work on platforms that omit it right?

@sayap
Copy link

sayap commented Jun 16, 2023

@JoelSpeed You are talking specifically about topology.kubernetes.io/zone-id? #300 (comment) proposed a way forward without setting this label.

@JoelSpeed
Copy link
Contributor

@sayap Yes, I'm trying to work out if there's merit to having this as a standard kube well known label that would work across clouds, seems like if there is precedent for multiple clouds, having a consistent label name would be preferential to specific cloud having their own right?

@isugimpy
Copy link

isugimpy commented Jul 5, 2023

Could this maybe be accomplished via opt-in using a flag on the binary to change topology.kubernetes.io/zone to the ID instead of the name? That'd retain existing compatibility while allowing people to use the feature.

@mmerkes
Copy link
Contributor

mmerkes commented Jul 13, 2023

I'm in favor of this approach and possibly later adding the ability to configure CCM to populate topology.kubernetes.io/zone differently. I'll start looking into the first idea.

@josh-ferrell This something your working on? Sounds reasonable to me.

@k8s-ci-robot
Copy link
Contributor

@mmerkes: GitHub didn't allow me to assign the following users: merkes.

Note that only kubernetes members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign merkes

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.

@mmerkes
Copy link
Contributor

mmerkes commented Jan 2, 2024

/assign mmerkes

@sftim
Copy link

sftim commented Feb 10, 2024

@rifelpet I think the consensus view is to use topology.k8s.aws/zone-id as the label key for the zone ID. If that looks right to you, would you be willing to update the issue description accordingly?

@rifelpet
Copy link
Member Author

@rifelpet I think the consensus view is to use topology.k8s.aws/zone-id as the label key for the zone ID. If that looks right to you, would you be willing to update the issue description accordingly?

I've updated the issue description to match

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.