[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1647 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76393992 --- Diff: packaging/systemd/cloudstack-agent.default --- @@ -0,0 +1,24 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +JAVA_HOME=/usr/lib/jvm/jre +if [ ! -f "$JAVA_HOME"/bin/java ] ; then +JAVA_HOME=$(readlink -f /usr/bin/java | sed 's:/bin/java::') +fi --- End diff -- Thanks @vincentbernat that's a nice idea. I've removed usage of JAVA_HOME, I checked and indeed all we want was path to java which I checked by default is at /usr/bin/java on most distros including the ones I tested - Ubuntu 12.04, 14.04, 16.04 and on CentOS 7(.2). Are we good to merge with the new change? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76392345
--- Diff: packaging/systemd/cloudstack-agent.default ---
@@ -0,0 +1,24 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+JAVA_HOME=/usr/lib/jvm/jre
+if [ ! -f "$JAVA_HOME"/bin/java ] ; then
+JAVA_HOME=$(readlink -f /usr/bin/java | sed 's:/bin/java::')
+fi
--- End diff --
I think the result is quite fragile. systemd just reads the first
JAVA_HOME, then the second one, ignoring the condition. You could just have
(without the condition):
```
JAVA_HOME=$(readlink -f /usr/bin/java | sed 's:/bin/java::')
```
The only use of JAVA_HOME seems to be to choose a Java vesion. Why not
just, in `/etc/default/...`:
```
JAVA=/usr/bin/java
```
And in the service file:
```
ExecStart=/bin/sh -ec '\
export ACP=`ls /usr/share/cloudstack-agent/lib/*.jar
/usr/share/cloudstack-agent/plugins/*.jar 2>/dev/null|tr "\\n" ":"`; \
export
CLASSPATH="$ACP:/etc/cloudstack/agent:/usr/share/cloudstack-common/scripts"; \
${JAVA} -Xms${JAVA_HEAP_INITIAL} -Xmx${JAVA_HEAP_MAX} -cp "$CLASSPATH"
$JAVA_CLASS'
```
People could still overload the version of Java used by changing the JAVA
variable.
If you are uncomfortable with this, at least, just remove the condition and
simplify the default file to be:
```
JAVA_HOME=$(readlink -f /usr/bin/java | sed 's:/bin/java::')
JAVA_HEAP_INITIAL=256m
JAVA_HEAP_MAX=2048m
JAVA_CLASS=com.cloud.agent.AgentShell
```
From your tests, it works both on Ubuntu and CentOS.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76323706 --- Diff: packaging/systemd/cloudstack-agent.default --- @@ -0,0 +1,24 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +JAVA_HOME=/usr/lib/jvm/jre +if [ ! -f "$JAVA_HOME"/bin/java ] ; then +JAVA_HOME=$(readlink -f /usr/bin/java | sed 's:/bin/java::') +fi --- End diff -- @vincentbernat yes this worked for me both on CentOS 7.2, Ubuntu 16.04 (systemd). Can you build packages and help with a secondary test. When the service starts it executes the environment/default file which gives us the $(readlink...) to replace $JAVA_HOME usage. I see this in my CentOS 7.2 environment (and similar on Ubuntu 16.04): ``` root 27929 0.0 0.0 115340 1616 ?Ss 20:57 0:00 /bin/sh -ec export UCP=`ls /usr/share/cloudstack-usage/cloud-usage-*.jar /usr/share/cloudstack-usage/lib/*.jar /usr/share/cloudstack-mysql-ha/lib/*.jar | tr "\n" ":"`; export CLASSPATH="$UCP:/etc/cloudstack/usage:/usr/share/java/mysql-connector-java.jar"; $(readlink -f /usr/bin/java | sed 's:/bin/java::')/bin/java -Dpid=$$ -Xms256m -Xmx2048m -cp "$CLASSPATH" $JAVA_CLASS root 27936 7.4 13.3 3616388 251860 ? Sl 20:57 0:07 java -Dpid=27929 -Xms256m -Xmx2048m -cp /usr/share/cloudstack-usage/cloud-usage-4.9.1.0-SNAPSHOT.jar:/usr/share/cloudstack-usage/lib/activation-1.1.jar:/usr/share/cloudstack-usage/lib/annotations-2.0.1.jar:/usr/share/cloudstack-usage/lib/antisamy-1.5.3.jar:/usr/share/cloudstack-usage/lib/aopalliance-1.0.jar ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76320932 --- Diff: packaging/systemd/cloudstack-agent.default --- @@ -0,0 +1,24 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +JAVA_HOME=/usr/lib/jvm/jre +if [ ! -f "$JAVA_HOME"/bin/java ] ; then +JAVA_HOME=$(readlink -f /usr/bin/java | sed 's:/bin/java::') +fi --- End diff -- Are you sure this will work with systemd? On Ubuntu, the best it could do is to keep `JAVA_HOME=/usr/lib/jvm/jre` which is not valid on Ubuntu. Maybe, it would be easier to push that inside the ExecStart (and equivalent in init.d): ``` if [ -n "$JAVA_HOME" ]; then export JAVA_HOME=$(readlink -f /usr/bin/java | sed 's:/bin/java::') fi ``` Or, keeping it for RHEL in the default file but using in ExecStart: ``` [ -d "$JAVA_HOME" ] || export JAVA_HOME=$(readlink -f /usr/bin/java | sed 's:/bin/java::') ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76320192 --- Diff: debian/rules --- @@ -35,13 +37,20 @@ override_dh_auto_install: # cloudstack-agent mkdir $(DESTDIR)/$(SYSCONFDIR)/$(PACKAGE)/agent mkdir $(DESTDIR)/$(SYSCONFDIR)/profile.d - mkdir $(DESTDIR)/var/log/$(PACKAGE)/agent mkdir $(DESTDIR)/usr/share/$(PACKAGE)-agent mkdir $(DESTDIR)/usr/share/$(PACKAGE)-agent/plugins install -D agent/target/cloud-agent-$(VERSION).jar $(DESTDIR)/usr/share/$(PACKAGE)-agent/lib/$(PACKAGE)-agent.jar install -D plugins/hypervisors/kvm/target/cloud-plugin-hypervisor-kvm-$(VERSION).jar $(DESTDIR)/usr/share/$(PACKAGE)-agent/lib/ install -D plugins/hypervisors/kvm/target/dependencies/* $(DESTDIR)/usr/share/$(PACKAGE)-agent/lib/ - install -D packaging/debian/init/cloud-agent $(DESTDIR)/$(SYSCONFDIR)/init.d/$(PACKAGE)-agent + + install -m0755 packaging/debian/$(PACKAGE)-agent.init $(DESTDIR)/$(SYSCONFDIR)/init.d/$(PACKAGE)-agent + install -d -m0755 debian/$(PACKAGE)-agent/lib/systemd/system + # Fix libvirt service name for Debian/Ubuntu + sed -i 's/Requires=libvirtd.service/Requires=libvirt-bin.service/g' packaging/systemd/$(PACKAGE)-agent.service + sed -i 's/After=libvirtd.service/After=libvirt-bin.service/g' packaging/systemd/$(PACKAGE)-agent.service + install -m0644 packaging/systemd/$(PACKAGE)-agent.service debian/$(PACKAGE)-agent/lib/systemd/system/$(PACKAGE)-agent.service + install -m0644 packaging/systemd/$(PACKAGE)-agent.default $(DESTDIR)/$(SYSCONFDIR)/default/$(PACKAGE)-agent --- End diff -- OK, the symlink comes with default-jre-headless package. If you don't have it, you don't have the symlink. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76284726 --- Diff: debian/rules --- @@ -35,13 +37,20 @@ override_dh_auto_install: # cloudstack-agent mkdir $(DESTDIR)/$(SYSCONFDIR)/$(PACKAGE)/agent mkdir $(DESTDIR)/$(SYSCONFDIR)/profile.d - mkdir $(DESTDIR)/var/log/$(PACKAGE)/agent mkdir $(DESTDIR)/usr/share/$(PACKAGE)-agent mkdir $(DESTDIR)/usr/share/$(PACKAGE)-agent/plugins install -D agent/target/cloud-agent-$(VERSION).jar $(DESTDIR)/usr/share/$(PACKAGE)-agent/lib/$(PACKAGE)-agent.jar install -D plugins/hypervisors/kvm/target/cloud-plugin-hypervisor-kvm-$(VERSION).jar $(DESTDIR)/usr/share/$(PACKAGE)-agent/lib/ install -D plugins/hypervisors/kvm/target/dependencies/* $(DESTDIR)/usr/share/$(PACKAGE)-agent/lib/ - install -D packaging/debian/init/cloud-agent $(DESTDIR)/$(SYSCONFDIR)/init.d/$(PACKAGE)-agent + + install -m0755 packaging/debian/$(PACKAGE)-agent.init $(DESTDIR)/$(SYSCONFDIR)/init.d/$(PACKAGE)-agent + install -d -m0755 debian/$(PACKAGE)-agent/lib/systemd/system + # Fix libvirt service name for Debian/Ubuntu + sed -i 's/Requires=libvirtd.service/Requires=libvirt-bin.service/g' packaging/systemd/$(PACKAGE)-agent.service + sed -i 's/After=libvirtd.service/After=libvirt-bin.service/g' packaging/systemd/$(PACKAGE)-agent.service + install -m0644 packaging/systemd/$(PACKAGE)-agent.service debian/$(PACKAGE)-agent/lib/systemd/system/$(PACKAGE)-agent.service + install -m0644 packaging/systemd/$(PACKAGE)-agent.default $(DESTDIR)/$(SYSCONFDIR)/default/$(PACKAGE)-agent --- End diff -- This has one issue if there is no /usr/lib/jvm/default-java installed on either Ubuntu 14.04/16.04 using the default repositories. I've reverted using the readlink solution now. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76283738 --- Diff: packaging/systemd/cloudstack-usage.default --- @@ -0,0 +1,25 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +JAVA_HOME=/usr/lib/jvm/jre +if [ ! -f "$JAVA_HOME"/bin/java ] ; then +JAVA_HOME=$(update-alternatives --display java | grep 'currently points to' | sed 's:.*currently points to ::' | sed 's:/bin/java::') +fi --- End diff -- @vincentbernat okay I'll revert it back to the readlink solution. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76281544 --- Diff: debian/rules --- @@ -35,13 +37,20 @@ override_dh_auto_install: # cloudstack-agent mkdir $(DESTDIR)/$(SYSCONFDIR)/$(PACKAGE)/agent mkdir $(DESTDIR)/$(SYSCONFDIR)/profile.d - mkdir $(DESTDIR)/var/log/$(PACKAGE)/agent mkdir $(DESTDIR)/usr/share/$(PACKAGE)-agent mkdir $(DESTDIR)/usr/share/$(PACKAGE)-agent/plugins install -D agent/target/cloud-agent-$(VERSION).jar $(DESTDIR)/usr/share/$(PACKAGE)-agent/lib/$(PACKAGE)-agent.jar install -D plugins/hypervisors/kvm/target/cloud-plugin-hypervisor-kvm-$(VERSION).jar $(DESTDIR)/usr/share/$(PACKAGE)-agent/lib/ install -D plugins/hypervisors/kvm/target/dependencies/* $(DESTDIR)/usr/share/$(PACKAGE)-agent/lib/ - install -D packaging/debian/init/cloud-agent $(DESTDIR)/$(SYSCONFDIR)/init.d/$(PACKAGE)-agent + + install -m0755 packaging/debian/$(PACKAGE)-agent.init $(DESTDIR)/$(SYSCONFDIR)/init.d/$(PACKAGE)-agent + install -d -m0755 debian/$(PACKAGE)-agent/lib/systemd/system + # Fix libvirt service name for Debian/Ubuntu + sed -i 's/Requires=libvirtd.service/Requires=libvirt-bin.service/g' packaging/systemd/$(PACKAGE)-agent.service + sed -i 's/After=libvirtd.service/After=libvirt-bin.service/g' packaging/systemd/$(PACKAGE)-agent.service + install -m0644 packaging/systemd/$(PACKAGE)-agent.service debian/$(PACKAGE)-agent/lib/systemd/system/$(PACKAGE)-agent.service + install -m0644 packaging/systemd/$(PACKAGE)-agent.default $(DESTDIR)/$(SYSCONFDIR)/default/$(PACKAGE)-agent --- End diff -- Here: ``` sed 's+JAVA_HOME=.*+JAVA_HOME=/usr/lib/jvm/default-java+' $(DESTDIR)/$(SYSCONFDIR)/default/$(PACKAGE)-agent ``` If people don't want to use the default JRE, they are free to change `/etc/default/cloudstack-agent`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76281415 --- Diff: packaging/systemd/cloudstack-usage.default --- @@ -0,0 +1,25 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +JAVA_HOME=/usr/lib/jvm/jre +if [ ! -f "$JAVA_HOME"/bin/java ] ; then +JAVA_HOME=$(update-alternatives --display java | grep 'currently points to' | sed 's:.*currently points to ::' | sed 's:/bin/java::') +fi --- End diff -- How is that supposed to work with systemd? Moreover, the `readlink -f` solution was far more robust than parsing `update-alternatives` output. Let me propose something simpler inside `debian/rules` instead. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76227828
--- Diff: debian/cloudstack-agent.postinst ---
@@ -41,6 +41,12 @@ case "$1" in
mkdir /etc/libvirt/hooks
fi
cp -a /usr/share/cloudstack-agent/lib/libvirtqemuhook
/etc/libvirt/hooks/qemu
+
+# Update JAVA_HOME in /etc/default/cloudstack-agent to default JRE
+JAVA_HOME=$(readlink -f /usr/bin/java | sed 's/\/jre.*java//g')
+if [ -d "$JAVA_HOME" ]; then
+sed -i "s:^JAVA_HOME=.*:JAVA_HOME=${JAVA_HOME}:"
/etc/default/cloudstack-agent
+fi
--- End diff --
@vincentbernat thanks, it makes sense. I'll see if we can move this to the
`default/files`. An alternative is to document this, we can keep some default
path and in our installation/upgrade documentation suggest the admin to fix the
JAVA_HOME here if it's something non-default. The code I've used here simply
replaces JAVA_HOME to whatever is set globally on that host.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76227609
--- Diff: packaging/centos7/cloud.spec ---
@@ -482,7 +488,7 @@ pip install --upgrade
/usr/share/cloudstack-marvin/Marvin-*.tar.gz
%dir %attr(0770,root,cloud) %{_localstatedir}/cache/%{name}/management/work
%dir %attr(0770,root,cloud) %{_localstatedir}/cache/%{name}/management/temp
%dir %attr(0770,root,cloud) %{_localstatedir}/log/%{name}/management
-%config(noreplace) %{_sysconfdir}/sysconfig/%{name}-management
--- End diff --
Cool, I see several packages hosting default config (such as what we've
here) at `/etc/default` so I'll keep 'em in `/etc/default` to make things
consistent.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76226635
--- Diff: packaging/centos7/cloud.spec ---
@@ -482,7 +488,7 @@ pip install --upgrade
/usr/share/cloudstack-marvin/Marvin-*.tar.gz
%dir %attr(0770,root,cloud) %{_localstatedir}/cache/%{name}/management/work
%dir %attr(0770,root,cloud) %{_localstatedir}/cache/%{name}/management/temp
%dir %attr(0770,root,cloud) %{_localstatedir}/log/%{name}/management
-%config(noreplace) %{_sysconfdir}/sysconfig/%{name}-management
--- End diff --
I am a Debian guy, so I can't give you an authoritative answer. I didn't
know that `/etc/default` already existed on CentOS too. I suppose that's up to
you. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76226387
--- Diff: packaging/centos7/cloud.spec ---
@@ -482,7 +488,7 @@ pip install --upgrade
/usr/share/cloudstack-marvin/Marvin-*.tar.gz
%dir %attr(0770,root,cloud) %{_localstatedir}/cache/%{name}/management/work
%dir %attr(0770,root,cloud) %{_localstatedir}/cache/%{name}/management/temp
%dir %attr(0770,root,cloud) %{_localstatedir}/log/%{name}/management
-%config(noreplace) %{_sysconfdir}/sysconfig/%{name}-management
--- End diff --
@vincentbernat thanks, I was not sure but found that /etc/default exists
and is used by several packages on CentOS and it would keep paths uniform
across distributions. Should I revert back to sysconfig for CentOS packages
then?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76226318
--- Diff: debian/cloudstack-agent.postinst ---
@@ -41,6 +41,12 @@ case "$1" in
mkdir /etc/libvirt/hooks
fi
cp -a /usr/share/cloudstack-agent/lib/libvirtqemuhook
/etc/libvirt/hooks/qemu
+
+# Update JAVA_HOME in /etc/default/cloudstack-agent to default JRE
+JAVA_HOME=$(readlink -f /usr/bin/java | sed 's/\/jre.*java//g')
+if [ -d "$JAVA_HOME" ]; then
+sed -i "s:^JAVA_HOME=.*:JAVA_HOME=${JAVA_HOME}:"
/etc/default/cloudstack-agent
+fi
--- End diff --
This would overwrite a change made by a user during upgrades. I would
suggest to move the first line directly in /etc/default/cloudstack-agent (if
it's compatible with RHEL). A user that didn't change anything will get the new
file and it will work as expected. A user that did alter the file will get a
prompt to solve the situation.
However, this won't work with systemd without further ugly hacks.
On my system, I notice that `/usr/lib/jvm/default-java` is a symlink. It
also seems to be the case on Precise. Maybe you could patch
`/etc/default/cloudstack-agent` after installing it in `debian/rules`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76225900
--- Diff: packaging/centos7/cloud.spec ---
@@ -482,7 +488,7 @@ pip install --upgrade
/usr/share/cloudstack-marvin/Marvin-*.tar.gz
%dir %attr(0770,root,cloud) %{_localstatedir}/cache/%{name}/management/work
%dir %attr(0770,root,cloud) %{_localstatedir}/cache/%{name}/management/temp
%dir %attr(0770,root,cloud) %{_localstatedir}/log/%{name}/management
-%config(noreplace) %{_sysconfdir}/sysconfig/%{name}-management
--- End diff --
RHEL users won't expect this file to be in /etc/default. In the systemd
unit:
```
EnvironmentFile=-/etc/default/cloudstack-agent
EnvironmentFile=-/etc/sysconfig/cloudstack-agent
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76206826 --- Diff: debian/rules --- @@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1) SYSCONFDIR = "/etc" DESTDIR = "debian/tmp" -ifeq ($(shell lsb_release -sr), 14.04) -SUBSTVARS = -Vjre:Depends="openjdk-7-jre-headless" -Vjdk:Depends="openjdk-7-jdk" -Vinit:"Depends:jsvc" -else -SUBSTVARS = -Vjre:Depends="openjdk-8-jre-headless" -Vjdk:Depends="openjdk-8-jdk" -Vinit:"Depends:systemd" -endif - --- End diff -- Oh, OK, that's why you removed it. OK. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76202771 --- Diff: debian/rules --- @@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1) SYSCONFDIR = "/etc" DESTDIR = "debian/tmp" -ifeq ($(shell lsb_release -sr), 14.04) -SUBSTVARS = -Vjre:Depends="openjdk-7-jre-headless" -Vjdk:Depends="openjdk-7-jdk" -Vinit:"Depends:jsvc" -else -SUBSTVARS = -Vjre:Depends="openjdk-8-jre-headless" -Vjdk:Depends="openjdk-8-jdk" -Vinit:"Depends:systemd" -endif - --- End diff -- @vincentbernat that's right, but if we use that then we'll need to build/host/publish two version of the deb repository, depending on whether the deb pkgs were built of Ubuntu 14.04 or 16.04+. I think the current is still a good compromise, I can add new dependency options to allow users to use oracle jre/jdk. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76200059 --- Diff: debian/rules --- @@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1) SYSCONFDIR = "/etc" DESTDIR = "debian/tmp" -ifeq ($(shell lsb_release -sr), 14.04) -SUBSTVARS = -Vjre:Depends="openjdk-7-jre-headless" -Vjdk:Depends="openjdk-7-jdk" -Vinit:"Depends:jsvc" -else -SUBSTVARS = -Vjre:Depends="openjdk-8-jre-headless" -Vjdk:Depends="openjdk-8-jdk" -Vinit:"Depends:systemd" -endif - --- End diff -- That's why I think that the use of `$SUBSTVARS` and `dh_gencontrol` override was a good idea. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76197541 --- Diff: debian/rules --- @@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1) SYSCONFDIR = "/etc" DESTDIR = "debian/tmp" -ifeq ($(shell lsb_release -sr), 14.04) -SUBSTVARS = -Vjre:Depends="openjdk-7-jre-headless" -Vjdk:Depends="openjdk-7-jdk" -Vinit:"Depends:jsvc" -else -SUBSTVARS = -Vjre:Depends="openjdk-8-jre-headless" -Vjdk:Depends="openjdk-8-jdk" -Vinit:"Depends:systemd" -endif - --- End diff -- @vincentbernat won't this force some release of Java8, what if the openjdk/oracle java8 pkg is not available on that system, say on 12.04 etc.? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user vincentbernat commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76039128 --- Diff: debian/rules --- @@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1) SYSCONFDIR = "/etc" DESTDIR = "debian/tmp" -ifeq ($(shell lsb_release -sr), 14.04) -SUBSTVARS = -Vjre:Depends="openjdk-7-jre-headless" -Vjdk:Depends="openjdk-7-jdk" -Vinit:"Depends:jsvc" -else -SUBSTVARS = -Vjre:Depends="openjdk-8-jre-headless" -Vjdk:Depends="openjdk-8-jdk" -Vinit:"Depends:systemd" -endif - --- End diff -- I would have kept something like that but using init-system-helpers instead of systemd. It is not unheard of for people to not install recommends. However, init-system-helpers is likely to be installed on the target system already (since everything that comes with a systemd unit pulls it). So, it's a bit nitpicking. About Java, you can change the dependency to: ``` default-jre-headless (>= 1:1.8) | java8-runtime-headless | java8-runtime ``` This would enable people to use Oracle JRE instead. Most packages for non-OpenJDK JRE are providing `java8-runtime-headless` or `java8-runtime`. For JDK, this would be: ``` default-jdk-headless (>= 1:1.8) | java8-sdk | java8-jdk ``` The normal virtual package is `java8-sdk` but some unofficial packages are using `java8-jdk` instead. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76007356 --- Diff: debian/rules --- @@ -5,8 +5,17 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1) SYSCONFDIR = "/etc" DESTDIR = "debian/tmp" +ifeq ($(shell lsb_release -sr), 14.04) +SUBSTVARS = -Vjre:Depends="openjdk-7-jre-headless" -Vjdk:Depends="openjdk-7-jdk" -Vinit:"Depends:jsvc" +else +SUBSTVARS = -Vjre:Depends="openjdk-8-jre-headless" -Vjdk:Depends="openjdk-8-jdk" -Vinit:"Depends:systemd" --- End diff -- Thanks @wido I'll try to build experimental packages to test if it just might work. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user wido commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76006449 --- Diff: debian/rules --- @@ -5,8 +5,17 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1) SYSCONFDIR = "/etc" DESTDIR = "debian/tmp" +ifeq ($(shell lsb_release -sr), 14.04) +SUBSTVARS = -Vjre:Depends="openjdk-7-jre-headless" -Vjdk:Depends="openjdk-7-jdk" -Vinit:"Depends:jsvc" +else +SUBSTVARS = -Vjre:Depends="openjdk-8-jre-headless" -Vjdk:Depends="openjdk-8-jdk" -Vinit:"Depends:systemd" --- End diff -- That might be possible, I am not sure. I think we could do it. Haven't tried that. I'm however very, very short on time these weeks, so I won't be able to work on it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1647: [lts] CLOUDSTACK-9462: Systemd support for Ub...
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1647#discussion_r76004158 --- Diff: debian/rules --- @@ -5,8 +5,17 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1) SYSCONFDIR = "/etc" DESTDIR = "debian/tmp" +ifeq ($(shell lsb_release -sr), 14.04) +SUBSTVARS = -Vjre:Depends="openjdk-7-jre-headless" -Vjdk:Depends="openjdk-7-jdk" -Vinit:"Depends:jsvc" +else +SUBSTVARS = -Vjre:Depends="openjdk-8-jre-headless" -Vjdk:Depends="openjdk-8-jdk" -Vinit:"Depends:systemd" --- End diff -- @wido should this be written like: `-Vinit:Depends="systemd"`. Would it be possible to support both systemd/initd environments by adding the dependency like `systemd | jsvc` -- this way it would try to install both? On (systemd based) Ubuntu 16.04 systemd will be already installed, we can install jsvc for all systems -- so why not just depend on jsvc alone? On 14.04, the pkgs can continue to use jsvc/initd script and on 16.04 it can uses systemd, both using the same packages. I want us to avoid hosting two separate deb repositories (one for non-systemd based and one for systemd based) if possible. Thoughts? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
