Fix MiqServer WorkerManager Process with Non Rails workers

[agrare]

We only added logic to set non-rails workers to started/stopped for kubernetes and systemd worker manager subclasses. This meant if you ran rake evm:start locally with a non-rails worker it wouldn't ever be marked as started.

Also updated the systemd side to be consistent with kubernetes in not returning a worker that was just marked as started.

Depends on:

• Fix non rails worker starting on podified #23116

[jrafanie]

Do we have non_rails_worker?. It might make more sense to select those instead of rejecting the rails ones.

Is it overkill to have these methods do something like:

def sync_starting_workers
  sync_from_system       # get the latest from process list
  sync_rails_workers     # no-op, workers do this on their own
  sync_non_rails_workers # move the new code here - it's an extra query but it's just as easy to update the non-rails worker rows by pid to started
  
  MiqWorker.find_all_starting.to_a
end

[agrare]

Do we have non_rails_worker?. It might make more sense to select those instead of rejecting the rails ones.

Ha we don't currently but I got this backwards multiple times while writing specs so I do think having a non_rails_worker? would be good.

[Fryguy]

Is it possible to do rails_worker as a query scope on MiqWorker? I'm thinking something similar concrete_subclasses where we can get a list of the possible types, then issue a .where(:type => those_subclasses). That way we can optimize the update to an update_all.

(Not necessary for this PR, but I can see it being more useful when chaining things later)

[agrare]

Yeah I started doing this as a scope but I think that'd be better off as a separate PR, also the switch to select(&:non_rails_worker?) because we'd want to change the other classes (systemd/kubernetes) as well.

[jrafanie]

Yeah, I like the scope by worker classes (rails vs non-rails) idea but agree we should do that across the board for other classes.

I'm fine with the existing code if you want to refactor it after to do the inverse non_rails_worker? + extract a method. Whatever is easier to verify.

[agrare]

I do like splitting this up into methods so let me get that in

[agrare]

Done, PTAL

[Fryguy]

LGTM - would also like @jrafanie to give another review.

[miq-bot]

Checked commits agrare/manageiq@b20a2d6~...217068b with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
4 files checked, 0 offenses detected
Everything looks fine. 🏆

[jrafanie]

I have some very minor comments that can be addressed in a followup PR if it makes sense. LGTM

[jrafanie]

super minor... swap the order of these methods for consistency

[jrafanie]

Again very minor... if the intent is to move the logic of setting starting/stopping from the rails workers and into here, maybe comment on how this logic is currently handled by the works so it's a no-op right now but to match systemd's logic, we might move the logic here.

[Fryguy]

Backported to radjabov in commit 2075352.

commit 2075352834988b549ae2697961bbda14bfab458e
Author: Joe Rafaniello <jrafanie@users.noreply.github.com>
Date:   Fri Aug 16 15:53:44 2024 -0400

    Merge pull request #23117 from agrare/non_rails_worker_process_starting
    
    Fix MiqServer WorkerManager Process with Non Rails workers
    
    (cherry picked from commit cffb072312449f4c922ca6d62755d7ecb479ab99)