1.6.0 RFE Assign location without foreman

[josephpisciotta]

No description provided.

[sideangleside]

I would change

if options.location:

to

if options.location and 'foreman' in options.skip:

While it doesn't hurt to do so, we really only want to write the location facts if/when --skip foreman is passed.

[josephpisciotta]

Added that

[sideangleside]

Did some testing on this. If we set location using the location.facts file above, then change the hosts location in the UI, on the next run of subscription-manager facts --update the host will be associated with the original location specified at registration.

Example:

• I have two locations, RDU and BOS
• I register a host to BOS using this patch ./bootstrap.py -o Example -L BOS -s foreman.example.com --skip foreman -a ak-Reg_To_Crash
• I update the host to use a new location hammer host update --name bootstrap.example.com --location RDU
• I run subscription-manager facts --update either manually or wait for it to occur at a later point.
• The host's location will be updated to BOS

Thus, I would advise to delete the location.facts file after the host is registered.

[josephpisciotta]

I have an alternative suggestion and I’m reading this while walking so I haven’t tested this but could we simply append the location parameter to the subscription-manager register command? Would that also solve the problem? I’ll test when I get home.

…

Sent from my iPhone

On Feb 1, 2018, at 4:08 PM, Rich Jerrido ***@***.***> wrote: Did some testing on this. If we set location using the location.facts file above, then change the hosts location in the UI, on the next run of subscription-manager facts --update the host will be associated with the original location specified at registration. Example: I have two locations, RDU and BOS I register a host to BOS using this patch ./bootstrap.py -o Example -L BOS -s foreman.example.com --skip foreman -a ak-Reg_To_Crash I update the host to use a new location hammer host update --name bootstrap.example.com --location RDU I run subscription-manager facts --update either manually or wait for it to occur at a later point. The host's location will be updated to BOS Thus, I would advise to delete the location.facts file after the host is registered. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[sideangleside]

Subscription-manager doesn't support Locations. (That's why we have to either create the host via the API or use the foreman_location fact).

[josephpisciotta]

My mistake. I was misremembering. I’ll fix it when I get home. Potentially would location be something to add to the subscription manager command as well? Maybe another philosophical question. Joe

…

Sent from my iPhone

On Feb 1, 2018, at 4:23 PM, Rich Jerrido ***@***.***> wrote: Subscription-manager doesn't support Locations. (That's why we have to either create the host via the API or use the foreman_location fact). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[evgeni]

Yeah, I agree with @sideangleside, we should yank the file after we finished registering.
Given this also applies for migrations, I think the removal should happen somewhere here: https://github.com/josephpisciotta/katello-client-bootstrap/blob/515f06f18f9a7efa5df9915ede13884057cfdf5d/bootstrap.py#L1259 (like: after all the branches are done).

[josephpisciotta]

I fixed that. I didn't see evgeni's suggestion but I removed the file after both registration and migration so all good.

[sideangleside]

You'd want the inverse here if 'foreman' in options.skip: (If foreman is in the list of options to be skipped, delete the file). It doesn't exist otherwise.

Also, place the file deletion in the location that @evgeni recommended. No need in having duplicated code.

Other than those changes, it looks good. Please squash this code to a singular commit Example

[josephpisciotta]

@evgeni do you mean outside of the else? because if it is inside the else it won't run in the case that rhn exists and migration is being ran. it would need to be in a different if block

[sideangleside]

You have a block of code here (https://github.com/josephpisciotta/katello-client-bootstrap/blob/master/bootstrap.py#L1257:L1258) that is unneeded. That code is unneeded (It attempts to delete the location.facts file when --skip foreman is NOT passed.

Other than that, the patches work as designed. Please fix the above and squash this work to a singluar commit. @evgeni , thoughts?

[evgeni]

LGTM