Skip to content

Commit

Permalink
OCPBUGS-17227: gcp: fix validation of custom instance types
Browse files Browse the repository at this point in the history
Users can create their own instance types in GCloud. Those types are not
returned when listing the machine types for a given project/region. This
would cause those installs with custom types to fail with the error:

```
Internal error: failed to fetch instance type, this error usually occurs if the region or the instance type is not found
```

after the validations introduced by
#7317 and
#7096.

To fix that, let's fallback to the previous way of fetching the specific
machine type whenever the aggregated list returns 0 elements.
  • Loading branch information
r4f4 committed Aug 2, 2023
1 parent 4f59664 commit 087ab4b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 21 deletions.
34 changes: 22 additions & 12 deletions pkg/asset/installconfig/gcp/client.go
Expand Up @@ -104,9 +104,6 @@ func GetMachineTypeList(ctx context.Context, svc *compute.Service, project, regi
}
return nil
})
if len(machines) == 0 {
return nil, errors.New("failed to fetch instance type, this error usually occurs if the region or the instance type is not found")
}

return machines, err
}
Expand All @@ -118,25 +115,38 @@ func (c *Client) GetMachineTypeWithZones(ctx context.Context, project, region, m
return nil, nil, err
}

pz, err := GetZones(ctx, svc, project, fmt.Sprintf("region eq .*%s", region))
if err != nil {
return nil, nil, err
}
projZones := sets.New[string]()
for _, zone := range pz {
projZones.Insert(zone.Name)
}

machines, err := GetMachineTypeList(ctx, svc, project, region, machineType, "")
if err != nil {
return nil, nil, err
}

// Custom machine types are not included in aggregated lists, so let's try
// to get the machine type directly before returning an error. Also
// fallback to all the zones in the project
if len(machines) == 0 {
cctx, cancel := context.WithTimeout(ctx, defaultTimeout)
defer cancel()
machine, err := svc.MachineTypes.Get(project, pz[0].Name, machineType).Context(cctx).Do()
if err != nil {
return nil, nil, fmt.Errorf("failed to fetch instance type: %w", err)
}
return machine, projZones, nil
}

zones := sets.New[string]()
for _, machine := range machines {
zones.Insert(machine.Zone)
}

// Restrict to zones avaialable in the project
pz, err := GetZones(ctx, svc, project, fmt.Sprintf("region eq .*%s", region))
if err != nil {
return nil, nil, err
}
projZones := sets.New[string]()
for _, zone := range pz {
projZones.Insert(zone.Name)
}
zones = zones.Intersection(projZones)

return machines[0], zones, nil
Expand Down
22 changes: 13 additions & 9 deletions pkg/asset/machines/gcp/zones.go
Expand Up @@ -64,25 +64,29 @@ func ZonesForInstanceType(project, region, instanceType string) ([]string, error
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()

pZones, err := gcpconfig.GetZones(ctx, svc, project, fmt.Sprintf("(region eq .*%s) (status eq UP)", region))
if err != nil {
return nil, fmt.Errorf("failed to get zones for project: %w", err)
}
pZoneNames := sets.New[string]()
for _, z := range pZones {
pZoneNames.Insert(z.Name)
}

machines, err := gcpconfig.GetMachineTypeList(ctx, svc, project, region, instanceType, "items/*/machineTypes(zone),nextPageToken")
if err != nil {
return nil, fmt.Errorf("failed to get zones for instance type: %w", err)
}
// Custom machine types do not show up in the list. Let's fallback to the project zones
if len(machines) == 0 {
return sets.List(pZoneNames), nil
}

zones := sets.New[string]()
for _, machine := range machines {
zones.Insert(machine.Zone)
}

// Not all instance zones might be available in the project
pZones, err := gcpconfig.GetZones(ctx, svc, project, fmt.Sprintf("(region eq .*%s) (status eq UP)", region))
if err != nil {
return nil, fmt.Errorf("failed to get zones for project: %w", err)
}
pZoneNames := sets.New[string]()
for _, z := range pZones {
pZoneNames.Insert(z.Name)
}

return sets.List(zones.Intersection(pZoneNames)), nil
}

0 comments on commit 087ab4b

Please sign in to comment.