Skip to content
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

msi: guard duplicated instance (not adding a new option) #648

Merged

Conversation

daipom
Copy link
Contributor

@daipom daipom commented Jun 25, 2024

Abstraction

Spec change

Before:

We can launch Fluentd by fluentd.bat even though fluentdwinsvc is running.
Launching multiple Fluentd with the same config may cause inconsistency of the buffers or the pos files.

After:

We can't launch Fluentd by fluent.bat with the default config path if fluentdwinsvc is running.
If one of the following options is specified, we can execute fluentd.bat as before.

  • --config (-c)
  • --dry-run
  • --reg-winsvc
  • --reg-winsvc-fluentdopt

Output

When the fluentdwinsvc is running, the behavior of fluentd.bat is as follows:

> fluentd
Error: Can't start duplicate Fluentd instance with the default config.

To start Fluentd, please do one of the following:
  (Caution: Please be careful not to start multiple instances with the same config.)
  - Stop the Fluentd Windows service 'fluentdwinsvc'.
  - Specify the config path explicitly by '-c' ('--config').

> fluentd -v
Error: Can't start duplicate Fluentd instance with the default config.

To take the version, please use '--version', not '-v' ('--verbose').

To start Fluentd, please do one of the following:
  (Caution: Please be careful not to start multiple instances with the same config.)
  - Stop the Fluentd Windows service 'fluentdwinsvc'.
  - Specify the config path explicitly by '-c' ('--config')

@daipom
Copy link
Contributor Author

daipom commented Jun 26, 2024

The msi installer is broken. I'm checking the cause.

@daipom
Copy link
Contributor Author

daipom commented Jun 26, 2024

Why is fluentdwinsvc service running during the installation??
The installer may not be behaving correctly in the first place.
=> This is no problem. StartServices action looks unnecessary, but it is harmless because the service is not registered at that point.

However, #622 does not have this problem. I wander what the difference is.
=> delayed expansion: #648 (comment)

msi.log

...
Action start 15:02:23: WriteEnvironmentStrings.
WriteEnvironmentStrings: Name: Updating environment strings, Value: , Action 
Action ended 15:02:23: WriteEnvironmentStrings. Return value 1.
Action 15:02:23: StartServices. Starting services
Action start 15:02:23: StartServices.
Action ended 15:02:23: StartServices. Return value 1.
Action 15:02:23: RegisterUser. Registering user
Action start 15:02:23: RegisterUser.
Action ended 15:02:23: RegisterUser. Return value 1.
Action 15:02:23: RegisterProduct. Registering product
Action start 15:02:23: RegisterProduct.
RegisterProduct: Registering product
Action ended 15:02:23: RegisterProduct. Return value 1.
Action 15:02:23: PublishFeatures. Publishing Product Features
Action start 15:02:23: PublishFeatures.
PublishFeatures: Feature: Publishing Product Features
Action ended 15:02:23: PublishFeatures. Return value 1.
Action 15:02:23: PublishProduct. Publishing product information
Action start 15:02:23: PublishProduct.
PublishProduct: 
Action ended 15:02:23: PublishProduct. Return value 1.
Action 15:02:23: SetCreateCompatTdAgentBat. 
Action start 15:02:23: SetCreateCompatTdAgentBat.
Action ended 15:02:23: SetCreateCompatTdAgentBat. Return value 1.
Action 15:02:23: CreateCompatTdAgentBat. 
Action start 15:02:23: CreateCompatTdAgentBat.
CreateCompatTdAgentBat: 
Action ended 15:02:23: CreateCompatTdAgentBat. Return value 1.
Action 15:02:23: SetCreateCompatTdAgentGemBat. 
Action start 15:02:23: SetCreateCompatTdAgentGemBat.
Action ended 15:02:23: SetCreateCompatTdAgentGemBat. Return value 1.
Action 15:02:23: CreateCompatTdAgentGemBat. 
Action start 15:02:23: CreateCompatTdAgentGemBat.
CreateCompatTdAgentGemBat: 
Action ended 15:02:23: CreateCompatTdAgentGemBat. Return value 1.
Action 15:02:23: InstallFinalize. 
Action start 15:02:23: InstallFinalize.
Action 15:02:23: ProcessComponents. Updating component registration
Action 15:02:27: CreateFolders. Creating folders
CreateFolders: Folder: C:\opt\fluent\
Action 15:02:27: InstallFiles. Copying new files
Action 15:02:44: InstallFluentdWinSvc. 
WixQuietExec64:  Error: Can't start duplicate Fluentd instance with the default config.
WixQuietExec64:  
WixQuietExec64:  To start Fluentd, please do one of the following:
WixQuietExec64:    (Caution: Please be careful not to start multiple instances with the same config.)
WixQuietExec64:    - Stop the Fluentd Windows service 'fluentdwinsvc'.
WixQuietExec64:    - Specify the config path explicitly by '-c' ('--config').
WixQuietExec64:  Error 0x80070002: Command line returned an error.
WixQuietExec64:  Error 0x80070002: QuietExec64 Failed
WixQuietExec64:  Error 0x80070002: Failed in ExecCommon method
CustomAction InstallFluentdWinSvc returned actual error code 1603 (note this may not be 100% accurate if translation happened inside sandbox)
Action ended 15:02:44: InstallFinalize. Return value 3.
Action 15:02:44: Rollback. Rolling back action:
Rollback: InstallFluentdWinSvc
...

@daipom daipom force-pushed the msi-prevent-launching-duplicate-instance branch from e192963 to 12dc74a Compare June 26, 2024 07:59
@daipom
Copy link
Contributor Author

daipom commented Jun 26, 2024

The msi installer is broken. I'm checking the cause.

This problem is resolved.
However, some test problems remains. I'm fixing them.
=> Fixed.

@daipom daipom force-pushed the msi-prevent-launching-duplicate-instance branch from 12dc74a to 31f118f Compare June 26, 2024 10:34
@kenhys
Copy link
Contributor

kenhys commented Jun 27, 2024

This approach may be better than #622.

@daipom daipom force-pushed the msi-prevent-launching-duplicate-instance branch from 31f118f to 6653d95 Compare June 27, 2024 02:09
@daipom
Copy link
Contributor Author

daipom commented Jun 27, 2024

Rebased to #647 as #647 got some changes.

@daipom
Copy link
Contributor Author

daipom commented Jun 27, 2024

This approach may be better than #622.

Thanks for considering.
If we need a force execution option or a strict check whether the config path is the same in the future, let's revive #622!

@kenhys
Copy link
Contributor

kenhys commented Jun 27, 2024

As #647 was merged into master, let's rebase with it.

@daipom daipom force-pushed the msi-prevent-launching-duplicate-instance branch from 6653d95 to faf185e Compare June 27, 2024 02:33
@daipom
Copy link
Contributor Author

daipom commented Jun 27, 2024

As #647 was merged into master, let's rebase with it.

Done!

Copy link
Contributor

@kenhys kenhys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@kenhys
Copy link
Contributor

kenhys commented Jun 27, 2024

Let's merge when all ci has passed.

@daipom
Copy link
Contributor Author

daipom commented Jun 27, 2024

Thanks for your review!

@daipom
Copy link
Contributor Author

daipom commented Jun 27, 2024

Can we put this in v5.0.4?

@kenhys
Copy link
Contributor

kenhys commented Jun 27, 2024

Can we put this in v5.0.4?

This should be released in v5.0.4 IMHO.

@kenhys kenhys added this to the 5.0.4 (T.B.D.) milestone Jun 27, 2024
Before:

  We can launch Fluentd by fluentd.bat even though fluentdwinsvc is
  running.
  Launching multiple Fluentd with the same config may cause
  inconsistency of the buffers or the pos files.

After:

  We can't launch Fluentd by fluent.bat with the default config path if
  fluentdwinsvc is running.
  If some options are specified, we can execute fluentd.bat as before.

Inspired by @kenhys's PR fluent#622.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Co-authored-by: Kentaro Hayashi <hayashi@clear-code.com>
@daipom daipom force-pushed the msi-prevent-launching-duplicate-instance branch from faf185e to 7cfd143 Compare June 27, 2024 04:04
@daipom
Copy link
Contributor Author

daipom commented Jun 27, 2024

Refactored the variable name NEED_DUPLICATE_LAUNCH_CHECK -> PREVENT_DUPLICATE_LAUNCH

@kenhys kenhys merged commit 3cc481e into fluent:master Jun 27, 2024
63 checks passed
@daipom daipom deleted the msi-prevent-launching-duplicate-instance branch June 27, 2024 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants