-
Notifications
You must be signed in to change notification settings - Fork 84
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
target groups: preserve last used arns in case of ROLLBACK_IN_PROGRESS
state
#673
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Mustafa Abdelrahman <[email protected]>
Signed-off-by: Mustafa Abdelrahman <[email protected]>
@@ -61,6 +61,7 @@ type Adapter struct { | |||
obsoleteInstances []string | |||
stackTerminationProtection bool | |||
stackTags map[string]string | |||
stackLastTargerGroupARNs map[string][]string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stackLastTargerGroupARNs -> stackLastTargetGroupARNs
if err != nil { | ||
return nil, err | ||
} | ||
return stacks, nil | ||
} | ||
|
||
func (a *Adapter) UpdateStackLastTargetGroupARNs(stack *Stack) { | ||
if _, ok := a.stackLastTargerGroupARNs[stack.Name]; !ok { | ||
if len(stack.TargetGroupARNs) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you check this here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check can be moved outside the function yes, I just thought it made sense here.
stacks := make([]*Stack, 0) | ||
err := svc.DescribeStacksPages(&cloudformation.DescribeStacksInput{}, | ||
func(page *cloudformation.DescribeStacksOutput, lastPage bool) bool { | ||
for _, s := range page.Stacks { | ||
if isManagedStack(s.Tags, clusterID, controllerID) { | ||
stacks = append(stacks, mapToManagedStack(s)) | ||
stack := mapToManagedStack(s) | ||
if len(stack.TargetGroupARNs) == 0 && stack.status == cloudformation.StackStatusRollbackInProgress { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why check len(stack.TargetGroupARNs) == 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still thinking should it be for all states without output or only for rollback
log.Warnf("stack %s is in rolling back state, falling back to last saved output", stack.Name) | ||
stack.TargetGroupARNs = stacksLastTargetGroupARNs[stack.Name] | ||
} else { | ||
log.Warnf("stack %s has no saved target groups, skipping", stack.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do you skip here? I don't see that you skip anything :)
Maybe use continue
to not add it to "stacks", then you do not need the code in worker.go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thanks. Forgot the continue
but I still need the code in worker.go that's where I reset and save "last" data
No description provided.