From 9ac6f728906c789c44dd6dff89d79bb2fd6a7508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Tue, 10 Jan 2023 16:18:32 +0100 Subject: [PATCH 1/6] feat(gke): allow synchronous teardown --- pkg/clusters/types/gke/builder.go | 29 +++++++++++------ pkg/clusters/types/gke/cluster.go | 52 +++++++++++++++++++++++++------ test/e2e/gke_cluster_test.go | 2 ++ 3 files changed, 64 insertions(+), 19 deletions(-) diff --git a/pkg/clusters/types/gke/builder.go b/pkg/clusters/types/gke/builder.go index d542d039..69bc43f7 100644 --- a/pkg/clusters/types/gke/builder.go +++ b/pkg/clusters/types/gke/builder.go @@ -7,7 +7,7 @@ import ( "sync" "time" - containerpb "cloud.google.com/go/container/apiv1/containerpb" + "cloud.google.com/go/container/apiv1/containerpb" "github.com/blang/semver/v4" "github.com/google/uuid" @@ -21,6 +21,7 @@ type Builder struct { Name string project, location string jsonCreds []byte + waitForTeardown bool addons clusters.Addons clusterVersion *semver.Version @@ -60,6 +61,15 @@ func (b *Builder) WithClusterMinorVersion(major, minor uint64) *Builder { return b } +// WithWaitForTeardown sets a flag telling whether the cluster should wait for +// a cleanup operation synchronously. +// +// Default: `false`. +func (b *Builder) WithWaitForTeardown(wait bool) *Builder { + b.waitForTeardown = wait + return b +} + // Build creates and configures clients for a GKE-based Kubernetes clusters.Cluster. func (b *Builder) Build(ctx context.Context) (clusters.Cluster, error) { // validate the credential contents by finding the IAM service account @@ -157,14 +167,15 @@ func (b *Builder) Build(ctx context.Context) (clusters.Cluster, error) { } cluster := &Cluster{ - name: b.Name, - project: b.project, - location: b.location, - jsonCreds: b.jsonCreds, - client: k8s, - cfg: restCFG, - addons: make(clusters.Addons), - l: &sync.RWMutex{}, + name: b.Name, + project: b.project, + location: b.location, + jsonCreds: b.jsonCreds, + waitForTeardown: b.waitForTeardown, + client: k8s, + cfg: restCFG, + addons: make(clusters.Addons), + l: &sync.RWMutex{}, } if err := utils.ClusterInitHooks(ctx, cluster); err != nil { diff --git a/pkg/clusters/types/gke/cluster.go b/pkg/clusters/types/gke/cluster.go index 40fcb414..0a66b10e 100644 --- a/pkg/clusters/types/gke/cluster.go +++ b/pkg/clusters/types/gke/cluster.go @@ -6,8 +6,10 @@ import ( "os" "strings" "sync" + "time" container "cloud.google.com/go/container/apiv1" + "cloud.google.com/go/container/apiv1/containerpb" "github.com/blang/semver/v4" "google.golang.org/api/option" "k8s.io/client-go/kubernetes" @@ -22,14 +24,15 @@ import ( // Cluster is a clusters.Cluster implementation backed by Google Kubernetes Engine (GKE) type Cluster struct { - name string - project string - location string - jsonCreds []byte - client *kubernetes.Clientset - cfg *rest.Config - addons clusters.Addons - l *sync.RWMutex + name string + project string + location string + jsonCreds []byte + waitForTeardown bool + client *kubernetes.Clientset + cfg *rest.Config + addons clusters.Addons + l *sync.RWMutex } // NewFromExistingWithEnv provides a new clusters.Cluster backed by an existing GKE cluster, @@ -111,13 +114,42 @@ func (c *Cluster) Cleanup(ctx context.Context) error { } defer mgrc.Close() - _, err = deleteCluster(ctx, mgrc, c.name, c.project, c.location) - return err + teardownOp, err := deleteCluster(ctx, mgrc, c.name, c.project, c.location) + if err != nil { + return err + } + + fullTeardownOpName := fmt.Sprintf("projects/%s/locations/%s/operations/%s", c.project, c.location, teardownOp.Name) + if err := waitForTeardown(ctx, mgrc, fullTeardownOpName); err != nil { + return err + } + + return nil } return nil } +func waitForTeardown(ctx context.Context, mgrc *container.ClusterManagerClient, teardownOpName string) error { + ticker := time.NewTicker(time.Second) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + op, err := mgrc.GetOperation(ctx, &containerpb.GetOperationRequest{Name: teardownOpName}) + if err != nil { + return err + } + if op.Status == containerpb.Operation_DONE { + return nil + } + } + } +} + func (c *Cluster) Client() *kubernetes.Clientset { return c.client } diff --git a/test/e2e/gke_cluster_test.go b/test/e2e/gke_cluster_test.go index 00e06cab..3b066550 100644 --- a/test/e2e/gke_cluster_test.go +++ b/test/e2e/gke_cluster_test.go @@ -54,6 +54,7 @@ func TestGKECluster(t *testing.T) { t.Logf("configuring the GKE cluster PROJECT=(%s) LOCATION=(%s)", gkeProject, gkeLocation) builder := gke.NewBuilder([]byte(gkeCreds), gkeProject, gkeLocation) builder.WithClusterMinorVersion(1, 23) + builder.WithWaitForTeardown(true) t.Logf("building cluster %s (this can take some time)", builder.Name) cluster, err := builder.Build(ctx) @@ -61,6 +62,7 @@ func TestGKECluster(t *testing.T) { t.Logf("setting up cleanup for cluster %s", cluster.Name()) defer func() { + t.Log("running cluster cleanup") assert.NoError(t, cluster.Cleanup(ctx)) }() From a194f748e3bc2e5d3afbbd1202859e271bcaba77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Tue, 10 Jan 2023 16:21:52 +0100 Subject: [PATCH 2/6] changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4f28595..5e954fe8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog -## Unreleased +## v0.25.0 + +- GKE cluster is able to wait for its cleanup synchronously. + [#491](https://github.com/Kong/kubernetes-testing-framework/pull/491) ## v0.24.1 From 7d4b28cbd13bfecfd813d56d4ba96f44e84b6c4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Tue, 10 Jan 2023 16:44:56 +0100 Subject: [PATCH 3/6] add cluster name to the log msg --- test/e2e/gke_cluster_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/gke_cluster_test.go b/test/e2e/gke_cluster_test.go index 3b066550..7142aaea 100644 --- a/test/e2e/gke_cluster_test.go +++ b/test/e2e/gke_cluster_test.go @@ -62,7 +62,7 @@ func TestGKECluster(t *testing.T) { t.Logf("setting up cleanup for cluster %s", cluster.Name()) defer func() { - t.Log("running cluster cleanup") + t.Logf("running cluster cleanup for %s", cluster.Name()) assert.NoError(t, cluster.Cleanup(ctx)) }() From e65512ae74342936808869b84761fd01077076f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Tue, 10 Jan 2023 16:46:27 +0100 Subject: [PATCH 4/6] don't mark release in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e954fe8..f0bd11cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## v0.25.0 +## Unreleased - GKE cluster is able to wait for its cleanup synchronously. [#491](https://github.com/Kong/kubernetes-testing-framework/pull/491) From 5001804b4d87ed502fef7f89d088d3c5cf132d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Tue, 10 Jan 2023 19:37:26 +0100 Subject: [PATCH 5/6] Update CHANGELOG.md --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3f4c4b4..d51af953 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,6 @@ [#490](https://github.com/Kong/kubernetes-testing-framework/pull/490) - GKE cluster is able to wait for its cleanup synchronously. [#491](https://github.com/Kong/kubernetes-testing-framework/pull/491) - - MetalLB addon will use an extended timeout when fetching manifests from GH which should improve its stability. [#492](https://github.com/Kong/kubernetes-testing-framework/pull/492) From d15c153d2e97201fc4b07fa3fa6cfffaa98f538f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Tue, 10 Jan 2023 19:54:28 +0100 Subject: [PATCH 6/6] use the flag --- pkg/clusters/types/gke/cluster.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/clusters/types/gke/cluster.go b/pkg/clusters/types/gke/cluster.go index 0a66b10e..c4b6b54c 100644 --- a/pkg/clusters/types/gke/cluster.go +++ b/pkg/clusters/types/gke/cluster.go @@ -119,9 +119,11 @@ func (c *Cluster) Cleanup(ctx context.Context) error { return err } - fullTeardownOpName := fmt.Sprintf("projects/%s/locations/%s/operations/%s", c.project, c.location, teardownOp.Name) - if err := waitForTeardown(ctx, mgrc, fullTeardownOpName); err != nil { - return err + if c.waitForTeardown { + fullTeardownOpName := fmt.Sprintf("projects/%s/locations/%s/operations/%s", c.project, c.location, teardownOp.Name) + if err := waitForTeardown(ctx, mgrc, fullTeardownOpName); err != nil { + return err + } } return nil