Skip to content

Fixes #560 - Add systemctl wants to foreman.target for httpd#561

Open
qcjames53 wants to merge 1 commit into
theforeman:masterfrom
qcjames53:560
Open

Fixes #560 - Add systemctl wants to foreman.target for httpd#561
qcjames53 wants to merge 1 commit into
theforeman:masterfrom
qcjames53:560

Conversation

@qcjames53

Copy link
Copy Markdown
Contributor

Why are you introducing these changes? (Problem description, related links)

When running systemctl start foreman.target, Apache (httpd) does not start on it's own. I attempted to include this as a part of the fix in #536 but biffed the implementation.

What are the changes introduced in this pull request?

  • Corrects foreman.target's wants to properly include httpd. The old add-wants block was skipped by systemctl due to httpd being in the multi-user.target (higher prio). This was fine unless the user manually wanted to run systemctl stop foreman.target and systemctl start foreman.target, which might sometimes be the case.

How to test this pull request

Steps to reproduce:

  1. Pull changes and rebuild the VMs.
  2. Run vagrant ssh quadlet after VM boot.
  3. Wait for services to spin up, then run systemctl list-dependencies foreman.target to verify httpd made the list.
  4. Run systemctl stop foreman.target and systemctl status httpd to ensure it stopped.
  5. Run systemctl start foreman.target and systemctl status httpd to ensure it is running.
  6. Validate unit test changes.

Checklist

  • Tests added/updated (if applicable)
  • Documentation updated (if applicable) N/A

Comment thread src/roles/httpd/tasks/main.yml Outdated
@qcjames53 qcjames53 force-pushed the 560 branch 2 times, most recently from a17696e to eb9b28a Compare June 11, 2026 21:05
Comment thread src/playbooks/deploy/deploy.yaml Outdated
certificate_checks_certificate: "{{ server_certificate }}"
certificate_checks_key: "{{ server_key }}"
certificate_checks_ca: "{{ server_ca_certificate }}"
- role: systemd_target

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did this need to move?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The foreman.target needed to be created before the httpd task used it for my previous add-wants approach. I just moved it back in the newest push.

Comment thread tests/httpd_test.py Outdated
assert drop_in.is_file
assert drop_in.contains("PartOf=foreman.target")
assert drop_in.contains("WantedBy=foreman.target")
assert not drop_in.contains("WantedBy=foreman.target")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Err sorry, this is kind of a weird assertion. I think if we are going to test anything, then we should test that the httpd.service has what we expect.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Asked Claude to fix the unit test but missed that it changed this line to something weird. It should assert on the actual string now.

@ianballou ianballou left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Working nicely for me:

[root@katello-production ~]# systemctl list-dependencies foreman.target 
foreman.target
● ├─candlepin.service
● ├─dynflow-sidekiq@orchestrator.service
● ├─dynflow-sidekiq@worker-hosts-queue.service
● ├─dynflow-sidekiq@worker.service
● ├─foreman-proxy.service
× ├─foreman-recurring@daily.service
● ├─foreman-recurring@daily.timer
× ├─foreman-recurring@hourly.service
● ├─foreman-recurring@hourly.timer
○ ├─foreman-recurring@monthly.service
● ├─foreman-recurring@monthly.timer
× ├─foreman-recurring@weekly.service
● ├─foreman-recurring@weekly.timer
● ├─foreman.service
● ├─httpd.service
● ├─postgresql.service
● ├─pulp-api.service
● ├─pulp-content.service
● ├─pulp-worker@1.service
● ├─pulp-worker@2.service
● ├─pulp-worker@3.service
● ├─pulp-worker@4.service
● ├─redis.service
● └─pulp-worker.target
●   ├─pulp-worker@1.service
●   ├─pulp-worker@2.service
●   ├─pulp-worker@3.service
●   └─pulp-worker@4.service

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Apache does not start on systemctl start with the rest of foreman.target

3 participants