kubernetes-sigs / cluster-api-provider-azure

Cluster API implementation for Microsoft Azure
https://capz.sigs.k8s.io/
Apache License 2.0
295 stars 425 forks source link

Shorten AKS Windows pool names when AzureManagedMachinePool name is too long #2422

Open jackfrancis opened 2 years ago

jackfrancis commented 2 years ago

/kind feature

Describe the solution you'd like [A clear and concise description of what you want to happen.]

The AKS API enforces a max 6 character limit for node pool names. Because this is not necessarily an intuitive requirement, let's consider ways to do this automatically on behalf of users.

Anything else you would like to add: [Miscellaneous information that will assist in solving the issue.]

A possible approach to follow is https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/1408

Environment:

CecileRobertMichon commented 2 years ago

/area managedcluster /help /good-first-issue

k8s-ci-robot commented 2 years ago

@CecileRobertMichon: This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to [this](https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/2422): >/area managedcluster >/help >/good-first-issue Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
k8s-ci-robot commented 2 years ago

@CecileRobertMichon: The label(s) area/managedcluster cannot be applied, because the repository doesn't have them.

In response to [this](https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/2422#issuecomment-1177931569): >/area managedcluster >/help >/good-first-issue Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
luthermonson commented 2 years ago

@jackfrancis @CecileRobertMichon i think this covers it: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/exp/api/v1beta1/azuremanagedmachinepool_webhook.go#L290-L301

CecileRobertMichon commented 2 years ago

indeed!

/close

k8s-ci-robot commented 2 years ago

@CecileRobertMichon: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/2422#issuecomment-1178262315): >indeed! > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
jackfrancis commented 2 years ago

So actually the foo that @luthermonson links to is validation that the user-configured pool name is no longer than 6 characters. I originally opened up this issue to track consideration of doing this automatically (truncating).

If we want to consider doing that (user inputs "my-pool-name-win" and we truncate to "win-kX" or something) we can re-open this issue.

CecileRobertMichon commented 2 years ago

Ah, understood. The title makes it sound like this is about validation I think.

/reopen /retitle Shorten AKS Windows pool names when AzureManagedMachinePool name is too long

k8s-ci-robot commented 2 years ago

@CecileRobertMichon: Reopened this issue.

In response to [this](https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/2422#issuecomment-1179211580): >Ah, understood. The title makes it sound like this is about validation I think. > >/reopen >/retitle Shorten AKS Windows pool names when AzureManagedMachinePool name is too long Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
sayantani11 commented 2 years ago

@jackfrancis @CecileRobertMichon I would like to get some more insight on this issue if possible!

jackfrancis commented 2 years ago

I'm not actually convinced we want to do this. @luthermonson do you have thoughts? Would you prefer a capz-enforced string concatenation and/or 6-char hash that replaces the inputted node pool name, or would you prefer capz to fail validation and force the user to purposefully choose a name that is 6 chars or fewer?

k8s-triage-robot commented 2 years ago

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:

You can:

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

/lifecycle stale