Refactor k8s_service to use new module_utils code #327
Conversation
Signed-off-by: Alina Buzachis <[email protected]>
|
recheck |
Signed-off-by: Alina Buzachis <[email protected]>
Akasurde
force-pushed
the
migrate_k8s_service
branch
from
ea04d2a
to
d4012b5
Compare
Signed-off-by: Alina Buzachis <[email protected]>
Signed-off-by: Alina Buzachis <[email protected]>
There was a problem hiding this comment.
Unfortunately, we can't use run_module() here as it's currently written. The selector, type, and ports parameters get lost. We should also force the kind value. I'll need to think about whether we want to change that function and how. It may be easiest for now to just replicate what you need from run_module() here. We don't need the continue on error functionality because this module currently only works with a single resource definition.
alinabuzachis
force-pushed
the
migrate_k8s_service
branch
3 times, most recently
from
fdd3251
to
4852313
Compare
Signed-off-by: Alina Buzachis <[email protected]>
alinabuzachis
force-pushed
the
migrate_k8s_service
branch
from
4852313
to
b6df034
Compare
|
recheck |
plugins/modules/k8s_service.py
Outdated
| if module.params["validate"] is not None: | ||
| module.warnings = validate(svc.client, module, definition) |
There was a problem hiding this comment.
This module does not support the validate param. I was a bit surprised to see that this passed CI since it should throw an AttributeError exception. Upon further investigation, I found that there is only a single test for this module and it uses ignore_errors: yes. 😱
There was a problem hiding this comment.
😭I wanted to replicate some things from the runner... but I didn't pay attention at the naming and the other things... my fault!
I also noticed k8s_service is not fully covered with integration tests.
There was a problem hiding this comment.
@gravesm Can we open some issues for the modules that are not fully covered by integration tests?
plugins/modules/k8s_service.py
Outdated
| except CoreException as e: | ||
| if module.warnings: | ||
| e["msg"] += "\n" + "\n ".join(module.warnings) | ||
| module.fail_json(msg=e) |
There was a problem hiding this comment.
I think msg needs to be a string.
There was a problem hiding this comment.
Yes, it needs. Thanks!
plugins/modules/k8s_service.py
Outdated
|
|
||
| try: | ||
| result = perform_action(svc, definition, module.params) |
There was a problem hiding this comment.
This function is infinitely recursive as it just keeps calling itself. Did you mean to import the perform_action() function from runner.py?
There was a problem hiding this comment.
It should replicate perform_action() from runner. But I did it wrong and trying to check if possible.
alinabuzachis
force-pushed
the
migrate_k8s_service
branch
4 times, most recently
from
33a81e9
to
43c4f7f
Compare
Signed-off-by: Alina Buzachis <[email protected]>
alinabuzachis
force-pushed
the
migrate_k8s_service
branch
from
43c4f7f
to
a0fdae2
Compare
![ansible-zuul[bot]](https://avatars.githubusercontent.com/in/27800?s=60&v=4){kind=small}
|
recheck |
1 similar comment
|
recheck |
|
recheck |
1 similar comment
|
recheck |
…s#327) Refactor k8s_service to use new module_utils code SUMMARY Refactor k8s_service to use new module_utils code ISSUE TYPE Feature Pull Request COMPONENT NAME k8s_service Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Alina Buzachis <None> Reviewed-by: None <None>
SUMMARY
Refactor k8s_service to use new module_utils code
ISSUE TYPE
COMPONENT NAME
k8s_service