search

Katello / katello-client-bootstrap

Bootstrap Script for migrating systems to Foreman & Katello
GNU General Public License v2.0
52 stars 63 forks source link

1.6.0 RFE Assign location without foreman #243

Closed josephpisciotta closed 6 years ago

sideangleside commented 6 years ago

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 commented 6 years ago

Added that

sideangleside commented 6 years ago

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:

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

josephpisciotta commented 6 years ago

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 notifications@github.com 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 commented 6 years ago

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 commented 6 years ago

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 notifications@github.com 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 commented 6 years ago

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 commented 6 years ago

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

josephpisciotta commented 6 years ago

@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 commented 6 years ago

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?