ec2_instance InvalidParameterCombination when calling ModifyInstanceAttribute #470
Comments
|
Leaving some notes here as I'll be away for several days. Replicating CI using the At https://github.com/ansible-collections/community.aws/blob/main/plugins/modules/ec2_instance.py#L1171 The problem here is that this vpc-id is the default VPC for us-east-1 in CI and sg-06ed7d30d22e0a670 (in this example) belongs to the test VPC, vpc-077adc0ebabb1f33b. So Which makes However if I run a simple playbook against a personal account using my actual default VPC, same failure |
|
This feels a lot like the instability we were seeing in #180 where filtered searches were (are) randomly returning empty results - I switched over to running the describe based on ID and it returned much more consistently. |
|
@tremble My concern is if this was a default behaviour that worked previously and is now resulting in new behaviour for Ansible users, I didn't find any obvious change notices on the AWS side but that's not shocking. |
* Explicitly pass the subnet the instance should live on - try to avoid https://github.com/ansible-collections/community.aws/issues/329 * Make sure we delete the Instance (and free the EIP) before we try to drop the IGW * Use IDs when cleaning up EC2 instances
|
@briantist I believe we've come to the conclusion that the fix here is to always be explicit and provide a |
|
Thanks @jillr , I think we're already always setting |
|
For anyone else who might come across this, from my debugging what I've been seeing is that some of the time filter based searches are returning an empty list when they shouldn't (or at least when historically they didn't). The account we use for CI often hits the API Rate Limits, so it's also possible that this is coming into play "somehow", either client side, or possibly on the Amazon side. I've been working around this by relying less on filter based searches where possible (ec2_eni used to make a lot of gratuitous filter based searches). If I keep seeing this get worse I'll likely try to put together a reproducer that we can throw at Amazon. |
…tions#333) * Explicitly pass the subnet the instance should live on - try to avoid https://github.com/ansible-collections/community.aws/issues/329 * Make sure we delete the Instance (and free the EIP) before we try to drop the IGW * Use IDs when cleaning up EC2 instances
…tions#333) * Explicitly pass the subnet the instance should live on - try to avoid https://github.com/ansible-collections/community.aws/issues/329 * Make sure we delete the Instance (and free the EIP) before we try to drop the IGW * Use IDs when cleaning up EC2 instances
ansibullbot
added
bug
…e-collections#470) * Make "unit" parameter optional and add support for check mode boto3 documentation explicitly suggests omitting this parameter: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/cloudwatch.html#CloudWatch.Client.put_metric_alarm Creating an alarm without the "unit" parameter specified fails: ``` - community.aws.ec2_metric_alarm: name: My alarm description: My description namespace: AWS/CertificateManager metric: DaysToExpiry statistic: Average comparison: LessThanOrEqualToThreshold threshold: 45 period: 86400 evaluation_periods: 1 dimensions: CertificateArn: "arn:aws:acm:us-east-1:123412341234:certificate/example" alarm_actions: - arn:aws:sns:us-east-1:123412341234:my-sns-topic ok_actions: - arn:aws:sns:us-east-1:123412341234:my-sns-topic treat_missing_data: ignore state: present ``` with the following error: ``` Invalid type for parameter Unit, value: None, type: <class 'NoneType'>, valid types: <class 'str'> ``` Apparently specifying `unit: None` in the example above is not the same as omitting the unit - it causes the alarm to be in "Insufficient data" state. * Fix module output for tests * Add tests for idempotency and check mode * Fix an error when the module creates a new alarm in check mode Alarm is not actuall created in check mode, and therefore `describe_alarms` returns an empty list. * Add tests for alarm creation with no unit attribute specified * Fix typo - MetricAlarms vs MetricsAlarms * Fix variable name - alarm_info_query_check vs alarm_info_check * Fix variable names in tests * Fix tests by ensuring that alarm doesn't exist before we begin * Fix variable name * Fix assertion * Ensure check mode is enabled when it is supposed to be * Enable check mode for alarm deletion * Fix variable name - alarm_info_no_unit vs alarm_info * Fix the test of creating the alarm without unit attribute * Fix variable name - alarm_info_query_no_unit vs alarm_info * Update changelogs/fragments/470-ec2_metric_alarm-unit-optional.yml * Update changelogs/fragments/470-ec2_metric_alarm-unit-optional.yml Co-authored-by: Mark Chappell <[email protected]>
…e-collections#470) * Make "unit" parameter optional and add support for check mode boto3 documentation explicitly suggests omitting this parameter: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/cloudwatch.html#CloudWatch.Client.put_metric_alarm Creating an alarm without the "unit" parameter specified fails: ``` - community.aws.ec2_metric_alarm: name: My alarm description: My description namespace: AWS/CertificateManager metric: DaysToExpiry statistic: Average comparison: LessThanOrEqualToThreshold threshold: 45 period: 86400 evaluation_periods: 1 dimensions: CertificateArn: "arn:aws:acm:us-east-1:123412341234:certificate/example" alarm_actions: - arn:aws:sns:us-east-1:123412341234:my-sns-topic ok_actions: - arn:aws:sns:us-east-1:123412341234:my-sns-topic treat_missing_data: ignore state: present ``` with the following error: ``` Invalid type for parameter Unit, value: None, type: <class 'NoneType'>, valid types: <class 'str'> ``` Apparently specifying `unit: None` in the example above is not the same as omitting the unit - it causes the alarm to be in "Insufficient data" state. * Fix module output for tests * Add tests for idempotency and check mode * Fix an error when the module creates a new alarm in check mode Alarm is not actuall created in check mode, and therefore `describe_alarms` returns an empty list. * Add tests for alarm creation with no unit attribute specified * Fix typo - MetricAlarms vs MetricsAlarms * Fix variable name - alarm_info_query_check vs alarm_info_check * Fix variable names in tests * Fix tests by ensuring that alarm doesn't exist before we begin * Fix variable name * Fix assertion * Ensure check mode is enabled when it is supposed to be * Enable check mode for alarm deletion * Fix variable name - alarm_info_no_unit vs alarm_info * Fix the test of creating the alarm without unit attribute * Fix variable name - alarm_info_query_no_unit vs alarm_info * Update changelogs/fragments/470-ec2_metric_alarm-unit-optional.yml * Update changelogs/fragments/470-ec2_metric_alarm-unit-optional.yml Co-authored-by: Mark Chappell <[email protected]>
…e-collections#470) * Make "unit" parameter optional and add support for check mode boto3 documentation explicitly suggests omitting this parameter: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/cloudwatch.html#CloudWatch.Client.put_metric_alarm Creating an alarm without the "unit" parameter specified fails: ``` - community.aws.ec2_metric_alarm: name: My alarm description: My description namespace: AWS/CertificateManager metric: DaysToExpiry statistic: Average comparison: LessThanOrEqualToThreshold threshold: 45 period: 86400 evaluation_periods: 1 dimensions: CertificateArn: "arn:aws:acm:us-east-1:123412341234:certificate/example" alarm_actions: - arn:aws:sns:us-east-1:123412341234:my-sns-topic ok_actions: - arn:aws:sns:us-east-1:123412341234:my-sns-topic treat_missing_data: ignore state: present ``` with the following error: ``` Invalid type for parameter Unit, value: None, type: <class 'NoneType'>, valid types: <class 'str'> ``` Apparently specifying `unit: None` in the example above is not the same as omitting the unit - it causes the alarm to be in "Insufficient data" state. * Fix module output for tests * Add tests for idempotency and check mode * Fix an error when the module creates a new alarm in check mode Alarm is not actuall created in check mode, and therefore `describe_alarms` returns an empty list. * Add tests for alarm creation with no unit attribute specified * Fix typo - MetricAlarms vs MetricsAlarms * Fix variable name - alarm_info_query_check vs alarm_info_check * Fix variable names in tests * Fix tests by ensuring that alarm doesn't exist before we begin * Fix variable name * Fix assertion * Ensure check mode is enabled when it is supposed to be * Enable check mode for alarm deletion * Fix variable name - alarm_info_no_unit vs alarm_info * Fix the test of creating the alarm without unit attribute * Fix variable name - alarm_info_query_no_unit vs alarm_info * Update changelogs/fragments/470-ec2_metric_alarm-unit-optional.yml * Update changelogs/fragments/470-ec2_metric_alarm-unit-optional.yml Co-authored-by: Mark Chappell <[email protected]>
SUMMARY
We're starting to see CI failures for tests that use ec2_instance when creating new instances:
"msg": "Could not apply change {'InstanceId': 'i-0c72055e2fe442009', 'Groups': []} to new instance.: An error occurred (InvalidParameterCombination) when calling the ModifyInstanceAttribute operation: No attributes specified.",For example, https://app.shippable.com/github/ansible-collections/community.aws/runs/1144/23/console
Appears to be a failure with https://github.com/ansible-collections/community.aws/blob/main/plugins/modules/ec2_instance.py#L1174 not returning any groups.
ISSUE TYPE
COMPONENT NAME
ec2_instance.py
ANSIBLE VERSION
2.9, 2.10, devel
OS / ENVIRONMENT
shippable
The text was updated successfully, but these errors were encountered: