Skip to content

Conversation

@anrayliu
Copy link
Collaborator

@anrayliu anrayliu commented Jul 31, 2025

Proposed Changes/Description

Adds a namespace label to filter for when running controller and scheduler. The plan is to use both a name and a label to filter for namespaces. Either of then can be disabled, increasing flexibility to select namespaces. Unit tests still pending.

Types of Changes

What types of changes does your code introduce to volume-cleaner? Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update
  • Other (if none of the other choices apply)

Related Issue/Ticket

closes #136

Checklist

  • README.md or the Github Wiki documentation updated - if appropriate
  • Unit and/or integration tests added/modified
  • Lint and Unit tests pass locally with my changes

anrayliu added 4 commits July 31, 2025 20:54
Namespace name and label will both be used to filter for namespaces.
Either of them can be disabled, increasing flexibility when filtering for namespaces.
Tests and functionality to come in later commits.
@anrayliu anrayliu self-assigned this Jul 31, 2025
@anrayliu anrayliu added the enhancement New feature or request label Jul 31, 2025
for _, ns := range NsList(kube, cfg.NsLabel) {
// skip if not in configured namespace
if ns.Name != cfg.Namespace && cfg.Namespace != "" {
continue
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This git diff looks weird but all I've done is wrap the red code inside the block above

@anrayliu
Copy link
Collaborator Author

anrayliu commented Aug 6, 2025

Oh hold for now because this pr is pretty drastic. Still deciding what to do with it.

// given a pvc, this function will aquire the details related to the pvc such as the owner of the pvc, their email, the bounded volume name and ID, and details about its deletion

func EmailDetails(kube kubernetes.Interface, pvc corev1.PersistentVolumeClaim, gracePeriod int) (string, structInternal.Personalisation) {
ns := pvc.Namespace
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match the code style from the rest of the repo. The rest of the code doesn't use temporary variables.

API_KEY: "Random APIKEY",
*/

type ControllerConfig struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we don't extract the common fields into their own shared config? Or why not have 1 config struct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The controller and scheduler were designed to be as loosely coupled as possible, so we didn't want to mix the configs. It would also be too unnecessarily complicated to create another struct for just a few values.

@anrayliu
Copy link
Collaborator Author

This PR is on hold. Maybe someone will re-implement this in the future. The issue with it currently is that it theoretically decreases runtime performance drastically by starting a goroutine for each namespace. In the future, someone may want to research better ways to implement a filter by labels, then test real-time performance.

#136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add label filter for namespace

3 participants