From b03e671a627839535104b850b7e0272769d0df55 Mon Sep 17 00:00:00 2001 From: Moritz Sanft <58110325+msanft@users.noreply.github.com> Date: Tue, 3 Dec 2024 22:44:38 +0100 Subject: [PATCH] constellation-node-operator: don't bail out on listing errors (#3522) If the GCP project has scaling groups for which our checks can't be performed (which is the case for regional scaling groups, as they "don't exist" for the operator, if deployed in another region) . In that case, we should not bail out directly but go on with the next group. An error should only be thrown if there are no matching groups at all. --- .../internal/cloud/gcp/client/scalinggroup.go | 19 +++++++++++++++---- .../cloud/gcp/client/scalinggroup_test.go | 5 ++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup.go b/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup.go index 658aaea965..3f10af753f 100644 --- a/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup.go +++ b/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup.go @@ -105,6 +105,8 @@ func (c *Client) GetAutoscalingGroupName(scalingGroupID string) (string, error) // ListScalingGroups retrieves a list of scaling groups for the cluster. func (c *Client) ListScalingGroups(ctx context.Context, uid string) ([]cspapi.ScalingGroup, error) { results := []cspapi.ScalingGroup{} + var retErr error + iter := c.instanceGroupManagersAPI.AggregatedList(ctx, &computepb.AggregatedListInstanceGroupManagersRequest{ Project: c.projectID, }) @@ -129,7 +131,8 @@ func (c *Client) ListScalingGroups(ctx context.Context, uid string) ([]cspapi.Sc } template, err := c.instanceTemplateAPI.Get(c.projectID, templateURI[len(templateURI)-1]) if err != nil { - return nil, fmt.Errorf("getting instance template: %w", err) + retErr = errors.Join(retErr, fmt.Errorf("getting instance template %q: %w", templateURI[len(templateURI)-1], err)) + continue } if template.Properties == nil || template.Properties.Labels == nil { continue @@ -140,14 +143,16 @@ func (c *Client) ListScalingGroups(ctx context.Context, uid string) ([]cspapi.Sc groupID, err := c.canonicalInstanceGroupID(ctx, *grpManager.SelfLink) if err != nil { - return nil, fmt.Errorf("normalizing instance group ID: %w", err) + retErr = errors.Join(retErr, fmt.Errorf("getting canonical instance group ID: %w", err)) + continue } role := updatev1alpha1.NodeRoleFromString(template.Properties.Labels["constellation-role"]) name, err := c.GetScalingGroupName(groupID) if err != nil { - return nil, fmt.Errorf("getting scaling group name: %w", err) + retErr = errors.Join(retErr, fmt.Errorf("getting scaling group name: %w", err)) + continue } nodeGroupName := template.Properties.Labels["constellation-node-group"] @@ -164,7 +169,8 @@ func (c *Client) ListScalingGroups(ctx context.Context, uid string) ([]cspapi.Sc autoscalerGroupName, err := c.GetAutoscalingGroupName(groupID) if err != nil { - return nil, fmt.Errorf("getting autoscaling group name: %w", err) + retErr = errors.Join(retErr, fmt.Errorf("getting autoscaling group name: %w", err)) + continue } results = append(results, cspapi.ScalingGroup{ @@ -176,6 +182,11 @@ func (c *Client) ListScalingGroups(ctx context.Context, uid string) ([]cspapi.Sc }) } } + + if len(results) == 0 { + return nil, errors.Join(errors.New("no scaling groups found"), retErr) + } + return results, nil } diff --git a/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup_test.go b/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup_test.go index 26983cb97d..01629951d2 100644 --- a/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup_test.go +++ b/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup_test.go @@ -427,8 +427,11 @@ func TestListScalingGroups(t *testing.T) { templateLabels: map[string]string{ "label": "value", }, + wantErr: true, + }, + "invalid instance group manager": { + wantErr: true, }, - "invalid instance group manager": {}, } for name, tc := range testCases {