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

feat: Allow users set upgrade_settings.max_surge property #9

Closed
wants to merge 3 commits into from

Conversation

mloskot
Copy link
Contributor

@mloskot mloskot commented Jan 12, 2024

Azure Kubernetes Service changed the default max surge
in October release, so that for clusters based
on Kubernetes >1.28 max surge defaults to 10%, see
https://github.com/Azure/AKS/releases/tag/2023-10-01

Previously it was left blank which implied use of
value 1 under the bonnet.

Using the current version of Terraform AzureRM 3.86.0
leads to implicit resetting of the max_surge:

~ default_node_pool {
            name                         = "default"
            ...
            # (23 unchanged attributes hidden)

          - upgrade_settings {
              - max_surge = "10%" -> null
            }
        }

        # (7 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

The only workaround to avoid such confusing annoyance is to
set the max_surge with explicit value e.g. default "10%".
But, this requires max_surge to be exposed to end-users of
this module. See also
hashicorp/terraform-provider-azurerm#24020

Closes #6

Azure Kubernetes Service changed the default max surge
in October release, so that if for clusters based
on Kubernetes >1.28 max surge defaults to 10%, see
https://github.com/Azure/AKS/releases/tag/2023-10-01

Previously it was left blank which implied use of
value 1 under the bonnet.

Using the current version of Terraform AzureRM 3.86.0
leads to implicit resetting of the max_surge:

  max_surge = "10%" -> null

The only workaround to avoid such confusing annoyance is to
set the max_surge with explicit value e.g. default "10%".
But, this requires max_surge to be exposed to end-users of
this module. See also
hashicorp/terraform-provider-azurerm#24020

Closes claranet#6

Signed-off-by: Mateusz Łoskot <mateusz@loskot.net>
r-aks.tf Outdated Show resolved Hide resolved
r-node-pools.tf Outdated Show resolved Hide resolved
@BzSpi
Copy link
Contributor

BzSpi commented Jan 12, 2024

Thanks @mloskot 👍

mloskot and others added 2 commits January 12, 2024 16:05
Co-authored-by: Spi <BzSpi@users.noreply.github.com>
Co-authored-by: Spi <BzSpi@users.noreply.github.com>
@RockyMM
Copy link
Contributor

RockyMM commented Feb 26, 2024

I'm guessing this PR has to be rebased? Actually, it seems obsolete?

@mloskot
Copy link
Contributor Author

mloskot commented Feb 26, 2024

@RockyMM Yes, it looks like the main commit 55a6808 replaces this PR.

@mloskot mloskot closed this Feb 26, 2024
@mloskot mloskot deleted the ml/feat-expose-max-surge branch February 26, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] Allow customization of upgrade_settings.max_surge
3 participants