From ec0df00fc1552faa19f64a2577170f3bd27a8041 Mon Sep 17 00:00:00 2001 From: Nick Cellino Date: Thu, 25 Jan 2024 12:27:36 -0500 Subject: [PATCH] Add finalizer to link resource (#20321) * Add finalizer to link resource * Update internal/hcp/internal/controllers/link/controller.go Co-authored-by: Semir Patel * Address PR style feedback --------- Co-authored-by: Semir Patel --- .../internal/controllers/link/controller.go | 17 ++++- .../controllers/link/controller_test.go | 33 ++++++++- .../internal/controllers/link/finalizer.go | 74 +++++++++++++++++++ 3 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 internal/hcp/internal/controllers/link/finalizer.go diff --git a/internal/hcp/internal/controllers/link/controller.go b/internal/hcp/internal/controllers/link/controller.go index ce27f371ae..d7ad99ba4b 100644 --- a/internal/hcp/internal/controllers/link/controller.go +++ b/internal/hcp/internal/controllers/link/controller.go @@ -7,10 +7,9 @@ import ( "context" "strings" + gnmmod "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/models" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - - gnmmod "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/models" "google.golang.org/protobuf/types/known/anypb" "github.com/hashicorp/consul/agent/hcp/bootstrap" @@ -18,6 +17,7 @@ import ( "github.com/hashicorp/consul/agent/hcp/config" "github.com/hashicorp/consul/internal/controller" "github.com/hashicorp/consul/internal/hcp/internal/types" + "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/storage" pbhcp "github.com/hashicorp/consul/proto-public/pbhcp/v2" "github.com/hashicorp/consul/proto-public/pbresource" @@ -108,6 +108,19 @@ func (r *linkReconciler) Reconcile(ctx context.Context, rt controller.Runtime, r return err } + if err = addFinalizer(ctx, rt, res); err != nil { + rt.Logger.Error("error adding finalizer to link resource", "error", err) + return err + } + + if resource.IsMarkedForDeletion(res) { + if err = cleanup(ctx, rt, res, r.dataDir); err != nil { + rt.Logger.Error("error cleaning up link resource", "error", err) + return err + } + return nil + } + // Validation - Ensure V2 Resource APIs are not enabled unless hcpAllowV2ResourceApis flag is set var newStatus *pbresource.Status if r.resourceApisEnabled && !r.hcpAllowV2ResourceApis { diff --git a/internal/hcp/internal/controllers/link/controller_test.go b/internal/hcp/internal/controllers/link/controller_test.go index b25bdf214e..d4b911390e 100644 --- a/internal/hcp/internal/controllers/link/controller_test.go +++ b/internal/hcp/internal/controllers/link/controller_test.go @@ -6,24 +6,28 @@ package link import ( "context" "fmt" + "os" + "path/filepath" "testing" + "github.com/hashicorp/go-uuid" + gnmmod "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/models" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - "github.com/hashicorp/go-uuid" - gnmmod "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/models" - svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" + "github.com/hashicorp/consul/agent/hcp/bootstrap" hcpclient "github.com/hashicorp/consul/agent/hcp/client" "github.com/hashicorp/consul/agent/hcp/config" "github.com/hashicorp/consul/internal/controller" "github.com/hashicorp/consul/internal/hcp/internal/types" + "github.com/hashicorp/consul/internal/resource" rtest "github.com/hashicorp/consul/internal/resource/resourcetest" pbhcp "github.com/hashicorp/consul/proto-public/pbhcp/v2" "github.com/hashicorp/consul/proto-public/pbresource" "github.com/hashicorp/consul/sdk/testutil" + "github.com/hashicorp/consul/sdk/testutil/retry" ) type controllerSuite struct { @@ -34,6 +38,7 @@ type controllerSuite struct { rt controller.Runtime tenancies []*pbresource.Tenancy + dataDir string } func mockHcpClientFn(t *testing.T) (*hcpclient.MockClient, HCPClientFn) { @@ -68,6 +73,18 @@ func TestLinkController(t *testing.T) { func (suite *controllerSuite) deleteResourceFunc(id *pbresource.ID) func() { return func() { suite.client.MustDelete(suite.T(), id) + + // Ensure hcp-config directory is removed + retry.Run(suite.T(), func(r *retry.R) { + if suite.dataDir != "" { + file := filepath.Join(suite.dataDir, bootstrap.SubDir) + if _, err := os.Stat(file); !os.IsNotExist(err) { + r.Fatalf("should have removed hcp-config directory") + } + } + }) + + suite.client.WaitForDeletion(suite.T(), id) } } @@ -90,6 +107,7 @@ func (suite *controllerSuite) TestController_Ok() { }, nil).Once() dataDir := testutil.TempDir(suite.T(), "test-link-controller") + suite.dataDir = dataDir mgr.Register(LinkController( false, false, @@ -112,6 +130,11 @@ func (suite *controllerSuite) TestController_Ok() { suite.T().Cleanup(suite.deleteResourceFunc(link.Id)) + // Ensure finalizer was added + suite.client.WaitForResourceState(suite.T(), link.Id, func(t rtest.T, res *pbresource.Resource) { + require.True(t, resource.HasFinalizer(res, StatusKey), "link resource does not have finalizer") + }) + suite.client.WaitForStatusCondition(suite.T(), link.Id, StatusKey, ConditionLinked(linkData.ResourceId)) var updatedLink pbhcp.Link updatedLinkResource := suite.client.WaitForNewVersion(suite.T(), link.Id, link.Version) @@ -138,6 +161,7 @@ func (suite *controllerSuite) TestController_Initialize() { } dataDir := testutil.TempDir(suite.T(), "test-link-controller") + suite.dataDir = dataDir mgr.Register(LinkController( false, @@ -176,6 +200,7 @@ func (suite *controllerSuite) TestControllerResourceApisEnabled_LinkDisabled() { mgr := controller.NewManager(suite.client, suite.rt.Logger) _, mockClientFunc := mockHcpClientFn(suite.T()) dataDir := testutil.TempDir(suite.T(), "test-link-controller") + suite.dataDir = dataDir mgr.Register(LinkController( true, false, @@ -217,6 +242,7 @@ func (suite *controllerSuite) TestControllerResourceApisEnabledWithOverride_Link }, nil).Once() dataDir := testutil.TempDir(suite.T(), "test-link-controller") + suite.dataDir = dataDir mgr.Register(LinkController( true, @@ -271,6 +297,7 @@ func (suite *controllerSuite) TestController_GetClusterError() { mockClient.EXPECT().GetCluster(mock.Anything).Return(nil, tc.expectErr) dataDir := testutil.TempDir(suite.T(), "test-link-controller") + suite.dataDir = dataDir mgr.Register(LinkController( true, true, diff --git a/internal/hcp/internal/controllers/link/finalizer.go b/internal/hcp/internal/controllers/link/finalizer.go new file mode 100644 index 0000000000..b29a7c70a4 --- /dev/null +++ b/internal/hcp/internal/controllers/link/finalizer.go @@ -0,0 +1,74 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package link + +import ( + "context" + "os" + "path/filepath" + + "github.com/hashicorp/consul/agent/hcp/bootstrap" + "github.com/hashicorp/consul/internal/controller" + "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/proto-public/pbresource" +) + +func cleanup(ctx context.Context, rt controller.Runtime, res *pbresource.Resource, dataDir string) error { + rt.Logger.Trace("cleaning up link resource") + if dataDir != "" { + hcpConfigDir := filepath.Join(dataDir, bootstrap.SubDir) + rt.Logger.Debug("deleting hcp-config dir", "dir", hcpConfigDir) + err := os.RemoveAll(hcpConfigDir) + if err != nil { + return err + } + } + + err := ensureDeleted(ctx, rt, res) + if err != nil { + return err + } + + return nil +} + +func addFinalizer(ctx context.Context, rt controller.Runtime, res *pbresource.Resource) error { + // The statusKey doubles as the finalizer name for the link resource + if resource.HasFinalizer(res, StatusKey) { + rt.Logger.Trace("already has finalizer") + return nil + } + + // Finalizer hasn't been written, so add it. + resource.AddFinalizer(res, StatusKey) + _, err := rt.Client.Write(ctx, &pbresource.WriteRequest{Resource: res}) + if err != nil { + return err + } + rt.Logger.Trace("added finalizer") + return err +} + +// ensureDeleted makes sure a link is finally deleted +func ensureDeleted(ctx context.Context, rt controller.Runtime, res *pbresource.Resource) error { + // Remove finalizer if present + if resource.HasFinalizer(res, StatusKey) { + resource.RemoveFinalizer(res, StatusKey) + _, err := rt.Client.Write(ctx, &pbresource.WriteRequest{Resource: res}) + if err != nil { + return err + } + rt.Logger.Trace("removed finalizer") + } + + // Finally, delete the link + _, err := rt.Client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id}) + if err != nil { + return err + } + + // Success + rt.Logger.Trace("finally deleted") + return nil +}