Fix recreate ResourceClaim bug (#114)

operator/resourcehandle.py

@@ -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
@@ -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:
@@ -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:

test/roles/poolboy_test_simple/tasks/test-recreate-01.yaml

@@ -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
+...

test/roles/poolboy_test_simple/tasks/test.yaml

@@ -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