adjust comments

Kong · Oct 17, 2024 · 208fa3b · 208fa3b
1 parent 48e31f0
commit 208fa3b
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 17 deletions.
diff --git a/controller/konnect/ops/ops.go b/controller/konnect/ops/ops.go
@@ -43,8 +43,8 @@ const (
 
 // Create creates a Konnect entity.
 func Create[
-	T constraints.SupportedKonnectEntityType,
-	TEnt constraints.EntityType[T],
+T constraints.SupportedKonnectEntityType,
+TEnt constraints.EntityType[T],
 ](
 	ctx context.Context,
 	sdk SDKWrapper,
@@ -148,8 +148,8 @@ func Create[
 // Delete deletes a Konnect entity.
 // It returns an error if the entity does not have a Konnect ID or if the operation fails.
 func Delete[
-	T constraints.SupportedKonnectEntityType,
-	TEnt constraints.EntityType[T],
+T constraints.SupportedKonnectEntityType,
+TEnt constraints.EntityType[T],
 ](ctx context.Context, sdk SDKWrapper, cl client.Client, e *T) error {
 	ent := TEnt(e)
 	if ent.GetKonnectStatus().GetKonnectID() == "" {
@@ -216,8 +216,8 @@ func Delete[
 }
 
 func shouldUpdate[
-	T constraints.SupportedKonnectEntityType,
-	TEnt constraints.EntityType[T],
+T constraints.SupportedKonnectEntityType,
+TEnt constraints.EntityType[T],
 ](
 	ctx context.Context,
 	ent TEnt,
@@ -255,8 +255,8 @@ func shouldUpdate[
 // Update updates a Konnect entity.
 // It returns an error if the entity does not have a Konnect ID or if the operation fails.
 func Update[
-	T constraints.SupportedKonnectEntityType,
-	TEnt constraints.EntityType[T],
+T constraints.SupportedKonnectEntityType,
+TEnt constraints.EntityType[T],
 ](
 	ctx context.Context,
 	sdk SDKWrapper,
@@ -284,7 +284,8 @@ func Update[
 	case *konnectv1alpha1.KonnectGatewayControlPlane:
 		err = updateControlPlane(ctx, sdk.GetControlPlaneSDK(), sdk.GetControlPlaneGroupSDK(), cl, ent)
 	case *configurationv1alpha1.KongService:
-		// TODO: modify the update* operation wrappers to return Konnect ID and error reason.
+		// 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.
 		err = updateService(ctx, sdk.GetServicesSDK(), ent)
 	case *configurationv1alpha1.KongRoute:
 		err = updateRoute(ctx, sdk.GetRoutesSDK(), ent)
@@ -346,8 +347,8 @@ func Update[
 }
 
 func logOpComplete[
-	T constraints.SupportedKonnectEntityType,
-	TEnt constraints.EntityType[T],
+T constraints.SupportedKonnectEntityType,
+TEnt constraints.EntityType[T],
 ](ctx context.Context, start time.Time, op Op, e TEnt, err error) {
 	keysAndValues := []interface{}{
 		"op", op,
@@ -373,8 +374,8 @@ func logOpComplete[
 // wrapErrIfKonnectOpFailed checks the response from the Konnect API and returns a uniform
 // error for all Konnect entities if the operation failed.
 func wrapErrIfKonnectOpFailed[
-	T constraints.SupportedKonnectEntityType,
-	TEnt constraints.EntityType[T],
+T constraints.SupportedKonnectEntityType,
+TEnt constraints.EntityType[T],
 ](err error, op Op, e TEnt) error {
 	if err != nil {
 		entityTypeName := constraints.EntityTypeName[T]()

diff --git a/controller/konnect/ops/ops_controlplane.go b/controller/konnect/ops/ops_controlplane.go
@@ -58,8 +58,9 @@ func getControlPlaneForUID(
 }
 
 // createControlPlane creates the ControlPlane as specified in provided ControlPlane's
-// spec. It returns the ID of created entity, a reason and an error.
-// The reason is provided if the err is not nil to indicate the failure reason.
+// spec. Besides creating the ControlPlane, it also creates the group membership if the
+// ControlPlane is a group. If the group membership creation fails, KonnectEntityCreatedButRelationsFailedError
+// is returned so it can be handled properly downstream.
 func createControlPlane(
 	ctx context.Context,
 	sdk ControlPlaneSDK,
@@ -136,8 +137,9 @@ func deleteControlPlane(
 
 // updateControlPlane updates a Konnect ControlPlane.
 // It is assumed that the Konnect ControlPlane has a Konnect ID.
-// When it succeeds it returns the ID of the updated entity.
-// It returns an error if the operation fails and a reason why the operation failed.
+// Besides updating the ControlPlane, it also updates the group membership if the ControlPlane is a group.
+// If the group membership update fails, KonnectEntityCreatedButRelationsFailedError is returned so it can
+// be handled properly downstream.
 func updateControlPlane(
 	ctx context.Context,
 	sdk ControlPlaneSDK,
@@ -184,6 +186,8 @@ func updateControlPlane(
 	id = *resp.ControlPlane.ID
 
 	if err := setGroupMembers(ctx, cl, cp, id, sdkGroups); err != nil {
+		// If we failed to set group membership, we should return a specific error with a reason
+		// so the downstream can handle it properly.
 		return KonnectEntityCreatedButRelationsFailedError{
 			KonnectID: id,
 			Err:       err,