Add logic to throw exception on workload group deletion with associated rules (#19502)

Signed-off-by: Kaushal Kumar <[email protected]>

Author: kaushalmahi12

CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Refactor to move prepareIndex and prepareDelete methods to Engine class ([#19551](https://github.com/opensearch-project/OpenSearch/pull/19551))
 
 ### Fixed
+- [WLM] add a check to stop workload group deletion having rules ([#19502](https://github.com/opensearch-project/OpenSearch/pull/19502))
 
 ### Dependencies
 - Bump `org.apache.zookeeper:zookeeper` from 3.9.3 to 3.9.4 ([#19535](https://github.com/opensearch-project/OpenSearch/pull/19535))

plugins/workload-management/src/internalClusterTest/java/org/opensearch/plugin/wlm/WlmAutoTaggingIT.java

@@ -784,7 +784,7 @@ public Collection<Object> createComponents(
                 wlmClusterSettingValuesProvider,
                 featureType
             );
-            return List.of(refreshMechanism);
+            return List.of(refreshMechanism, rulePersistenceService, featureType);
         }
 
         @Override

plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPlugin.java

@@ -171,7 +171,7 @@ public Collection<Object> createComponents(
             wlmClusterSettingValuesProvider,
             featureType
         );
-        return List.of(refreshMechanism);
+        return List.of(refreshMechanism, featureType, rulePersistenceService);
     }
 
     @Override

plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportDeleteWorkloadGroupAction.java

@@ -8,22 +8,34 @@
 
 package org.opensearch.plugin.wlm.action;
 
+import org.opensearch.ResourceNotFoundException;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.clustermanager.AcknowledgedResponse;
 import org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.block.ClusterBlockException;
 import org.opensearch.cluster.block.ClusterBlockLevel;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
+import org.opensearch.cluster.metadata.WorkloadGroup;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.inject.Inject;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.common.io.stream.StreamInput;
+import org.opensearch.plugin.wlm.rule.WorkloadGroupFeatureType;
 import org.opensearch.plugin.wlm.service.WorkloadGroupPersistenceService;
+import org.opensearch.rule.RulePersistenceService;
+import org.opensearch.rule.action.GetRuleRequest;
+import org.opensearch.rule.action.GetRuleResponse;
+import org.opensearch.rule.autotagging.FeatureType;
+import org.opensearch.rule.autotagging.Rule;
+import org.opensearch.rule.service.IndexStoredRulePersistenceService;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportService;
 
 import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 
 /**
  * Transport action for delete WorkloadGroup
@@ -35,6 +47,8 @@ public class TransportDeleteWorkloadGroupAction extends TransportClusterManagerN
     AcknowledgedResponse> {
 
     private final WorkloadGroupPersistenceService workloadGroupPersistenceService;
+    private final RulePersistenceService rulePersistenceService;
+    private final FeatureType featureType;
 
     /**
      * Constructor for TransportDeleteWorkloadGroupAction
@@ -45,6 +59,8 @@ public class TransportDeleteWorkloadGroupAction extends TransportClusterManagerN
      * @param threadPool - a {@link ThreadPool} object
      * @param indexNameExpressionResolver - a {@link IndexNameExpressionResolver} object
      * @param workloadGroupPersistenceService - a {@link WorkloadGroupPersistenceService} object
+     * @param persistenceService - a {@link IndexStoredRulePersistenceService} instance
+     * @param featureType - workloadManagement feature type
      */
     @Inject
     public TransportDeleteWorkloadGroupAction(
@@ -53,7 +69,9 @@ public TransportDeleteWorkloadGroupAction(
         ActionFilters actionFilters,
         ThreadPool threadPool,
         IndexNameExpressionResolver indexNameExpressionResolver,
-        WorkloadGroupPersistenceService workloadGroupPersistenceService
+        WorkloadGroupPersistenceService workloadGroupPersistenceService,
+        IndexStoredRulePersistenceService persistenceService,
+        WorkloadGroupFeatureType featureType
     ) {
         super(
             DeleteWorkloadGroupAction.NAME,
@@ -65,6 +83,8 @@ public TransportDeleteWorkloadGroupAction(
             indexNameExpressionResolver
         );
         this.workloadGroupPersistenceService = workloadGroupPersistenceService;
+        this.rulePersistenceService = persistenceService;
+        this.featureType = featureType;
     }
 
     @Override
@@ -73,12 +93,18 @@ protected void clusterManagerOperation(
         ClusterState state,
         ActionListener<AcknowledgedResponse> listener
     ) throws Exception {
-        workloadGroupPersistenceService.deleteInClusterStateMetadata(request, listener);
+        threadPool.executor(executor()).submit(() -> {
+            try {
+                checkNoAssociatedRulesExist(request, listener, state);
+            } catch (Exception e) {
+                listener.onFailure(e);
+            }
+        });
     }
 
     @Override
     protected String executor() {
-        return ThreadPool.Names.SAME;
+        return ThreadPool.Names.GET;
     }
 
     @Override
@@ -90,4 +116,48 @@ protected AcknowledgedResponse read(StreamInput in) throws IOException {
     protected ClusterBlockException checkBlock(DeleteWorkloadGroupRequest request, ClusterState state) {
         return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE);
     }
+
+    private void checkNoAssociatedRulesExist(
+        DeleteWorkloadGroupRequest request,
+        ActionListener<AcknowledgedResponse> listener,
+        ClusterState state
+    ) {
+        Collection<WorkloadGroup> workloadGroups = WorkloadGroupPersistenceService.getFromClusterStateMetadata(request.getName(), state);
+        if (workloadGroups.isEmpty()) {
+            throw new ResourceNotFoundException("No WorkloadGroup exists with the provided name: " + request.getName());
+        }
+
+        WorkloadGroup workloadGroup = workloadGroups.iterator().next();
+        rulePersistenceService.getRule(
+            new GetRuleRequest(null, Collections.emptyMap(), null, featureType),
+            new ActionListener<GetRuleResponse>() {
+                @Override
+                public void onResponse(GetRuleResponse getRuleResponse) {
+                    List<Rule> associatedRules = getRuleResponse.getRules()
+                        .stream()
+                        .filter(rule -> rule.getFeatureValue().equals(workloadGroup.get_id()))
+                        .toList();
+
+                    if (!associatedRules.isEmpty()) {
+                        listener.onFailure(
+                            new IllegalStateException(
+                                workloadGroup.getName()
+                                    + " workload group has rules with ids: "
+                                    + associatedRules
+                                    + " ."
+                                    + "Please delete them first otherwise system will be an inconsistent state."
+                            )
+                        );
+                        return;
+                    }
+                    workloadGroupPersistenceService.deleteInClusterStateMetadata(request, listener);
+                }
+
+                @Override
+                public void onFailure(Exception e) {
+                    listener.onFailure(e);
+                }
+            }
+        );
+    }
 }

plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/WorkloadGroupPersistenceService.java

@@ -236,14 +236,18 @@ protected AcknowledgedResponse newResponse(boolean acknowledged) {
      */
     ClusterState deleteWorkloadGroupInClusterState(final String name, final ClusterState currentClusterState) {
         final Metadata metadata = currentClusterState.metadata();
-        final WorkloadGroup workloadGroupToRemove = metadata.workloadGroups()
+        final WorkloadGroup workloadGroupToRemove = getWorkloadGroup(name, metadata);
+
+        return ClusterState.builder(currentClusterState).metadata(Metadata.builder(metadata).remove(workloadGroupToRemove).build()).build();
+    }
+
+    private static WorkloadGroup getWorkloadGroup(String name, Metadata metadata) {
+        return metadata.workloadGroups()
             .values()
             .stream()
             .filter(workloadGroup -> workloadGroup.getName().equals(name))
             .findAny()
             .orElseThrow(() -> new ResourceNotFoundException("No WorkloadGroup exists with the provided name: " + name));
-
-        return ClusterState.builder(currentClusterState).metadata(Metadata.builder(metadata).remove(workloadGroupToRemove).build()).build();
     }
 
     /**

plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/WorkloadManagementPluginTests.java

@@ -50,6 +50,7 @@
 import java.util.function.Supplier;
 
 import static org.opensearch.plugin.wlm.WorkloadManagementPlugin.PRINCIPAL_ATTRIBUTE_NAME;
+import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
@@ -187,8 +188,7 @@ public void testCreateComponentsReturnsRefreshMechanism() {
             mockRepositoriesServiceSupplier
         );
 
-        assertEquals(1, components.size());
-        assertTrue(components.iterator().next() instanceof RefreshBasedSyncMechanism);
+        assertThat(components.stream().filter(c -> c instanceof RefreshBasedSyncMechanism).count(), equalTo(1L));
     }
 
     public void testSetAttributesWithMock() {