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 image_cleaner & image_cleaner_interval_hours #342

Closed
1 task done
aevlogiev opened this issue Mar 31, 2023 · 7 comments
Closed
1 task done

Support for image_cleaner & image_cleaner_interval_hours #342

aevlogiev opened this issue Mar 31, 2023 · 7 comments

Comments

@aevlogiev
Copy link

aevlogiev commented Mar 31, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Description

image_cleaner_enabled - (Optional) Specifies whether Image Cleaner is enabled.
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/kubernetes_cluster#image_cleaner_enabled

image_cleaner_interval_hours - (Optional) Specifies the interval in hours when images should be cleaned up. Defaults to 48
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/kubernetes_cluster#image_cleaner_interval_hours

This feature helps clean the unused images, although it can be updated manually on a cluster level, doing it manually triggers terraform change, along with the missing lifecycle option, we need to enable it each time we run terraform

This requires that the Preview Feature Microsoft.ContainerService/EnableImageCleanerPreview is enabled and the Resource Provider is re-registered, see the documentation for more information.

New or Affected Resource(s)/Data Source(s)

module.aks.azurerm_kubernetes_cluster.main

Potential Terraform Configuration

module "aks" {
  source  = "Azure/aks/azurerm"
  version = "6.7.1"
  image_cleaner_enabled = true/false (defaults to false)
  image_cleaner_interval_hours = 48
}

References

No response

@zioproto
Copy link
Collaborator

@aevlogiev I would propose we implement this as soon as the feature is GA.
The shift from Public Preview to GA is tracked here: Azure/AKS#2912

Would this work for you ?

@lonegunmanb
Copy link
Member

@aevlogiev I would propose we implement this as soon as the feature is GA. The shift from Public Preview to GA is tracked here: Azure/AKS#2912

Would this work for you ?

Agreed, I'd like to support this feature when it's not preview.

@aevlogiev
Copy link
Author

Perfect @zioproto @lonegunmanb thanks

@skolobov
Copy link
Contributor

skolobov commented Apr 10, 2023

@aevlogiev I would propose we implement this as soon as the feature is GA.

@zioproto In my opinion, this is a useful feature to start using now and I don't see a point in waiting for it to become GA, as it is low-risk: it is only about adding a couple of optional variables and thus should not break any existing setups that are not enabling the use of image cleaner at the moment.

This PR already implemented these new variables.

@lonegunmanb
Copy link
Member

lonegunmanb commented May 23, 2023

Hi @skolobov sorry for the late reply and thanks for your pr, unfortunately I agree with @zioproto, since the feature is still in preview, I'd like to wait for its GA.

A preview feature's API might be changed significantly, or even been revoked. We can only introduce breaking change to this module when we'd like to publish a major version upgrade, and we take these kinds of breaking changes very seriously. So, I'd like to wait for GA, please accept my apology. When the feature is GA would you please update your pr and ping me to kick off the e2e test? Thanks for your contribution and understanding!

@amoosbr
Copy link

amoosbr commented Sep 28, 2023

The AKS image cleaner feature is now GA.
Perhaps the merge conflict of the existing PR can be solved and the PR merged.

@skolobov
Copy link
Contributor

Thanks @amoosbr!
I have updated the PR (#343) to resolve the merge conflicts and it can now be applied cleanly again.

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 a pull request may close this issue.

5 participants