Issue #3534: Tag compute resources according to the tool's metadata -…

… cleanup
epam · May 23, 2024 · e0a8b63 · e0a8b63
1 parent d35113c
commit e0a8b63
Show file tree

Hide file tree

Showing 13 changed files with 78 additions and 37 deletions.
diff --git a/api/src/main/java/com/epam/pipeline/manager/cloud/CloudFacade.java b/api/src/main/java/com/epam/pipeline/manager/cloud/CloudFacade.java
@@ -113,5 +113,5 @@ RunInstance scaleUpNode(Long runId, RunInstance instance, Map<String, String> ru
 
     boolean instanceScalingSupported(Long cloudRegionId);
 
-    void deleteInstanceTags(Long regionId, String instanceId, Set<String> tagNames);
+    void deleteInstanceTags(Long regionId, String runId, Set<String> tagNames);
 }
diff --git a/api/src/main/java/com/epam/pipeline/manager/cloud/CloudFacadeImpl.java b/api/src/main/java/com/epam/pipeline/manager/cloud/CloudFacadeImpl.java
@@ -322,9 +322,9 @@ public boolean instanceScalingSupported(final Long regionId) {
         return region.getProvider() != CloudProvider.LOCAL;
     }
 
-    public void deleteInstanceTags(final Long regionId, final String instanceId, final Set<String> tagNames) {
+    public void deleteInstanceTags(final Long regionId, final String runId, final Set<String> tagNames) {
         final AbstractCloudRegion region = regionManager.loadOrDefault(regionId);
-        getInstanceService(region).deleteInstanceTags(region, instanceId, tagNames);
+        getInstanceService(region).deleteInstanceTags(region, runId, tagNames);
     }
 
     private AbstractCloudRegion getRegionByRunId(final Long runId) {

diff --git a/api/src/main/java/com/epam/pipeline/manager/cloud/CloudInstanceService.java b/api/src/main/java/com/epam/pipeline/manager/cloud/CloudInstanceService.java
@@ -211,5 +211,5 @@ default boolean isNodeExpired(T region, Long runId, Integer keepAliveMinutes) {
 
     void adjustOfferRequest(InstanceOfferRequestVO requestVO);
 
-    void deleteInstanceTags(T region, String instanceId, Set<String> tagNames);
+    void deleteInstanceTags(T region, String runId, Set<String> tagNames);
 }
diff --git a/api/src/main/java/com/epam/pipeline/manager/cloud/aws/AWSInstanceService.java b/api/src/main/java/com/epam/pipeline/manager/cloud/aws/AWSInstanceService.java
@@ -364,8 +364,8 @@ public void adjustOfferRequest(final InstanceOfferRequestVO requestVO) {
     }
 
     @Override
-    public void deleteInstanceTags(final AwsRegion region, final String instanceId, final Set<String> tagNames) {
-        ec2Helper.deleteInstanceTags(region, instanceId, tagNames);
+    public void deleteInstanceTags(final AwsRegion region, final String runId, final Set<String> tagNames) {
+        ec2Helper.deleteInstanceTags(region, runId, tagNames);
     }
 
     private String buildNodeUpCommand(final AwsRegion region,

diff --git a/api/src/main/java/com/epam/pipeline/manager/cloud/aws/EC2Helper.java b/api/src/main/java/com/epam/pipeline/manager/cloud/aws/EC2Helper.java
@@ -82,6 +82,7 @@
 
 import java.time.LocalDateTime;
 import java.time.ZoneId;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
@@ -359,10 +360,16 @@ public void createAndAttachVolume(final String runId, final Long size,
         enableVolumeDeletionOnInstanceTermination(client, instance.getInstanceId(), device);
     }
 
-    public void deleteInstanceTags(final AwsRegion awsRegion, final String instanceId, final Set<String> tags) {
+    public void deleteInstanceTags(final AwsRegion awsRegion, final String runId, final Set<String> tags) {
         final AmazonEC2 client = getEC2Client(awsRegion);
+        final Instance instance = getAliveInstance(runId, awsRegion);
+
+        final List<String> resourcesIds = new ArrayList<>();
+        resourcesIds.add(instance.getInstanceId());
+        resourcesIds.addAll(getVolumeIds(instance));
+
         client.deleteTags(new DeleteTagsRequest()
-                .withResources(instanceId)
+                .withResources(resourcesIds)
                 .withTags(tags.stream()
                         .map(Tag::new)
                         .collect(Collectors.toList())));

diff --git a/api/src/main/java/com/epam/pipeline/manager/cloud/azure/AzureInstanceService.java b/api/src/main/java/com/epam/pipeline/manager/cloud/azure/AzureInstanceService.java
@@ -291,7 +291,7 @@ public void adjustOfferRequest(final InstanceOfferRequestVO requestVO) {
     }
 
     @Override
-    public void deleteInstanceTags(final AzureRegion region, final String instanceId, final Set<String> tagNames) {
+    public void deleteInstanceTags(final AzureRegion region, final String runId, final Set<String> tagNames) {
 
     }
 

diff --git a/api/src/main/java/com/epam/pipeline/manager/cloud/gcp/GCPInstanceService.java b/api/src/main/java/com/epam/pipeline/manager/cloud/gcp/GCPInstanceService.java
@@ -251,7 +251,7 @@ public void adjustOfferRequest(final InstanceOfferRequestVO requestVO) {
     }
 
     @Override
-    public void deleteInstanceTags(final GCPRegion region, final String instanceId, final Set<String> tagNames) {
+    public void deleteInstanceTags(final GCPRegion region, final String runId, final Set<String> tagNames) {
 
     }
 

diff --git a/api/src/main/java/com/epam/pipeline/manager/cloud/local/LocalInstanceService.java b/api/src/main/java/com/epam/pipeline/manager/cloud/local/LocalInstanceService.java
@@ -190,7 +190,7 @@ public void adjustOfferRequest(InstanceOfferRequestVO requestVO) {
     }
 
     @Override
-    public void deleteInstanceTags(final LocalRegion region, final String instanceId, final Set<String> tagNames) {
+    public void deleteInstanceTags(final LocalRegion region, final String runId, final Set<String> tagNames) {
 
     }
 

diff --git a/api/src/main/java/com/epam/pipeline/manager/metadata/MetadataManager.java b/api/src/main/java/com/epam/pipeline/manager/metadata/MetadataManager.java
@@ -47,6 +47,7 @@
 import org.apache.commons.collections4.MapUtils;
 import org.apache.commons.collections4.ListUtils;
 import org.apache.commons.lang.StringUtils;
+import org.apache.logging.log4j.util.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -60,7 +61,16 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.nio.charset.StandardCharsets;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -346,7 +356,6 @@ public Set<String> getMetadataKeys(final AclClass entityClass) {
 
     public Map<String, String> buildCustomInstanceTags(final PipelineRun run) {
         final Map<String, String> customTags = resolveCommonCustomInstanceTags(run);
-
         final Tool tool = toolManager.loadByNameOrId(run.getDockerImage());
         final MetadataEntry toolMetadata = loadMetadataItem(tool.getId(), AclClass.TOOL);
         return resolveInstanceTagsFromMetadata(toolMetadata, customTags);
@@ -420,8 +429,11 @@ private Map<String, String> resolveInstanceTagsFromMetadata(final MetadataEntry
         if (MapUtils.isEmpty(metadataData)) {
             return customTags;
         }
-        final Set<String> instanceTagsKeys = preferenceManager.getPreference(
-                SystemPreferences.CLUSTER_INSTANCE_ALLOWED_CUSTOM_TAGS);
+        final Set<String> instanceTagsKeys = new HashSet<>(Arrays.asList(preferenceManager.findPreference(
+                SystemPreferences.CLUSTER_INSTANCE_ALLOWED_CUSTOM_TAGS)
+                .filter(StringUtils::isNotBlank)
+                .map(value -> value.split(","))
+                .orElse(Strings.EMPTY_ARRAY)));
         if (CollectionUtils.isEmpty(instanceTagsKeys)) {
             return customTags;
         }

diff --git a/api/src/main/java/com/epam/pipeline/manager/pipeline/PipelineRunManager.java b/api/src/main/java/com/epam/pipeline/manager/pipeline/PipelineRunManager.java
@@ -672,6 +672,9 @@ public PipelineRun updatePipelineStatusIfNotFinal(Long runId, TaskStatus status)
                     pipelineRun.getId(), pipelineRun.getStatus());
             return pipelineRun;
         }
+        if (status.isFinal()) {
+            removeInstanceTags(pipelineRun);
+        }
         if (pipelineRun.getExecutionPreferences().getEnvironment() == ExecutionEnvironment.DTS
                 && status == TaskStatus.STOPPED) {
             configurationProviderManager.stop(runId, pipelineRun.getExecutionPreferences());
@@ -697,10 +700,6 @@ public PipelineRun updatePipelineStatus(final PipelineRun run) {
      */
     @Transactional(propagation = Propagation.REQUIRED)
     public PipelineRun stop(Long runId) {
-        final PipelineRun run = loadPipelineRun(runId);
-        final Map<String, String> tags = metadataManager.buildCustomInstanceTags(run);
-        final RunInstance instance = run.getInstance();
-        cloudFacade.deleteInstanceTags(instance.getCloudRegionId(), instance.getNodeName(), tags.keySet());
         return updatePipelineStatusIfNotFinal(runId, TaskStatus.STOPPED);
     }
 
@@ -1810,4 +1809,17 @@ private ContextualPreferenceExternalResource getRegionContextualPreference(final
                 : regionId.toString();
         return new ContextualPreferenceExternalResource(ContextualPreferenceLevel.REGION, regionIdStr);
     }
+
+    @SuppressWarnings("PMD.AvoidCatchingGenericException")
+    private void removeInstanceTags(final PipelineRun run) {
+        try {
+            final Map<String, String> tags = metadataManager.buildCustomInstanceTags(run);
+            if (MapUtils.isNotEmpty(tags)) {
+                final RunInstance instance = run.getInstance();
+                cloudFacade.deleteInstanceTags(instance.getCloudRegionId(), run.getId().toString(), tags.keySet());
+            }
+        } catch (Exception e) {
+            log.error("An error occurred during cloud resource tags removal for run '{}'", run.getId(), e);
+        }
+    }
 }
diff --git a/api/src/main/java/com/epam/pipeline/manager/preference/PreferenceValidators.java b/api/src/main/java/com/epam/pipeline/manager/preference/PreferenceValidators.java
@@ -32,10 +32,10 @@
 import com.epam.pipeline.security.ExternalServiceEndpoint;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import org.apache.commons.collections4.SetUtils;
 import org.apache.commons.lang.math.NumberUtils;
 import org.apache.commons.lang3.EnumUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.util.Strings;
 import org.springframework.scheduling.support.CronTrigger;
 import org.springframework.util.AntPathMatcher;
 
@@ -45,7 +45,11 @@
 import java.net.URLConnection;
 import java.nio.file.Files;
 import java.nio.file.Paths;
-import java.util.*;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Optional;
 import java.util.function.BiPredicate;
 
 import static com.epam.pipeline.manager.preference.SystemPreferences.DOCKER_SECURITY_TOOL_SCAN_CLAIR_ROOT_URL;
@@ -300,15 +304,16 @@ public static BiPredicate<String, Map<String, Preference>> isNullOrValidEnum(fin
         );
 
     public static final BiPredicate<String, Map<String, Preference>> isValidInstanceCustomTags =
-            isNullOrValidJson(new TypeReference<Set<String>>() {})
-                    .and((pref, dependencies) -> {
-                        final Set<String> customTags = JsonMapper.parseData(pref, new TypeReference<Set<String>>() {});
-                        if (SetUtils.emptyIfNull(customTags).stream()
-                                .anyMatch(tag -> tag.toUpperCase(Locale.ROOT).equals(NOT_ALLOWED_INSTANCE_TAG))) {
-                            throw new IllegalArgumentException("Tag 'Name' is not allowed for custom instance tags.");
-                        }
-                        return true;
-                    });
+            (pref, dependencies) -> {
+        if (Arrays.stream(Optional.ofNullable(pref)
+                        .filter(StringUtils::isNotBlank)
+                        .map(value -> value.split(","))
+                        .orElse(Strings.EMPTY_ARRAY))
+                .anyMatch(tag -> tag.toUpperCase(Locale.ROOT).equals(NOT_ALLOWED_INSTANCE_TAG))) {
+            throw new IllegalArgumentException("Tag 'Name' is not allowed for custom instance tags.");
+        }
+        return true;
+    };
 
     private PreferenceValidators() {
         // No-op

diff --git a/api/src/main/java/com/epam/pipeline/manager/preference/SystemPreferences.java b/api/src/main/java/com/epam/pipeline/manager/preference/SystemPreferences.java
@@ -655,12 +655,10 @@ public class SystemPreferences {
     public static final ObjectPreference<Map<CommonCustomInstanceTagsTypes, String>> CLUSTER_INSTANCE_TAGS =
             new ObjectPreference<>("cluster.instances.tags", null,
                     new TypeReference<Map<CommonCustomInstanceTagsTypes, String>>() {}, CLUSTER_GROUP,
-                    isValidInstanceCustomTags);
+                    isNullOrValidJson(new TypeReference<Map<CommonCustomInstanceTagsTypes, String>>() {}));
 
-    public static final ObjectPreference<Set<String>> CLUSTER_INSTANCE_ALLOWED_CUSTOM_TAGS =
-            new ObjectPreference<>("cluster.instances.allowed.custom.tags", null,
-                    new TypeReference<Set<String>>() {}, CLUSTER_GROUP,
-                    isNullOrValidJson(new TypeReference<Set<String>>() {}));
+    public static final StringPreference CLUSTER_INSTANCE_ALLOWED_CUSTOM_TAGS = new StringPreference(
+            "cluster.instances.allowed.custom.tags", null, CLUSTER_GROUP, isValidInstanceCustomTags);
 
     //LAUNCH_GROUP
     public static final StringPreference LAUNCH_CMD_TEMPLATE = new StringPreference("launch.cmd.template",

diff --git a/api/src/main/java/com/epam/pipeline/manager/security/metadata/MetadataPermissionManager.java b/api/src/main/java/com/epam/pipeline/manager/security/metadata/MetadataPermissionManager.java
@@ -33,9 +33,13 @@
 import org.apache.commons.collections4.ListUtils;
 import org.apache.commons.collections4.MapUtils;
 import org.apache.commons.collections4.SetUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.logging.log4j.util.Strings;
 import org.springframework.stereotype.Service;
 
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -162,8 +166,11 @@ private boolean isSameUser(final Long entityId) {
     }
 
     private boolean isMetadataContainsRestrictedInstanceValues(final MetadataVO metadata) {
-        final Set<String> allowedCustomTags = SetUtils.emptyIfNull(preferenceManager
-                .getPreference(SystemPreferences.CLUSTER_INSTANCE_ALLOWED_CUSTOM_TAGS));
+        final Set<String> allowedCustomTags = new HashSet<>(Arrays.asList(preferenceManager.findPreference(
+                        SystemPreferences.CLUSTER_INSTANCE_ALLOWED_CUSTOM_TAGS)
+                .filter(StringUtils::isNotBlank)
+                .map(value -> value.split(","))
+                .orElse(Strings.EMPTY_ARRAY)));
         if (CollectionUtils.isEmpty(allowedCustomTags)) {
             return false;
         }