-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Allow scaling events to be logged during cluster validation #16354
Changes from all commits
9fc0365
bd049f5
56b53f8
9b6c417
39246bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1196,13 +1196,29 @@ func awsBuildCloudInstanceGroup(ctx context.Context, c AWSCloud, cluster *kops.C | |
return nil, fmt.Errorf("failed to fetch instances: %v", err) | ||
} | ||
|
||
scalingReq := &autoscaling.DescribeScalingActivitiesInput{ | ||
AutoScalingGroupName: g.AutoScalingGroupName, | ||
} | ||
scalingEvents := make([]cloudinstances.ScalingEvent, 0) | ||
c.Autoscaling().DescribeScalingActivitiesPagesWithContext(ctx, scalingReq, func(p *autoscaling.DescribeScalingActivitiesOutput, lastPage bool) bool { | ||
for _, activity := range p.Activities { | ||
event := cloudinstances.ScalingEvent{ | ||
Timestamp: aws.TimeValue(activity.StartTime), | ||
Description: aws.StringValue(activity.Description), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like there's some extra fields; I'm not sure if Description has all/most of the information, but you might consider including the |
||
} | ||
scalingEvents = append(scalingEvents, event) | ||
} | ||
return true | ||
}) | ||
|
||
cg := &cloudinstances.CloudInstanceGroup{ | ||
HumanName: aws.StringValue(g.AutoScalingGroupName), | ||
InstanceGroup: ig, | ||
MinSize: int(aws.Int64Value(g.MinSize)), | ||
TargetSize: int(aws.Int64Value(g.DesiredCapacity)), | ||
MaxSize: int(aws.Int64Value(g.MaxSize)), | ||
Raw: g, | ||
Events: scalingEvents, | ||
} | ||
|
||
for _, i := range g.Instances { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -673,6 +673,7 @@ type InstanceGroupManagerClient interface { | |
Delete(project, zone, name string) (*compute.Operation, error) | ||
Get(project, zone, name string) (*compute.InstanceGroupManager, error) | ||
List(ctx context.Context, project, zone string) ([]*compute.InstanceGroupManager, error) | ||
ListErrors(ctx context.Context, project, zone, name string) ([]*compute.InstanceManagedByIgmError, error) | ||
ListManagedInstances(ctx context.Context, project, zone, name string) ([]*compute.ManagedInstance, error) | ||
RecreateInstances(project, zone, name, id string) (*compute.Operation, error) | ||
SetTargetPools(project, zone, name string, targetPools []string) (*compute.Operation, error) | ||
|
@@ -709,6 +710,17 @@ func (c *instanceGroupManagerClientImpl) List(ctx context.Context, project, zone | |
return ms, nil | ||
} | ||
|
||
func (c *instanceGroupManagerClientImpl) ListErrors(ctx context.Context, project, zone, name string) ([]*compute.InstanceManagedByIgmError, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aside: I sort of regret our having this layer in GCE, I'm not sure it adds much! |
||
var igmErrors []*compute.InstanceManagedByIgmError | ||
if err := c.srv.ListErrors(project, zone, name).Pages(ctx, func(page *compute.InstanceGroupManagersListErrorsResponse) error { | ||
igmErrors = append(igmErrors, page.Items...) | ||
return nil | ||
}); err != nil { | ||
return nil, err | ||
} | ||
return igmErrors, nil | ||
} | ||
|
||
func (c *instanceGroupManagerClientImpl) ListManagedInstances(ctx context.Context, project, zone, name string) ([]*compute.ManagedInstance, error) { | ||
var instances []*compute.ManagedInstance | ||
if err := c.srv.ListManagedInstances(project, zone, name).Pages(ctx, func(page *compute.InstanceGroupManagersListManagedInstancesResponse) error { | ||
|
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 don't know whether this gets expensive, but I wonder if we should plumb through a flag here as to whether we want this information (or add a method or func to CloudInstanceGroup that could query it on-demand)