k8s: wait for delete to be complete on trigger/full rebuild (#5262)

This adapts the code originally added in #4626 to apply to other delete cases. Previously, it was only done in `deleteAndCreate`. However, `Delete` is also called explicitly as part of a resource trigger/full rebuild, so the same eventual issue applies where Tilt is going to (almost) immediately re-create the entities it just deleted and might hit conflicts as a result.
tilt-dev · Dec 9, 2021 · 0edb9ad · 0edb9ad
1 parent ad3edb9
commit 0edb9ad
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 29 deletions.
diff --git a/internal/cli/down.go b/internal/cli/down.go
@@ -136,7 +136,7 @@ func deleteK8sEntities(ctx context.Context, manifests []model.Manifest, updateSe
 	errs := []error{}
 	if len(entities) > 0 {
 		dCtx, cancel := context.WithTimeout(ctx, updateSettings.K8sUpsertTimeout())
-		err = downDeps.kClient.Delete(dCtx, entities)
+		err = downDeps.kClient.Delete(dCtx, entities, false)
 		cancel()
 		if err != nil {
 			errs = append(errs, errors.Wrap(err, "Deleting k8s entities"))

diff --git a/internal/controllers/core/kubernetesapply/reconciler.go b/internal/controllers/core/kubernetesapply/reconciler.go
@@ -684,7 +684,7 @@ func (r *Reconciler) bestEffortDelete(ctx context.Context, toDelete deleteSpec)
 			l.Infof("→ %s", displayName)
 		}
 
-		err := r.k8sClient.Delete(ctx, toDelete.entities)
+		err := r.k8sClient.Delete(ctx, toDelete.entities, false)
 		if err != nil {
 			l.Errorf("Error garbage collecting Kubernetes resources: %v", err)
 		}

diff --git a/internal/engine/buildcontrol/image_build_and_deployer.go b/internal/engine/buildcontrol/image_build_and_deployer.go
@@ -367,7 +367,8 @@ func (ibd *ImageBuildAndDeployer) delete(ctx context.Context, k8sTarget model.K8
 
 	entities = k8s.ReverseSortedEntities(entities)
 
-	return ibd.k8sClient.Delete(ctx, entities)
+	// wait for entities to be fully deleted from the server so that it's safe to re-create them
+	return ibd.k8sClient.Delete(ctx, entities, true)
 }
 
 // Create a new ImageTarget with the Dockerfiles rewritten with the injected images.

diff --git a/internal/k8s/client.go b/internal/k8s/client.go
@@ -89,11 +89,11 @@ type Client interface {
 	// than they were passed in) and with UUIDs from the Kube API
 	Upsert(ctx context.Context, entities []K8sEntity, timeout time.Duration) ([]K8sEntity, error)
 
-	// Deletes all given entities.
+	// Delete all given entities, optionally waiting for them to be fully deleted.
 	//
 	// Currently ignores any "not found" errors, because that seems like the correct
 	// behavior for our use cases.
-	Delete(ctx context.Context, entities []K8sEntity) error
+	Delete(ctx context.Context, entities []K8sEntity, wait bool) error
 
 	GetMetaByReference(ctx context.Context, ref v1.ObjectReference) (metav1.Object, error)
 	ListMeta(ctx context.Context, gvk schema.GroupVersionKind, ns Namespace) ([]metav1.Object, error)
@@ -481,25 +481,8 @@ func (k *K8sClient) deleteAndCreate(list kube.ResourceList) (*kube.Result, error
 		return nil, errors.Wrap(err, "kubernetes delete")
 	}
 
-	var wg sync.WaitGroup
-
-	for _, r := range list {
-
-		wg.Add(1)
-		go func(resourceInfo *resource.Info) {
-			waitOpt := &wait.WaitOptions{
-				DynamicClient: k.dynamic,
-				IOStreams:     genericclioptions.NewTestIOStreamsDiscard(),
-				Timeout:       30 * time.Second,
-				ForCondition:  "delete",
-			}
-
-			_, _, _ = wait.IsDeleted(resourceInfo, waitOpt)
-			wg.Done()
-		}(r)
-	}
-
-	wg.Wait()
+	// ensure the delete has finished before attempting to recreate
+	k.waitForDelete(list)
 
 	result, err := k.resourceClient.Create(list)
 	if err != nil {
@@ -572,7 +555,7 @@ func maybeAnnotationsTooLong(stderr string) (string, bool) {
 //
 // Currently ignores any "not found" errors, because that seems like the correct
 // behavior for our use cases.
-func (k *K8sClient) Delete(ctx context.Context, entities []K8sEntity) error {
+func (k *K8sClient) Delete(ctx context.Context, entities []K8sEntity, wait bool) error {
 	l := logger.Get(ctx)
 	l.Infof("Deleting kubernetes objects:")
 	for _, e := range entities {
@@ -597,6 +580,10 @@ func (k *K8sClient) Delete(ctx context.Context, entities []K8sEntity) error {
 		return errors.Wrap(err, "kubernetes delete")
 	}
 
+	if wait {
+		k.waitForDelete(resources)
+	}
+
 	return nil
 }
 
@@ -622,6 +609,25 @@ func (k *K8sClient) forceDiscovery(ctx context.Context, gvk schema.GroupVersionK
 	return rm.Resource, nil
 }
 
+func (k *K8sClient) waitForDelete(list kube.ResourceList) {
+	var wg sync.WaitGroup
+	for _, r := range list {
+		wg.Add(1)
+		go func(resourceInfo *resource.Info) {
+			waitOpt := &wait.WaitOptions{
+				DynamicClient: k.dynamic,
+				IOStreams:     genericclioptions.NewTestIOStreamsDiscard(),
+				Timeout:       30 * time.Second,
+				ForCondition:  "delete",
+			}
+
+			_, _, _ = wait.IsDeleted(resourceInfo, waitOpt)
+			wg.Done()
+		}(r)
+	}
+	wg.Wait()
+}
+
 func (k *K8sClient) ListMeta(ctx context.Context, gvk schema.GroupVersionKind, ns Namespace) ([]metav1.Object, error) {
 	gvr, err := k.forceDiscovery(ctx, gvk)
 	if err != nil {

diff --git a/internal/k8s/client_test.go b/internal/k8s/client_test.go
@@ -54,7 +54,7 @@ func TestDelete(t *testing.T) {
 	f := newClientTestFixture(t)
 	postgres, err := ParseYAMLFromString(testyaml.PostgresYAML)
 	assert.Nil(t, err)
-	err = f.client.Delete(f.ctx, postgres)
+	err = f.client.Delete(f.ctx, postgres, true)
 	assert.Nil(t, err)
 	assert.Equal(t, 5, len(f.resourceClient.deletes))
 }
@@ -70,7 +70,7 @@ func TestDeleteMissingKind(t *testing.T) {
 
 	postgres, err := ParseYAMLFromString(testyaml.PostgresYAML)
 	assert.Nil(t, err)
-	err = f.client.Delete(f.ctx, postgres)
+	err = f.client.Delete(f.ctx, postgres, true)
 	assert.Nil(t, err)
 	assert.Equal(t, 4, len(f.resourceClient.deletes))
 

diff --git a/internal/k8s/exploding_client.go b/internal/k8s/exploding_client.go
@@ -27,7 +27,7 @@ func (ec *explodingClient) Upsert(ctx context.Context, entities []K8sEntity, tim
 	return nil, errors.Wrap(ec.err, "could not set up kubernetes client")
 }
 
-func (ec *explodingClient) Delete(ctx context.Context, entities []K8sEntity) error {
+func (ec *explodingClient) Delete(ctx context.Context, entities []K8sEntity, wait bool) error {
 	return errors.Wrap(ec.err, "could not set up kubernetes client")
 }
 

diff --git a/internal/k8s/fake_client.go b/internal/k8s/fake_client.go
@@ -377,7 +377,7 @@ func (c *FakeK8sClient) Upsert(_ context.Context, entities []K8sEntity, timeout
 	return result, nil
 }
 
-func (c *FakeK8sClient) Delete(ctx context.Context, entities []K8sEntity) error {
+func (c *FakeK8sClient) Delete(_ context.Context, entities []K8sEntity, wait bool) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()