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.

Sign up for GitHub

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

Jump to bottom

fix(systemd): fix service detection #94

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(systemd): fix service detection #94

wants to merge 2 commits into from

Conversation

ze42
Copy link

@ze42 ze42 commented Jun 25, 2021

Fix service detection to properly ensure the service is running.
Command also updated at other service.running/dead onlyif.

Fixes #93 with the change suggested there

noelmcloughlin
noelmcloughlin approved these changes Jun 25, 2021
View reviewed changes
Copy link
Member

@noelmcloughlin noelmcloughlin left a comment

Choose a reason for hiding this comment

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

LGTM thanks.

Note: I did a quick search (grep -ir list-units) and see this in Airflow & Prometheus formulas but only used in their clean states so not an issue.

@noelmcloughlin
Copy link
Member

Not sure why Amazonlinux fails.

@ze42
Copy link
Author

ze42 commented Jun 25, 2021

mmm... might be a bug in salt I've seen some time ago, and might still be there.

Enabling and launching at the same time fails. Splitting into 2 states (service.enabled and service.running) might do the trick.

@daks
Copy link
Member

daks commented May 18, 2022

HI, this PR is quite old now but I' interested in reviewing it to merge it if possible because I'm facing the same bug.

daks
daks reviewed May 18, 2022
View reviewed changes
Copy link
Member

@daks daks left a comment •
edited
Loading

Choose a reason for hiding this comment

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

edit: previous comment was wrong

mongodb/service/clean.sls
@@ -29,7 +29,7 @@ include:
service.dead:
- name: {{ servicename }}
{% if grains.kernel|lower == 'linux' %}
- onlyif: systemctl list-units |grep {{ servicename }} >/dev/null 2>&1
- onlyif: systemctl list-unit-files |grep {{ servicename }} >/dev/null 2>&1
Copy link
Member

Choose a reason for hiding this comment

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

why do we need those onlyif in service.* states?

Copy link
Author

Choose a reason for hiding this comment

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

... ensure it is dead only if the service exists?

Just fixing it, have not checked further if it would work without it at all.

@daks
Copy link
Member

daks commented May 18, 2022

I rebased this branch on master, added an Inspec test

diff --git a/test/integration/default/controls/archive_spec.rb b/test/integration/default/controls/archive_spec.rb
index 432ad9a..a2a2c24 100644
--- a/test/integration/default/controls/archive_spec.rb
+++ b/test/integration/default/controls/archive_spec.rb
@@ -3,6 +3,9 @@
 control 'mongodb components' do
   title 'should be installed'
 
+  describe service('mongod') do
+    it { should be_running }
+  end
   # describe package('unzip') do
   #   it { should be_installed }
   # end

and I still have a problem with service not running

  ×  mongodb components: should be installed (1 failed)
     ×  Service mongod is expected to be running
     expected that `Service mongod` is running

@daks
Copy link
Member

daks commented May 18, 2022

FYI: even removing those onlyif, I don't have mongod service running, even if the converge pass

fix(systemd): fix service detection
96b02bc 
Fix service detection to properly ensure the service is running.
Command also updated at other service.running/dead onlyif.
@ze42 ze42 force-pushed the systemd-service-detection branch from d8fa40a to 96b02bc Compare July 7, 2022 12:41
@ze42
Copy link
Author

ze42 commented Jul 7, 2022

Testing daks test, and getting the same error, but...

Salt states in is running while applying...

                 ID: mongodb-service-running-database-mongod
           Function: service.running
               Name: mongod
             Result: True
            Comment: Service mongod has been enabled, and is running
            Started: 12:46:32.712630
           Duration: 637.408 ms
            Changes:   
              ----------
              mongod:
                  True

And tests states it is not

  ×  mongodb components: should be installed (1 failed)
     ×  Service mongod is expected to be running
     expected that `Service mongod` is running

Removing that test. If anyone has ideas about it...

@ze42 ze42 force-pushed the systemd-service-detection branch from e853bf5 to 96b02bc Compare July 7, 2022 12:52
fix(services): split enable/running mongos
ce9c356 
Test shows:
  Service mongos has been enabled, and is dead

Attempt to split running and enable states
@ze42 ze42 force-pushed the systemd-service-detection branch 2 times, most recently from 3c235c5 to ce9c356 Compare July 7, 2022 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daks daks daks left review comments

@noelmcloughlin noelmcloughlin noelmcloughlin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] service not launched on install
3 participants
@ze42 @noelmcloughlin @daks