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 #28067 - dynflow sidekiq services config #761

Merged
merged 1 commit into from
Jan 20, 2020

Conversation

ezr-ondrej
Copy link
Member

@ezr-ondrej ezr-ondrej commented Oct 16, 2019

Install necessary sidekiq configs and dependencies.

manifests/config.pp Outdated Show resolved Hide resolved
@ezr-ondrej ezr-ondrej force-pushed the dynflow_sidekiq branch 6 times, most recently from 63c29e1 to ab7fcff Compare October 22, 2019 16:25
manifests/service.pp Outdated Show resolved Hide resolved
manifests/init.pp Outdated Show resolved Hide resolved
manifests/config.pp Outdated Show resolved Hide resolved
manifests/config.pp Outdated Show resolved Hide resolved
manifests/config.pp Outdated Show resolved Hide resolved
@ezr-ondrej
Copy link
Member Author

ezr-ondrej commented Oct 22, 2019

the tests doesn't seem to kick on It's on

@tbrisker
Copy link
Member

Tests are failing?

@ezr-ondrej
Copy link
Member Author

Tests are failing?

As we have it default on, it needs the package, that is not in yet

@tbrisker
Copy link
Member

To summarize our discussion from https://community.theforeman.org/t/release-meeting-notes-2019-10-23/15883 - this will be merged only post branching and will be supported in 1.25.

@ezr-ondrej ezr-ondrej force-pushed the dynflow_sidekiq branch 3 times, most recently from 67c867f to 7e087ba Compare January 14, 2020 13:01
@ezr-ondrej ezr-ondrej force-pushed the dynflow_sidekiq branch 2 times, most recently from f21a8fe to aef973d Compare January 15, 2020 13:54
@ehelms
Copy link
Member

ehelms commented Jan 15, 2020

I tested this out on a Katello nightly. Installation was successful but bats test are failing for most anything task related.

My forklift box:

dynflow:
  box: centos7-katello-nightly
  ansible:
    variables:
      foreman_repositories_environment: staging
      katello_repositories_environment: staging
      foreman_installer_module_prs:
        - theforeman/foreman/761

After running vagrant up dynflow, I ran the bats tests:

ansible-playbook playbook/bats.yml -l dynflow

@ezr-ondrej
Copy link
Member Author

I think you need this: theforeman/foreman-installer#400

The failure seems to be in the ping command though. It fails on status dynflowd but I don't see it validated in the ping command 🤔

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

@ehelms could you have a look?

The Debian acceptance tests will fail until we actually build a new package with the packaging change (https://ci.theforeman.org/job/foreman-develop-package-release/336/) and promote it.

@ehelms
Copy link
Member

ehelms commented Jan 17, 2020

Will run through a few tests and report back

@ekohl
Copy link
Member

ekohl commented Jan 17, 2020

I've kicked off https://ci.theforeman.org/job/foreman-nightly-deb-pipeline/263/ and will restart the Debian tests after that.

@ehelms
Copy link
Member

ehelms commented Jan 17, 2020

I'm getting less bats failures now, woot! But I am still seeing an issue with one related to package installation failing:

 ok 29 enable content view repo
    not ok 30 install katello-host-tools
    # (from function `tIsRedHatCompatible' in file os_helper.bash, line 4,
    #  from function `tPackageInstall' in file os_helper.bash, line 89,
    #  in test file fb-content-katello.bats, line 194)
    #   `tPackageInstall katello-host-tools && tPackageExists katello-host-tools' failed
    # Loaded plugins: fastestmirror, product-id, search-disabled-repos, subscription-
    #               : manager
    # 1 local certificate has been deleted.
    # Loading mirror speeds from cached hostfile
    #  * base: mirror.math.princeton.edu
    #  * centos-sclo-rh: mirror.fileplanet.com
    #  * epel: download-ib01.fedoraproject.org
    #  * extras: mirrors.mit.edu
    #  * updates: repo1.ash.innoscale.net
    # No package katello-host-tools available.
    # Error: Nothing to do

This makes me think one of the previous tasks is failing in some way or incomplete but is not registering as such. Needs further investigation before we merge. As this will surely result in a broken nightly.

My steps were the same as before. I spin up a machine with this puppet module patch, and latest nightly and ran bats tests against it.

@ekohl
Copy link
Member

ekohl commented Jan 17, 2020

Perhaps it was an issue that the old dynflowd was still running? After theforeman/foreman-packaging#4442 is merged, that should be fixed in a yum update but until then it might interfere.

@ehelms
Copy link
Member

ehelms commented Jan 17, 2020

It appeared dead when I checked:

[root@dynflow vagrant]# systemctl status dynflowd
● dynflowd.service - Foreman jobs daemon
   Loaded: loaded (/usr/lib/systemd/system/dynflowd.service; disabled; vendor preset: disabled)
   Active: inactive (dead)
     Docs: https://theforeman.org

@mmoll
Copy link
Contributor

mmoll commented Jan 17, 2020

Debian seems to fail on the dynflow-orchestrator service now 💔

@adamruzicka
Copy link
Contributor

+Jan 17 21:49:32 debian10-64.example.com systemd[1]: Started Foreman jobs daemon - orchestrator on sidekiq.
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]: 2020-01-17T21:49:33.658Z 9064 TID-gpi086500 INFO: ==================================================================
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]: 2020-01-17T21:49:33.659Z 9064 TID-gpi086500 INFO:   Please point sidekiq to a Rails 4/5 application or a Ruby file
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]: 2020-01-17T21:49:33.659Z 9064 TID-gpi086500 INFO:   to load your worker classes with -r [DIR|FILE].
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]: 2020-01-17T21:49:33.659Z 9064 TID-gpi086500 INFO: ==================================================================
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]: 2020-01-17T21:49:33.659Z 9064 TID-gpi086500 INFO: sidekiq [options]
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -c, --concurrency INT            processor threads to use
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -d, --daemon                     Daemonize process
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -e, --environment ENV            Application environment
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -g, --tag TAG                    Process tag for procline
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -i, --index INT                  unique process index on this machine
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -q, --queue QUEUE[,WEIGHT]       Queues to process with optional weights
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -r, --require [PATH|DIR]         Location of Rails application with workers or file to require
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -t, --timeout NUM                Shutdown timeout
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -v, --verbose                    Print more verbose output
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -C, --config PATH                path to YAML config file
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -L, --logfile PATH               path to writable logfile
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -P, --pidfile PATH               path to pidfile
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -V, --version                    Print version and exit
+Jan 17 21:49:33 debian10-64.example.com dynflow-sidekiq@orchestrator[9064]:     -h, --help                       Show help

It looks like the systemd unit is somehow wrong

@ekohl
Copy link
Member

ekohl commented Jan 20, 2020

Looks like it's not included in Debian

# ls /usr/share/foreman/extras/dynflow-sidekiq.rb
ls: cannot access '/usr/share/foreman/extras/dynflow-sidekiq.rb': No such file or directory

I was looking at systemd conditionals to check if $DYNFLOW_SIDEKIQ_SCRIPT exists but I can't really get them to work. Probably best to first just ship it and later look at other guarantees.

Speaking of guarantees: using type=notify per https://github.com/mperham/sidekiq/blob/3510de7df3c2eaacec9061184666aa853a0abb13/examples/systemd/sidekiq.service#L28-L35 looks like a good idea but does require some code in core.

@ekohl
Copy link
Member

ekohl commented Jan 20, 2020

theforeman/foreman-packaging#4620 intends to fix the Debian packaging side.

@ehelms
Copy link
Member

ehelms commented Jan 20, 2020

Looks like I am seeing two failures, but latest nightly has the same bats failures. So I am good from that perspective. We will need this to land close to the foreman-maintain change + a build of foreman-maintain or else nightly will fail with the status check.

@adamruzicka
Copy link
Contributor

I updated the f-m PR to have the correct redis service names

@ekohl
Copy link
Member

ekohl commented Jan 20, 2020

https://ci.theforeman.org/job/foreman-develop-package-release/343/ should include the Debian fix. Once complete I'll kick off https://ci.theforeman.org/job/foreman-nightly-deb-pipeline/ as well to get green tests.

I updated the f-m PR to have the correct redis service names

This is theforeman/foreman_maintain#292

@ekohl
Copy link
Member

ekohl commented Jan 20, 2020

https://ci.theforeman.org/job/foreman-nightly-deb-pipeline/268/ should make that available.

@ekohl
Copy link
Member

ekohl commented Jan 20, 2020

I think the same statsd failure is seen in master now, haven't looked into it yet.

@mmoll
Copy link
Contributor

mmoll commented Jan 20, 2020

Exactly 💚

@mmoll mmoll merged commit bdd1730 into theforeman:master Jan 20, 2020
@mmoll
Copy link
Contributor

mmoll commented Jan 20, 2020

merged, díky @ezr-ondrej!

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.

8 participants