Skip to content

Commit

Permalink
fix(konnect): handle KongRoute conflict on creation
Browse files Browse the repository at this point in the history
  • Loading branch information
pmalek committed Oct 17, 2024
1 parent 8e6269f commit c87d1dd
Show file tree
Hide file tree
Showing 12 changed files with 432 additions and 141 deletions.
1 change: 1 addition & 0 deletions controller/konnect/ops/kongroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ type RoutesSDK interface {
CreateRoute(ctx context.Context, controlPlaneID string, route sdkkonnectcomp.RouteInput, opts ...sdkkonnectops.Option) (*sdkkonnectops.CreateRouteResponse, error)
UpsertRoute(ctx context.Context, req sdkkonnectops.UpsertRouteRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.UpsertRouteResponse, error)
DeleteRoute(ctx context.Context, controlPlaneID, routeID string, opts ...sdkkonnectops.Option) (*sdkkonnectops.DeleteRouteResponse, error)
ListRoute(ctx context.Context, request sdkkonnectops.ListRouteRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.ListRouteResponse, error)
}
74 changes: 74 additions & 0 deletions controller/konnect/ops/kongroute_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

56 changes: 51 additions & 5 deletions controller/konnect/ops/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ func Create[
err = createControlPlane(ctx, sdk.GetControlPlaneSDK(), sdk.GetControlPlaneGroupSDK(), cl, ent)
case *configurationv1alpha1.KongService:
err = createService(ctx, sdk.GetServicesSDK(), ent)

// TODO: modify the create* operation wrappers to not set Programmed conditions and return
// a KonnectEntityCreatedButRelationsFailedError if the entity was created but its relations assignment failed.

case *configurationv1alpha1.KongRoute:
err = createRoute(ctx, sdk.GetRoutesSDK(), ent)

Expand All @@ -72,6 +68,10 @@ func Create[

case *configurationv1.KongConsumer:
err = createConsumer(ctx, sdk.GetConsumersSDK(), sdk.GetConsumerGroupsSDK(), cl, ent)

// TODO: modify the create* operation wrappers to not set Programmed conditions and return
// a KonnectEntityCreatedButRelationsFailedError if the entity was created but its relations assignment failed.

case *configurationv1beta1.KongConsumerGroup:
err = createConsumerGroup(ctx, sdk.GetConsumerGroupsSDK(), ent)
case *configurationv1alpha1.KongPluginBinding:
Expand Down Expand Up @@ -126,7 +126,7 @@ func Create[

switch {
case errors.As(err, &errConflict) ||
// Service API returns 400 for conflicts
// ControlPlane resource APIs return 400 for conflicts
errSdkBody.Code == 3 && errSdkBody.Message == "data constraint error":
// If there was a conflict on the create request, we can assume the entity already exists.
// We'll get its Konnect ID by listing all entities of its type filtered by the Kubernetes object UID.
Expand All @@ -136,6 +136,8 @@ func Create[
id, err = getControlPlaneForUID(ctx, sdk.GetControlPlaneSDK(), ent)
case *configurationv1alpha1.KongService:
id, err = getKongServiceForUID(ctx, sdk.GetServicesSDK(), ent)
case *configurationv1alpha1.KongRoute:
id, err = getKongRouteForUID(ctx, sdk.GetRoutesSDK(), ent)
case *configurationv1alpha1.KongSNI:
id, err = getSNIForUID(ctx, sdk.GetSNIsSDK(), ent)
// ---------------------------------------------------------------------
Expand Down Expand Up @@ -419,3 +421,47 @@ func logEntityNotFoundRecreating[
"id", id,
)
}

type entityWithID interface {
GetID() *string
}

func fromSliceToSlice[T any](
slice []T,
) []entityWithID {
result := make([]entityWithID, 0, len(slice))
for _, item := range slice {
converted, ok := any(&item).(entityWithID)
if !ok {
panic(fmt.Sprintf("failed to convert %T to entityWithID", item))
}
result = append(result, converted)
}
return result
}

// getMatchingEntryFromListResponseData returns the ID of the first entry in the list response data.
// It returns an error if no entry with a non-empty ID was found.
// It is used in conjunction with the list operation to get the ID of the entity that matches the UID
// hence no filtering is done here because it is assumed that the provided list response data is already filtered.
func getMatchingEntryFromListResponseData(
data []entityWithID,
entity entity,
) (string, error) {
var id string
for _, entry := range data {
entryID := entry.GetID()
if entryID != nil && *entryID != "" {
id = *entryID
break
}
}

if id == "" {
return "", EntityWithMatchingUIDNotFoundError{
Entity: entity,
}
}

return id, nil
}
66 changes: 26 additions & 40 deletions controller/konnect/ops/ops_controlplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,46 +19,6 @@ import (
konnectv1alpha1 "github.com/kong/kubernetes-configuration/api/konnect/v1alpha1"
)

// getControlPlaneForUID returns the Konnect ID of the Konnect ControlPlane
// that matches the UID of the provided KonnectGatewayControlPlane.
func getControlPlaneForUID(
ctx context.Context,
sdk ControlPlaneSDK,
cp *konnectv1alpha1.KonnectGatewayControlPlane,
) (string, error) {
reqList := sdkkonnectops.ListControlPlanesRequest{
// NOTE: only filter on object's UID.
// Other fields like name might have changed in the meantime but that's OK.
// Those will be enforced via subsequent updates.
Labels: lo.ToPtr(UIDLabelForObject(cp)),
}

respList, err := sdk.ListControlPlanes(ctx, reqList)
if err != nil {
return "", err
}

if respList == nil || respList.ListControlPlanesResponse == nil {
return "", fmt.Errorf("failed listing ControlPlanes: %w", ErrNilResponse)
}

var id string
for _, entry := range respList.ListControlPlanesResponse.Data {
if entry.ID != nil && *entry.ID != "" {
id = *entry.ID
break
}
}

if id == "" {
return "", EntityWithMatchingUIDNotFoundError{
Entity: cp,
}
}

return id, nil
}

// createControlPlane creates the ControlPlane as specified in provided ControlPlane's
// spec. Besides creating the ControlPlane, it also creates the group membership if the
// ControlPlane is a group. If the group membership creation fails, KonnectEntityCreatedButRelationsFailedError
Expand Down Expand Up @@ -254,3 +214,29 @@ type membersByID []sdkkonnectcomp.Members
func (m membersByID) Len() int { return len(m) }
func (m membersByID) Less(i, j int) bool { return *m[i].ID < *m[j].ID }
func (m membersByID) Swap(i, j int) { m[i], m[j] = m[j], m[i] }

// getControlPlaneForUID returns the Konnect ID of the Konnect ControlPlane
// that matches the UID of the provided KonnectGatewayControlPlane.
func getControlPlaneForUID(
ctx context.Context,
sdk ControlPlaneSDK,
cp *konnectv1alpha1.KonnectGatewayControlPlane,
) (string, error) {
reqList := sdkkonnectops.ListControlPlanesRequest{
// NOTE: only filter on object's UID.
// Other fields like name might have changed in the meantime but that's OK.
// Those will be enforced via subsequent updates.
Labels: lo.ToPtr(UIDLabelForObject(cp)),
}

respList, err := sdk.ListControlPlanes(ctx, reqList)
if err != nil {
return "", err
}

if respList == nil || respList.ListControlPlanesResponse == nil {
return "", fmt.Errorf("failed listing %s: %w", cp.GetTypeName(), ErrNilResponse)
}

return getMatchingEntryFromListResponseData(fromSliceToSlice(respList.ListControlPlanesResponse.Data), cp)
}
13 changes: 7 additions & 6 deletions controller/konnect/ops/ops_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,21 @@ import (
"errors"
"fmt"

"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// ErrNilResponse is an error indicating that a Konnect operation returned an empty response.
// This can sometimes happen regardless of the err being nil.
var ErrNilResponse = errors.New("nil response received")

type entity interface {
client.Object
GetTypeName() string
}

// EntityWithMatchingUIDNotFoundError is an error indicating that an entity with a matching UID was not found on Konnect.
type EntityWithMatchingUIDNotFoundError struct {
Entity interface {
GetTypeName() string
GetName() string
GetUID() types.UID
}
Entity entity
}

// Error implements the error interface.
Expand Down
72 changes: 61 additions & 11 deletions controller/konnect/ops/ops_kongroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
sdkkonnectcomp "github.com/Kong/sdk-konnect-go/models/components"
sdkkonnectops "github.com/Kong/sdk-konnect-go/models/operations"
sdkkonnecterrs "github.com/Kong/sdk-konnect-go/models/sdkerrors"
"github.com/samber/lo"
"sigs.k8s.io/controller-runtime/pkg/client"
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"

Expand All @@ -25,16 +26,18 @@ func createRoute(
}

resp, err := sdk.CreateRoute(ctx, route.Status.Konnect.ControlPlaneID, kongRouteToSDKRouteInput(route))
// TODO: handle already exists
// Can't adopt it as it will cause conflicts between the controller
// that created that entity and already manages it, hm
// that created that entity and already manages it.
// TODO: implement entity adoption https://github.com/Kong/gateway-operator/issues/460
if errWrap := wrapErrIfKonnectOpFailed(err, CreateOp, route); errWrap != nil {
SetKonnectEntityProgrammedConditionFalse(route, "FailedToCreate", errWrap.Error())
return errWrap
}

route.Status.Konnect.SetKonnectID(*resp.Route.ID)
SetKonnectEntityProgrammedCondition(route)
if resp == nil || resp.Route == nil || resp.Route.ID == nil {
return fmt.Errorf("failed creating %s: %w", route.GetTypeName(), ErrNilResponse)
}

route.SetKonnectID(*resp.Route.ID)

return nil
}
Expand All @@ -53,22 +56,42 @@ func updateRoute(
return fmt.Errorf("can't update %T %s without a Konnect ControlPlane ID", route, client.ObjectKeyFromObject(route))
}

id := route.GetKonnectStatus().GetKonnectID()
_, err := sdk.UpsertRoute(ctx, sdkkonnectops.UpsertRouteRequest{
ControlPlaneID: cpID,
RouteID: route.Status.Konnect.ID,
RouteID: id,
Route: kongRouteToSDKRouteInput(route),
})

// TODO: handle already exists
// Can't adopt it as it will cause conflicts between the controller
// that created that entity and already manages it, hm
// that created that entity and already manages it.
// TODO: implement entity adoption https://github.com/Kong/gateway-operator/issues/460
if errWrap := wrapErrIfKonnectOpFailed(err, UpdateOp, route); errWrap != nil {
SetKonnectEntityProgrammedConditionFalse(route, "FailedToUpdate", errWrap.Error())
// Route update operation returns an SDKError instead of a NotFoundError.
var sdkError *sdkkonnecterrs.SDKError
if errors.As(errWrap, &sdkError) {
switch sdkError.StatusCode {
case 404:
logEntityNotFoundRecreating(ctx, route, id)
if err := createRoute(ctx, sdk, route); err != nil {
return FailedKonnectOpError[configurationv1alpha1.KongRoute]{
Op: UpdateOp,
Err: err,
}
}
// Create succeeded, createService sets the status so no need to do this here.
return nil
default:
return FailedKonnectOpError[configurationv1alpha1.KongRoute]{
Op: UpdateOp,
Err: sdkError,
}
}
}

return errWrap
}

SetKonnectEntityProgrammedCondition(route)

return nil
}

Expand Down Expand Up @@ -136,3 +159,30 @@ func kongRouteToSDKRouteInput(
}
return r
}

// getKongRouteForUID returns the Konnect ID of the KongRoute
// that matches the UID of the provided KongRoute.
func getKongRouteForUID(
ctx context.Context,
sdk RoutesSDK,
r *configurationv1alpha1.KongRoute,
) (string, error) {
reqList := sdkkonnectops.ListRouteRequest{
// NOTE: only filter on object's UID.
// Other fields like name might have changed in the meantime but that's OK.
// Those will be enforced via subsequent updates.
Tags: lo.ToPtr(UIDLabelForObject(r)),
ControlPlaneID: r.GetControlPlaneID(),
}

respList, err := sdk.ListRoute(ctx, reqList)
if err != nil {
return "", err
}

if respList == nil || respList.Object == nil {
return "", fmt.Errorf("failed listing %s: %w", r.GetTypeName(), ErrNilResponse)
}

return getMatchingEntryFromListResponseData(fromSliceToSlice(respList.Object.Data), r)
}
Loading

0 comments on commit c87d1dd

Please sign in to comment.