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

ekazachkova · 2024-05-28T09:40:51Z

The current PR provides implementation for issue #3534

The following changes were added:

a new preferences cluster.instances.tags. This preference supports tool/run_id/owner keys only.

{
  "owner": "CP_OWNER",
  "run_id": "CP_RUN_ID",
  "tool": "CP_TOOL"
}

a new preference cluster.instances.allowed.tags. This preferences allows to specify comma-separated list of string values. If tool's metadata key exactly matches the value from this list the corresponding metadata (a key-value pair) shall be added to compute resources tags. Name value is not allowed.
stop run behaviour modification: tags shall be removed from compute resources if run status changed to final.
attach disks: tags shall be added to a new disks.
nodeup and reassign_node scripts support new --tags option. This option accepts a list of <tag_name>=<tag_value> string values. If this list provided the corresponding tags shall be added to instances and ebs volumes.
if CP_CAP_SHARE_FS_TYPE=lustre corresponding tags shall be added to created FS.

NOTE: all changes implemented for AWS provider only.

… initial API part

… delete tags method

… cleanup

… remove instance tags during pipeline stop

… support --custom_tags for nodeup

… support --custom_tags for reassign

… cleanup

… test fix

… attach tags with disk

… cleanup

… tests

… tag ebs volumes when reassign

… supported for aws provider only

… cleanup

… support tags for lustre

tcibinan · 2024-05-29T12:52:32Z

api/src/main/java/com/epam/pipeline/manager/cloud/aws/EC2Helper.java

+        client.deleteTags(new DeleteTagsRequest()
+                .withResources(resourcesIds)
+                .withTags(tags.stream()
+                        .map(Tag::new)
+                        .collect(Collectors.toList())));


Let's extract deleteTags method similarly to how createTags is extracted.

tcibinan · 2024-05-29T13:02:32Z

api/src/main/java/com/epam/pipeline/manager/cluster/autoscale/ReassignHandler.java

+        final Optional<PipelineRun> run = optionalRun.isPresent() ? optionalRun : pipelineRunManager.findRun(runId);
+        if (!run.isPresent()) {
+            return Collections.emptyMap();
+        }
+        return metadataManager.prepareCustomInstanceTags(run.get());


We can use more optional-friendly implementation here.

Suggested change

final Optional<PipelineRun> run = optionalRun.isPresent() ? optionalRun : pipelineRunManager.findRun(runId);

if (!run.isPresent()) {

return Collections.emptyMap();

}

return metadataManager.prepareCustomInstanceTags(run.get());

return optionalRun.map(Optional::of)

.orElseGet(() -> pipelineRunManager.findRun(runId))

.map(metadataManager::prepareCustomInstanceTags)

.orElseGet(Collections::emptyMap);

tcibinan · 2024-05-29T13:10:51Z

api/src/main/java/com/epam/pipeline/manager/metadata/MetadataManager.java

+        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)));


We can use Stream API here.

Suggested change

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)));

final Set<String> instanceTagsKeys = preferenceManager.findPreference(

SystemPreferences.CLUSTER_INSTANCE_ALLOWED_CUSTOM_TAGS)

.filter(StringUtils::isNotBlank)

.map(value -> value.split(","))

.map(Arrays::stream)

.orElseGet(Stream::empty)

.filter(StringUtils::isNotBlank)

.collect(Collectors.toSet());

tcibinan · 2024-05-29T13:12:50Z

api/src/main/java/com/epam/pipeline/manager/metadata/MetadataManager.java

+        final Map<String, PipeConfValue> metadataData = MapUtils.emptyIfNull(Objects.isNull(toolMetadata)
+                ? null
+                : toolMetadata.getData());


Optionals are perfect for such a situation.

Suggested change

final Map<String, PipeConfValue> metadataData = MapUtils.emptyIfNull(Objects.isNull(toolMetadata)

? null

: toolMetadata.getData());

final Map<String, PipeConfValue> metadataData = Optional.ofNullable(toolMetadata)

.map(MetadataEntry::getData)

.orElseGet(Collections::emptyMap);

tcibinan · 2024-05-29T13:15:18Z

api/src/main/java/com/epam/pipeline/manager/metadata/MetadataManager.java

+            final Map<String, String> customTags = resolveCommonCustomInstanceTags(run);
+            return resolveInstanceTagsFromMetadata(run.getDockerImage(), customTags);


I would suggest using a simpler approach for collecting tags from various sources.

Suggested change

final Map<String, String> customTags = resolveCommonCustomInstanceTags(run);

return resolveInstanceTagsFromMetadata(run.getDockerImage(), customTags);

final Map<String, String> implicitTags = resolveCommonCustomInstanceTags(run);

final Map<String, String> explicitTags = resolveInstanceTagsFromMetadata(run);

return CommonUtils.mergeMaps(implicitTags, explicitTags);

tcibinan · 2024-05-29T13:26:54Z

api/src/main/java/com/epam/pipeline/entity/metadata/CommonCustomInstanceTagsTypes.java

+ * - run_id - Integer ID of a run
+ * - owner - Username of a run owner
+ */
+public enum CommonCustomInstanceTagsTypes {


I think the name of the class is too confusing. It's both common and custom. Can we just call it CommonInstanceTagType? I think that for the most part of the code in this pull request there is no actual need in having custom in name.

… review fixes

… cleanup namings

… cleanup

… comma separated list refactor

… cleanup

…3544) (cherry picked from commit cb8c9fd)

…3544) (cherry picked from commit cb8c9fd) (cherry picked from commit 6eb95b5)

ekazachkova added 15 commits May 22, 2024 18:35

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

b45f2b9

… initial API part

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

4ebeefe

… delete tags method

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

a596c9d

… cleanup

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

e89bdcb

… remove instance tags during pipeline stop

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

574f423

… support --custom_tags for nodeup

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

d35113c

… support --custom_tags for reassign

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

e0a8b63

… cleanup

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

ee8e1f5

… test fix

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

bcc82d7

… attach tags with disk

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

22170a0

… cleanup

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

a159aaa

… tests

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

ea1a306

… tag ebs volumes when reassign

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

af745da

… supported for aws provider only

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

6d6f768

… cleanup

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

a835e2a

… support tags for lustre

ekazachkova requested review from mzueva and tcibinan and removed request for mzueva May 28, 2024 15:34

tcibinan reviewed May 29, 2024

View reviewed changes

ekazachkova added 5 commits May 29, 2024 19:48

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

94f24b7

… review fixes

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

5552586

… cleanup namings

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

bb7ab70

… cleanup

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

a1a13fa

… comma separated list refactor

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

7f456ad

… cleanup

tcibinan approved these changes May 30, 2024

View reviewed changes

ekazachkova merged commit cb8c9fd into develop May 30, 2024
4 checks passed

ekazachkova added a commit that referenced this pull request May 31, 2024

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

6eb95b5

…3544) (cherry picked from commit cb8c9fd)

sidoruka pushed a commit that referenced this pull request Jun 17, 2024

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

74fb0da

…3544) (cherry picked from commit cb8c9fd) (cherry picked from commit 6eb95b5)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ekazachkova commented May 28, 2024 •

edited by sidoruka

Loading

tcibinan May 29, 2024

tcibinan May 29, 2024

tcibinan May 29, 2024

tcibinan May 29, 2024

tcibinan May 29, 2024

tcibinan May 29, 2024

-        final Map<String, PipeConfValue> metadataData = MapUtils.emptyIfNull(Objects.isNull(toolMetadata)
-                ? null
-                : toolMetadata.getData());
+        final Map<String, PipeConfValue> metadataData = Optional.ofNullable(toolMetadata)
+                .map(MetadataEntry::getData)
+                .orElseGet(Collections::emptyMap);

		final Map<String, String> customTags = resolveCommonCustomInstanceTags(run);
		return resolveInstanceTagsFromMetadata(run.getDockerImage(), customTags);

-            final Map<String, String> customTags = resolveCommonCustomInstanceTags(run);
-            return resolveInstanceTagsFromMetadata(run.getDockerImage(), customTags);
+            final Map<String, String> implicitTags = resolveCommonCustomInstanceTags(run);
+            final Map<String, String> explicitTags = resolveInstanceTagsFromMetadata(run);
+            return CommonUtils.mergeMaps(implicitTags, explicitTags);

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

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

Conversation

ekazachkova commented May 28, 2024 • edited by sidoruka Loading

tcibinan May 29, 2024

Choose a reason for hiding this comment

tcibinan May 29, 2024

Choose a reason for hiding this comment

tcibinan May 29, 2024

Choose a reason for hiding this comment

tcibinan May 29, 2024

Choose a reason for hiding this comment

tcibinan May 29, 2024

Choose a reason for hiding this comment

tcibinan May 29, 2024

Choose a reason for hiding this comment

ekazachkova commented May 28, 2024 •

edited by sidoruka

Loading