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

Fix a bunch of the AWS recipe issue #75

Closed
wants to merge 6 commits into from

Conversation

zsiddique
Copy link

  1. You dont need to install the dep for fog using bootstrap. You can use run_action to have the dependences be installed at compile time before chef_gem is ran. The build-essential cookbook works this way and is needed to have compile time flag set to true.

  2. The security group stuff for AWS was incorrect according to the plugin docs: https://github.com/elasticsearch/elasticsearch-cloud-aws see EC2 Discovery. I also rev'ed the version since I was editing the file.

  3. I was getting a discovery_tags nil error since I was not setting that and while it rescues the nil value it still try to enumerate over it. Set a unless block to protect this.

karmi pushed a commit that referenced this pull request Mar 6, 2013
Fixing issue with AWS plugin, attribute for security group should be discovery.ec2.group
karmi pushed a commit that referenced this pull request Mar 6, 2013
* Force installation of packages required for the Fog gem before the Chef run
* Use `run_action` for the Fog gem
karmi pushed a commit that referenced this pull request Mar 6, 2013
karmi added a commit that referenced this pull request Mar 6, 2013
karmi pushed a commit that referenced this pull request Mar 6, 2013
karmi added a commit that referenced this pull request Mar 6, 2013
Revert "[#75] Need to rescue the data attribute as it will be nil if your not using that recipe (like in dev)"

This should not be needed:

    data = raise('BAM') rescue {}

    setting = data['devices'] || {}

    p setting

This reverts commit 45eb5ec.
karmi pushed a commit that referenced this pull request Mar 6, 2013
The "elasticsearch-cloud-aws" plugin can't be named just "aws" in the plugin attributes,
as the plugin recipe will tries to install it, and fail,
causing Elasticsearch to restart on every Chef run.
karmi pushed a commit that referenced this pull request Mar 6, 2013
The "elasticsearch-cloud-aws" plugin can't be named just "aws" in the plugin attributes,
as the plugin recipe will tries to install it, and fail,
causing Elasticsearch to restart on every Chef run.
karmi pushed a commit that referenced this pull request Mar 6, 2013
* Force installation of packages required for the Fog gem before the Chef run
* Use `run_action` for the Fog gem
karmi pushed a commit that referenced this pull request Mar 6, 2013
karmi added a commit that referenced this pull request Mar 6, 2013
karmi pushed a commit that referenced this pull request Mar 6, 2013
karmi added a commit that referenced this pull request Mar 6, 2013
Revert "[#75] Need to rescue the data attribute as it will be nil if your not using that recipe (like in dev)"

This should not be needed:

    data = raise('BAM') rescue {}

    setting = data['devices'] || {}

    p setting

This reverts commit 45eb5ec.
@karmi
Copy link
Contributor

karmi commented Mar 6, 2013

Hi, thanks for all the patches! Finally found time to review and merge it.

I've retouched it a bit and also reverted two commits:

  • 560597f is fixed by 4905e52, the structure should be a Hash. Verified in Vagrant.
  • 45eb5ec was replaced in 9e0e770, I'm wondering about any issue you should have with it, since on a pure Ruby level, this should work (see commit message)
  • 705c796 was split it two, to separate the different concerns in the code

Verifying with Chef Server now...

@karmi karmi closed this Mar 6, 2013
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