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

Add priority to plugin hook #806

Merged
merged 3 commits into from
Feb 29, 2024
Merged

Add priority to plugin hook #806

merged 3 commits into from
Feb 29, 2024

Conversation

essa
Copy link
Contributor

@essa essa commented Feb 28, 2024

Continue from #805

The Datadog agent must be invoked after the instance has been set up, otherwise it will alert about setup activity, such as installing a yum package.

This PR adds a priority to Plugin and updates the calling order of plugin hooks so that we can be sure that datadog is installed and invoked last.

This plugin should be registered with max priority in the district.

$ bcn district put-plugin -a api_key=8e53.... -a hook_priority=10 ec-staging datadog

And it also updates the instance type of bastion to 't3.small'. 't3.micro' was too small for all the plugin and sometimes the cloud-init fails.

@degikko
Copy link

degikko commented Feb 28, 2024

@showwin can you help us review this PR, please?

@degikko degikko requested a review from showwin February 28, 2024 06:26
@essa
Copy link
Contributor Author

essa commented Feb 28, 2024

@showwin
Sorry, hold the review please.
I found one issue to fix.

Copy link
Contributor

@showwin showwin left a comment

Choose a reason for hiding this comment

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

LGTM 👍

db/migrate/20240228032657_add_hook_order_to_plugin.rb Outdated Show resolved Hide resolved
@essa
Copy link
Contributor Author

essa commented Feb 28, 2024

@showwin
I moved the hook_priority from a model attribute to a plugin attribute.
To add the priority by bcn cli, it must be a plugin attribute.

And I also make the default value to zero as your suggestion.

Could you review it again? 🙏

@essa essa merged commit db6595f into master Feb 29, 2024
6 checks passed
@showwin
Copy link
Contributor

showwin commented Feb 29, 2024

LGTM about the latest code👍

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.

3 participants