Skip to content

Perform definition import after plugins are enabled #2384

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.

Sign up for GitHub

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

Jump to bottom
Closed
michaelklishin opened this issue Jun 16, 2020 · 4 comments · Fixed by #2396
Closed

Perform definition import after plugins are enabled #2384

michaelklishin opened this issue Jun 16, 2020 · 4 comments · Fixed by #2396
Assignees
@michaelklishin
Labels
usability
Milestone
3.8.6

Comments

@michaelklishin
Copy link
Collaborator

Currently there is a chicken-and-egg problem triangle between

which can be problematic when definitions related to or dependent on plugins are involved, e.g. an exchange of a custom plugin-provided type.

We can potentially untangle this with a few low risk changes:

  • Avoid default data seeding when load_definitions is configured
  • Load definitions not in a core boot step but explicitly in a "post-run" phase, after plugins have been enabled
  • As a bonus, consider enabling client connection listeners after all plugins have been enabled (but this can be problematic: consider a Shovel that has to connect to the local node upon startup)

While definitions can be imported at any point after node startup, it would be convenient for Shovel and custom exchange type users if this limitation of load_definitions was lifted.

See #2381 for the background.
@michaelklishin michaelklishin added this to the 3.8.6 milestone Jun 16, 2020
@michaelklishin michaelklishin self-assigned this Jun 16, 2020
@michaelklishin michaelklishin mentioned this issue Jun 18, 2020
Closed
@michaelklishin michaelklishin mentioned this issue Jun 26, 2020
Closed
michaelklishin added a commit that referenced this issue Jun 26, 2020
@michaelklishin
Export definitions on boot after all plugins have been enabled
9aead56 
This way definitions that depend on a plugin, such as federation upstreams
or exchanges of custom types, can be imported successfully.

Closes #2384
michaelklishin added a commit that referenced this issue Jun 26, 2020
@michaelklishin
Avoid seeding default data if there are configured definitions to loa…
0523b44 
…d on boot

This is a behavior we use to provide because definitions were
imported before the empty DB boot step. Now when the definitions
are imported in the post-launch phase, we'd always end up with
default data seeded in addition to definitions being imported.

References #2384
@michaelklishin michaelklishin mentioned this issue Jun 26, 2020
Merged
This was referenced Jun 28, 2020
Closed
Closed
Closed
@michaelklishin michaelklishin mentioned this issue Jul 26, 2020
Closed
@michaelklishin michaelklishin mentioned this issue Aug 5, 2020
Closed
@tbnguyen1407
Copy link

Hello, this seems still not working for x-consistent-hash exchange plugin when an exchange of that type is defined in definitions.

I removed the exchange definition and see the plugin bootstep is still performed after definition import

rabbitmq-definitions

Can you help take a look?

@lukebakken
Copy link
Collaborator

@tbnguyen1407 please provide complete instructions for how to reproduce, including a definitions file that will reproduce the issue. We don't have time to guess. Thank you.

@michaelklishin
Copy link
Collaborator Author

michaelklishin commented Aug 28, 2020 •
edited
Loading

@tbnguyen1407 this does work for the consistent hash exchange plugin. In fact, this plugin was specifically affected by certain boot path refactoring in 3.8.4 and caused us to notice that the problem (which is not new) has worsened.

I can do another test but you have provided no details of what you actually did. Very likely you are using management.load_definitions which runs when the management plugin is activated, and not load_definitions which is a built-in option in 3.8.

docker-library/rabbitmq#429 and docker-library/rabbitmq#430 provide some details on the difference between the two.

@tbnguyen1407
Copy link

@michaelklishin yes seem I missed the new "load_definitions". It can work now. Thank you.

@michaelklishin michaelklishin mentioned this issue Sep 16, 2020
Closed
@michaelklishin michaelklishin mentioned this issue Oct 16, 2020
Closed
@michaelklishin michaelklishin mentioned this issue Jan 5, 2021
Closed
michaelklishin added a commit that referenced this issue Jan 14, 2021
@michaelklishin
Make sure legacy rabbit_management_load_definitions step does not blo…
07a0dd7 
…ck boot

Follow-up to #2656, #2384.
michaelklishin added a commit that referenced this issue Jan 14, 2021
@michaelklishin
Make sure legacy rabbit_management_load_definitions step does not blo…
3470ffb 
…ck boot

Follow-up to #2656, #2384.

(cherry picked from commit 07a0dd7)
lukebakken added a commit that referenced this issue Mar 22, 2023
@lukebakken
Always import default data if rabbit_table:needs_default_data/0 is true
dbd4448 
Fixes #7678

References:
* #2384
* #2396

PR #2396 preserved the old behavior where definitions import took
priority over the default data and environment data that may be present.
This behavior continues to confuse users where they expect
`RABBITMQ_DEFAULT_USER` / `RABBITMQ_DEFAULT_PASS` to be imported,
especially if there is no `users` data in the definitions file.

This PR allows default data and the environment to be imported first,
then possibly overwritten by the definitions file.
@lukebakken lukebakken mentioned this issue Mar 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michaelklishin michaelklishin

Labels
usability
Projects
None yet
Milestone
3.8.6
Development

Successfully merging a pull request may close this issue.

Import definitions in the post-launch phase rabbitmq/rabbitmq-server
3 participants
@michaelklishin @lukebakken @tbnguyen1407