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 recreate ResourceClaim bug #114

Merged
merged 1 commit into from
Jul 10, 2024
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
14 changes: 12 additions & 2 deletions operator/resourcehandle.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@ async def bind_handle_to_claim(
# Check if there is already an assigned claim
resource_handle = cls.bound_instances.get((resource_claim.namespace, resource_claim.name))
if resource_handle:
return resource_handle
if await resource_handle.refetch():
logger.warning(f"Rebinding {resource_handle} to {resource_claim}")
return resource_handle
else:
logger.warning(f"Deleted {resource_handle} was still in memory cache")

matched_resource_handles = []
matched_resource_handle = None
Expand Down Expand Up @@ -556,14 +560,18 @@ def __register(self) -> None:
Add ResourceHandle to register of bound or unbound instances.
This method must be called with the ResourceHandle.lock held.
"""
# Ensure deleting resource handles are not cached
if self.is_deleting:
self.__unregister()
return
self.all_instances[self.name] = self
if self.is_bound:
self.bound_instances[(
self.resource_claim_namespace,
self.resource_claim_name
)] = self
self.unbound_instances.pop(self.name, None)
elif not self.is_deleting:
else:
self.unbound_instances[self.name] = self

def __unregister(self) -> None:
Expand Down Expand Up @@ -831,6 +839,8 @@ async def handle_delete(self, logger: kopf.ObjectLogger) -> None:
if resource_pool:
await resource_pool.manage(logger=logger)

self.__unregister()

async def handle_resource_event(self,
logger: Union[logging.Logger, logging.LoggerAdapter],
) -> None:
Expand Down
180 changes: 180 additions & 0 deletions test/roles/poolboy_test_simple/tasks/test-recreate-01.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
---
# Test to resolve issue of resource claim being recreated binds to deleted handle
- name: Create ResourceProvider test-recreate-01
kubernetes.core.k8s:
definition:
apiVersion: "{{ poolboy_domain }}/v1"
kind: ResourceProvider
metadata:
name: test-recreate-01
namespace: "{{ poolboy_namespace }}"
labels: >-
{{ {
poolboy_domain ~ "/test": "simple"
} }}
spec:
override:
apiVersion: "{{ poolboy_domain }}/v1"
kind: ResourceClaimTest
metadata:
namespace: "{{ poolboy_test_namespace }}"
spec:
foo: bar
validation:
openAPIV3Schema:
additionalProperties: false
properties:
metadata:
additionalProperties: false
properties:
name:
pattern: ^test-recreate-01-[a-z0-9]+$
type: string
required:
- name
type: object

- name: Create ResourceClaim test-recreate-01
kubernetes.core.k8s:
definition:
apiVersion: "{{ poolboy_domain }}/v1"
kind: ResourceClaim
metadata:
name: test-recreate-01
namespace: "{{ poolboy_test_namespace }}"
labels: >-
{{ {
poolboy_domain ~ "/test": "simple"
} }}
spec:
resources:
- provider:
name: test-recreate-01
template:
metadata:
name: test-recreate-01-a

- name: Verify handling of ResourceClaim test-recreate-01
kubernetes.core.k8s_info:
api_version: "{{ poolboy_domain }}/v1"
kind: ResourceClaim
name: test-recreate-01
namespace: "{{ poolboy_test_namespace }}"
register: r_get_resource_claim
failed_when: >-
r_get_resource_claim.resources[0].status.resources[0].state is undefined
until: r_get_resource_claim is success
delay: 1
retries: 10

- name: Save facts from for ResourceClaim test-recreate-01
set_fact:
resource_handle_name: >-
{{ r_get_resource_claim.resources[0].status.resourceHandle.name }}

- name: Verify state of ResourceClaim test-recreate-01
vars:
__state: "{{ r_get_resource_claim.resources[0] }}"
assert:
that:
- __state.status.resources | length == 1
- __state.status.resources[0].state.metadata.name == 'test-recreate-01-a'

- name: Verify creation of ResourceClaimTest test-recreate-01-a
kubernetes.core.k8s_info:
api_version: "{{ poolboy_domain }}/v1"
kind: ResourceClaimTest
name: test-recreate-01-a
namespace: "{{ poolboy_test_namespace }}"
register: r_get_resource_claim_test
failed_when: r_get_resource_claim_test.resources | length != 1
until: r_get_resource_claim_test is success
delay: 1
retries: 10

- name: Verify state of ResourceClaimTest test-recreate-01-a
vars:
__state: "{{ r_get_resource_claim_test.resources[0] }}"
assert:
that:
- __state.spec.foo == 'bar'

- name: Delete ResourceClaim test-recreate-01
kubernetes.core.k8s:
api_version: "{{ poolboy_domain }}/v1"
kind: ResourceClaim
name: test-recreate-01
namespace: "{{ poolboy_test_namespace }}"
state: absent

- name: Verify delete of ResourceClaim test-recreate-01
kubernetes.core.k8s_info:
api_version: "{{ poolboy_domain }}/v1"
kind: ResourceClaim
name: test-recreate-01
namespace: "{{ poolboy_test_namespace }}"
register: r_get_resource_claim
failed_when: r_get_resource_claim.resources | length != 0
until: r_get_resource_claim is success
retries: 5
delay: 1

- name: Verify delete of ResourceHandle for test-recreate-01
kubernetes.core.k8s_info:
api_version: "{{ poolboy_domain }}/v1"
kind: ResourceHandle
name: "{{ resource_handle_name }}"
namespace: "{{ poolboy_namespace }}"
register: r_get_resource_handle
failed_when: r_get_resource_handle.resources | length != 0
until: r_get_resource_handle is success
retries: 5
delay: 1

- name: Verify delete of ResourceClaimTest test-recreate-01-a
kubernetes.core.k8s_info:
api_version: "{{ poolboy_domain }}/v1"
kind: ResourceClaimTest
name: test-recreate-01-a
namespace: "{{ poolboy_test_namespace }}"
register: r_get_resource_claim_test
failed_when: r_get_resource_claim_test.resources | length != 0
until: r_get_resource_claim_test is success
delay: 1
retries: 10

- name: Receate ResourceClaim test-recreate-01
kubernetes.core.k8s:
definition:
apiVersion: "{{ poolboy_domain }}/v1"
kind: ResourceClaim
metadata:
name: test-recreate-01
namespace: "{{ poolboy_test_namespace }}"
labels: >-
{{ {
poolboy_domain ~ "/test": "simple"
} }}
spec:
resources:
- provider:
name: test-recreate-01
template:
metadata:
name: test-recreate-01-a

- name: Verify recreate handling of ResourceClaim test-recreate-01
kubernetes.core.k8s_info:
api_version: "{{ poolboy_domain }}/v1"
kind: ResourceClaim
name: test-recreate-01
namespace: "{{ poolboy_test_namespace }}"
register: r_get_resource_claim
failed_when: >-
r_get_resource_claim.resources[0].status.resources[0].state is undefined or
r_get_resource_claim.resources[0].status.resourceHandle.name is undefined or
r_get_resource_claim.resources[0].status.resourceHandle.name == resource_handle_name
until: r_get_resource_claim is success
delay: 1
retries: 10
...
1 change: 1 addition & 0 deletions test/roles/poolboy_test_simple/tasks/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- test-pool-02.yaml
- test-pool-03.yaml
- test-pool-04.yaml
- test-recreate-01.yaml
- test-vars-01.yaml
- test-vars-02.yaml
- test-vars-03.yaml
Expand Down
Loading