[Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Olga Arkhangelskaia]

After taratool installation on Debuan/Ubuntu from repo, example instance
was automatically started over 3301 port. On the other hand example instance
on RHEL/CentOS is started manually. Patch does the same for Debian/Ubuntu.

Closes #4507

@TarantoolBot document
Title: manage example instance after installation
After tarantool installation from repos on all distros example instance
should be managed manually.
---
Branch:
https://github.com/tarantool/tarantool/tree/gh-4507-disable-start-example-instance
 debian/tarantool-common.postinst | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
index e2eda3415..03e4b2215 100644
--- a/debian/tarantool-common.postinst
+++ b/debian/tarantool-common.postinst
@@ -22,14 +22,6 @@ case "$1" in
         install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
         install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
         install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
-
-        # Enable example.lua by default
-        if [ -z $2 ] && [ ! -e /etc/tarantool/instances.enabled/example.lua ] &&
-           [ -d /etc/tarantool/instances.enabled ] &&
-           [ -d /etc/tarantool/instances.available/ ]; then
-          ln -s /etc/tarantool/instances.available/example.lua \
-                /etc/tarantool/instances.enabled/example.lua
-        fi
     ;;
 esac
 
-- 
2.20.1 (Apple Git-117)

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Igor Munkin]

Olya,

Thanks for the patch, LGTM except some nits regarding commit message.
However, it would be great to see a green full CI pipeline for such
changes especially Debian/Ubuntu packaging routines tests to confirm
nothing is broken after your change. Since this patch is trivial and
doesn't directly relate to package build process, I guess manual check
is enough.

On 29.04.20, Olga Arkhangelskaia wrote:
> After taratool installation on Debuan/Ubuntu from repo, example instance

Typo: s/taratool/tarantool/.
Typo: s/Debuan/Debian/.

> was automatically started over 3301 port. On the other hand example instance

Typo: s/over/on/.
Typo: s/On the other hand/At the same time/.

> on RHEL/CentOS is started manually. Patch does the same for Debian/Ubuntu.
> 
> Closes #4507
> 
> @TarantoolBot document
> Title: manage example instance after installation
> After tarantool installation from repos on all distros example instance
> should be managed manually.
> ---

Please provide a ChangeLog entry.

> Branch:
> https://github.com/tarantool/tarantool/tree/gh-4507-disable-start-example-instance
>  debian/tarantool-common.postinst | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
> index e2eda3415..03e4b2215 100644
> --- a/debian/tarantool-common.postinst
> +++ b/debian/tarantool-common.postinst
> @@ -22,14 +22,6 @@ case "$1" in
>          install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>          install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>          install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
> -
> -        # Enable example.lua by default
> -        if [ -z $2 ] && [ ! -e /etc/tarantool/instances.enabled/example.lua ] &&
> -           [ -d /etc/tarantool/instances.enabled ] &&
> -           [ -d /etc/tarantool/instances.available/ ]; then
> -          ln -s /etc/tarantool/instances.available/example.lua \
> -                /etc/tarantool/instances.enabled/example.lua
> -        fi
>      ;;
>  esac
>  
> -- 
> 2.20.1 (Apple Git-117)
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Olga Arkhangelskaia]

Hi Igor!

Have fixed commit message:

  After tarantool installation on Debian/Ubuntu from repo, example instance
  was automatically started on 3301 port. At the same time example instance
  on RHEL/CentOS is started manually. Patch does the same for 
Debian/Ubuntu.

   Closes #4507

   @TarantoolBot document
   Title: manage example instance after installation
   After tarantool installation from repos on all distros example instance
   should be managed manually.

  @ChangeLog:
   * Do not start example instance on 3301 port after tarantool 
installation
   from Debian/Ubuntu repo.

25.05.2020 16:03, Igor Munkin пишет:
> Olya,
>
> Thanks for the patch, LGTM except some nits regarding commit message.
> However, it would be great to see a green full CI pipeline for such
> changes especially Debian/Ubuntu packaging routines tests to confirm
> nothing is broken after your change. Since this patch is trivial and
> doesn't directly relate to package build process, I guess manual check
> is enough.
>
> On 29.04.20, Olga Arkhangelskaia wrote:
>> After taratool installation on Debuan/Ubuntu from repo, example instance
> Typo: s/taratool/tarantool/.
> Typo: s/Debuan/Debian/.
>
>> was automatically started over 3301 port. On the other hand example instance
> Typo: s/over/on/.
> Typo: s/On the other hand/At the same time/.
>
>> on RHEL/CentOS is started manually. Patch does the same for Debian/Ubuntu.
>>
>> Closes #4507
>>
>> @TarantoolBot document
>> Title: manage example instance after installation
>> After tarantool installation from repos on all distros example instance
>> should be managed manually.
>> ---
> Please provide a ChangeLog entry.
>
>> Branch:
>> https://github.com/tarantool/tarantool/tree/gh-4507-disable-start-example-instance
>>   debian/tarantool-common.postinst | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
>> index e2eda3415..03e4b2215 100644
>> --- a/debian/tarantool-common.postinst
>> +++ b/debian/tarantool-common.postinst
>> @@ -22,14 +22,6 @@ case "$1" in
>>           install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
>> -
>> -        # Enable example.lua by default
>> -        if [ -z $2 ] && [ ! -e /etc/tarantool/instances.enabled/example.lua ] &&
>> -           [ -d /etc/tarantool/instances.enabled ] &&
>> -           [ -d /etc/tarantool/instances.available/ ]; then
>> -          ln -s /etc/tarantool/instances.available/example.lua \
>> -                /etc/tarantool/instances.enabled/example.lua
>> -        fi
>>       ;;
>>   esac
>>   
>> -- 
>> 2.20.1 (Apple Git-117)
>>

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Oleg Babin]

Hi! Thanks for your patch! LGTM after changes.

On 25/05/2020 19:27, Olga Arkhangelskaia wrote:
> Hi Igor!
> 
> Have fixed commit message:
> 
>   After tarantool installation on Debian/Ubuntu from repo, example instance
>   was automatically started on 3301 port. At the same time example instance
>   on RHEL/CentOS is started manually. Patch does the same for 
> Debian/Ubuntu.
> 
>    Closes #4507
> 
>    @TarantoolBot document
>    Title: manage example instance after installation
>    After tarantool installation from repos on all distros example instance
>    should be managed manually.
> 
>   @ChangeLog:
>    * Do not start example instance on 3301 port after tarantool 
> installation
>    from Debian/Ubuntu repo.
> 
> 25.05.2020 16:03, Igor Munkin пишет:
>> Olya,
>>
>> Thanks for the patch, LGTM except some nits regarding commit message.
>> However, it would be great to see a green full CI pipeline for such
>> changes especially Debian/Ubuntu packaging routines tests to confirm
>> nothing is broken after your change. Since this patch is trivial and
>> doesn't directly relate to package build process, I guess manual check
>> is enough.
>>
>> On 29.04.20, Olga Arkhangelskaia wrote:
>>> After taratool installation on Debuan/Ubuntu from repo, example instance
>> Typo: s/taratool/tarantool/.
>> Typo: s/Debuan/Debian/.
>>
>>> was automatically started over 3301 port. On the other hand example 
>>> instance
>> Typo: s/over/on/.
>> Typo: s/On the other hand/At the same time/.
>>
>>> on RHEL/CentOS is started manually. Patch does the same for 
>>> Debian/Ubuntu.
>>>
>>> Closes #4507
>>>
>>> @TarantoolBot document
>>> Title: manage example instance after installation
>>> After tarantool installation from repos on all distros example instance
>>> should be managed manually.
>>> ---
>> Please provide a ChangeLog entry.
>>
>>> Branch:
>>> https://github.com/tarantool/tarantool/tree/gh-4507-disable-start-example-instance 
>>>
>>>   debian/tarantool-common.postinst | 8 --------
>>>   1 file changed, 8 deletions(-)
>>>
>>> diff --git a/debian/tarantool-common.postinst 
>>> b/debian/tarantool-common.postinst
>>> index e2eda3415..03e4b2215 100644
>>> --- a/debian/tarantool-common.postinst
>>> +++ b/debian/tarantool-common.postinst
>>> @@ -22,14 +22,6 @@ case "$1" in
>>>           install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>>>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>>>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
>>> -
>>> -        # Enable example.lua by default
>>> -        if [ -z $2 ] && [ ! -e 
>>> /etc/tarantool/instances.enabled/example.lua ] &&
>>> -           [ -d /etc/tarantool/instances.enabled ] &&
>>> -           [ -d /etc/tarantool/instances.available/ ]; then
>>> -          ln -s /etc/tarantool/instances.available/example.lua \
>>> -                /etc/tarantool/instances.enabled/example.lua
>>> -        fi
>>>       ;;
>>>   esac
>>> -- 
>>> 2.20.1 (Apple Git-117)
>>>

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Igor Munkin]

Olya,

Thanks, but I have last (but not least) nit: a ChangeLog entry need to
be plased aside from the commit message, i.e. under three dashes or in a
cover letter. Please adjust your commit message regarding this remark.

On 25.05.20, Olga Arkhangelskaia wrote:
> Hi Igor!
> 
> Have fixed commit message:
> 
>   After tarantool installation on Debian/Ubuntu from repo, example instance
>   was automatically started on 3301 port. At the same time example instance
>   on RHEL/CentOS is started manually. Patch does the same for 
> Debian/Ubuntu.
> 
>    Closes #4507
> 
>    @TarantoolBot document
>    Title: manage example instance after installation
>    After tarantool installation from repos on all distros example instance
>    should be managed manually.
> 
>   @ChangeLog:
>    * Do not start example instance on 3301 port after tarantool 
> installation
>    from Debian/Ubuntu repo.
> 

<snipped>

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Olga Arkhangelskaia]

Hi!

I have removed @ChangeLog from commit message.

I guess I jut leave it here:

@ChangeLog:
  * Do not start example instance on 3301 port after tarantool installation
   from Debian/Ubuntu repo.


26.05.2020 19:00, Igor Munkin пишет:
> Olya,
>
> Thanks, but I have last (but not least) nit: a ChangeLog entry need to
> be plased aside from the commit message, i.e. under three dashes or in a
> cover letter. Please adjust your commit message regarding this remark.
>
> On 25.05.20, Olga Arkhangelskaia wrote:
>> Hi Igor!
>>
>> Have fixed commit message:
>>
>>    After tarantool installation on Debian/Ubuntu from repo, example instance
>>    was automatically started on 3301 port. At the same time example instance
>>    on RHEL/CentOS is started manually. Patch does the same for
>> Debian/Ubuntu.
>>
>>     Closes #4507
>>
>>     @TarantoolBot document
>>     Title: manage example instance after installation
>>     After tarantool installation from repos on all distros example instance
>>     should be managed manually.
>>
>>    @ChangeLog:
>>     * Do not start example instance on 3301 port after tarantool
>> installation
>>     from Debian/Ubuntu repo.
>>
> <snipped>
>

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Alexander Turenko]

I was wondered why the instance is started automatically and found past
discussions on this topic, see [1].

I also added some thoughts whether it is right to change this: I think
so, we should disable the default example instance.

See comments for the patch itself below.

[1]: https://github.com/tarantool/tarantool/issues/4507#issuecomment-634567994

WBR, Alexander Turenko.

> core: don't start example instance in postinstall 

Nit: I would use prefix 'build' here. 'core' is usually about the code
that resides in src/lib/core.

On Wed, Apr 29, 2020 at 03:20:38PM +0300, Olga Arkhangelskaia wrote:
> After taratool installation on Debuan/Ubuntu from repo, example instance
> was automatically started over 3301 port. On the other hand example instance
> on RHEL/CentOS is started manually. Patch does the same for Debian/Ubuntu.
> 
> Closes #4507
> 
> @TarantoolBot document
> Title: manage example instance after installation
> After tarantool installation from repos on all distros example instance
> should be managed manually.

I verified, where the old behaviour is mentioned in the documentation:

 | Depending on the release, during installation Tarantool may start a
 | demonstrative global example.lua instance that listens to the 3301
 | port by default. <...>
 |
 | <...>
 |
 | However, we encourage you to perform the instance startup manually,
 | so you can learn.

Cited from [1] and [2].

[1]: https://www.tarantool.io/en/doc/2.4/getting_started/getting_started_db/#using-a-binary-package
[2]: https://www.tarantool.io/en/doc/2.4/getting_started/using_binary/

In fact this wording does not contradicts with neither of the
behaviours. I guess it is better to left the documentation as is for a
while.

> ---
> Branch:
> https://github.com/tarantool/tarantool/tree/gh-4507-disable-start-example-instance

I would check it on a -full-ci branch just in case. Debian package build
is changed and we should at least ensure that packages are build.

Hope we'll add such builds to verify each push to a developer branch,
but now it is only on -full-ci and release branches.

>  debian/tarantool-common.postinst | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
> index e2eda3415..03e4b2215 100644
> --- a/debian/tarantool-common.postinst
> +++ b/debian/tarantool-common.postinst
> @@ -22,14 +22,6 @@ case "$1" in
>          install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>          install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>          install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
> -
> -        # Enable example.lua by default
> -        if [ -z $2 ] && [ ! -e /etc/tarantool/instances.enabled/example.lua ] &&
> -           [ -d /etc/tarantool/instances.enabled ] &&
> -           [ -d /etc/tarantool/instances.available/ ]; then
> -          ln -s /etc/tarantool/instances.available/example.lua \
> -                /etc/tarantool/instances.enabled/example.lua
> -        fi

/etc/tarantool/instances.enabled/example.lua will not be removed
automatically even after tarantool-common upgrade, because the symlink
will be removed in debian/tarantool-common.postrm only at `apt-get purge
tarantool-common`.

One possible way is to left everything as is and mention necessary steps
to get rid from example.lua symlink (say, `apt-get purge
tarantool-common`, `apt-get install tarantool-common`) in release notes.

However it looks as a lie: as user I would expect that the new behaviour
will become in effect just after upgrade. But we should not remove the
symlink every time when tarantool-common is updated, because a user may
want to use this example instance.

I looked over the Debian maintainer scripts documentation ([2], [3]) and
found several ways to stop and disable an instance on upgrade.

[2]: https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html
[3]: https://www.debian.org/doc/debian-policy/ap-flowcharts.html

First, we can compare 'previously configured' and 'newly configured'
versions on 'postinst configure' stage of tarantool-common. This,
however, both ugly and fragile. It is ugly, because we don't know exact
four-digits version at which the patch will land into all long-term
branches: so we'll likely remove the symlink during several upgrades,
not only one. It is fragile, because it will not be reliable if we'll
decide to push the patch to, say, 1.10 later (it should be included into
the condition of the master patch).

It would look like so (not tested):

diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
index 03e4b2215..17d6982c0 100644
--- a/debian/tarantool-common.postinst
+++ b/debian/tarantool-common.postinst
@@ -22,6 +22,13 @@ case "$1" in
         install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
         install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
         install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
+
+        if [ -e /etc/tarantool/instances.enabled/example.lua ] &&
+           [ -n "$2" ] && (dpkg --compare-versions $2 lt 2.4.2 ||
+           (dpkg --compare-versions $2 gt 2.5.0 &&
+           dpkg --compare-versions $2 lt 2.5.1)); then
+            rm /etc/tarantool/instances.enabled/example.lua
+        fi
     ;;
 esac

Then next way I considered is to add /etc/tarantool/debian/keep
directory, which will hold files that should be hold during upgrade. But
it looks weird: there is only one file we want to remove from the old
configuration during upgrade and unlikely there will be more. This
mechanism looks artificial here.

So I implemented it in the migration-style way, see the patch below the
email.

Please, note that enabled instances directory now can be empty and it
would be good to handle this case in the systemd unit generator file (it
is in the patch below too).

To be honest, I still don't very like the result (those upgrade markers
right in /etc), but at least it looks like a general mechanism that may
be used again (but I hope it will not). If dpkg would provide some
database for performing migrations during updates, I would use it. But
it seems, no: it is overkill for dpkg (I don't found anything like
this).

Olya, Igor, Oleg, can you look into this change and say what do you
think about?

----

diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
index 03e4b2215..e368cdd2e 100644
--- a/debian/tarantool-common.postinst
+++ b/debian/tarantool-common.postinst
@@ -22,6 +22,46 @@ case "$1" in
         install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
         install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
         install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
+
+        # Track configuration updates we want to perform only
+        # once.
+        UPGRADE_DIR="/etc/tarantool/debian/upgrade"
+        mkdir -p "${UPGRADE_DIR}"
+
+        # Add README file to /etc/tarantool/debian/upgrade
+        # to don't confuse a user with this directory.
+        UPGRADE_FILE_0000="${UPGRADE_DIR}/0000-add-readme.done"
+        if [ ! -e "${UPGRADE_FILE_0000}" ]; then
+            cat << EOF > "${UPGRADE_DIR}/README"
+There are rare cases, when existing configuration files
+content or structure should be upgraded when a new package
+version is installed.
+
+This directory tracks performed updates.
+EOF
+            touch "${UPGRADE_FILE_0000}"
+        fi
+
+        # Stop and disable example.lua if it was started/enabled,
+        # but do it only once in case a user actually want the
+        # instance be enabled.
+        UPGRADE_FILE_0001="${UPGRADE_DIR}/0001-disable-example-instance.done"
+        if [ ! -e "${UPGRADE_FILE_0001}" ]; then
+            # Stop the instance if it was started.
+            if command -v invoke-rc.d > /dev/null; then
+                invoke-rc.d tarantool@example stop || true
+            else
+                /etc/init.d/tarantool@example stop || true
+            fi
+
+            # Disable the instance, but keep it in
+            # instances.available.
+            if [ -e /etc/tarantool/instances.enabled/example.lua ]; then
+                rm /etc/tarantool/instances.enabled/example.lua
+            fi
+
+            touch "${UPGRADE_FILE_0001}"
+        fi
     ;;
 esac
 
diff --git a/extra/dist/tarantool-generator.in b/extra/dist/tarantool-generator.in
index f6a6a2540..2048b7517 100755
--- a/extra/dist/tarantool-generator.in
+++ b/extra/dist/tarantool-generator.in
@@ -13,6 +13,7 @@ mkdir -p "$wantdir"
 
 for file in @TARANTOOL_ENABLEDDIR@/*.lua; do
        instance=`basename $file .lua`
+       [ "${instance}" = "*" ] && break  # skip empty directory
        ln -s "$service" "$wantdir/tarantool@$instance.service"
 done

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Olga Arkhangelskaia]

I have got branch for full CI.

See https://gitlab.com/tarantool/tarantool/pipelines/150018512

27.05.2020 19:02, Alexander Turenko пишет:
> I was wondered why the instance is started automatically and found past
> discussions on this topic, see [1].
>
> I also added some thoughts whether it is right to change this: I think
> so, we should disable the default example instance.
>
> See comments for the patch itself below.
>
> [1]: https://github.com/tarantool/tarantool/issues/4507#issuecomment-634567994
>
> WBR, Alexander Turenko.
>
>> core: don't start example instance in postinstall
> Nit: I would use prefix 'build' here. 'core' is usually about the code
> that resides in src/lib/core.
>
> On Wed, Apr 29, 2020 at 03:20:38PM +0300, Olga Arkhangelskaia wrote:
>> After taratool installation on Debuan/Ubuntu from repo, example instance
>> was automatically started over 3301 port. On the other hand example instance
>> on RHEL/CentOS is started manually. Patch does the same for Debian/Ubuntu.
>>
>> Closes #4507
>>
>> @TarantoolBot document
>> Title: manage example instance after installation
>> After tarantool installation from repos on all distros example instance
>> should be managed manually.
> I verified, where the old behaviour is mentioned in the documentation:
>
>   | Depending on the release, during installation Tarantool may start a
>   | demonstrative global example.lua instance that listens to the 3301
>   | port by default. <...>
>   |
>   | <...>
>   |
>   | However, we encourage you to perform the instance startup manually,
>   | so you can learn.
>
> Cited from [1] and [2].
>
> [1]: https://www.tarantool.io/en/doc/2.4/getting_started/getting_started_db/#using-a-binary-package
> [2]: https://www.tarantool.io/en/doc/2.4/getting_started/using_binary/
>
> In fact this wording does not contradicts with neither of the
> behaviours. I guess it is better to left the documentation as is for a
> while.
>
>> ---
>> Branch:
>> https://github.com/tarantool/tarantool/tree/gh-4507-disable-start-example-instance
> I would check it on a -full-ci branch just in case. Debian package build
> is changed and we should at least ensure that packages are build.
>
> Hope we'll add such builds to verify each push to a developer branch,
> but now it is only on -full-ci and release branches.
>
>>   debian/tarantool-common.postinst | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
>> index e2eda3415..03e4b2215 100644
>> --- a/debian/tarantool-common.postinst
>> +++ b/debian/tarantool-common.postinst
>> @@ -22,14 +22,6 @@ case "$1" in
>>           install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
>> -
>> -        # Enable example.lua by default
>> -        if [ -z $2 ] && [ ! -e /etc/tarantool/instances.enabled/example.lua ] &&
>> -           [ -d /etc/tarantool/instances.enabled ] &&
>> -           [ -d /etc/tarantool/instances.available/ ]; then
>> -          ln -s /etc/tarantool/instances.available/example.lua \
>> -                /etc/tarantool/instances.enabled/example.lua
>> -        fi
> /etc/tarantool/instances.enabled/example.lua will not be removed
> automatically even after tarantool-common upgrade, because the symlink
> will be removed in debian/tarantool-common.postrm only at `apt-get purge
> tarantool-common`.
>
> One possible way is to left everything as is and mention necessary steps
> to get rid from example.lua symlink (say, `apt-get purge
> tarantool-common`, `apt-get install tarantool-common`) in release notes.
>
> However it looks as a lie: as user I would expect that the new behaviour
> will become in effect just after upgrade. But we should not remove the
> symlink every time when tarantool-common is updated, because a user may
> want to use this example instance.
>
> I looked over the Debian maintainer scripts documentation ([2], [3]) and
> found several ways to stop and disable an instance on upgrade.
>
> [2]: https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html
> [3]: https://www.debian.org/doc/debian-policy/ap-flowcharts.html
>
> First, we can compare 'previously configured' and 'newly configured'
> versions on 'postinst configure' stage of tarantool-common. This,
> however, both ugly and fragile. It is ugly, because we don't know exact
> four-digits version at which the patch will land into all long-term
> branches: so we'll likely remove the symlink during several upgrades,
> not only one. It is fragile, because it will not be reliable if we'll
> decide to push the patch to, say, 1.10 later (it should be included into
> the condition of the master patch).
>
> It would look like so (not tested):
>
> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
> index 03e4b2215..17d6982c0 100644
> --- a/debian/tarantool-common.postinst
> +++ b/debian/tarantool-common.postinst
> @@ -22,6 +22,13 @@ case "$1" in
>           install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
> +
> +        if [ -e /etc/tarantool/instances.enabled/example.lua ] &&
> +           [ -n "$2" ] && (dpkg --compare-versions $2 lt 2.4.2 ||
> +           (dpkg --compare-versions $2 gt 2.5.0 &&
> +           dpkg --compare-versions $2 lt 2.5.1)); then
> +            rm /etc/tarantool/instances.enabled/example.lua
> +        fi
>       ;;
>   esac
>
> Then next way I considered is to add /etc/tarantool/debian/keep
> directory, which will hold files that should be hold during upgrade. But
> it looks weird: there is only one file we want to remove from the old
> configuration during upgrade and unlikely there will be more. This
> mechanism looks artificial here.
>
> So I implemented it in the migration-style way, see the patch below the
> email.
>
> Please, note that enabled instances directory now can be empty and it
> would be good to handle this case in the systemd unit generator file (it
> is in the patch below too).
>
> To be honest, I still don't very like the result (those upgrade markers
> right in /etc), but at least it looks like a general mechanism that may
> be used again (but I hope it will not). If dpkg would provide some
> database for performing migrations during updates, I would use it. But
> it seems, no: it is overkill for dpkg (I don't found anything like
> this).
>
> Olya, Igor, Oleg, can you look into this change and say what do you
> think about?
>
> ----
>
> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
> index 03e4b2215..e368cdd2e 100644
> --- a/debian/tarantool-common.postinst
> +++ b/debian/tarantool-common.postinst
> @@ -22,6 +22,46 @@ case "$1" in
>           install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
> +
> +        # Track configuration updates we want to perform only
> +        # once.
> +        UPGRADE_DIR="/etc/tarantool/debian/upgrade"
> +        mkdir -p "${UPGRADE_DIR}"
> +
> +        # Add README file to /etc/tarantool/debian/upgrade
> +        # to don't confuse a user with this directory.
> +        UPGRADE_FILE_0000="${UPGRADE_DIR}/0000-add-readme.done"
> +        if [ ! -e "${UPGRADE_FILE_0000}" ]; then
> +            cat << EOF > "${UPGRADE_DIR}/README"
> +There are rare cases, when existing configuration files
> +content or structure should be upgraded when a new package
> +version is installed.
> +
> +This directory tracks performed updates.
> +EOF
> +            touch "${UPGRADE_FILE_0000}"
> +        fi
> +
> +        # Stop and disable example.lua if it was started/enabled,
> +        # but do it only once in case a user actually want the
> +        # instance be enabled.
> +        UPGRADE_FILE_0001="${UPGRADE_DIR}/0001-disable-example-instance.done"
> +        if [ ! -e "${UPGRADE_FILE_0001}" ]; then
> +            # Stop the instance if it was started.
> +            if command -v invoke-rc.d > /dev/null; then
> +                invoke-rc.d tarantool@example stop || true
> +            else
> +                /etc/init.d/tarantool@example stop || true
> +            fi
> +
> +            # Disable the instance, but keep it in
> +            # instances.available.
> +            if [ -e /etc/tarantool/instances.enabled/example.lua ]; then
> +                rm /etc/tarantool/instances.enabled/example.lua
> +            fi
> +
> +            touch "${UPGRADE_FILE_0001}"
> +        fi
>       ;;
>   esac
>   
> diff --git a/extra/dist/tarantool-generator.in b/extra/dist/tarantool-generator.in
> index f6a6a2540..2048b7517 100755
> --- a/extra/dist/tarantool-generator.in
> +++ b/extra/dist/tarantool-generator.in
> @@ -13,6 +13,7 @@ mkdir -p "$wantdir"
>   
>   for file in @TARANTOOL_ENABLEDDIR@/*.lua; do
>          instance=`basename $file .lua`
> +       [ "${instance}" = "*" ] && break  # skip empty directory
>          ln -s "$service" "$wantdir/tarantool@$instance.service"
>   done

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Olga Arkhangelskaia]

[-- Attachment #1: Type: text/plain, Size: 10520 bytes --]

Hi Alexander!

Thank you very much for detailed feedback and pointing out the things I 
even did not think about.

As I understand (correct me if I am wrong) in 2 cases:

1) Installing a package when previous that was removed but not purged

2) Upgrading the package.

So if package already exists - all postrm, prerm are useless.

The only places we actually can take advantage is preinst and postinst 
scripts.

Moreover,  when Installing a package that was previously removed, but 
not purged [2] preinst scripts will be called from

previous version. So only postinst is left to stop existing example 
instance and remove the link.

I am confused with the idea of pushing patch in later versions, because 
it is rather surprising behaviour.

So I do not mind the first patch, however we still need to stop running 
instances (in case they are still running).

The second thought I have that tracking  one-time-actions is actually 
good idea and may be useful so far(not only in this case). And confuse 
user with disabling example instances (in case when they are enabled 
again)every time it is bad idea. So the second patch wins.

I guess I need to test it with full CI?

Moreover, I would write a comment why it is done in such a way (with 
links to the diagram). Because I failed to find them when writing the 
first version of patch.


27.05.2020 19:02, Alexander Turenko пишет:
> I was wondered why the instance is started automatically and found past
> discussions on this topic, see [1].
>
> I also added some thoughts whether it is right to change this: I think
> so, we should disable the default example instance.
>
> See comments for the patch itself below.
>
> [1]: https://github.com/tarantool/tarantool/issues/4507#issuecomment-634567994
>
> WBR, Alexander Turenko.
>
>> core: don't start example instance in postinstall
> Nit: I would use prefix 'build' here. 'core' is usually about the code
> that resides in src/lib/core.
>
> On Wed, Apr 29, 2020 at 03:20:38PM +0300, Olga Arkhangelskaia wrote:
>> After taratool installation on Debuan/Ubuntu from repo, example instance
>> was automatically started over 3301 port. On the other hand example instance
>> on RHEL/CentOS is started manually. Patch does the same for Debian/Ubuntu.
>>
>> Closes #4507
>>
>> @TarantoolBot document
>> Title: manage example instance after installation
>> After tarantool installation from repos on all distros example instance
>> should be managed manually.
> I verified, where the old behaviour is mentioned in the documentation:
>
>   | Depending on the release, during installation Tarantool may start a
>   | demonstrative global example.lua instance that listens to the 3301
>   | port by default. <...>
>   |
>   | <...>
>   |
>   | However, we encourage you to perform the instance startup manually,
>   | so you can learn.
>
> Cited from [1] and [2].
>
> [1]: https://www.tarantool.io/en/doc/2.4/getting_started/getting_started_db/#using-a-binary-package
> [2]: https://www.tarantool.io/en/doc/2.4/getting_started/using_binary/
>
> In fact this wording does not contradicts with neither of the
> behaviours. I guess it is better to left the documentation as is for a
> while.
>
>> ---
>> Branch:
>> https://github.com/tarantool/tarantool/tree/gh-4507-disable-start-example-instance
> I would check it on a -full-ci branch just in case. Debian package build
> is changed and we should at least ensure that packages are build.
>
> Hope we'll add such builds to verify each push to a developer branch,
> but now it is only on -full-ci and release branches.
>
>>   debian/tarantool-common.postinst | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
>> index e2eda3415..03e4b2215 100644
>> --- a/debian/tarantool-common.postinst
>> +++ b/debian/tarantool-common.postinst
>> @@ -22,14 +22,6 @@ case "$1" in
>>           install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
>> -
>> -        # Enable example.lua by default
>> -        if [ -z $2 ] && [ ! -e /etc/tarantool/instances.enabled/example.lua ] &&
>> -           [ -d /etc/tarantool/instances.enabled ] &&
>> -           [ -d /etc/tarantool/instances.available/ ]; then
>> -          ln -s /etc/tarantool/instances.available/example.lua \
>> -                /etc/tarantool/instances.enabled/example.lua
>> -        fi
> /etc/tarantool/instances.enabled/example.lua will not be removed
> automatically even after tarantool-common upgrade, because the symlink
> will be removed in debian/tarantool-common.postrm only at `apt-get purge
> tarantool-common`.
>
> One possible way is to left everything as is and mention necessary steps
> to get rid from example.lua symlink (say, `apt-get purge
> tarantool-common`, `apt-get install tarantool-common`) in release notes.
>
> However it looks as a lie: as user I would expect that the new behaviour
> will become in effect just after upgrade. But we should not remove the
> symlink every time when tarantool-common is updated, because a user may
> want to use this example instance.
>
> I looked over the Debian maintainer scripts documentation ([2], [3]) and
> found several ways to stop and disable an instance on upgrade.
>
> [2]: https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html
> [3]: https://www.debian.org/doc/debian-policy/ap-flowcharts.html
>
> First, we can compare 'previously configured' and 'newly configured'
> versions on 'postinst configure' stage of tarantool-common. This,
> however, both ugly and fragile. It is ugly, because we don't know exact
> four-digits version at which the patch will land into all long-term
> branches: so we'll likely remove the symlink during several upgrades,
> not only one. It is fragile, because it will not be reliable if we'll
> decide to push the patch to, say, 1.10 later (it should be included into
> the condition of the master patch).
>
> It would look like so (not tested):
>
> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
> index 03e4b2215..17d6982c0 100644
> --- a/debian/tarantool-common.postinst
> +++ b/debian/tarantool-common.postinst
> @@ -22,6 +22,13 @@ case "$1" in
>           install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
> +
> +        if [ -e /etc/tarantool/instances.enabled/example.lua ] &&
> +           [ -n "$2" ] && (dpkg --compare-versions $2 lt 2.4.2 ||
> +           (dpkg --compare-versions $2 gt 2.5.0 &&
> +           dpkg --compare-versions $2 lt 2.5.1)); then
> +            rm /etc/tarantool/instances.enabled/example.lua
> +        fi
>       ;;
>   esac
>
> Then next way I considered is to add /etc/tarantool/debian/keep
> directory, which will hold files that should be hold during upgrade. But
> it looks weird: there is only one file we want to remove from the old
> configuration during upgrade and unlikely there will be more. This
> mechanism looks artificial here.
>
> So I implemented it in the migration-style way, see the patch below the
> email.
>
> Please, note that enabled instances directory now can be empty and it
> would be good to handle this case in the systemd unit generator file (it
> is in the patch below too).
>
> To be honest, I still don't very like the result (those upgrade markers
> right in /etc), but at least it looks like a general mechanism that may
> be used again (but I hope it will not). If dpkg would provide some
> database for performing migrations during updates, I would use it. But
> it seems, no: it is overkill for dpkg (I don't found anything like
> this).
>
> Olya, Igor, Oleg, can you look into this change and say what do you
> think about?
>
> ----
>
> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
> index 03e4b2215..e368cdd2e 100644
> --- a/debian/tarantool-common.postinst
> +++ b/debian/tarantool-common.postinst
> @@ -22,6 +22,46 @@ case "$1" in
>           install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>           install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
> +
> +        # Track configuration updates we want to perform only
> +        # once.
> +        UPGRADE_DIR="/etc/tarantool/debian/upgrade"
> +        mkdir -p "${UPGRADE_DIR}"
> +
> +        # Add README file to /etc/tarantool/debian/upgrade
> +        # to don't confuse a user with this directory.
> +        UPGRADE_FILE_0000="${UPGRADE_DIR}/0000-add-readme.done"
> +        if [ ! -e "${UPGRADE_FILE_0000}" ]; then
> +            cat << EOF > "${UPGRADE_DIR}/README"
> +There are rare cases, when existing configuration files
> +content or structure should be upgraded when a new package
> +version is installed.
> +
> +This directory tracks performed updates.
> +EOF
> +            touch "${UPGRADE_FILE_0000}"
> +        fi
> +
> +        # Stop and disable example.lua if it was started/enabled,
> +        # but do it only once in case a user actually want the
> +        # instance be enabled.
> +        UPGRADE_FILE_0001="${UPGRADE_DIR}/0001-disable-example-instance.done"
> +        if [ ! -e "${UPGRADE_FILE_0001}" ]; then
> +            # Stop the instance if it was started.
> +            if command -v invoke-rc.d > /dev/null; then
> +                invoke-rc.d tarantool@example stop || true
> +            else
> +                /etc/init.d/tarantool@example stop || true
> +            fi
> +
> +            # Disable the instance, but keep it in
> +            # instances.available.
> +            if [ -e /etc/tarantool/instances.enabled/example.lua ]; then
> +                rm /etc/tarantool/instances.enabled/example.lua
> +            fi
> +
> +            touch "${UPGRADE_FILE_0001}"
> +        fi
>       ;;
>   esac
>   
> diff --git a/extra/dist/tarantool-generator.in b/extra/dist/tarantool-generator.in
> index f6a6a2540..2048b7517 100755
> --- a/extra/dist/tarantool-generator.in
> +++ b/extra/dist/tarantool-generator.in
> @@ -13,6 +13,7 @@ mkdir -p "$wantdir"
>   
>   for file in @TARANTOOL_ENABLEDDIR@/*.lua; do
>          instance=`basename $file .lua`
> +       [ "${instance}" = "*" ] && break  # skip empty directory
>          ln -s "$service" "$wantdir/tarantool@$instance.service"
>   done

[-- Attachment #2: Type: text/html, Size: 12743 bytes --]

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Igor Munkin]

Sasha,

Many thanks for the complex research and clarification! I dumped parts
of our offline discussion below. Feel free to correct me if I'm wrong
and add anything I missed.

Long story short: comparing both approaches I personally prefer to apply
the one from the origin patch and take some changes you provided below
(e.g. empty <enabled> directory handling).

I also Cced Yaroslav as the person who faced the issue "for several
times across these years".

On 27.05.20, Alexander Turenko wrote:
> I was wondered why the instance is started automatically and found past
> discussions on this topic, see [1].
> 
> I also added some thoughts whether it is right to change this: I think
> so, we should disable the default example instance.

We need to define "disable" here. As we discussed there are two ways:
* Just stop enabling (i.e. creating symlink for available instance to
  <enabled> directory) the example.lua instance after installing or
  upgrading Tarantool and let users to control it on their own in
  future.
* In addition to everything above, if example.lua instance is already
  running, stop it and disable (i.e. remove symlink to <enable>
  directory).

Please correct me if I'm wrong here.

> 
> See comments for the patch itself below.
> 
> [1]: https://github.com/tarantool/tarantool/issues/4507#issuecomment-634567994
> 
> WBR, Alexander Turenko.
> 

<snipped>

> > 
> > @TarantoolBot document
> > Title: manage example instance after installation
> > After tarantool installation from repos on all distros example instance
> > should be managed manually.
> 
> I verified, where the old behaviour is mentioned in the documentation:
> 
>  | Depending on the release, during installation Tarantool may start a
>  | demonstrative global example.lua instance that listens to the 3301
>  | port by default. <...>
>  |
>  | <...>
>  |
>  | However, we encourage you to perform the instance startup manually,
>  | so you can learn.
> 
> Cited from [1] and [2].
> 
> [1]: https://www.tarantool.io/en/doc/2.4/getting_started/getting_started_db/#using-a-binary-package
> [2]: https://www.tarantool.io/en/doc/2.4/getting_started/using_binary/
> 
> In fact this wording does not contradicts with neither of the
> behaviours. I guess it is better to left the documentation as is for a
> while.

Agree.

> 

<snipped>

> 
> >  debian/tarantool-common.postinst | 8 --------
> >  1 file changed, 8 deletions(-)
> > 
> > diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
> > index e2eda3415..03e4b2215 100644
> > --- a/debian/tarantool-common.postinst
> > +++ b/debian/tarantool-common.postinst
> > @@ -22,14 +22,6 @@ case "$1" in
> >          install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
> >          install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
> >          install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
> > -
> > -        # Enable example.lua by default
> > -        if [ -z $2 ] && [ ! -e /etc/tarantool/instances.enabled/example.lua ] &&
> > -           [ -d /etc/tarantool/instances.enabled ] &&
> > -           [ -d /etc/tarantool/instances.available/ ]; then
> > -          ln -s /etc/tarantool/instances.available/example.lua \
> > -                /etc/tarantool/instances.enabled/example.lua
> > -        fi
> 
> /etc/tarantool/instances.enabled/example.lua will not be removed
> automatically even after tarantool-common upgrade, because the symlink
> will be removed in debian/tarantool-common.postrm only at `apt-get purge
> tarantool-common`.

I guess it should not be removed (or even added) via package removal
(installation). As we discussed Tarantool instances are not similar to
NGINX or MySQL ones: user can run multiple instances and ought to
maintain them all by himself. I see Tarantool as a *platform* that can
be used in various ways: stateless application server, database or
cache, or even Lua interpreter.

Side note: It can substitute any of the letters or a combination of them
in LAMP except the L one (well, honestly A is better substituted with
NGINX, but no one forbids to bind Tarantool HTTP server on 80 port).

> 
> One possible way is to left everything as is and mention necessary steps
> to get rid from example.lua symlink (say, `apt-get purge
> tarantool-common`, `apt-get install tarantool-common`) in release notes.

AFAIR, it can be done without package re-installation but using service
managing tools (i.e. systemctl, since systemd is the sole init system on
deb-based distros today).

> 
> However it looks as a lie: as user I would expect that the new behaviour
> will become in effect just after upgrade. But we should not remove the
> symlink every time when tarantool-common is updated, because a user may
> want to use this example instance.

Yes, but listen, if the example instance is not bothering you for a long
period of time, you likely might have no idea it is running. Otherwise
you've already disabled it once and hope it will not start again. But
since the instance is *re-enabled* as a result of every update on
deb-based distros, it makes you either change the distro or just crazy.

> 
> I looked over the Debian maintainer scripts documentation ([2], [3]) and
> found several ways to stop and disable an instance on upgrade.
> 
> [2]: https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html
> [3]: https://www.debian.org/doc/debian-policy/ap-flowcharts.html
> 

Oh, I snipped everything related to the first approach you described
for the one reason: it's too fragile, artificial, ugly and what's more
important doesn't differ from the one provided by Olya, since it just
drops a symlink with no other benefits (at least I see not a one).

<snipped>

> 
> So I implemented it in the migration-style way, see the patch below the
> email.
> 
> Please, note that enabled instances directory now can be empty and it
> would be good to handle this case in the systemd unit generator file (it
> is in the patch below too).

Good catch! I guess this change should be applied anyway.

> 
> To be honest, I still don't very like the result (those upgrade markers
> right in /etc), but at least it looks like a general mechanism that may
> be used again (but I hope it will not). If dpkg would provide some
> database for performing migrations during updates, I would use it. But
> it seems, no: it is overkill for dpkg (I don't found anything like
> this).
> 
> Olya, Igor, Oleg, can you look into this change and say what do you
> think about?

Personally, I think this solution is overcomplicated. User can (and
must) manage his services by himself and I see the next problem is the
root cause of the issue: even when user found the example instance
running on his machine, once stopped and disabled it, the instance is
*re-enabled* and ergo started after upgrading/reinstalling the package.
As a result user has to fix the issue manually again and again.

> 
> ----
> 
> diff --git a/debian/tarantool-common.postinst b/debian/tarantool-common.postinst
> index 03e4b2215..e368cdd2e 100644
> --- a/debian/tarantool-common.postinst
> +++ b/debian/tarantool-common.postinst
> @@ -22,6 +22,46 @@ case "$1" in
>          install -d -o$SYSUSER -gadm -m2750 /var/log/tarantool
>          install -d -o$SYSUSER -g$SYSUSER -m750 /var/run/tarantool
>          install -d -o$SYSUSER -g$SYSUSER -m750 /var/lib/tarantool
> +
> +        # Track configuration updates we want to perform only
> +        # once.
> +        UPGRADE_DIR="/etc/tarantool/debian/upgrade"
> +        mkdir -p "${UPGRADE_DIR}"
> +
> +        # Add README file to /etc/tarantool/debian/upgrade
> +        # to don't confuse a user with this directory.
> +        UPGRADE_FILE_0000="${UPGRADE_DIR}/0000-add-readme.done"
> +        if [ ! -e "${UPGRADE_FILE_0000}" ]; then
> +            cat << EOF > "${UPGRADE_DIR}/README"
> +There are rare cases, when existing configuration files
> +content or structure should be upgraded when a new package
> +version is installed.
> +
> +This directory tracks performed updates.
> +EOF
> +            touch "${UPGRADE_FILE_0000}"
> +        fi
> +
> +        # Stop and disable example.lua if it was started/enabled,
> +        # but do it only once in case a user actually want the
> +        # instance be enabled.
> +        UPGRADE_FILE_0001="${UPGRADE_DIR}/0001-disable-example-instance.done"
> +        if [ ! -e "${UPGRADE_FILE_0001}" ]; then
> +            # Stop the instance if it was started.
> +            if command -v invoke-rc.d > /dev/null; then
> +                invoke-rc.d tarantool@example stop || true
> +            else
> +                /etc/init.d/tarantool@example stop || true
> +            fi
> +
> +            # Disable the instance, but keep it in
> +            # instances.available.
> +            if [ -e /etc/tarantool/instances.enabled/example.lua ]; then
> +                rm /etc/tarantool/instances.enabled/example.lua
> +            fi
> +
> +            touch "${UPGRADE_FILE_0001}"
> +        fi
>      ;;
>  esac

AFAICS, there are some files left on the system after this "migration"
that are not tracked by the package. So they are not removed anymore
even after the package is deinstalled.

>  
> diff --git a/extra/dist/tarantool-generator.in b/extra/dist/tarantool-generator.in
> index f6a6a2540..2048b7517 100755
> --- a/extra/dist/tarantool-generator.in
> +++ b/extra/dist/tarantool-generator.in
> @@ -13,6 +13,7 @@ mkdir -p "$wantdir"
>  
>  for file in @TARANTOOL_ENABLEDDIR@/*.lua; do
>         instance=`basename $file .lua`
> +       [ "${instance}" = "*" ] && break  # skip empty directory
>         ln -s "$service" "$wantdir/tarantool@$instance.service"
>  done

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Alexander Turenko]

> Long story short: comparing both approaches I personally prefer to apply
> the one from the origin patch and take some changes you provided below
> (e.g. empty <enabled> directory handling).
> 
> I also Cced Yaroslav as the person who faced the issue "for several
> times across these years".

I don't mind any of these approaches. I don't know which one would be
better. So I would say that I have no preference for one over another.

> AFAICS, there are some files left on the system after this "migration"
> that are not tracked by the package. So they are not removed anymore
> even after the package is deinstalled.

They will not be removed at `apt-get remove`, but will be at `apt-get
purge` as well as everything in /etc/tarantool. See:

 | $ cat debian/tarantool-common.postrm 
 | #!/bin/sh
 |
 | set -e
 |
 | case "$1" in
 |     purge)
 |         rm -fr  \
 |             /etc/tarantool \
 |             /var/log/tarantool \
 |             /var/run/tarantool \
 |             /var/lib/tarantool
 |     ;;
 | esac
 |
 | #DEBHELPER#

Re: [Tarantool-patches] [PATCH] core: don't start example instance in postinstall

[Yaroslav Dynnikov]

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

Hi!

As a person who "faced the issue for several times across these years" I
can only add
that that most important is fresh installatoin. As far as I can see,
the original
patch
<https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/016443.html>
is enough
to solve the problem. So, LGTM.

P.S. I won't comment other parts of discussion because they're beyond my
understanding of deb scripts.


On Fri, 5 Jun 2020 at 09:55, Alexander Turenko <
alexander.turenko@tarantool.org> wrote:

> > Long story short: comparing both approaches I personally prefer to apply
> > the one from the origin patch and take some changes you provided below
> > (e.g. empty <enabled> directory handling).
> >
> > I also Cced Yaroslav as the person who faced the issue "for several
> > times across these years".
>
> I don't mind any of these approaches. I don't know which one would be
> better. So I would say that I have no preference for one over another.
>
> > AFAICS, there are some files left on the system after this "migration"
> > that are not tracked by the package. So they are not removed anymore
> > even after the package is deinstalled.
>
> They will not be removed at `apt-get remove`, but will be at `apt-get
> purge` as well as everything in /etc/tarantool. See:
>
>  | $ cat debian/tarantool-common.postrm
>  | #!/bin/sh
>  |
>  | set -e
>  |
>  | case "$1" in
>  |     purge)
>  |         rm -fr  \
>  |             /etc/tarantool \
>  |             /var/log/tarantool \
>  |             /var/run/tarantool \
>  |             /var/lib/tarantool
>  |     ;;
>  | esac
>  |
>  | #DEBHELPER#
>


-- 
С уважением.
Дынников Ярослав.

[-- Attachment #2: Type: text/html, Size: 2441 bytes --]