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

fix multiple issues with dry_run logic #561

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelogs/fragments/561-fix-dry-run.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
bugfixes:
- Fix dry_run logic - Pass the value dry_run=All instead of dry_run=True to the client, add conditional check on kubernetes client version as this feature is supported only for kubernetes >= 18.20.0 (https://github.com/ansible-collections/kubernetes.core/pull/561).
8 changes: 5 additions & 3 deletions plugins/module_utils/k8s/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ class K8SClient:
If there is a need for other methods or attributes to be proxied, they can be added here.
"""

K8S_SERVER_DRY_RUN = "All"

def __init__(self, configuration, client, dry_run: bool = False) -> None:
self.configuration = configuration
self.client = client
Expand Down Expand Up @@ -305,7 +307,7 @@ def resource(self, kind: str, api_version: str) -> Resource:

def _ensure_dry_run(self, params: Dict) -> Dict:
if self.dry_run:
params["dry_run"] = True
params["dry_run"] = self.K8S_SERVER_DRY_RUN
return params

def validate(
Expand Down Expand Up @@ -354,8 +356,8 @@ def get_api_client(module=None, **kwargs: Optional[Any]) -> K8SClient:
raise CoreException(msg) from e

dry_run = False
if module:
dry_run = module.params.get("dry_run", False)
if module and module.server_side_dry_run:
dry_run = True

k8s_client = K8SClient(
configuration=configuration,
Expand Down
4 changes: 4 additions & 0 deletions plugins/module_utils/k8s/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ def __init__(self, **kwargs) -> None:
def check_mode(self):
return self._module.check_mode

@property
def server_side_dry_run(self):
return self.check_mode and self.has_at_least("kubernetes", "18.20.0")

@property
def _diff(self):
return self._module._diff
Expand Down
29 changes: 10 additions & 19 deletions plugins/module_utils/k8s/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ def __init__(self, client, module) -> None:
self.client = client
self.module = module

@property
def _client_side_dry_run(self):
return self.module.check_mode and not self.client.dry_run

def find_resource(
self, kind: str, api_version: str, fail: bool = False
) -> Optional[Resource]:
Expand Down Expand Up @@ -153,8 +157,6 @@ def patch_resource(
)
try:
params = dict(name=name, namespace=namespace)
if self.module.check_mode:
params["dry_run"] = "All"
if merge_type:
params["content_type"] = "application/{0}-patch+json".format(merge_type)
return self.client.patch(resource, definition, **params).to_dict()
Expand Down Expand Up @@ -306,15 +308,12 @@ def create(self, resource: Resource, definition: Dict) -> Dict:
namespace = definition["metadata"].get("namespace")
name = definition["metadata"].get("name")

if self.module.check_mode and not self.client.dry_run:
if self._client_side_dry_run:
k8s_obj = _encode_stringdata(definition)
else:
params = {}
if self.module.check_mode:
params["dry_run"] = "All"
try:
k8s_obj = self.client.create(
resource, definition, namespace=namespace, **params
resource, definition, namespace=namespace
).to_dict()
except ConflictError:
# Some resources, like ProjectRequests, can't be created multiple times,
Expand Down Expand Up @@ -344,7 +343,7 @@ def apply(
server_side_apply = self.module.params.get("server_side_apply")
if server_side_apply:
requires("kubernetes", "19.15.0", reason="to use server side apply")
if self.module.check_mode and not self.client.dry_run:
if self._client_side_dry_run:
ignored, patch = apply_object(resource, _encode_stringdata(definition))
if existing:
k8s_obj = dict_merge(existing.to_dict(), patch)
Expand All @@ -353,8 +352,6 @@ def apply(
else:
try:
params = {}
if self.module.check_mode:
params["dry_run"] = "All"
if server_side_apply:
params["server_side"] = True
params.update(server_side_apply)
Expand All @@ -377,20 +374,16 @@ def replace(
name = definition["metadata"].get("name")
namespace = definition["metadata"].get("namespace")

if self.module.check_mode and not self.module.client.dry_run:
if self._client_side_dry_run:
k8s_obj = _encode_stringdata(definition)
else:
params = {}
if self.module.check_mode:
params["dry_run"] = "All"
try:
k8s_obj = self.client.replace(
resource,
definition,
name=name,
namespace=namespace,
append_hash=append_hash,
**params
).to_dict()
except Exception as e:
reason = e.body if hasattr(e, "body") else e
Expand All @@ -404,7 +397,7 @@ def update(
name = definition["metadata"].get("name")
namespace = definition["metadata"].get("namespace")

if self.module.check_mode and not self.client.dry_run:
if self._client_side_dry_run:
k8s_obj = dict_merge(existing.to_dict(), _encode_stringdata(definition))
else:
exception = None
Expand Down Expand Up @@ -445,7 +438,7 @@ def delete(
return {}

# Delete the object
if self.module.check_mode and not self.client.dry_run:
if self._client_side_dry_run:
return {}

if name:
Expand All @@ -465,8 +458,6 @@ def delete(
body.update(delete_options)
params["body"] = body

if self.module.check_mode:
params["dry_run"] = "All"
try:
k8s_obj = self.client.delete(resource, **params).to_dict()
except Exception as e:
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/targets/k8s_check_mode/aliases
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
time=30
k8s
k8s_info
10 changes: 10 additions & 0 deletions tests/integration/targets/k8s_check_mode/defaults/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
test_namespace: "check-mode-ns"

test_config:
- k8s_release: "kubernetes < 18.20.0"
dry_run: false
virtualenv: 'env1'
- k8s_release: "kubernetes >= 18.20.0"
dry_run: true
virtualenv: 'env2'
3 changes: 3 additions & 0 deletions tests/integration/targets/k8s_check_mode/meta/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
dependencies:
- remove_namespace
32 changes: 32 additions & 0 deletions tests/integration/targets/k8s_check_mode/tasks/check_mode.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
- name: Create virtualenv with kubernetes release
pip:
name:
- '{{ item.k8s_release }}'
virtualenv_command: "virtualenv --python {{ ansible_python_interpreter }}"
virtualenv: "{{ tmpdir }}/{{ item.virtualenv }}"

- name: Create resource using check mode
k8s:
kind: Namespace
name: '{{ test_namespace }}'
check_mode: true
register: _create

- name: Ensure namespace was not created
k8s_info:
kind: Namespace
name: '{{ test_namespace }}'
register: info
failed_when: info.resources | length > 0

- name: Ensure server side dry_run has being used
assert:
that:
- '"creationTimestamp" in _create.result.metadata'
when: item.dry_run

- name: Ensure server side dry_run was not used
assert:
that:
- '"creationTimestamp" in _create.result.metadata'
when: not item.dry_run
19 changes: 19 additions & 0 deletions tests/integration/targets/k8s_check_mode/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
- name: create temporary directory for tests
tempfile:
suffix: .k8s
state: directory
register: _path

- block:
- include_tasks: tasks/check_mode.yml
with_items: '{{ test_config }}'

vars:
tmpdir: '{{ _path.path }}'

always:
- name: Delete temporaray directory
file:
state: absent
path: '{{ tmpdir }}'
ignore_errors: true