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

[BUG] kubernetes.azure.com/scalesetpriority:spot is user-overrideable #3152

Closed
jackfrancis opened this issue Aug 25, 2022 · 5 comments
Closed

Comments

@jackfrancis
Copy link
Member

jackfrancis commented Aug 25, 2022

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  1. Create an AKS cluster with a spot node pool
  2. Issue an AKS node pool label update against the spot pool, e.g.
$ az aks nodepool update --resource-group my-cluster-rg --cluster-name my-cluster --name pool-spot --labels foo=bar
  1. The label update will be accepted, and the new (foo=bar) label applied to the pool, and to all of its nodes; but, the kubernetes.azure.com/scalesetpriority:spot label will be deleted.

After the above happens, all spot-targeted workloads will be disrupted, as the node affinity requirements are no longer fulfilled.

The above behavior seems to be because the spot-required node pool label is shared with the user-configurable node pool label surface area. The documentation here explains that node pool label updates overwrite the entire set of existing labels:

(And thus you must always issue updates as an entire "set" of "user" labels.)

Additionally, here we document that when you use the Spot feature you get the kubernetes.azure.com/scalesetpriority:spot label:

The documentation does not mention that user labels maintenance also includes the requirement to include that label with any subsequent node pool label update (which is effectively what the current implementation requires a user to do).

Arguably AKS should not impose such a requirement on the user, and should instead expose a user-configurable interface to manage node pool labels that is entirely distinct from kubernetes.azure.com/-prefixed labels that are required in order for foundational Kubernetes + Azure functionality.

Expected behavior
I would expect the user node pool label update to be successful, but not to effect any "system" labels such as kubernetes.azure.com/scalesetpriority:spot.

Screenshots
If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

  • CLI Version [e.g. 3.22]
  • Kubernetes version [e.g. 1.24.3]
  • CLI Extension version [e.g. 1.7.5] if applicable
  • Browser [e.g. chrome, safari] is applicable

Additional context
Add any other context about the problem here.

@jackfrancis
Copy link
Member Author

We might also investigate if the kubernetes.azure.com/scalesetpriority=spot:NoSchedule node pool taint is also subject to the same user-override pathology as labels, I haven't had time to look into that.

@jackfrancis
Copy link
Member Author

cc @zmalik

@NovemberZulu
Copy link

We might also investigate if the kubernetes.azure.com/scalesetpriority=spot:NoSchedule node pool taint is also subject to the same user-override pathology as labels, I haven't had time to look into that.

I believe no, at least I never observed lost taints, only labels were lost.

@ghost ghost added the action-required label Sep 20, 2022
@ghost
Copy link

ghost commented Sep 25, 2022

Action required from @Azure/aks-pm

@ghost ghost added the Needs Attention 👋 Issues needs attention/assignee/owner label Sep 25, 2022
@ghost ghost removed action-required Needs Attention 👋 Issues needs attention/assignee/owner labels Oct 6, 2022
@ghost ghost added the action-required label Oct 31, 2022
@kaarthis
Copy link
Contributor

This is fixed pls check now.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants