feat(#702): serviceaccount removal #724
Conversation
- add registry url cli option for easy registry change - fix service annotations - fix replacement of kwargs with empty dict by filtering only the empty values
- feat: add service account deletion in operator terminate handler - add registry url to up command
|
I merged the other PR because I wanted to test it locally and provide a custom registry url |
|
This PR addresses #702 |
| key, value = annotation[0].split("=") | ||
| stowaway_annotations[key] = value | ||
| except ValueError: | ||
| # handle preset values |
There was a problem hiding this comment.
We should probably display a warning or something if this happens and the parameter is ignored no?
| kube_config_file=kubeconfig, | ||
| kube_context=kubecontext, | ||
| ignore_docker=True, | ||
| registry_url=kwargs.get("registry_url", ""), |
There was a problem hiding this comment.
Why not make a dedicated kw? I think that would be cleaner and nicer for code completion in IDEs
There was a problem hiding this comment.
Because of the construction of GefyraInstallOptions in lines 66-67. Making registry_url a dedicated keyword pops it from kwargs leading to the option not being respected anymore
There was a problem hiding this comment.
Still I think it would be better to make it explicit, just for code completion and typing.
You could add it to the kwargs in case a preset is set.
| if not all(kwargs.values()): | ||
| kwargs = {} | ||
| # filter out empty kwargs | ||
| kwargs = {k: v for k, v in kwargs.items() if v} |
There was a problem hiding this comment.
It would be more transparent to show a warning when things are filtered out no? Otherwise users might wonder.
This PR adds automatic removal of service accounts in operator terminate handler
closes #702