diff --git a/docs/guide/usage/crud-operations.md b/docs/guide/usage/crud-operations.md index 1fb2b4f..fd33baa 100644 --- a/docs/guide/usage/crud-operations.md +++ b/docs/guide/usage/crud-operations.md @@ -184,25 +184,35 @@ The `delete()` method accepts optional parameters for fine-grained control: public function delete( array $query = ['pretty' => 1], ?int $gracePeriod = null, - string $propagationPolicy = 'Foreground' + PropagationPolicy|string $propagationPolicy = PropagationPolicy::FOREGROUND ): bool ``` Example with options: ```php +use RenokiCo\PhpK8s\Enums\PropagationPolicy; + // Delete with 30-second grace period $pod->delete( query: ['pretty' => 1], gracePeriod: 30, - propagationPolicy: 'Foreground' + propagationPolicy: PropagationPolicy::FOREGROUND ); ``` **Propagation Policies:** -- `Foreground` - Waits for dependents to be deleted first -- `Background` - Deletes immediately, dependents deleted in background -- `Orphan` - Leaves dependents orphaned +- `PropagationPolicy::FOREGROUND` - Waits for dependents to be deleted first +- `PropagationPolicy::BACKGROUND` - Deletes immediately, dependents deleted in background +- `PropagationPolicy::ORPHAN` - Leaves dependents orphaned + +The equivalent strings (`'Foreground'`, `'Background'`, `'Orphan'`) are still +accepted; anything else raises an `InvalidArgumentException` before the request +is sent, rather than being rejected by the API. + +Under `Foreground`, the resource stays readable until its dependents are gone, +so a `get()` immediately after `delete()` returns the resource with a +`deletionTimestamp` instead of a 404. ## Creating or Updating Resources diff --git a/src/Enums/PropagationPolicy.php b/src/Enums/PropagationPolicy.php new file mode 100644 index 0000000..833ac98 --- /dev/null +++ b/src/Enums/PropagationPolicy.php @@ -0,0 +1,13 @@ + 1]): bool /** * Delete the resource. * - * @param null|int $gracePeriod - * * @throws KubernetesAPIException */ - public function delete(array $query = ['pretty' => 1], $gracePeriod = null, string $propagationPolicy = 'Foreground'): bool + public function delete(array $query = ['pretty' => 1], ?int $gracePeriod = null, PropagationPolicy|string $propagationPolicy = PropagationPolicy::FOREGROUND): bool { if (! $this->isSynced()) { return true; } - $this->setAttribute('preconditions', [ - 'resourceVersion' => $this->getResourceVersion(), - 'uid' => $this->getResourceUid(), - 'propagationPolicy' => $propagationPolicy, - 'gracePeriodSeconds' => $gracePeriod, - ]); - $this->refresh(); + $payload = json_encode( + $this->deleteOptions($gracePeriod, $propagationPolicy), + JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES + ); + $this->cluster ->setResourceClass(get_class($this)) ->runOperation( Operation::DELETE, $this->resourcePath(), - $this->toJsonPayload('DeleteOptions'), + $payload, $query ); @@ -266,6 +264,48 @@ public function delete(array $query = ['pretty' => 1], $gracePeriod = null, stri return true; } + /** + * Build the DeleteOptions payload for the delete operation. + * The propagationPolicy and gracePeriodSeconds fields are top-level + * DeleteOptions fields; only the resource uid belongs in preconditions. + * + * @return array + * + * @throws InvalidArgumentException + */ + public function deleteOptions(?int $gracePeriod = null, PropagationPolicy|string $propagationPolicy = PropagationPolicy::FOREGROUND): array + { + $policy = $propagationPolicy instanceof PropagationPolicy + ? $propagationPolicy + : PropagationPolicy::tryFrom($propagationPolicy); + + if (is_null($policy)) { + throw new InvalidArgumentException(sprintf( + 'Invalid propagation policy "%s"; expected one of %s', + $propagationPolicy, + implode(', ', array_column(PropagationPolicy::cases(), 'value')) + )); + } + + $options = [ + 'apiVersion' => 'v1', + 'kind' => 'DeleteOptions', + 'propagationPolicy' => $policy->value, + ]; + + if (! is_null($gracePeriod)) { + $options['gracePeriodSeconds'] = $gracePeriod; + } + + if (! is_null($uid = $this->getResourceUid())) { + $options['preconditions'] = [ + 'uid' => $uid, + ]; + } + + return $options; + } + /** * Apply the resource using server-side apply. * diff --git a/tests/CSIDriverTest.php b/tests/CSIDriverTest.php index fec5a16..50ed962 100644 --- a/tests/CSIDriverTest.php +++ b/tests/CSIDriverTest.php @@ -139,6 +139,8 @@ public function runDeletionTests() $this->assertTrue($csiDriver->delete()); + $this->waitUntilDeleted($csiDriver); + $this->expectException(KubernetesAPIException::class); $this->cluster->getCSIDriverByName('test-csi-driver.example.com'); diff --git a/tests/DeleteOptionsTest.php b/tests/DeleteOptionsTest.php new file mode 100644 index 0000000..1343ca1 --- /dev/null +++ b/tests/DeleteOptionsTest.php @@ -0,0 +1,156 @@ +cluster->job()->setName('reap'); + + $job->syncWith([ + 'metadata' => [ + 'name' => 'reap', + 'uid' => 'aa11bb22-cc33-dd44-ee55-ff6677889900', + 'resourceVersion' => '12345', + ], + ]); + + $options = $job->deleteOptions(null, 'Foreground'); + + $this->assertSame('DeleteOptions', $options['kind']); + $this->assertSame('v1', $options['apiVersion']); + $this->assertSame('Foreground', $options['propagationPolicy']); + $this->assertSame(['uid' => 'aa11bb22-cc33-dd44-ee55-ff6677889900'], $options['preconditions']); + $this->assertArrayNotHasKey('propagationPolicy', $options['preconditions']); + $this->assertArrayNotHasKey('gracePeriodSeconds', $options['preconditions']); + $this->assertArrayNotHasKey('gracePeriodSeconds', $options); + } + + public function test_grace_period_is_a_top_level_delete_options_field() + { + $job = $this->cluster->job()->setName('reap'); + + $job->syncWith([ + 'metadata' => [ + 'name' => 'reap', + 'uid' => 'aa11bb22-cc33-dd44-ee55-ff6677889900', + ], + ]); + + $options = $job->deleteOptions(30, 'Background'); + + $this->assertSame('Background', $options['propagationPolicy']); + $this->assertSame(30, $options['gracePeriodSeconds']); + $this->assertSame(['uid' => 'aa11bb22-cc33-dd44-ee55-ff6677889900'], $options['preconditions']); + } + + public function test_preconditions_are_omitted_without_a_resource_uid() + { + $job = $this->cluster->job()->setName('reap'); + + $options = $job->deleteOptions(null, 'Foreground'); + + $this->assertSame('Foreground', $options['propagationPolicy']); + $this->assertArrayNotHasKey('preconditions', $options); + } + + public function test_invalid_propagation_policy_is_rejected_before_the_request() + { + $job = $this->cluster->job()->setName('reap'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid propagation policy "foreground"'); + + $job->deleteOptions(null, 'foreground'); + } + + public function test_propagation_policy_accepts_the_enum() + { + $job = $this->cluster->job()->setName('reap'); + + $this->assertSame( + 'Orphan', + $job->deleteOptions(null, PropagationPolicy::ORPHAN)['propagationPolicy'] + ); + } + + public function test_delete_sends_the_propagation_policy_at_the_top_level() + { + $cluster = (new RecordingCluster('http://127.0.0.1:8080')) + ->respondWith([ + ['kind' => 'Job', 'apiVersion' => 'batch/v1', 'metadata' => [ + 'name' => 'reap', + 'uid' => 'aa11bb22-cc33-dd44-ee55-ff6677889900', + ]], + ['kind' => 'Status', 'apiVersion' => 'v1', 'status' => 'Success'], + ]); + + $job = $cluster->job()->setName('reap'); + $job->syncWith([ + 'metadata' => [ + 'name' => 'reap', + 'uid' => 'aa11bb22-cc33-dd44-ee55-ff6677889900', + ], + ]); + + $this->assertTrue($job->delete(gracePeriod: 30, propagationPolicy: PropagationPolicy::BACKGROUND)); + + $requests = $cluster->requests(); + + $this->assertCount(2, $requests, 'delete() refreshes the resource, then deletes it'); + $this->assertSame('GET', $requests[0]->getMethod()); + + $delete = $requests[1]; + $this->assertSame('DELETE', $delete->getMethod()); + + $sent = json_decode((string) $delete->getBody(), true); + + $this->assertSame('Background', $sent['propagationPolicy'] ?? null); + $this->assertSame(30, $sent['gracePeriodSeconds'] ?? null); + $this->assertSame( + ['uid' => 'aa11bb22-cc33-dd44-ee55-ff6677889900'], + $sent['preconditions'] ?? null, + 'only the uid belongs in preconditions' + ); + } + + public function test_delete_options_payload_shape_matches_the_kubernetes_api() + { + $job = $this->cluster->job()->setName('reap'); + + $job->syncWith([ + 'metadata' => [ + 'name' => 'reap', + 'uid' => 'aa11bb22-cc33-dd44-ee55-ff6677889900', + ], + 'spec' => [ + 'template' => [ + 'spec' => [ + 'containers' => [], + ], + ], + ], + ]); + + $payload = $job->deleteOptions(null, 'Foreground'); + + $this->assertSame( + [ + 'apiVersion' => 'v1', + 'kind' => 'DeleteOptions', + 'propagationPolicy' => 'Foreground', + 'preconditions' => [ + 'uid' => 'aa11bb22-cc33-dd44-ee55-ff6677889900', + ], + ], + $payload + ); + + $this->assertArrayNotHasKey('spec', $payload); + $this->assertArrayNotHasKey('metadata', $payload); + } +} diff --git a/tests/IngressTest.php b/tests/IngressTest.php index 2ce0e4d..0c65aab 100644 --- a/tests/IngressTest.php +++ b/tests/IngressTest.php @@ -178,6 +178,8 @@ public function runDeletionTests() $this->assertTrue($ingress->delete()); + $this->waitUntilDeleted($ingress); + $this->expectException(KubernetesAPIException::class); $this->cluster->getIngressByName('nginx'); diff --git a/tests/RecordingCluster.php b/tests/RecordingCluster.php new file mode 100644 index 0000000..d4c5e64 --- /dev/null +++ b/tests/RecordingCluster.php @@ -0,0 +1,73 @@ + */ + public array $transactions = []; + + protected ?MockHandler $handler = null; + + protected ?Client $client = null; + + /** + * @param array> $bodies + */ + public function respondWith(array $bodies): static + { + $this->handler ??= new MockHandler; + + foreach ($bodies as $body) { + $this->handler->append( + new Response(200, ['Content-Type' => 'application/json'], json_encode($body)) + ); + } + + return $this; + } + + /** + * One client, and therefore one response queue, for the life of the + * cluster: a fresh handler per call would replay the first response to + * every request and no multi-request flow could be modelled. + */ + #[Override] + public function getClient(): Client + { + if ($this->client === null) { + $this->handler ??= new MockHandler; + + $stack = HandlerStack::create($this->handler); + $stack->push(Middleware::history($this->transactions)); + + $this->client = new Client(['handler' => $stack]); + } + + return $this->client; + } + + /** + * @return array + */ + public function requests(): array + { + return array_map( + fn (array $transaction): RequestInterface => $transaction['request'], + $this->transactions + ); + } +} diff --git a/tests/SecretTest.php b/tests/SecretTest.php index 20e644c..6f5ab17 100644 --- a/tests/SecretTest.php +++ b/tests/SecretTest.php @@ -149,6 +149,8 @@ public function runDeletionTests() $this->assertTrue($secret->delete()); + $this->waitUntilDeleted($secret); + $this->expectException(KubernetesAPIException::class); $this->cluster->getSecretByName('passwords'); diff --git a/tests/StorageClassTest.php b/tests/StorageClassTest.php index ecd96bd..8dac68c 100644 --- a/tests/StorageClassTest.php +++ b/tests/StorageClassTest.php @@ -130,6 +130,8 @@ public function runDeletionTests() $this->assertTrue($sc->delete()); + $this->waitUntilDeleted($sc); + $this->expectException(KubernetesAPIException::class); $this->cluster->getStorageClassByName('io1'); diff --git a/tests/TestCase.php b/tests/TestCase.php index d15c88a..36f2375 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -7,6 +7,7 @@ use RenokiCo\PhpK8s\Instances\Container; use RenokiCo\PhpK8s\K8s; use RenokiCo\PhpK8s\Kinds\K8sPod; +use RenokiCo\PhpK8s\Kinds\K8sResource; use RenokiCo\PhpK8s\KubernetesCluster; use RenokiCo\PhpK8s\PhpK8sServiceProvider; @@ -65,6 +66,28 @@ public function getEnvironmentSetUp($app) // } + /** + * Foreground propagation keeps a deleted resource readable until its + * dependents are gone, so a 404 is not immediate after delete(). + */ + protected function waitUntilDeleted(K8sResource $resource, int $timeout = 60): void + { + $deadline = time() + $timeout; + + while ($resource->exists()) { + if (time() >= $deadline) { + self::fail(sprintf( + '%s "%s" still exists %d seconds after delete()', + $resource::getKind(), + $resource->getName(), + $timeout + )); + } + + sleep(1); + } + } + /** * Create a standard mariadb container with common configuration. *