slavkap commented on a change in pull request #5043:
URL: https://github.com/apache/cloudstack/pull/5043#discussion_r641555972
##########
File path:
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -2803,29 +2803,9 @@ public ServiceOffering updateServiceOffering(final
UpdateServiceOfferingCmd cmd)
offering.setSortKey(sortKey);
}
- // Note: tag editing commented out for now; keeping the code intact,
- // might need to re-enable in next releases
- // if (tags != null)
- // {
- // if (tags.trim().isEmpty() && offeringHandle.getTags() == null)
- // {
- // //no new tags; no existing tags
- // offering.setTagsArray(csvTagsToList(null));
- // }
- // else if (!tags.trim().isEmpty() && offeringHandle.getTags() != null)
- // {
- // //new tags + existing tags
- // List<String> oldTags = csvTagsToList(offeringHandle.getTags());
- // List<String> newTags = csvTagsToList(tags);
- // oldTags.addAll(newTags);
- // offering.setTagsArray(oldTags);
- // }
- // else if(!tags.trim().isEmpty())
- // {
- // //new tags; NO existing tags
- // offering.setTagsArray(csvTagsToList(tags));
- // }
- // }
+ updateOfferingTagsIfIsNotNull(cmd.getTags(), offering);
+
+ updateServiceOfferingHostTagsIfNotNull(cmd.getHostTag(), offering);
Review comment:
Thanks, @sureshanaparti, for the review! The call can set tags, replace
the existing ones or add more tags. And yes, there will be inconsistency
between old VMs/volumes and the new ones, as @shwstppr mentioned. And maybe
there is a need for discussion which option is better:
- to migrate the VMs/volumes on the right host/storage with the new tags
- to disable this option on already created VMs with the respective offering
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
