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

Resolves issue-16844 - systemd notify by default #16845

Merged
merged 1 commit into from
Jun 2, 2023

Conversation

drawks
Copy link
Contributor

@drawks drawks commented Mar 31, 2023

Description

To resolve issue #16844 and switch packaged systemd unit to use systemd's notify mechanism by default.

Testing & Reproduction steps

See #16844 where the steps to reproduce the current and updated behavior are documented

Links

#2121
31a310f
https://developer.hashicorp.com/consul/docs/agent#understanding-the-agent-startup-output
https://developer.hashicorp.com/consul/tutorials/production-deploy/deployment-guide#configure-the-consul-process

PR Checklist

  • updated test coverage
  • external facing docs updated
  • not a security concern

@drawks drawks requested a review from a team March 31, 2023 22:46
@drawks drawks requested a review from a team as a code owner March 31, 2023 22:46
@drawks drawks requested review from sarahethompson and alvin-huang and removed request for a team March 31, 2023 22:46
@github-actions
Copy link

This pull request has been automatically flagged for inactivity because it has not been acted upon in the last 60 days. It will be closed if no new activity occurs in the next 30 days. Please feel free to re-open to resurrect the change if you feel this has happened by mistake. Thank you for your contributions.

@github-actions github-actions bot added the meta/stale Automatically flagged for inactivity by stalebot label May 31, 2023
@drawks
Copy link
Contributor Author

drawks commented Jun 1, 2023

Any updates on this PR? It is a very minor change and shouldn't require any updated tests.

@alvin-huang @sarahethompson I see you got assigned as reviewers

@drawks
Copy link
Contributor Author

drawks commented Jun 1, 2023

I've rebased this since it hasn't been looked at in a while. Tests are passing except for the one requiring a label, which needs a maintainer to add either a no-backport or backport to current supported versions (this change is super low risk and should be totally non-disruptive in 99% of cases). The changelog update check doesn't give any advice as to the expected changelog update style/semantics. I'd be happy to make whatever changes are needed to get this merged.

On a human note it feels pretty ridiculous that hashicorp sends me all sorts of "core contributor" swag in the mail but can't be bothered to even comment on this TINY pr which I've submitted.

@loshz loshz added backport/1.13 backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. labels Jun 1, 2023
@loshz
Copy link
Contributor

loshz commented Jun 1, 2023

Hey @drawks, thanks for fixing this issue and sorry for the delayed response. You are right in that the systemd unit should contain this, as our docs clearly state the requirement.

I'm happy to accept this if you wouldn't mind adding a changelog. The file should be .changelog/16845.txt:
```release-note:improvement
systemd: set service type to notify.
```

@loshz loshz requested review from loshz and removed request for a team, sarahethompson and alvin-huang June 1, 2023 23:05
@loshz loshz added type/bug Feature does not function as expected and removed meta/stale Automatically flagged for inactivity by stalebot labels Jun 1, 2023
@drawks
Copy link
Contributor Author

drawks commented Jun 2, 2023

@loshz thanks for the response, I've just squashed the changelog update as requested into my branch. I'm looking forward to systemd being able to order my services correctly (without any local gyrations) :)

@loshz
Copy link
Contributor

loshz commented Jun 2, 2023

@loshz thanks for the response, I've just squashed the changelog update as requested into my branch. I'm looking forward to systemd being able to order my services correctly (without any local gyrations) :)

Apologies, but the changlelog has to include the back ticks (for reason's unknown to me):

```release-note:improvement
systemd: set service type to notify.
```

* updates `consul.service` systemd service unit to use `Type=notify` to
  resolve issue hashicorp#16844
* add changelog update to match
@drawks
Copy link
Contributor Author

drawks commented Jun 2, 2023

@loshz ha! I've just squshed in a fix for that

@loshz
Copy link
Contributor

loshz commented Jun 2, 2023

Thanks again, @drawks. I can help with the backports if they don't automerge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. type/bug Feature does not function as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants