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

Enable workers as containers #19451

Merged
merged 5 commits into from
Nov 4, 2019
Merged

Enable workers as containers #19451

merged 5 commits into from
Nov 4, 2019

Conversation

carbonin
Copy link
Member

This PR reverts the commit which disabled workers to be run as containers in OpenShift.

It also removes some information around artemis which is no longer planned to be used and fixes some additional bugs.

Notably it changes the MiqWorker.workers method to return a count of 0 when we are missing a required role for the specific worker type. Previously this method would return the configured type regardless of the roles on the server. In OpenShift, this will allow us to scale down workers

@carbonin
Copy link
Member Author

@Fryguy I can contain the last commit to a hack in the containers code, but I figured this is probably the way it should have worked either way.

app/models/miq_worker.rb Outdated Show resolved Hide resolved
@Fryguy
Copy link
Member

Fryguy commented Oct 31, 2019

Have you verified that the existing non-container worker management still works as expected?

Copy link
Member

@Fryguy Fryguy left a comment

Choose a reason for hiding this comment

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

Overall LGTM, with a few questions/comments.

Artemis is not being used anymore and ENV["DATABASE_SERVICE_NAME"]
is not set on the orchestrator pod.
This makes it easier for developers to specify alternate namespaces
and registries for the worker images.

The orchestrator will be provided the value as a part of the deploy
process and will pass it along to all the worker deployments
Previously MiqWorker.workers would return the configured count
even if the required roles for the worker type were not met.
@carbonin
Copy link
Member Author

Have you verified that the existing non-container worker management still works as expected?

Yup.

@miq-bot
Copy link
Member

miq-bot commented Nov 1, 2019

Checked commits carbonin/manageiq@9d279ed~...469f730 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
10 files checked, 0 offenses detected
Everything looks fine. 🍪

@Fryguy Fryguy merged commit e2f0281 into ManageIQ:master Nov 4, 2019
@Fryguy Fryguy added this to the Sprint 124 Ending Nov 11, 2019 milestone Nov 4, 2019
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