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

Fixes #18757 - Handle dynflow service in foreman core #602

Merged
merged 2 commits into from
Jan 15, 2018

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Oct 24, 2017

The dynflow service was moved into core with 1.16. That renamed it to dynflowd and means the service should be started by default.

For compatibility parameters are provided. This means we must move the service name detection to the jobs.pp file because that's the only place we know the actual value.

This replaces #530. It's mostly the same, but split into two commits to make reviews easier. The first is a compatible refactor where the second is just the introduction of dynflow in core.

@theforeman-bot
Copy link
Member

@ekohl, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).


This message was auto-generated by Foreman's prprocessor

@ekohl ekohl changed the title Fixes #18575 - Handle dynflow service in foreman core Fixes #18757 - Handle dynflow service in foreman core Oct 24, 2017
Copy link
Member

@dLobatog dLobatog left a comment

Choose a reason for hiding this comment

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

@ekohl I don't see many changes different to #530 , is there anything left to be done before merge?

@ekohl ekohl requested a review from mmoll November 22, 2017 11:53
@ekohl
Copy link
Member Author

ekohl commented Nov 22, 2017

@mmoll I'm looking at merging the RPM packaging PR. Is there anything that should be done for the deb side?

@mmoll
Copy link
Contributor

mmoll commented Nov 22, 2017

Something like theforeman/foreman-packaging#1549 would need to get added to the DEB package also...

@mmoll
Copy link
Contributor

mmoll commented Nov 22, 2017

If the service is also called "dynflowd" on Debian, this should work OOTB, AFAICT 👍

@ekohl
Copy link
Member Author

ekohl commented Nov 22, 2017

I'll have a look at adding dynflowd to Debian. It was my intention to unify them and simplify our instructions/packaging/configuration.

@dLobatog
Copy link
Member

The dynflow service was moved into core with 1.17. That renamed it to
dynflowd and means the service should be started by default.

For compatibility parameters are provided. This means we must move the
service name detection to the jobs.pp file because that's the only place
we know the actual value.
@ekohl
Copy link
Member Author

ekohl commented Jan 15, 2018

Rebased and updated to state it's 1.17+. Now that it's in both debs and rpms this can be merged once tests pass.

Copy link
Member

@dLobatog dLobatog left a comment

Choose a reason for hiding this comment

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

@ekohl 👍 can be merged now

@ekohl ekohl merged commit ee1b7b4 into theforeman:master Jan 15, 2018
@ekohl ekohl deleted the dynflow-in-core branch January 15, 2018 10:02
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.

4 participants