Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Add MembersReferenceResolved condition for cp groups #824

Merged
merged 4 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions controller/konnect/ops/conditions.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ops

import (
sdkkonnectcomp "github.com/Kong/sdk-konnect-go/models/components"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/kong/gateway-operator/pkg/consts"
Expand Down Expand Up @@ -60,3 +61,67 @@ func _setKonnectEntityProgrammedConditon(
obj,
)
}

const (
// ControlPlaneGroupMembersReferenceResolvedConditionType sets the condition for control plane groups
// to show whether all of its members are programmed and attached to the group.
ControlPlaneGroupMembersReferenceResolvedConditionType = "MembersReferenceResolved"
// ControlPlaneGroupMembersReferenceResolvedReasonNoMembers indicates that there are no members specified in the control plane group.
ControlPlaneGroupMembersReferenceResolvedReasonNoMembers consts.ConditionReason = "NoMembers"
// ControlPlaneGroupMembersReferenceResolvedReasonResolved indicates that all members of the control plane group
// are created and attached to the group in Konnect.
ControlPlaneGroupMembersReferenceResolvedReasonResolved consts.ConditionReason = "Resolved"
// ControlPlaneGroupMembersReferenceResolvedReasonPartialNotResolved indicates that some members of the control plane group
// are not resolved (not found or not created in Konnect).
ControlPlaneGroupMembersReferenceResolvedReasonPartialNotResolved consts.ConditionReason = "SomeMemberNotResolved"
// ControlPlaneGroupMembersReferenceResolvedReasonFailedToSet indicates that error happened on setting control plane as
// member of the control plane.
ControlPlaneGroupMembersReferenceResolvedReasonFailedToSet consts.ConditionReason = "SetGroupMemberFailed"
)

// SetControlPlaneGroupMembersReferenceResolvedCondition sets MembersReferenceResolved condition of control plane to True.
func SetControlPlaneGroupMembersReferenceResolvedCondition(
cpGroup *konnectv1alpha1.KonnectGatewayControlPlane,
) {
_setControlPlaneGroupMembersReferenceResolvedCondition(
cpGroup,
metav1.ConditionTrue,
ControlPlaneGroupMembersReferenceResolvedReasonResolved,
"",
)
}

// SetControlPlaneGroupMembersReferenceResolvedConditionFalse sets MembersReferenceResolved condition of control plane to False.
func SetControlPlaneGroupMembersReferenceResolvedConditionFalse(
cpGroup *konnectv1alpha1.KonnectGatewayControlPlane,
reason consts.ConditionReason,
msg string,
) {
_setControlPlaneGroupMembersReferenceResolvedCondition(
cpGroup,
metav1.ConditionFalse,
reason,
msg,
)
}

func _setControlPlaneGroupMembersReferenceResolvedCondition(
cpGroup *konnectv1alpha1.KonnectGatewayControlPlane,
status metav1.ConditionStatus,
reason consts.ConditionReason,
msg string,
) {
if cpGroup.Spec.ClusterType == nil || *cpGroup.Spec.ClusterType != sdkkonnectcomp.CreateControlPlaneRequestClusterTypeClusterTypeControlPlaneGroup {
return
}
k8sutils.SetCondition(
k8sutils.NewConditionWithGeneration(
ControlPlaneGroupMembersReferenceResolvedConditionType,
status,
reason,
msg,
cpGroup.GetGeneration(),
),
cpGroup,
)
}
29 changes: 29 additions & 0 deletions controller/konnect/ops/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,32 @@ type KonnectEntityCreatedButRelationsFailedError struct {
func (e KonnectEntityCreatedButRelationsFailedError) Error() string {
return fmt.Sprintf("Konnect entity (ID: %s) created but relations failed: %s: %v", e.KonnectID, e.Reason, e.Err)
}

// GetControlPlaneGroupMemberFailedError is an error type returned when
// failed to get member of control plane group.
type GetControlPlaneGroupMemberFailedError struct {
MemberName string
Err error
}

// Error implements the error interface.
func (e GetControlPlaneGroupMemberFailedError) Error() string {
return fmt.Sprintf("failed to get control plane group member %s: %v", e.MemberName, e.Err.Error())
}

// Unwrap returns the underlying error.
func (e GetControlPlaneGroupMemberFailedError) Unwrap() error {
return e.Err
}

// ControlPlaneGroupMemberNoKonnectIDError is an error type returned when
// member of control plane group does not have a Konnect ID.
type ControlPlaneGroupMemberNoKonnectIDError struct {
GroupName string
MemberName string
}

// Error implements the error interface.
func (e ControlPlaneGroupMemberNoKonnectIDError) Error() string {
return fmt.Sprintf("control plane group %s member %s has no Konnect ID", e.GroupName, e.MemberName)
}
2 changes: 1 addition & 1 deletion controller/konnect/ops/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func Create[
var id string
switch ent := any(e).(type) {
case *konnectv1alpha1.KonnectGatewayControlPlane:
id, err = getControlPlaneForUID(ctx, sdk.GetControlPlaneSDK(), ent)
id, err = getControlPlaneForUID(ctx, sdk.GetControlPlaneSDK(), sdk.GetControlPlaneGroupSDK(), cl, ent)
case *configurationv1alpha1.KongService:
id, err = getKongServiceForUID(ctx, sdk.GetServicesSDK(), ent)
case *configurationv1alpha1.KongRoute:
Expand Down
55 changes: 50 additions & 5 deletions controller/konnect/ops/ops_controlplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,21 @@ func setGroupMembers(
id string,
sdkGroups sdkops.ControlPlaneGroupSDK,
) error {
if len(cp.Spec.Members) == 0 ||
cp.Spec.ClusterType == nil ||
if cp.Spec.ClusterType == nil ||
*cp.Spec.ClusterType != sdkkonnectcomp.CreateControlPlaneRequestClusterTypeClusterTypeControlPlaneGroup {
return nil
}

// Set MembersReferenceResolved condition to False if there are no members in the CP group.
if len(cp.Spec.Members) == 0 {
SetControlPlaneGroupMembersReferenceResolvedConditionFalse(
cp,
ControlPlaneGroupMembersReferenceResolvedReasonNoMembers,
"No members in the control plane group",
)
return nil
}

members, err := iter.MapErr(cp.Spec.Members,
func(member *corev1.LocalObjectReference) (sdkkonnectcomp.Members, error) {
var (
Expand All @@ -145,17 +154,28 @@ func setGroupMembers(
)
if err := cl.Get(ctx, nn, &memberCP); err != nil {
return sdkkonnectcomp.Members{},
fmt.Errorf("failed to get control plane group member %s: %w", member.Name, err)
GetControlPlaneGroupMemberFailedError{
MemberName: memberCP.Name,
Err: err,
}
}
if memberCP.GetKonnectID() == "" {
return sdkkonnectcomp.Members{},
fmt.Errorf("control plane group %s member %s has no Konnect ID", cp.Name, member.Name)
ControlPlaneGroupMemberNoKonnectIDError{
GroupName: cp.Name,
MemberName: memberCP.Name,
}
}
return sdkkonnectcomp.Members{
ID: lo.ToPtr(memberCP.GetKonnectID()),
}, nil
})
if err != nil {
SetControlPlaneGroupMembersReferenceResolvedConditionFalse(
cp,
ControlPlaneGroupMembersReferenceResolvedReasonPartialNotResolved,
err.Error(),
)
return fmt.Errorf("failed to set group members, some members couldn't be found: %w", err)
}

Expand All @@ -165,11 +185,19 @@ func setGroupMembers(
}
_, err = sdkGroups.PutControlPlanesIDGroupMemberships(ctx, id, &gm)
if err != nil {
SetControlPlaneGroupMembersReferenceResolvedConditionFalse(
cp,
ControlPlaneGroupMembersReferenceResolvedReasonFailedToSet,
err.Error(),
)
return fmt.Errorf("failed to set members on control plane group %s: %w",
client.ObjectKeyFromObject(cp), err,
)
}

SetControlPlaneGroupMembersReferenceResolvedCondition(
cp,
)
return nil
}

Expand All @@ -184,6 +212,8 @@ func (m membersByID) Swap(i, j int) { m[i], m[j] = m[j], m[i] }
func getControlPlaneForUID(
ctx context.Context,
sdk sdkops.ControlPlaneSDK,
sdkGroups sdkops.ControlPlaneGroupSDK,
cl client.Client,
cp *konnectv1alpha1.KonnectGatewayControlPlane,
) (string, error) {
reqList := sdkkonnectops.ListControlPlanesRequest{
Expand All @@ -202,5 +232,20 @@ func getControlPlaneForUID(
return "", fmt.Errorf("failed listing %s: %w", cp.GetTypeName(), ErrNilResponse)
}

return getMatchingEntryFromListResponseData(sliceToEntityWithIDSlice(resp.ListControlPlanesResponse.Data), cp)
id, err := getMatchingEntryFromListResponseData(sliceToEntityWithIDSlice(resp.ListControlPlanesResponse.Data), cp)
if err != nil {
return "", err
}

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 id, KonnectEntityCreatedButRelationsFailedError{
KonnectID: id,
Err: err,
Reason: konnectv1alpha1.KonnectGatewayControlPlaneProgrammedReasonFailedToSetControlPlaneGroupMembers,
}
}

return id, nil
randmonkey marked this conversation as resolved.
Show resolved Hide resolved
}
34 changes: 27 additions & 7 deletions controller/konnect/ops/ops_controlplane_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

sdkmocks "github.com/kong/gateway-operator/controller/konnect/ops/sdk/mocks"
"github.com/kong/gateway-operator/modules/manager/scheme"
"github.com/kong/gateway-operator/pkg/consts"

konnectv1alpha1 "github.com/kong/kubernetes-configuration/api/konnect/v1alpha1"
)
Expand Down Expand Up @@ -638,11 +639,13 @@ func TestCreateAndUpdateControlPlane_KubernetesMetadataConsistency(t *testing.T)

func TestSetGroupMembers(t *testing.T) {
testcases := []struct {
name string
group *konnectv1alpha1.KonnectGatewayControlPlane
cps []client.Object
sdk func(t *testing.T) *sdkmocks.MockControlPlaneGroupSDK
expectedErr bool
name string
group *konnectv1alpha1.KonnectGatewayControlPlane
cps []client.Object
sdk func(t *testing.T) *sdkmocks.MockControlPlaneGroupSDK
expectedErr bool
memberRefResolvedStatus metav1.ConditionStatus
memberRefResolvedReason consts.ConditionReason
}{
{
name: "no members",
Expand All @@ -662,6 +665,8 @@ func TestSetGroupMembers(t *testing.T) {
sdk := sdkmocks.NewMockControlPlaneGroupSDK(t)
return sdk
},
memberRefResolvedStatus: metav1.ConditionFalse,
memberRefResolvedReason: ControlPlaneGroupMembersReferenceResolvedReasonNoMembers,
},
{
name: "1 member with Konnect Status ID",
Expand Down Expand Up @@ -715,6 +720,8 @@ func TestSetGroupMembers(t *testing.T) {
)
return sdk
},
memberRefResolvedStatus: metav1.ConditionTrue,
memberRefResolvedReason: ControlPlaneGroupMembersReferenceResolvedReasonResolved,
},
{
name: "1 member without Konnect Status ID",
Expand Down Expand Up @@ -748,7 +755,9 @@ func TestSetGroupMembers(t *testing.T) {
sdk := sdkmocks.NewMockControlPlaneGroupSDK(t)
return sdk
},
expectedErr: true,
expectedErr: true,
memberRefResolvedStatus: metav1.ConditionFalse,
memberRefResolvedReason: ControlPlaneGroupMembersReferenceResolvedReasonPartialNotResolved,
},
{
name: "2 member with Konnect Status IDs",
Expand Down Expand Up @@ -819,6 +828,8 @@ func TestSetGroupMembers(t *testing.T) {
)
return sdk
},
memberRefResolvedStatus: metav1.ConditionTrue,
memberRefResolvedReason: ControlPlaneGroupMembersReferenceResolvedReasonResolved,
},
{
name: "2 member, 1 with Konnect Status IDs, 1 without it",
Expand Down Expand Up @@ -865,7 +876,9 @@ func TestSetGroupMembers(t *testing.T) {
sdk := sdkmocks.NewMockControlPlaneGroupSDK(t)
return sdk
},
expectedErr: true,
expectedErr: true,
memberRefResolvedStatus: metav1.ConditionFalse,
memberRefResolvedReason: ControlPlaneGroupMembersReferenceResolvedReasonPartialNotResolved,
},
}

Expand All @@ -885,6 +898,13 @@ func TestSetGroupMembers(t *testing.T) {
}
assert.NoError(t, err)
assert.True(t, sdk.AssertExpectations(t))

membersRefResolvedCondition, conditionFound := lo.Find(tc.group.Status.Conditions, func(c metav1.Condition) bool {
return c.Type == ControlPlaneGroupMembersReferenceResolvedConditionType
})
assert.True(t, conditionFound, "Should find MembersReferenceResolved condition")
assert.Equal(t, tc.memberRefResolvedStatus, membersRefResolvedCondition.Status, "Should have expected MembersReferenceResolved status")
assert.Equal(t, string(tc.memberRefResolvedReason), membersRefResolvedCondition.Reason, "Should have expected MembersReferenceResolved reason")
})
}
}
Loading
Loading