Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Refactor: Ancillary K8s modules should not inherit from KubernetesRawModule #219

Closed
4 of 6 tasks
tima opened this issue Sep 2, 2020 · 2 comments
Closed
4 of 6 tasks
Assignees
Labels
priority/critical type/meta Meta-issue that collections multiple tasks and other issues

Comments

@tima
Copy link
Collaborator

tima commented Sep 2, 2020

[Derived from feedback from @fabianvf]

Ancillary K8s modules such as k8s_service should not inherit from KubernetesRawModule. This is problematic in that the inheritance is too broad and includes a lot of implicitly inherited behavior This makes it unclear what is actually shared and sometimes carries through functions or parameters not applicable to the module being implemented.

Examples of inheritance from the KubernetesRawModule:

Which are based on these classes:

These ancillary K8s modules should inherit from AnsibleModule directly rather KubernetesRawModule and use either bare methods in the common.py or if the state is relevant belong in something like the K8sAnsibleMixin (https://github.com/ansible-collections/community.kubernetes/blob/main/plugins/module_utils/common.py#L162), which should not implement AnsibleModule functionality.

Conversely, any broadly useful functions or shared code should be identified and moved out of specific modules and into `common.py.

Related to this, the argspec property should be not be included in K8sAnsibleMixin to avoid similar unintended inheritance with modules where arguments are not relevant.

  • Relocate generally applicable functions inherited from KubernetesRawModule to K8sAnsibleMixin or common.py depending on use
  • Move any identified broadly useful functions or shared code in specific modules to common.py
  • Change inheritance of ancillary k8s modules to use AnsibleModule and apply relocated functions from K8sAnsibleMixin or common.py as necessary.
  • Remove argspec property from K8sAnsibleMixin moving to specific argspecs in each module
  • Identify any ways the code could be refactored to be "more testable"
  • Adjust unit tests as needed to work with refactored code
@tima tima added priority/critical type/meta Meta-issue that collections multiple tasks and other issues labels Sep 2, 2020
@Akasurde
Copy link
Member

All PRs are merged and can be closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority/critical type/meta Meta-issue that collections multiple tasks and other issues
Projects
None yet
Development

No branches or pull requests

2 participants