-
-
Notifications
You must be signed in to change notification settings - Fork 745
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
Remove EL6 support #4984
Remove EL6 support #4984
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this!
I just provided a couple more pointers.
Also don't worry about TravisCI tests, - they're unrelated to this change.
CHANGELOG.rst
Outdated
@@ -6,6 +6,8 @@ in development | |||
|
|||
Added | |||
~~~~~ | |||
* EL6 deprecation. Removed building of EL6 RPMs and associated jobs | |||
Contributed by Amanda McGuinness (@amanda11 Ammeon Solutions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fits into Changed
sub-category under the Changelog. Please take a look at examples from the previous release. Also please try to format this changelog message based on those examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry - didn't see the different sub-sections.
I see in 3.2.0 there actually was a Removed sub-category with "Removed Ubuntu 14.04 from test matrix #4897" in it.
Would you prefer it in Changed or Removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes absolutely, Removed
is even better 👍
Didn't know we had that section. Thanks for noticing that!
st2actions/bin/runners.sh
Outdated
@@ -28,7 +28,7 @@ elif [ -z "$sv" ] && ( /sbin/start 2>&1 | grep -q "missing job name" ); then | |||
sv=upstart | |||
svbin=$UPSTARTCTL | |||
else | |||
# Old debians, redhats and centos, amazon etc | |||
# Old debians, amazon etc | |||
sv=sysv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good that you found this file!
EL6
was the last platform with sysv
init system which relies on /etc/init.d/
service startup files. All OS systems we support right now have systemd
only.
Please try to cleanup this file based on that information, I'll help where I can.
@@ -107,7 +107,7 @@ jobs: | |||
|
|||
# Build & Test st2 packages | |||
packages: | |||
parallelism: 5 | |||
parallelism: 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -27,13 +27,6 @@ rpm: | |||
cp packaging/rpm/$(COMPONENTS).spec $(RPM_SPECS_DIR)/ | |||
cd $(RPM_SPECS_DIR) && rpmbuild --clean --rmsource -ba $(COMPONENTS).spec | |||
|
|||
.PHONY: rhel-rpm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just FYI the rpm/deb logic in the Makefiles of st2
repository is really outdated and unused code we're guilty to remove before.
All packaging happens in https://github.com/stackstorm/st2-packages
Given comment about init.d then there is a python file that checks whether it’s systemd , upstart or sysv and complains if it’s none of those.
Shall I update that as well to remove the allowance of sysv?
|
It was runners.sh that also mentioned sysv. So I've updated that - but I'm not sure if its used, as the st2-packages/runners.sh seems to be what is installed - but that still mentions sysv - so not sure if there's a separate issue raised for updating st2-packages/runners.sh to remove the sysv support. |
Yeah, I will handle st2-packages separately in another PR, per StackStorm/community#39 |
@@ -115,10 +115,8 @@ function service_manager() { | |||
/sbin/initctl $action $svcname | |||
elif command -v service > /dev/null 2>&1; then | |||
service $svcname $action | |||
elif [ -x /etc/init.d/${1} ]; then | |||
/etc/init.d/$svcname $action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch for st2ctl
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Thanks for removing EL6 in the st2
repo!
EL6 is being deprecated. Remove running of builds in EL6.
Remove rhel-rpm in Makefiles, and keep the rpm for EL7 and above. I was a bit confused with the name rhel-rpm - it seemed to just use the el6 specs - and so I was presuming it was no-longer required - but would need confirmation.