search

StackStorm / st2-packages

StackStorm deb/rpm packages (automated docker build pipeline)
https://stackstorm.com/
27 stars 59 forks source link

Use st2.conf to bind ip:port for st2auth, st2api and st2stream #706

Closed nzlosh closed 2 years ago

nzlosh commented 3 years ago

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 https://github.com/StackStorm/st2-packages/issues/686 https://github.com/StackStorm/st2/issues/3356 https://github.com/StackStorm/st2/issues/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 commented 3 years ago

This PR requires merge of https://github.com/StackStorm/st2/pull/5286 to be complete.

nzlosh commented 3 years ago

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.

cognifloyd commented 2 years ago

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 commented 2 years ago

Also, I dropped the generator scripts under rpm because, as it turns out, we only need the debian copy of these generator scripts: https://github.com/StackStorm/st2-packages/blob/82422e925d2f394c82e71832f147b9e2c6fcdf8d/rpmspec/helpers.spec#L28-L34

And %default_install is used in rpm.spec here: https://github.com/StackStorm/st2-packages/blob/da36b56e315982b136660ae0f3276fb50a3966aa/packages/st2/rpm/st2.spec#L57-L58

%default_install installs things based on these files:

minsis commented 2 years ago

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 commented 2 years ago

Good catch @minsis

It was documented in the changelog https://github.com/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 commented 2 years ago

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 commented 2 years ago

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

nzlosh commented 2 years ago

@armab https://github.com/StackStorm/st2docs/pull/1098

arm4b commented 2 years ago

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

we need it in both places