Bug 1479765

Summary: [RFE] Commands for creating, updating and deleting compute profiles and attributes
Product: Red Hat Satellite Reporter: Tomas Strachota <tstrachota>
Component: Compute ResourcesAssignee: Shira Maximov <mshira>
Status: CLOSED ERRATA QA Contact: Lukáš Hellebrandt <lhellebr>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 6.3.0CC: bkearney, chris.brown, chris.snell, dahorak, dmoessne, lhellebr, mhulan, mmccune, oprazak, orabin, pcreech, rvdwees
Target Milestone: 6.7.0Keywords: FutureFeature
Target Release: Unused   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-04-14 13:22:23 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Tomas Strachota 2017-08-09 11:13:51 UTC
Hammer should provide commands for modifying operations around compute profiles and attributes.

# Commands for managing compute profiles:

Compute profile commands should be quite simple:

  hammer compute-profile create --name 1-Small
  hammer compute-profile update --name 1-Small --new-name 1-Smaller
  hammer compute-profile delete --name 1-Smaller

# Commands for managing attributes:
set-attributes
- replaces the whole set of attributes for a compute resource

  hammer compute-profile set-attributes --compute-resorce tstracho-laptop --compute-profile 1-Small \
  --attributes flavor=m1.small,cpus=2,memory=4GB \
  --interface type=network,bridge=br0 \
  --interface type=network,bridge=br1 \
  --volume size=40GB

update-attributes
- partially updates enumerated attributes, keeps the previous values for the rest
- attributes for specific volume or interface can be updated when users specify its index (position in the list)
- without an index the command adds a new interface/volume

  hammer compute-profile update-attributes --compute-resorce tstracho-laptop --compute-profile 1-Small \
  --attributes cpus=2,memory=4GB \
  --interface type=network,bridge=br1,index=1 \
  --volume size=40GB,index=2

erase-attributes
- completely removes all compute attributes from a compute profile
- we can consider other names for this command, eg. "remove-attributes" or "delete-attributes"

  hammer compute-profile erase-attributes --compute-resorce tstracho-laptop --compute-profile 1-Small

# Managing interfaces and volumes:

- add-interface adds a new interface at the end of the interface list
- remove-interface requires interface's index (position in the interface list)
- update-interface changes attributes for an existing interface definition
- commands for volumes (add-volume, remove-volume, update-volume) work similarly

  hammer compute-profile add-interface --compute-resorce tstracho-laptop --compute-profile 1-Small --attributes type=network,bridge=br1
  hammer compute-profile remove-interface --compute-resorce tstracho-laptop --compute-profile 1-Small --interface-index=1
  hammer compute-profile update-interface --compute-resorce tstracho-laptop --compute-profile 1-Small --interface-index=1 --attributes type=network,bridge=br1

Comment 2 Satellite Program 2018-01-11 17:22:30 UTC
Upstream bug assigned to mshira

Comment 3 Satellite Program 2018-01-11 17:22:37 UTC
Upstream bug assigned to mshira

Comment 4 Bryan Kearney 2019-02-07 12:09:27 UTC
The Satellite Team is attempting to provide an accurate backlog of bugzilla requests which we feel will be resolved in the next few releases. We do not believe this bugzilla will meet that criteria, and have plans to close it out in 1 month. This is not a reflection on the validity of the request, but a reflection of the many priorities for the product. If you have any concerns about this, feel free to contact Red Hat Technical Support or your account team. If we do not hear from you, we will close this bug out. Thank you.

Comment 6 Bryan Kearney 2019-02-07 21:10:10 UTC
interesting, my script missed this. I will look to update the script. Keeping this open.

Comment 7 Bryan Kearney 2019-08-20 14:00:55 UTC
Moving this bug to POST for triage into Satellite 6 since the upstream issue https://projects.theforeman.org/issues/20538 has been resolved.

Comment 8 Lukáš Hellebrandt 2020-01-30 12:02:39 UTC
FailedQA with Sat 6.7 snap 10.

1) It's possible to set nonsense values, e.g.:
# hammer compute-profile values update --compute-resource-id 1 --compute-profile-id 1 --compute-attributes memory=whatever
Compute profile attributes updated.
# hammer compute-profile info --id 1
...
VM attributes:    {"memory"=>"whatever", "volumes_attributes"=>{}, "interfaces_attributes"=>{}}
...

2) Updating a value actually deletes other values (seems related to https://github.com/theforeman/hammer-cli-foreman/pull/398#issuecomment-460232201 , maybe just missing cherrypick?) AND updating adds empty volumes_attributes, interfaces_attributes while creating doesn't:
# hammer compute-profile info --id 1
...
VM attributes:    {"cluster"=>"5aaaaa41-02db-027c-0190-0000000001c8", "template"=>"", "instance_type"=>"", "cores"=>"1", "sockets"=>"1", "memory"=>"1073741824", "ha"=>"0", "display"=>{"type"=>"spice", "keyboard_layout"=>"en-us"}}
...
# hammer compute-profile values update --compute-profile-id 1 --compute-resource-id 3 --compute-attributes cores=42
Compute profile attributes updated.
# hammer compute-profile info --id 1
...
VM attributes:    {"cores"=>"42", "volumes_attributes"=>{}, "interfaces_attributes"=>{}}
...

Comment 9 Shira Maximov 2020-02-16 12:35:24 UTC
Hi lukas, 
1. legit
2. This is the correct scenario, if you are updating the --compute-attributes then it will override the all --compute-attributes. 

since 1 is a minor issue (which should be fix in foreman and not in hammer CLI), it would be great if you can verify this one and open a new BZ for validating the compute-attributes data.

Comment 10 Lukáš Hellebrandt 2020-03-04 09:15:52 UTC
1) I agree this is not a regression because it has already been possible to do the same through API.
2) Based on the comment I linked in comment 8 and on comment 0, the values should be kept ("partially updates enumerated attributes, keeps the previous values for the rest").

Comment 11 Shira Maximov 2020-03-04 09:39:28 UTC
Hi Lukas, 
nope, this was designed to override the values, and also this is how it's done in the API as well.

Comment 12 Lukáš Hellebrandt 2020-03-04 13:42:42 UTC
Are you sure? This has been reported in the linked PR multiple times and you never said it was intended behavior. Also, user can expect Hammer to be more friendly about this than bare API calls. I would expect "update" to be non-destructive.

Ori, what do you think? If you both insist this is the correct behavior, then there still remains the second part of issue 2): updating adds empty volumes_attributes, interfaces_attributes while creating doesn't

Comment 13 Shira Maximov 2020-03-04 14:40:45 UTC
yes, I'm sure, this is the correct behavior, For the second part, it's the same, updating the compute profile will override the existing params in the vm_attributes, volumes and interfaces, if you will not create the volume then it will be empty. 

Hammer should reflect the API, and therefore we decided to override the values as we are doing in the API./

Comment 14 Mike McCune 2020-03-06 16:21:42 UTC
+1 to Shira's argument that we file a new bug to fix the core API to address the issue found during the testing of this RFE. That is arguably a separate issue from the addition of this functionality.

I'm going to move this back ON_QA for a check on verification with this consideration in place.

Comment 15 orabin 2020-03-08 12:34:04 UTC
I agree with Shira, hammer tries to be friendlier than the API but the expectation isn't to fix/change all API behaviors that are not the most user friendly. As mentioned in Comment 14, this can be opened as a RFE to the api.

Comment 16 Lukáš Hellebrandt 2020-03-09 09:54:10 UTC
Verified with Sat 6.7 snap 10 as per comment 8. There are some unexpected behaviors caused by API, out of scope of this BZ based on development.

Comment 19 errata-xmlrpc 2020-04-14 13:22:23 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2020:1454