Upstream bug assigned to mshira

Upstream bug assigned to mshira

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.

interesting, my script missed this. I will look to update the script. Keeping this open.

Moving this bug to POST for triage into Satellite 6 since the upstream issue https://projects.theforeman.org/issues/20538 has been resolved.

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"=>{}}
...

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.

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").

Hi Lukas, 
nope, this was designed to override the values, and also this is how it's done in the API as well.

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

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./

+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.

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.

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.

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