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

Integrate vagrant-triggers plugin functionality into core Vagrant #9713

Merged
merged 95 commits into from
Apr 24, 2018

Conversation

briancain
Copy link
Member

@briancain briancain commented Apr 20, 2018

This pull request integrates the community plugin vagrant-triggers functionality as a core feature of Vagrant. It is a complete rewrite of the plugin.

Resolves #3849
Related issue: emyl/vagrant-triggers#85 /cc @emyl
Related pull request: hashicorp/vagrant-spec#28
Related demo environment: briancain/vagrant-triggers-demo

Send the defined before and or after triggers in the merge function if
triggers exist already
@briancain briancain added this to the 2.1 milestone Apr 20, 2018
KFishner
KFishner previously approved these changes Apr 21, 2018
Copy link
Contributor

@KFishner KFishner left a comment

Choose a reason for hiding this comment

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

lgtm! some small copy tweaks

page_title: "Vagrant Triggers Configuration"
sidebar_current: "triggers-configuration"
description: |-
Documentation of various configuration options for Vagrnat Triggers
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Vagrnat/Vagrant

# Configuration

Vagrant Triggers has a few various options that can be set which define how the
trigger should behave.
Copy link
Contributor

Choose a reason for hiding this comment

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

Vagrant Triggers has a few options to define trigger behavior


The trigger class takes various options.

* `action` (symbol, array) - Expected to be a single symbol value, an array of symbols, or a _splat_ of symbols. The first argument that comes after either __before__ or __after__ when defining a new trigger. Can be any valid Vagrant command. It also accepts a special value `:all` which will make the trigger fire for every action. An action can be ignored with the `ignore` setting if desired.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe link to the Vagrant commands here? https://www.vagrantup.com/docs/cli/

_after_ Vagrant commands.

Each trigger is expected to be given a command key for when it should be fired
during the Vagrant command life cycle. These could be defined as a single key or
Copy link
Contributor

Choose a reason for hiding this comment

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

lifecycle

end
```

Global and machine scopped triggers will execute in the order that they are
Copy link
Contributor

Choose a reason for hiding this comment

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

machine-scoped

@emyl
Copy link
Contributor

emyl commented Apr 21, 2018

@briancain Such an awesome job! 🤘

Feel so honored.

Copy link
Member

@chrisroberts chrisroberts left a comment

Choose a reason for hiding this comment

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

🎆

#
# @param [Object] env Vagrant environment
# @param [Object] config Trigger configuration
# @param [Object] machine Active Machine
Copy link
Member

Choose a reason for hiding this comment

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

These should be updated with actual types expected

triggers.each do |trigger|
@logger.debug("Running trigger #{trigger.id}...")

if !trigger.name.nil?
Copy link
Member

Choose a reason for hiding this comment

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

if trigger.name

# but can be set as a single String or Symbol
#
# Guests are stored internally as strings
if !@only_on.nil?
Copy link
Member

Choose a reason for hiding this comment

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

if @only_on

end

# Commands must be stored internally as symbols
if !@ignore.nil?
Copy link
Member

Choose a reason for hiding this comment

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

if @ignore

errors.concat errorz["shell provisioner"] if !errorz.empty?
end

if !@name.nil? && !@name.is_a?(String)
Copy link
Member

Choose a reason for hiding this comment

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

if @name &&

errors << I18n.t("vagrant.config.triggers.name_bad_type", cmd: @command)
end

if !@info.nil? && !@info.is_a?(String)
Copy link
Member

Choose a reason for hiding this comment

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

if @info &&

errors << I18n.t("vagrant.config.triggers.info_bad_type", cmd: @command)
end

if !@warn.nil? && !@warn.is_a?(String)
Copy link
Member

Choose a reason for hiding this comment

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

if @warn &&

index = triggers.index(trigger) unless match == true
end

if !index.nil?
Copy link
Member

Choose a reason for hiding this comment

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

if index


The trigger class takes various options.

* `action` (symbol, array) - Expected to be a single symbol value, an array of symbols, or a _splat_ of symbols. The first argument that comes after either __before__ or __after__ when defining a new trigger. Can be any valid Vagrant command. It also accepts a special value `:all` which will make the trigger fire for every action. An action can be ignored with the `ignore` setting if desired. A list of valid core commands can be found [here](/docs/cli).
Copy link
Member

Choose a reason for hiding this comment

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

It should probably be called out here explicitly which commands the triggers will apply to at this point since they will be limited to provider dependent commands.

@briancain briancain merged commit 5643ba0 into hashicorp:master Apr 24, 2018
@briancain briancain deleted the vagrant-triggers-config branch October 2, 2018 17:46
@leopold-p
Copy link

@briancain Are there any plans to integrate triggers as provisioners feature? As in emyl/vagrant-triggers#21 (comment)

@briancain
Copy link
Member Author

@zloyleopold - One of the next things I plan on working on with triggers is the ability to define triggers around various events like the provision step rather than just commands, so yep! Soon. I believe the issue tracking that is here: #8500

@ghost
Copy link

ghost commented Mar 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate triggers into core?
5 participants