Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MiqWorker service_base_name to match unit file prefix #21149

Merged
merged 4 commits into from
Apr 6, 2021

Conversation

agrare
Copy link
Member

@agrare agrare commented Apr 6, 2021

After we renamed the systemd services to have "manageiq-" and "manageiq-providers-" prefixes the service_base_name wasn't updated to match the new names.

Also adds a spec test to ensure all workers have systemd unit files and that there are no extra unit files without workers.

Depends on:

@agrare
Copy link
Member Author

agrare commented Apr 6, 2021

In splitting up #20983 and #21120 the commit that did this service file rename got dropped in the shuffle

describe ".service_base_name" do
before { MiqWorkerType.seed }

it "every worker has a matching systemd target and service file" do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really want to make this a providers_common spec but we'll need a good way to determine which workers come from which engine.

This is how the container_common_spec is testing all worker deployment name lengths

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we? It can't hurt to check all workers in all engines. If someone changes a particular engine, then the probability they break their own engine is higher than breaking others, especially since we won't merge on red.

@agrare agrare force-pushed the fix_systemd_service_names branch from 7204ae3 to 8e186ca Compare April 6, 2021 22:50
@kbrock kbrock self-assigned this Apr 6, 2021
@kbrock kbrock merged commit 6607b29 into ManageIQ:master Apr 6, 2021
@agrare agrare deleted the fix_systemd_service_names branch April 6, 2021 23:19
[service_file, target_file]
end

expect(expected_units).to match_array(found_units)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized this should have been the other way around...

      expect(found_units).to match_array(expected_units)

but it's fine.

Copy link
Member Author

@agrare agrare Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fryguy I think this was actually correct originally. I swapped this in the follow-up PR but now that I'm running one switching this around prints missing unit files as "extra" rather than "missing". Nevermind there were issues with the worker seed for subclassed workers, thought it was the missing unit files but was actually missing worker classes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants