Optimize API Calls (#235)

Any handler (regions, clusters) that needed access to either the
identity service or the region service would unconditionally create a
client, and that would involve TLS handshakes and token exchange.  For
things like clusters, they only need to update allocations on
create/update/delete, not read, and only need access to flavors and
images on create/update, thus there is an implicit penalty for APIs that
don't need those clients.  This makes API access to those client lazy,
and they will only create a client if necessary.  Secondly, much like
the region service, we expect things like regions, flavors and images to
be relatively static, so we can put a cache in front of these calls with
an hour timeout.

The result is that on the standard "List Kubernetes Clusters" UI view,
the API response times are now less that 100ms, down from over 1000ms.

Author: spjmurray

charts/kubernetes/templates/server/clusterrole.yaml

@@ -36,3 +36,10 @@ rules:
   verbs:
   - list
   - watch
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - list
+  - watch

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/prometheus/client_golang v1.22.0
 	github.com/spf13/pflag v1.0.6
 	github.com/stretchr/testify v1.10.0
-	github.com/unikorn-cloud/core v0.1.95
+	github.com/unikorn-cloud/core v0.1.96-rc1
 	github.com/unikorn-cloud/identity v0.2.63
 	github.com/unikorn-cloud/region v0.1.54
 	go.opentelemetry.io/otel v1.35.0
@@ -33,6 +33,7 @@ require (
 	github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/blang/semver/v4 v4.0.0 // indirect
+	github.com/brunoga/deep v1.2.4 // indirect
 	github.com/cenkalti/backoff/v4 v4.3.0 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/coreos/go-oidc/v3 v3.14.1 // indirect

go.sum

@@ -10,6 +10,8 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r
 github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM=
 github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ=
 github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w=
+github.com/brunoga/deep v1.2.4 h1:Aj9E9oUbE+ccbyh35VC/NHlzzjfIVU69BXu2mt2LmL8=
+github.com/brunoga/deep v1.2.4/go.mod h1:GDV6dnXqn80ezsLSZ5Wlv1PdKAWAO4L5PnKYtv2dgaI=
 github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
 github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
 github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
@@ -173,8 +175,8 @@ github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOf
 github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE=
 github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg=
-github.com/unikorn-cloud/core v0.1.95 h1:i5jyWnleYbFxvniKfneiEeeNL2oCtkJjdGtRJipYH9Y=
-github.com/unikorn-cloud/core v0.1.95/go.mod h1:OdZlqXlkO0EFaThARFYRctKAtsVKimhSZid72OaC43Y=
+github.com/unikorn-cloud/core v0.1.96-rc1 h1:TgSMyHhUWYmWgLXiTWNUsroO3F2LtTJ/k+nEDgDnOAo=
+github.com/unikorn-cloud/core v0.1.96-rc1/go.mod h1:stInT6j9sM9KzDHgNxBtmrdDIAxQuIZI1/TCGo0jNK8=
 github.com/unikorn-cloud/identity v0.2.63 h1:cG3Aa3LmweqOwNtOeq9W29/aoJ4wY0uiulLNzWwn7TY=
 github.com/unikorn-cloud/identity v0.2.63/go.mod h1:xuOIyB4wDAz4+kJfZk2q+8MqGj+9IhVbd0Q38iqBY24=
 github.com/unikorn-cloud/region v0.1.54 h1:orGCMLIMUmSWLOlBBha6q5SgXKAlzFqQJRVhneD4/so=

pkg/provisioners/managers/cluster/provisioner.go

@@ -53,7 +53,6 @@ import (
 	"github.com/unikorn-cloud/kubernetes/pkg/provisioners/helmapplications/openstackcloudprovider"
 	"github.com/unikorn-cloud/kubernetes/pkg/provisioners/helmapplications/openstackplugincindercsi"
 	"github.com/unikorn-cloud/kubernetes/pkg/provisioners/helmapplications/vcluster"
-	"github.com/unikorn-cloud/kubernetes/pkg/server/handler/region"
 	regionclient "github.com/unikorn-cloud/region/pkg/client"
 	regionapi "github.com/unikorn-cloud/region/pkg/openapi"
 
@@ -428,6 +427,21 @@ func (p *Provisioner) getRegionClient(ctx context.Context, traceName string) (co
 	return ctx, client, nil
 }
 
+func (p *Provisioner) getFlavors(ctx context.Context, client regionapi.ClientWithResponsesInterface, organizationID, regionID string) ([]regionapi.Flavor, error) {
+	resp, err := client.GetApiV1OrganizationsOrganizationIDRegionsRegionIDFlavorsWithResponse(ctx, organizationID, regionID)
+	if err != nil {
+		return nil, err
+	}
+
+	if resp.StatusCode() != http.StatusOK {
+		return nil, coreapiutils.ExtractError(resp.StatusCode(), resp)
+	}
+
+	flavors := *resp.JSON200
+
+	return flavors, nil
+}
+
 func (p *Provisioner) getIdentity(ctx context.Context, client regionapi.ClientWithResponsesInterface) (*regionapi.IdentityRead, error) {
 	log := log.FromContext(ctx)
 
@@ -538,7 +552,7 @@ func (p *Provisioner) identityOptions(ctx context.Context, client regionapi.Clie
 		return nil, err
 	}
 
-	flavors, err := region.Flavors(ctx, client, p.cluster.Labels[coreconstants.OrganizationLabel], p.cluster.Spec.RegionID)
+	flavors, err := p.getFlavors(ctx, client, p.cluster.Labels[coreconstants.OrganizationLabel], p.cluster.Spec.RegionID)
 	if err != nil {
 		return nil, err
 	}

pkg/server/handler/cluster/client.go

@@ -40,6 +40,7 @@ import (
 	"github.com/unikorn-cloud/kubernetes/pkg/provisioners/helmapplications/vcluster"
 	"github.com/unikorn-cloud/kubernetes/pkg/server/handler/clustermanager"
 	"github.com/unikorn-cloud/kubernetes/pkg/server/handler/common"
+	"github.com/unikorn-cloud/kubernetes/pkg/server/handler/identity"
 	"github.com/unikorn-cloud/kubernetes/pkg/server/handler/region"
 	regionapi "github.com/unikorn-cloud/region/pkg/openapi"
 
@@ -92,14 +93,14 @@ type Client struct {
 	options *Options
 
 	// identity is a client to access the identity service.
-	identity identityapi.ClientWithResponsesInterface
+	identity *identity.Client
 
 	// region is a client to access regions.
-	region regionapi.ClientWithResponsesInterface
+	region *region.Client
 }
 
 // NewClient returns a new client with required parameters.
-func NewClient(client client.Client, namespace string, options *Options, identity identityapi.ClientWithResponsesInterface, region regionapi.ClientWithResponsesInterface) *Client {
+func NewClient(client client.Client, namespace string, options *Options, identity *identity.Client, region *region.Client) *Client {
 	return &Client{
 		client:    client,
 		namespace: namespace,
@@ -200,7 +201,7 @@ func (c *Client) GetKubeconfig(ctx context.Context, organizationID, projectID, c
 }
 
 func (c *Client) generateAllocations(ctx context.Context, organizationID string, resource *unikornv1.KubernetesCluster) (*identityapi.AllocationWrite, error) {
-	flavors, err := region.Flavors(ctx, c.region, organizationID, resource.Spec.RegionID)
+	flavors, err := c.region.Flavors(ctx, organizationID, resource.Spec.RegionID)
 	if err != nil {
 		return nil, err
 	}
@@ -280,7 +281,12 @@ func (c *Client) createAllocation(ctx context.Context, organizationID, projectID
 		return nil, err
 	}
 
-	resp, err := c.identity.PostApiV1OrganizationsOrganizationIDProjectsProjectIDAllocationsWithResponse(ctx, organizationID, projectID, *allocations)
+	client, err := c.identity.Client(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	resp, err := client.PostApiV1OrganizationsOrganizationIDProjectsProjectIDAllocationsWithResponse(ctx, organizationID, projectID, *allocations)
 	if err != nil {
 		return nil, err
 	}
@@ -298,7 +304,12 @@ func (c *Client) updateAllocation(ctx context.Context, organizationID, projectID
 		return err
 	}
 
-	resp, err := c.identity.PutApiV1OrganizationsOrganizationIDProjectsProjectIDAllocationsAllocationIDWithResponse(ctx, organizationID, projectID, resource.Annotations[constants.AllocationAnnotation], *allocations)
+	client, err := c.identity.Client(ctx)
+	if err != nil {
+		return err
+	}
+
+	resp, err := client.PutApiV1OrganizationsOrganizationIDProjectsProjectIDAllocationsAllocationIDWithResponse(ctx, organizationID, projectID, resource.Annotations[constants.AllocationAnnotation], *allocations)
 	if err != nil {
 		return err
 	}
@@ -311,7 +322,12 @@ func (c *Client) updateAllocation(ctx context.Context, organizationID, projectID
 }
 
 func (c *Client) deleteAllocation(ctx context.Context, organizationID, projectID, allocationID string) error {
-	resp, err := c.identity.DeleteApiV1OrganizationsOrganizationIDProjectsProjectIDAllocationsAllocationIDWithResponse(ctx, organizationID, projectID, allocationID)
+	client, err := c.identity.Client(ctx)
+	if err != nil {
+		return err
+	}
+
+	resp, err := client.DeleteApiV1OrganizationsOrganizationIDProjectsProjectIDAllocationsAllocationIDWithResponse(ctx, organizationID, projectID, allocationID)
 	if err != nil {
 		return err
 	}
@@ -342,7 +358,12 @@ func (c *Client) createIdentity(ctx context.Context, organizationID, projectID,
 		},
 	}
 
-	resp, err := c.region.PostApiV1OrganizationsOrganizationIDProjectsProjectIDIdentitiesWithResponse(ctx, organizationID, projectID, request)
+	client, err := c.region.Client(ctx)
+	if err != nil {
+		return nil, errors.OAuth2ServerError("unable to create region client").WithError(err)
+	}
+
+	resp, err := client.PostApiV1OrganizationsOrganizationIDProjectsProjectIDIdentitiesWithResponse(ctx, organizationID, projectID, request)
 	if err != nil {
 		return nil, errors.OAuth2ServerError("unable to create identity").WithError(err)
 	}
@@ -380,7 +401,12 @@ func (c *Client) createPhysicalNetworkOpenstack(ctx context.Context, organizatio
 		},
 	}
 
-	resp, err := c.region.PostApiV1OrganizationsOrganizationIDProjectsProjectIDIdentitiesIdentityIDNetworksWithResponse(ctx, organizationID, projectID, identity.Metadata.Id, request)
+	client, err := c.region.Client(ctx)
+	if err != nil {
+		return nil, errors.OAuth2ServerError("unable to create region client").WithError(err)
+	}
+
+	resp, err := client.PostApiV1OrganizationsOrganizationIDProjectsProjectIDIdentitiesIdentityIDNetworksWithResponse(ctx, organizationID, projectID, identity.Metadata.Id, request)
 	if err != nil {
 		return nil, errors.OAuth2ServerError("unable to physical network").WithError(err)
 	}
@@ -392,30 +418,6 @@ func (c *Client) createPhysicalNetworkOpenstack(ctx context.Context, organizatio
 	return resp.JSON201, nil
 }
 
-func (c *Client) getRegion(ctx context.Context, organizationID, regionID string) (*regionapi.RegionRead, error) {
-	// TODO: Need a straight get interface rather than a list.
-	resp, err := c.region.GetApiV1OrganizationsOrganizationIDRegionsWithResponse(ctx, organizationID)
-	if err != nil {
-		return nil, errors.OAuth2ServerError("unable to get region").WithError(err)
-	}
-
-	if resp.StatusCode() != http.StatusOK {
-		return nil, errors.OAuth2ServerError("unable to get region")
-	}
-
-	results := *resp.JSON200
-
-	index := slices.IndexFunc(results, func(region regionapi.RegionRead) bool {
-		return region.Metadata.Id == regionID
-	})
-
-	if index < 0 {
-		return nil, errors.OAuth2ServerError("unable to get region")
-	}
-
-	return &results[index], nil
-}
-
 func (c *Client) applyCloudSpecificConfiguration(ctx context.Context, organizationID, projectID, regionID string, allocation *identityapi.AllocationRead, identity *regionapi.IdentityRead, cluster *unikornv1.KubernetesCluster) error {
 	// Save the identity ID for later cleanup.
 	if cluster.Annotations == nil {
@@ -426,7 +428,7 @@ func (c *Client) applyCloudSpecificConfiguration(ctx context.Context, organizati
 	cluster.Annotations[constants.IdentityAnnotation] = identity.Metadata.Id
 
 	// Apply any region specific configuration based on feature flags.
-	region, err := c.getRegion(ctx, organizationID, regionID)
+	region, err := c.region.Get(ctx, organizationID, regionID)
 	if err != nil {
 		return err
 	}
@@ -576,10 +578,6 @@ func (c *Client) Update(ctx context.Context, organizationID, projectID, clusterI
 		return errors.OAuth2ServerError("failed to merge annotations").WithError(err)
 	}
 
-	if err := c.updateAllocation(ctx, organizationID, projectID, required); err != nil {
-		return errors.OAuth2ServerError("failed to update quota allocation").WithError(err)
-	}
-
 	// Preserve networking options as if they change it'll be fairly catastrophic.
 	required.Spec.Network = current.Spec.Network
 
@@ -590,6 +588,10 @@ func (c *Client) Update(ctx context.Context, organizationID, projectID, clusterI
 	updated.Annotations = required.Annotations
 	updated.Spec = required.Spec
 
+	if err := c.updateAllocation(ctx, organizationID, projectID, updated); err != nil {
+		return errors.OAuth2ServerError("failed to update quota allocation").WithError(err)
+	}
+
 	if err := c.client.Patch(ctx, updated, client.MergeFrom(current)); err != nil {
 		return errors.OAuth2ServerError("failed to patch cluster").WithError(err)
 	}

pkg/server/handler/cluster/conversion.go

@@ -60,7 +60,7 @@ type generator struct {
 	// options allows access to resource defaults.
 	options *Options
 	// region is a client to access regions.
-	region regionapi.ClientWithResponsesInterface
+	region *region.Client
 	// namespace the resource is provisioned in.
 	namespace string
 	// organizationID is the unique organization identifier.
@@ -74,7 +74,7 @@ type generator struct {
 	existing *unikornv1.KubernetesCluster
 }
 
-func newGenerator(client client.Client, options *Options, region regionapi.ClientWithResponsesInterface, namespace, organizationID, projectID string) *generator {
+func newGenerator(client client.Client, options *Options, region *region.Client, namespace, organizationID, projectID string) *generator {
 	return &generator{
 		client:         client,
 		options:        options,
@@ -241,7 +241,7 @@ func (g *generator) defaultApplicationBundle(ctx context.Context) (*unikornv1.Ku
 
 // defaultControlPlaneFlavor returns a default control plane flavor.
 func (g *generator) defaultControlPlaneFlavor(ctx context.Context, request *openapi.KubernetesClusterWrite) (*regionapi.Flavor, error) {
-	flavors, err := region.Flavors(ctx, g.region, g.organizationID, request.Spec.RegionId)
+	flavors, err := g.region.Flavors(ctx, g.organizationID, request.Spec.RegionId)
 	if err != nil {
 		return nil, errors.OAuth2ServerError("failed to list flavors").WithError(err)
 	}
@@ -278,20 +278,11 @@ func (g *generator) defaultControlPlaneFlavor(ctx context.Context, request *open
 // defaultImage returns a default image for either control planes or workload pools
 // based on the specified Kubernetes version.
 func (g *generator) defaultImage(ctx context.Context, request *openapi.KubernetesClusterWrite) (*regionapi.Image, error) {
-	images, err := region.Images(ctx, g.region, g.organizationID, request.Spec.RegionId)
+	images, err := g.region.Images(ctx, g.organizationID, request.Spec.RegionId)
 	if err != nil {
 		return nil, errors.OAuth2ServerError("failed to list images").WithError(err)
 	}
 
-	// Only get the version asked for.
-	images = slices.DeleteFunc(images, func(x regionapi.Image) bool {
-		return (*x.Spec.SoftwareVersions)["kubernetes"] != request.Spec.Version
-	})
-
-	if len(images) == 0 {
-		return nil, errors.OAuth2ServerError("unable to select an image")
-	}
-
 	return &images[0], nil
 }