Merge pull request #156 from pnguyen44/HYPERFLEET-1041/nodepool-force-delete

HYPERFLEET-1041 - feat: add force-delete endpoint for nodepools

Author: openshift-merge-bot[bot]

pkg/api/force_delete_types.go

@@ -0,0 +1,7 @@
+package api
+
+// TODO(HYPERFLEET-1075): Move to hyperfleet-api-spec and generate via oapi-codegen.
+// ForceDeleteRequest is the request body for force-delete endpoints.
+type ForceDeleteRequest struct {
+	Reason string `json:"reason"`
+}

pkg/handlers/cluster_nodepools.go

@@ -138,6 +138,30 @@ func (h ClusterNodePoolsHandler) SoftDelete(w http.ResponseWriter, r *http.Reque
 	handleSoftDelete(w, r, cfg, http.StatusAccepted)
 }
 
+// ForceDelete permanently removes a nodepool that is in Finalizing state.
+func (h ClusterNodePoolsHandler) ForceDelete(w http.ResponseWriter, r *http.Request) {
+	var req api.ForceDeleteRequest
+	cfg := &handlerConfig{
+		MarshalInto: &req,
+		Validate: []validate{
+			validateNotEmpty(&req, "Reason", "reason"),
+		},
+		Action: func() (interface{}, *errors.ServiceError) {
+			clusterID := mux.Vars(r)["id"]
+			nodePoolID := mux.Vars(r)["nodepool_id"]
+			ctx := r.Context()
+			if _, err := h.nodePoolService.GetByIDAndOwner(ctx, nodePoolID, clusterID); err != nil {
+				return nil, err
+			}
+			if err := h.nodePoolService.ForceDelete(ctx, nodePoolID, req.Reason); err != nil {
+				return nil, err
+			}
+			return nil, nil
+		},
+	}
+	handleForceDelete(w, r, cfg)
+}
+
 // Patch patches a specific nodepool for a cluster
 func (h ClusterNodePoolsHandler) Patch(w http.ResponseWriter, r *http.Request) {
 	var patch api.NodePoolPatchRequest

pkg/handlers/cluster_nodepools_test.go

@@ -553,3 +553,125 @@ func TestClusterNodePoolsHandler_Patch(t *testing.T) {
 		})
 	}
 }
+
+func TestClusterNodePoolsHandler_ForceDelete(t *testing.T) {
+	RegisterTestingT(t)
+
+	clusterID := testClusterID
+	nodePoolID := testNodePoolID
+
+	tests := []struct {
+		setupMocks         func(ctrl *gomock.Controller) (*services.MockClusterService, *services.MockNodePoolService)
+		name               string
+		nodePoolID         string
+		body               string
+		expectedStatusCode int
+	}{
+		{
+			name:       "Success 204 - nodepool force-deleted",
+			nodePoolID: nodePoolID,
+			body:       `{"reason": "Stuck in finalizing for 2 hours"}`,
+			setupMocks: func(ctrl *gomock.Controller) (*services.MockClusterService, *services.MockNodePoolService) {
+				mockClusterSvc := services.NewMockClusterService(ctrl)
+				mockNodePoolSvc := services.NewMockNodePoolService(ctrl)
+				mockNodePoolSvc.EXPECT().
+					GetByIDAndOwner(gomock.Any(), nodePoolID, clusterID).
+					Return(&api.NodePool{}, nil)
+				mockNodePoolSvc.EXPECT().
+					ForceDelete(gomock.Any(), nodePoolID, "Stuck in finalizing for 2 hours").
+					Return(nil)
+				return mockClusterSvc, mockNodePoolSvc
+			},
+			expectedStatusCode: http.StatusNoContent,
+		},
+		{
+			name:       "Error 404 - nodepool not found",
+			nodePoolID: "non-existent-id",
+			body:       `{"reason": "some reason"}`,
+			setupMocks: func(ctrl *gomock.Controller) (*services.MockClusterService, *services.MockNodePoolService) {
+				mockClusterSvc := services.NewMockClusterService(ctrl)
+				mockNodePoolSvc := services.NewMockNodePoolService(ctrl)
+				mockNodePoolSvc.EXPECT().
+					GetByIDAndOwner(gomock.Any(), "non-existent-id", clusterID).
+					Return(nil, errors.NotFound("NodePool with id='non-existent-id' not found"))
+				return mockClusterSvc, mockNodePoolSvc
+			},
+			expectedStatusCode: http.StatusNotFound,
+		},
+		{
+			name:       "Error 409 - nodepool not in Finalizing state",
+			nodePoolID: nodePoolID,
+			body:       `{"reason": "some reason"}`,
+			setupMocks: func(ctrl *gomock.Controller) (*services.MockClusterService, *services.MockNodePoolService) {
+				mockClusterSvc := services.NewMockClusterService(ctrl)
+				mockNodePoolSvc := services.NewMockNodePoolService(ctrl)
+				mockNodePoolSvc.EXPECT().
+					GetByIDAndOwner(gomock.Any(), nodePoolID, clusterID).
+					Return(&api.NodePool{}, nil)
+				mockNodePoolSvc.EXPECT().
+					ForceDelete(gomock.Any(), nodePoolID, "some reason").
+					Return(errors.ConflictState("NodePool '%s' is not in Finalizing state", nodePoolID))
+				return mockClusterSvc, mockNodePoolSvc
+			},
+			expectedStatusCode: http.StatusConflict,
+		},
+		{
+			name:       "Error 400 - empty reason",
+			nodePoolID: nodePoolID,
+			body:       `{"reason": ""}`,
+			setupMocks: func(ctrl *gomock.Controller) (*services.MockClusterService, *services.MockNodePoolService) {
+				mockClusterSvc := services.NewMockClusterService(ctrl)
+				mockNodePoolSvc := services.NewMockNodePoolService(ctrl)
+				return mockClusterSvc, mockNodePoolSvc
+			},
+			expectedStatusCode: http.StatusBadRequest,
+		},
+		{
+			name:       "Error 400 - malformed JSON",
+			nodePoolID: nodePoolID,
+			body:       `not json`,
+			setupMocks: func(ctrl *gomock.Controller) (*services.MockClusterService, *services.MockNodePoolService) {
+				mockClusterSvc := services.NewMockClusterService(ctrl)
+				mockNodePoolSvc := services.NewMockNodePoolService(ctrl)
+				return mockClusterSvc, mockNodePoolSvc
+			},
+			expectedStatusCode: http.StatusBadRequest,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			RegisterTestingT(t)
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockClusterSvc, mockNodePoolSvc := tt.setupMocks(ctrl)
+			handler := NewClusterNodePoolsHandler(mockClusterSvc, mockNodePoolSvc)
+
+			reqURL := "/api/hyperfleet/v1/clusters/" + clusterID + "/nodepools/" + tt.nodePoolID + "/force-delete"
+			req := httptest.NewRequest(http.MethodPost, reqURL, strings.NewReader(tt.body))
+			req.Header.Set("Content-Type", "application/json")
+			req = mux.SetURLVars(req, map[string]string{
+				"id":          clusterID,
+				"nodepool_id": tt.nodePoolID,
+			})
+
+			rr := httptest.NewRecorder()
+			handler.ForceDelete(rr, req)
+
+			Expect(rr.Code).To(Equal(tt.expectedStatusCode))
+
+			if tt.expectedStatusCode == http.StatusNoContent {
+				Expect(rr.Body.Len()).To(Equal(0))
+			}
+
+			if tt.expectedStatusCode == http.StatusConflict {
+				var errResp openapi.Error
+				err := json.Unmarshal(rr.Body.Bytes(), &errResp)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(*errResp.Detail).To(ContainSubstring("not in Finalizing state"))
+			}
+		})
+	}
+}

pkg/handlers/framework.go

@@ -135,6 +135,42 @@ func handleList(w http.ResponseWriter, r *http.Request, cfg *handlerConfig) {
 	writeJSONResponse(w, r, http.StatusOK, results)
 }
 
+// handleForceDelete reads the request body, unmarshals it into cfg.MarshalInto,
+// runs validators, executes the action, and returns 204 No Content on success.
+func handleForceDelete(w http.ResponseWriter, r *http.Request, cfg *handlerConfig) {
+	if cfg.ErrorHandler == nil {
+		cfg.ErrorHandler = handleError
+	}
+
+	bytes, err := io.ReadAll(r.Body)
+	if err != nil {
+		handleError(r, w, errors.MalformedRequest("Unable to read request body: %s", err))
+		return
+	}
+
+	err = json.Unmarshal(bytes, &cfg.MarshalInto)
+	if err != nil {
+		handleError(r, w, errors.MalformedRequest("Invalid request format: %s", err))
+		return
+	}
+
+	for _, v := range cfg.Validate {
+		err := v()
+		if err != nil {
+			cfg.ErrorHandler(r, w, err)
+			return
+		}
+	}
+
+	_, serviceErr := cfg.Action()
+	if serviceErr != nil {
+		cfg.ErrorHandler(r, w, serviceErr)
+		return
+	}
+
+	w.WriteHeader(http.StatusNoContent)
+}
+
 // handleCreateWithNoContent handles create requests that may return 204 No Content
 // If action returns (nil, nil), it means a no-op and returns 204 No Content
 // Otherwise, it returns 201 Created with the result

pkg/services/cluster_test.go

@@ -106,7 +106,9 @@ func (d *mockClusterDao) All(ctx context.Context) (api.ClusterList, error) {
 var _ dao.ClusterDao = &mockClusterDao{}
 
 type mockAdapterStatusDao struct {
-	statuses map[string]*api.AdapterStatus
+	statuses            map[string]*api.AdapterStatus
+	findByResourceErr   error
+	deleteByResourceErr error
 }
 
 func newMockAdapterStatusDao() *mockAdapterStatusDao {
@@ -157,6 +159,9 @@ func (d *mockAdapterStatusDao) Delete(ctx context.Context, id string) error {
 }
 
 func (d *mockAdapterStatusDao) DeleteByResource(ctx context.Context, resourceType, resourceID string) error {
+	if d.deleteByResourceErr != nil {
+		return d.deleteByResourceErr
+	}
 	for key, s := range d.statuses {
 		if s.ResourceType == resourceType && s.ResourceID == resourceID {
 			delete(d.statuses, key)
@@ -169,6 +174,9 @@ func (d *mockAdapterStatusDao) FindByResource(
 	ctx context.Context,
 	resourceType, resourceID string,
 ) (api.AdapterStatusList, error) {
+	if d.findByResourceErr != nil {
+		return nil, d.findByResourceErr
+	}
 	var result api.AdapterStatusList
 	for _, s := range d.statuses {
 		if s.ResourceType == resourceType && s.ResourceID == resourceID {
@@ -278,6 +286,9 @@ func (m *mockNodePoolService) CascadeSoftDelete(
 }
 
 func (m *mockNodePoolService) Delete(context.Context, string) *errors.ServiceError { return nil }
+func (m *mockNodePoolService) ForceDelete(context.Context, string, string) *errors.ServiceError {
+	return nil
+}
 
 func (m *mockNodePoolService) All(context.Context) (api.NodePoolList, *errors.ServiceError) {
 	return nil, nil

pkg/services/node_pool.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api"
+	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/auth"
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/config"
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/dao"
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors"
@@ -29,6 +30,7 @@ type NodePoolService interface {
 		ctx context.Context, nodePools api.NodePoolList, deletedBy string, deletedTime time.Time,
 	) *errors.ServiceError
 	Delete(ctx context.Context, id string) *errors.ServiceError
+	ForceDelete(ctx context.Context, id string, reason string) *errors.ServiceError
 	All(ctx context.Context) (api.NodePoolList, *errors.ServiceError)
 
 	FindByIDs(ctx context.Context, ids []string) (api.NodePoolList, *errors.ServiceError)
@@ -244,6 +246,61 @@ func (s *sqlNodePoolService) Delete(ctx context.Context, id string) *errors.Serv
 	return nil
 }
 
+func (s *sqlNodePoolService) ForceDelete(ctx context.Context, id string, reason string) *errors.ServiceError {
+	nodePool, err := s.nodePoolDao.GetForUpdate(ctx, id)
+	if err != nil {
+		return handleGetError("NodePool", "id", id, err)
+	}
+
+	if nodePool.DeletedTime == nil {
+		return errors.ConflictState("NodePool '%s' is not in Finalizing state", id)
+	}
+
+	statuses, err := s.adapterStatusDao.FindByResource(ctx, "NodePool", id)
+	if err != nil {
+		return errors.GeneralError("Failed to fetch adapter statuses for nodepool '%s': %s", id, err)
+	}
+
+	caller := auth.GetUsernameFromContext(ctx)
+	if caller == "" {
+		caller = "unknown"
+	}
+
+	type adapterSummary struct {
+		Conditions map[string]string `json:"conditions"`
+		Adapter    string            `json:"adapter"`
+	}
+	summaries := make([]adapterSummary, 0, len(statuses))
+	for _, st := range statuses {
+		conds := make(map[string]string)
+		var parsed []api.AdapterCondition
+		if err := json.Unmarshal(st.Conditions, &parsed); err == nil {
+			for _, c := range parsed {
+				conds[c.Type] = string(c.Status)
+			}
+		}
+		summaries = append(summaries, adapterSummary{Adapter: st.Adapter, Conditions: conds})
+	}
+
+	logger.With(ctx,
+		"nodepool_id", id,
+		"resource_type", "NodePool",
+		"caller", caller,
+		"reason", reason,
+		"adapter_statuses", summaries,
+	).Info("Force-deleting nodepool")
+
+	if err := s.adapterStatusDao.DeleteByResource(ctx, "NodePool", id); err != nil {
+		return errors.GeneralError("Failed to delete adapter statuses for nodepool '%s': %s", id, err)
+	}
+
+	if err := s.nodePoolDao.Delete(ctx, id); err != nil {
+		return errors.GeneralError("Failed to force-delete nodepool '%s': %s", id, err)
+	}
+
+	return nil
+}
+
 func (s *sqlNodePoolService) FindByIDs(ctx context.Context, ids []string) (api.NodePoolList, *errors.ServiceError) {
 	nodePools, err := s.nodePoolDao.FindByIDs(ctx, ids)
 	if err != nil {