Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

fleetd: process dependencies in [Install] section #1655

Merged
merged 1 commit into from
Nov 2, 2016

Conversation

dongsupark
Copy link
Contributor

@dongsupark dongsupark commented Aug 8, 2016

Introduce a section [Install] in unit files, to allow WantedBy in the section to be able to enable the dependent units.
This PR is just a rebased version of #1523, except for minor changes to fix build errors.

Resolves #1382
Supersedes #1523

/cc @kayrus

@dongsupark
Copy link
Contributor Author

Hmm, interesting. TestInstallUnit fails only on semaphoreci, hanging forever on "fleetctl load". OTOH local test runs without any problem.
Do not merge this PR until the test issue could get fixed.

@dongsupark dongsupark changed the title fleetd: process dependencies in [Install] section [WIP] fleetd: process dependencies in [Install] section Aug 9, 2016
@dongsupark dongsupark force-pushed the dongsu/install_section branch 2 times, most recently from 22b0396 to 5c5e1d0 Compare August 9, 2016 12:10
@kayrus
Copy link
Contributor

kayrus commented Aug 9, 2016

@dongsupark last one doesn't relate to install stuff.

@dongsupark
Copy link
Contributor Author

@kayrus Right. I was just taking a try-and-error approach. But none of the reverted patches was a culprit.
Though I think I just figured out how to reproduce the hanging problem on my local laptop, testing with CoreOS alpha.
I'll let you know if I find more about this.

@dongsupark dongsupark force-pushed the master branch 3 times, most recently from 150d30c to 4002bf5 Compare August 16, 2016 08:54
@dongsupark dongsupark force-pushed the master branch 7 times, most recently from 3b60e93 to 875d938 Compare August 23, 2016 11:00
@dongsupark dongsupark changed the title [WIP] fleetd: process dependencies in [Install] section fleetd: process dependencies in [Install] section Aug 29, 2016
@dongsupark
Copy link
Contributor Author

To sum up, this PR works fine on CoreOS alpha v1151.0.0 (with systemd v231), while it still fails with CoreOS beta(1122) & stable(1068) (with systemd v229). So the issue of failing functional test was clearly not a fleet issue.
Once CoreOS stable got updated to 1151, I would merge this PR.

@dongsupark
Copy link
Contributor Author

CoreOS stable was updated to 1185.3.0 with systemd v231. So finally this PR can be merged.

However it's still pending due to other test failures. It's blocked by #1700.

@dongsupark
Copy link
Contributor Author

All right. As the blocker #1700 was merged, I'll also merge this PR.

@dongsupark dongsupark merged commit e98df4c into coreos:master Nov 2, 2016
@dongsupark dongsupark deleted the dongsu/install_section branch November 2, 2016 12:31
@jonboulle
Copy link
Contributor

Bit late to this. Code LGTM, but @dongsupark what do you think about making this configurable, since it can seem a bit unintuitive?

@dongsupark
Copy link
Contributor Author

@jonboulle I agree that it's unintuitive, but I'm not sure about how to make it configurable for users. AFAICS users anyway need to write own unit files for dependencies, with either Wants or WantedBy. Can you please explain?

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

Successfully merging this pull request may close these issues.

WantedBy doesn't lead to starting service
3 participants