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

Clean up the handling for .spec.provider.workers[].machine.volume.type=standard #190

Open
ialidzhikov opened this issue Nov 16, 2020 · 3 comments · May be fixed by #353
Open

Clean up the handling for .spec.provider.workers[].machine.volume.type=standard #190

ialidzhikov opened this issue Nov 16, 2020 · 3 comments · May be fixed by #353
Assignees
Labels
kind/cleanup Something that is not needed anymore and can be cleaned up lifecycle/stale Nobody worked on this for 6 months (will further age) platform/azure Microsoft Azure platform/infrastructure

Comments

@ialidzhikov
Copy link
Member

gardener-attic/gardener-extensions#401 introduces the following code fragment for backwards-compatibility reasons:

// In the past the volume type information was not passed to the machineclass.
// In consequence the Machine controller manager has created machines always
// with the default volume type of the requested machine type. Existing clusters
// respectively their worker pools could have an invalid volume configuration
// which was not applied. To do not damage existing cluster we will set for
// now the volume type only if it's a valid Azure volume type.
// Otherwise we will still use the default volume of the machine type.
if pool.Volume.Type != nil && (*pool.Volume.Type == "Standard_LRS" || *pool.Volume.Type == "StandardSSD_LRS" || *pool.Volume.Type == "Premium_LRS") {
osDisk["type"] = *pool.Volume.Type
}

Prior to gardener-attic/gardener-extensions#401, the azure machines were always with the default os disk belonging to the requested machine type.

We already have in place the validation that prevents creation of a new cluster with volume type != ["Standard_LRS", "StandardSSD_LRS", "Premium_LRS"].

$ k apply -f ~/bar.yaml --dry-run=server
Error from server (Forbidden): error when creating "/Users/foo/bar.yaml": shoots.core.gardener.cloud "test" is forbidden: [spec.provider.workers[0].volume.type: Unsupported value: core.Volume{Name:(*string)(nil), Type:(*string)(0xc013ff5820), VolumeSize:"50Gi", Encrypted:(*bool)(nil)}: supported values: "Standard_LRS", "StandardSSD_LRS", "Premium_LRS"]

We still have a small number of legacy Shoots which still use .spec.provider.workers[].machine.volume.type=standard. We should actively ping them to migrate (a rolling update of the Nodes will be caused by change of volume.type).

After that we can clean up the above explicit check for ["Standard_LRS", "StandardSSD_LRS", "Premium_LRS"].

/kind cleanup
/platform azure
/priority normal
/cc @dkistner

@gardener-robot gardener-robot added kind/cleanup Something that is not needed anymore and can be cleaned up platform/azure Microsoft Azure platform/infrastructure priority/normal labels Nov 16, 2020
@dkistner
Copy link
Member

I wrote a small script to detect the affected clusters and their owners:

#/bin/bash -e -x pipefail

shoot_list=$(kubectl get shoots -o json -A | jq -r '.items[] | select(.spec.provider.type == "azure") | select(.spec.provider.workers[] | .volume.type == "standard") | [.metadata.namespace,.metadata.name,.metadata.creationTimestamp] | @json' | sed s/garden-//g | uniq)

for a in $shoot_list; do
  echo "---"
  echo "Project: $(echo $a | jq -r .[0])"
  echo "Shoot: $(echo $a | jq -r .[1])"
  echo "Owner: $(kubectl get project $(echo $a | jq -r .[0]) -o json | jq -r .spec.owner.name)"
  echo "Created at $(echo $a | jq -r .[2])"
done

@dkistner
Copy link
Member

/assign

@gardener-robot gardener-robot added the lifecycle/stale Nobody worked on this for 6 months (will further age) label Jan 26, 2022
@gardener-robot gardener-robot added lifecycle/rotten Nobody worked on this for 12 months (final aging stage) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels Jul 25, 2022
@kon-angelo
Copy link
Contributor

/remove lifecycle/rotten

@gardener-robot gardener-robot removed the lifecycle/rotten Nobody worked on this for 12 months (final aging stage) label Mar 11, 2023
@gardener-robot gardener-robot added the lifecycle/stale Nobody worked on this for 6 months (will further age) label Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Something that is not needed anymore and can be cleaned up lifecycle/stale Nobody worked on this for 6 months (will further age) platform/azure Microsoft Azure platform/infrastructure
Projects
None yet
4 participants