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

provider/aws: wait for ASG capacity on update #3947

Merged
merged 1 commit into from
Jan 6, 2016

Conversation

phinze
Copy link
Contributor

@phinze phinze commented Nov 17, 2015

It's a bit confusing to have Terraform poll until instances come up on
ASG creation but not on update. This changes update to also poll if
min_size or desired_capacity are changed.

This changes the waiting behavior to wait for precisely the desired
number of instances instead of that number as a "minimum". I believe
this shouldn't have any undue side effects, and the behavior can still
be opted out of by setting wait_for_capacity_timeout to 0.

@phinze
Copy link
Contributor Author

phinze commented Nov 17, 2015

cc @pshima, @catsby

Wondering if perhaps we should wait behavior to poll for == instead of >= so it covers decreases as well? I'm guessing that's the least surprising behavior. What do you think?

@pshima
Copy link
Contributor

pshima commented Nov 17, 2015

I think we should cover capacity decreases like you suggest as well as we are still in the middle of mutating the infrastructure.

@jen20
Copy link
Contributor

jen20 commented Nov 17, 2015

This seems like the behaviour I'd expect. Probably >= is better if we also want to cover decreases.

It's a bit confusing to have Terraform poll until instances come up on
ASG creation but not on update. This changes update to also poll if
min_size or desired_capacity are changed.

This changes the waiting behavior to wait for precisely the desired
number of instances instead of that number as a "minimum". I believe
this shouldn't have any undue side effects, and the behavior can still
be opted out of by setting `wait_for_capacity_timeout` to 0.
@phinze
Copy link
Contributor Author

phinze commented Nov 17, 2015

Okay updated to change waitFor behavior to look for exactly the number of desired instances. This makes the behavior cover decreases as well as increases.

I had to rename the min_elb_capacity field to wait_for_elb_capacity so it is not confusing for decreases.

@catsby
Copy link
Contributor

catsby commented Nov 17, 2015

@jen20 seemed to think >= was the better choice, but we have ==. Have we agreed on == or is that still up for discussion?

@phinze
Copy link
Contributor Author

phinze commented Dec 2, 2015

@jen20 was your last comment a typo? I believe we need to wait for == to cover both increases and decreases.

@jen20
Copy link
Contributor

jen20 commented Jan 6, 2016

Sorry - yes == is what I meant. Typo - sorry!

@phinze
Copy link
Contributor Author

phinze commented Jan 6, 2016

Alrighty - given that, bringing this in!

phinze added a commit that referenced this pull request Jan 6, 2016
provider/aws: wait for ASG capacity on update
@phinze phinze merged commit 319f7d7 into master Jan 6, 2016
@phinze phinze deleted the phinze/asg-wait-on-update branch January 6, 2016 21:55
phinze added a commit that referenced this pull request Jan 27, 2016
…waiting

It was a mistake to switched fully to `==` when activating waiting for
capacity on updates in #3947. Users that didn't set `min_elb_capacity ==
desired_capacity` and instead treated it as an actual "minimum" would
see timeouts for every create, since their target numbers would never be
reached exactly.

Here, we fix that regression by restoring the minimum waiting behavior
during creates.

In order to preserve all the stated behavior, I had to split out
different criteria for create and update, criteria which are now
exhaustively unit tested.

The set of fields that affect capacity waiting behavior has become a bit
of a mess. Next major release I'd like to rework all of these into a
more consistently named block of config. For now, just getting the
behavior correct and documented.

Fixes #4792
phinze added a commit that referenced this pull request Jan 27, 2016
It was a mistake to switched fully to `==` when activating waiting for
capacity on updates in #3947. Users that didn't set `min_elb_capacity ==
desired_capacity` and instead treated it as an actual "minimum" would
see timeouts for every create, since their target numbers would never be
reached exactly.

Here, we fix that regression by restoring the minimum waiting behavior
during creates.

In order to preserve all the stated behavior, I had to split out
different criteria for create and update, criteria which are now
exhaustively unit tested.

The set of fields that affect capacity waiting behavior has become a bit
of a mess. Next major release I'd like to rework all of these into a
more consistently named block of config. For now, just getting the
behavior correct and documented.

Fixes #4792
phinze added a commit that referenced this pull request Jan 27, 2016
It was a mistake to switched fully to `==` when activating waiting for
capacity on updates in #3947. Users that didn't set `min_elb_capacity ==
desired_capacity` and instead treated it as an actual "minimum" would
see timeouts for every create, since their target numbers would never be
reached exactly.

Here, we fix that regression by restoring the minimum waiting behavior
during creates.

In order to preserve all the stated behavior, I had to split out
different criteria for create and update, criteria which are now
exhaustively unit tested.

The set of fields that affect capacity waiting behavior has become a bit
of a mess. Next major release I'd like to rework all of these into a
more consistently named block of config. For now, just getting the
behavior correct and documented.

Fixes #4792
phinze added a commit that referenced this pull request Jan 27, 2016
It was a mistake to switched fully to `==` when activating waiting for
capacity on updates in #3947. Users that didn't set `min_elb_capacity ==
desired_capacity` and instead treated it as an actual "minimum" would
see timeouts for every create, since their target numbers would never be
reached exactly.

Here, we fix that regression by restoring the minimum waiting behavior
during creates.

In order to preserve all the stated behavior, I had to split out
different criteria for create and update, criteria which are now
exhaustively unit tested.

The set of fields that affect capacity waiting behavior has become a bit
of a mess. Next major release I'd like to rework all of these into a
more consistently named block of config. For now, just getting the
behavior correct and documented.

Fixes #4792
phinze added a commit that referenced this pull request Jan 27, 2016
It was a mistake to switched fully to `==` when activating waiting for
capacity on updates in #3947. Users that didn't set `min_elb_capacity ==
desired_capacity` and instead treated it as an actual "minimum" would
see timeouts for every create, since their target numbers would never be
reached exactly.

Here, we fix that regression by restoring the minimum waiting behavior
during creates.

In order to preserve all the stated behavior, I had to split out
different criteria for create and update, criteria which are now
exhaustively unit tested.

The set of fields that affect capacity waiting behavior has become a bit
of a mess. Next major release I'd like to rework all of these into a
more consistently named block of config. For now, just getting the
behavior correct and documented.

(Also removes all the fixed names from the ASG tests as I was hitting
collision issues running them over here.)

Fixes #4792
joshmyers pushed a commit to joshmyers/terraform that referenced this pull request Feb 18, 2016
It was a mistake to switched fully to `==` when activating waiting for
capacity on updates in hashicorp#3947. Users that didn't set `min_elb_capacity ==
desired_capacity` and instead treated it as an actual "minimum" would
see timeouts for every create, since their target numbers would never be
reached exactly.

Here, we fix that regression by restoring the minimum waiting behavior
during creates.

In order to preserve all the stated behavior, I had to split out
different criteria for create and update, criteria which are now
exhaustively unit tested.

The set of fields that affect capacity waiting behavior has become a bit
of a mess. Next major release I'd like to rework all of these into a
more consistently named block of config. For now, just getting the
behavior correct and documented.

(Also removes all the fixed names from the ASG tests as I was hitting
collision issues running them over here.)

Fixes hashicorp#4792
bigkraig pushed a commit to bigkraig/terraform that referenced this pull request Mar 1, 2016
It was a mistake to switched fully to `==` when activating waiting for
capacity on updates in hashicorp#3947. Users that didn't set `min_elb_capacity ==
desired_capacity` and instead treated it as an actual "minimum" would
see timeouts for every create, since their target numbers would never be
reached exactly.

Here, we fix that regression by restoring the minimum waiting behavior
during creates.

In order to preserve all the stated behavior, I had to split out
different criteria for create and update, criteria which are now
exhaustively unit tested.

The set of fields that affect capacity waiting behavior has become a bit
of a mess. Next major release I'd like to rework all of these into a
more consistently named block of config. For now, just getting the
behavior correct and documented.

(Also removes all the fixed names from the ASG tests as I was hitting
collision issues running them over here.)

Fixes hashicorp#4792
omeid pushed a commit to omeid/terraform that referenced this pull request Mar 30, 2018
…x-match-set

New Resource: aws_waf_regex_match_set
@ghost
Copy link

ghost commented Apr 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 Apr 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants