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 launch config creation params #34014

Merged
merged 1 commit into from
Jun 15, 2016

Conversation

jnhmcknight
Copy link
Contributor

What does this PR do?

This fixes an issue where the launch configuration could not be created because it was connecting to the default region instead of the specified region.

What issues does this PR fix or reference?

There was no submitted issue for this. It was an issue that was hit this morning for us and we debugged until it was fixed.

Previous Behavior

Given:

    Ensure mylc exists:
      boto_lc.present:
        - name: mylc
        - region: sa-east-1
        - image_id: ami-0b9c9f62
        - profile: myprofile

the state will fail with due to connecting to the wrong region:

<ErrorResponse xmlns="http://autoscaling.amazonaws.com/doc/2011-01-01/">
  <Error>
    <Type>Sender</Type>
    <Code>ValidationError</Code>
    <Message>AMI ami-0b9c9f62 is invalid: The image id '[ami-0b9c9f62]' does not exist</Message>
  </Error>
  <RequestId>b60ae124-3269-11e6-9c98-016ce5ab63a5</RequestId>
</ErrorResponse>

The arguments to boto_asg.create_launch_configuration were taken as positional instead of named, and there were additional arguments to boto_asg.create_launch_configuration. This created an inconsistency between argument lists and therefore the specified region was getting passed into the volume_type argument and consequently the local region variable was always None for the actual creation of the launch configuration.

New Behavior

All parameters have been named to prevent this error.

Tests written?

No

@rallytime
Copy link
Contributor

@jnhmcknight Thank you for cleaning this up. This has bitten us in a couple of places in the boto files where positional kwargs are being used instead of explicitly naming them.

rallytime pushed a commit that referenced this pull request Jun 15, 2016
@rallytime rallytime merged commit f598f96 into saltstack:2016.3 Jun 15, 2016
@scoates scoates deleted the boto_lc_region_fix branch June 15, 2016 17:05
gitebra pushed a commit to gitebra/salt that referenced this pull request Jun 16, 2016
* upstream/develop: (42 commits)
  Typo fix (saltstack#34066)
  states.disk: rewrite unit tests
  states.disk.status: validate percent values
  states.disk: add documentation
  _active_mounts_openbsd: unbreak output for special filesystems (saltstack#34057)
  Use the develop version of the disk state
  fix saltstack#34038 (saltstack#34042)
  Merge pull request saltstack#34025 from rallytime/merge-2016.3
  Update saltmod tests to use master __opts__
  Documentation to describe orchestration support for masterless minions
  Support `salt.state` orchestration formats on masterless minions
  Import the orchestrate runner as an execution module function
  Updated latest release to 2016.3.1 Clean up installation instructions code-block type updates Add link to jinja tutorial
  fix launch config creation params (saltstack#34014)
  Always make chanes to minion config if set (saltstack#34021)
  `states.postgres_privileges` expects a real list, not a comma-separated string for `privileges`
  one clause to set OS grain from CPE_NAME
  fix redis_return's clean_old_jobs.
  Adds new Understanding Jinja topic, and fixes several Jinja doc issues. Removes the "Full list of builtin ..." from each module reference list, leaving just the module type for scanability.
  Call `sys.exit()` instead of `exit()`
  ...
@jnhmcknight jnhmcknight restored the boto_lc_region_fix branch October 18, 2016 20:37
@jnhmcknight jnhmcknight deleted the boto_lc_region_fix branch November 4, 2016 12:33
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.

None yet

2 participants