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 for AKS CSI Storage Driver #12826

Closed
philwinder opened this issue Aug 3, 2021 · 14 comments
Closed

Support for AKS CSI Storage Driver #12826

philwinder opened this issue Aug 3, 2021 · 14 comments

Comments

@philwinder
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Add the feature flags necessary to enable CSI driver support: https://docs.microsoft.com/en-us/azure/aks/csi-storage-drivers

New or Affected Resource(s)

  • azurerm_kubernetes_cluster

Potential Terraform Configuration

{
...
  azure_csi_driver {
    enabled = true
  }
}

References

@crstin
Copy link

crstin commented Aug 6, 2021

This is important because it seems you can't enable this on an existing cluster with az aks update. This renders terraform approaches useless when you want to have things like volume snapshots. Correct me if wrong, please.

@poddm
Copy link

poddm commented Aug 23, 2021

This is important because it seems you can't enable this on an existing cluster with az aks update. This renders terraform approaches useless when you want to have things like volume snapshots. Correct me if wrong, please.

I see this covered in the docs here: https://docs.microsoft.com/en-us/azure/aks/csi-secrets-store-driver#upgrade-an-existing-aks-cluster-with-secrets-store-csi-driver-support

To upgrade an existing AKS cluster with Secrets Store CSI Driver capability, use the az aks enable-addons command with the addon azure-keyvault-secrets-provider:

az aks enable-addons --addons azure-keyvault-secrets-provider --name myAKSCluster --resource-group myResourceGroup

As stated above, the addon creates a user-assigned managed identity that can be used to authenticate to Azure Key Vault.

@primeroz
Copy link

I think the request is more about enabling CSI in general on the cluster, and as specified here https://docs.microsoft.com/en-us/azure/aks/csi-storage-drivers#limitations it can't be done other than at cluster creation time.

I think that the addon you are referencing will require the CSI to be enabled in the first place , then the addon can be installed.

@favoretti
Copy link
Collaborator

Hi there and thank you for reporting this. This appears to be a duplicate of an earlier reported issue #12783, so I'm going to close this one to keep the discussion concise.

@primeroz
Copy link

@favoretti i might be wrong but i don't think they are duplicate.

This request is about enabling the CSI addon which is a pre-requisite of enabling the secrets one in #12783.

@favoretti
Copy link
Collaborator

@primeroz from what I understand those will be addon parameters, so will need to be tackled/implemented in one go, WDYT?

@primeroz
Copy link

I was not really talking about the implementaion, more about the scope of the Github issue :)

Also everytime i visit the docs on azure for CSI they seem to have changed !

As of now the CSI support is enabled by setting : --aks-custom-headers EnableAzureDiskFileCSIDriver=true this only enable support for CSI in the control plane ( at least that's my understanding ) but it does not install any manifest / resource for CSI in the cluster which is still up to the operator to do

for the Secrets one i understand we need to enable the addon as described in the docs

So i think that with this issue we could track adding support for CSI in the control plane ( which requires a new resource creation since it can only be set at cluster creation time ) , then people ( like me ) might want to add any number of CSI Drivers on the cluster in a self-managed fashion

@favoretti favoretti reopened this Sep 21, 2021
@favoretti
Copy link
Collaborator

@primeroz re-opened, let's see when someone gets to it. That said, since it's a preview feature, as you can see it changes quite often still, so I'm not sure we can expect sensible API support for it soon. But I'm interested in this one as well, so I'll try to dive into it.

@primeroz
Copy link

👍 i think csi is finally GA on azure since it will be default soon

@avinashpancham
Copy link

@favoretti I can understand that it might not be worthwhile to add this preview feature to azurerm as it might change in the future.

For users that still want to activate this preview feature on their AKS cluster a solution for now would be installing it via the Secrets Store CSI Driver Helm chart.

resource "helm_release" "csi-secrets-store-provider-azure" {
  name       = "csi-secrets-store-provider-azure"
  repository = "https://raw.githubusercontent.com/Azure/secrets-store-csi-driver-provider-azure/master/charts"
  chart      = "csi-secrets-store-provider-azure"
  version    = "0.2.1"  # latest version at the time of writing
}

A Helm chart is available for other preview features as well, such as Azure Pod Identity. See #9885 (comment) for how I got that working.

@thy143
Copy link

thy143 commented Oct 1, 2021

I took a quick look at implementing the --addons azure-keyvault-secrets-provider function, however, as it’s preview unless your subscription has it enabled it cause terraform to fail as the api reports you need to enable it.

I new at TF code, so not sure how I make the configuration option only present in the api request if you have declared it in your tf code.

@DrewRomanyk
Copy link

@favoretti As far as I can tell this feature has been GA since July 25th according to this issue, but it requires setting this property EnableAzureDiskFileCSIDriver to true on the request header when creating the cluster, which can likely be done generically for other custom header values with this terraform provider azurerm issue. So if anything we can close this as a duplicate to that issue.

I posted in that provider issue what I think might be a solution to do it, but I'm not well versed in terraform provider source code. Happy to try to fix this if given the proper guidance, but also happy to let someone more experienced fix this faster.

@aristosvo
Copy link
Collaborator

Starting in Kubernetes version 1.21, Kubernetes will use CSI drivers only and by default. These drivers are the future of storage support in Kubernetes.

With 1.21 being GA for a while, this header is only useful for current supported version 1.19 and 1.20. These being out of support Nov 2021 (1.19) and Feb 2022 (1.20), I'm going to close this issue.

Custom header support can still be tracked in #6793, which (if properly implemented) would also support this use-case.

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 13, 2021
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

10 participants