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: label review app namespaces #370

Merged
merged 4 commits into from
Aug 22, 2023
Merged

feat: label review app namespaces #370

merged 4 commits into from
Aug 22, 2023

Conversation

artsyjian
Copy link
Contributor

@artsyjian artsyjian commented Aug 18, 2023

Supports PHIRE-179

We have a script that cleans up old Review Apps. It does so by deleting their Kubernetes namespaces.

The script must know which namespaces are Review Apps. Currently there is no way to determine that from the name of a namespace. Therefore, the script is hardcoded with a list of non-Review-App namespaces and it deletes namespaces not on that list.

However, the list has become a burden to upkeep. This PR proposes to label Review App namespaces with app-phase=review. This allows the script to filter namespaces by that label, then we can do away with the non-Review-App list.

Un-related fix:

  • Instruct to run setuptools_scm for local dev.

@artsyjian artsyjian requested a review from a team August 18, 2023 18:50
@artsyjian artsyjian force-pushed the artsyjian/reviewapp branch 2 times, most recently from bac005e to 44a2e3c Compare August 18, 2023 20:56
@artsyjian artsyjian force-pushed the artsyjian/reviewapp branch from 44a2e3c to 89f2229 Compare August 18, 2023 20:57
```
python -m setuptools_scm
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if this command is not run, hokusai version reports v0.0.0 (the dummy string in pyproject.toml). that causes conflict with projects that require certain versions of Hokusai (e.g. hokusai-sandbox).

'name': clean_string(app_name)
'name': clean_string(app_name),
'labels': {
'app-phase': 'review'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

potentially another use case for a ~/.hokusai.conf config file. i would have a var there like org=acme, then this label can become acme/app-phase: review which should never conflict with any label k8s might automatically add to this resource in the future.

Copy link
Contributor

@mc-jones mc-jones left a comment

Choose a reason for hiding this comment

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

This LGTM though, admittedly, I'm not familiar with the test patterns. I'll hold on the merge until in the event there is something that you want to follow up on after.

@artsyjian
Copy link
Contributor Author

thanks it should be good to merge.

@mc-jones mc-jones merged commit e4af53a into main Aug 22, 2023
@mc-jones mc-jones deleted the artsyjian/reviewapp branch August 22, 2023 14:11
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.

2 participants