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

Replace Container ENV var EMS_IDS->_ID #23195

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

agrare
Copy link
Member

@agrare agrare commented Sep 19, 2024

Workers having a comma separated list of EMS IDs has been replaced with moving child-manager refresh logic into the parent-manager refresher.

I noticed this while investigating the OpentofuWorker, it is also odd that deployment-per-worker also assumes that it is an EMS worker which isn't necessarily the case.

I think a better approach would be to move the environment variable logic out of how the workers are managed by kubernetes, and unify it with how systemd handles environment variables per-worker with the unit_environment_variables method (which already handles EMS_ID).

Co-depends:

Workers having a comma separated list of EMS IDs has been replaced with
moving child-manager refresh logic into the parent-manager refresher.
@agrare agrare force-pushed the deployment_per_worker_ems_ids branch from 4703bf3 to 41d5fad Compare September 19, 2024 20:52
@miq-bot
Copy link
Member

miq-bot commented Sep 19, 2024

Checked commit agrare@41d5fad with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🏆

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

It feels like you've been changing array EMS ID everywhere. It's the gift that keeps on giving.

@agrare
Copy link
Member Author

agrare commented Sep 19, 2024

Ha yeah I keep finding more, I think this is the last of them though (famous last words)

@jrafanie jrafanie merged commit 4063077 into ManageIQ:master Sep 20, 2024
8 checks passed
@jrafanie jrafanie self-assigned this Sep 20, 2024
@agrare agrare deleted the deployment_per_worker_ems_ids branch September 20, 2024 14:14
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