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.

Sign up for GitHub

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

refactor: refactor Gateway API's route parent status update code #6877

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: refactor Gateway API's route parent status update code #6877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
156 changes: 156 additions & 0 deletions internal/controllers/gateway/conditions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package gateway

import (
"context"

"github.com/samber/lo"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"

"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
)

func newCondition(
typ string,
status metav1.ConditionStatus,
reason string,
generation int64,
) metav1.Condition {
return metav1.Condition{
Type: typ,
Status: status,
Reason: reason,
ObservedGeneration: generation,
LastTransitionTime: metav1.Now(),
}
}

func newProgrammedConditionUnknown(obj interface{ GetGeneration() int64 }) metav1.Condition {
return newCondition(
ConditionTypeProgrammed,
metav1.ConditionUnknown,
string(ConditionReasonProgrammedUnknown),
obj.GetGeneration(),
)
}

// sameCondition returns true if the conditions in parameter has the same type, status, reason and message.
func sameCondition(a, b metav1.Condition) bool {
return a.Type == b.Type &&
a.Status == b.Status &&
a.Reason == b.Reason &&
a.Message == b.Message &&
a.ObservedGeneration == b.ObservedGeneration
}

func setRouteParentStatusCondition(parentStatus *gatewayapi.RouteParentStatus, newCondition metav1.Condition) bool {
var conditionFound, changed bool
for i, condition := range parentStatus.Conditions {
if condition.Type == newCondition.Type {
conditionFound = true
if !sameCondition(condition, newCondition) {
parentStatus.Conditions[i] = newCondition
changed = true
}
}
}

if !conditionFound {
parentStatus.Conditions = append(parentStatus.Conditions, newCondition)
changed = true
}
return changed
}

func parentStatusHasProgrammedCondition(parentStatus *gatewayapi.RouteParentStatus) bool {
for _, condition := range parentStatus.Conditions {
if condition.Type == ConditionTypeProgrammed {
return true
}
}
return false
}

// ensureParentsProgrammedCondition ensures that provided route's parent statuses
// have Programmed condition set properly. It returns a boolean flag indicating
// whether an update to the provided route has been performed.
//
// Use the condition argument to specify the Reason, Status and Message.
// Type will be set to Programmed whereas ObservedGeneration and LastTransitionTime
// will be set accordingly based on the route's generation and current time.
func ensureParentsProgrammedCondition[
routeT gatewayapi.RouteT,
](
ctx context.Context,
client client.SubResourceWriter,
route routeT,
routeParentStatuses []gatewayapi.RouteParentStatus,
gateways []supportedGatewayWithCondition,
condition metav1.Condition,
) (bool, error) {
// map the existing parentStatues to avoid duplications
parentStatuses := getParentStatuses(route, routeParentStatuses)

condition.Type = ConditionTypeProgrammed
condition.ObservedGeneration = route.GetGeneration()
condition.LastTransitionTime = metav1.Now()

statusChanged := false
for _, g := range gateways {
gateway := g.gateway

parentRefKey := routeParentStatusKey(route, g)
parentStatus, ok := parentStatuses[parentRefKey]
if ok {
// update existing parent in status.
changed := setRouteParentStatusCondition(parentStatus, condition)
if changed {
parentStatuses[parentRefKey] = parentStatus
setRouteParentInStatusForParent(route, *parentStatus, g)
}
statusChanged = statusChanged || changed
} else {
// add a new parent if the parent is not found in status.
newParentStatus := gatewayapi.RouteParentStatus{
ParentRef: gatewayapi.ParentReference{
Namespace: lo.ToPtr(gatewayapi.Namespace(gateway.Namespace)),
Name: gatewayapi.ObjectName(gateway.Name),
Kind: lo.ToPtr(gatewayapi.Kind("Gateway")),
Group: lo.ToPtr(gatewayapi.Group(gatewayv1.GroupName)),
SectionName: func() *gatewayapi.SectionName {
// We don't need to check whether the listener matches route's spec
// because that should already be done via getSupportedGatewayForRoute
// at this point.
if g.listenerName != "" {
return lo.ToPtr(gatewayapi.SectionName(g.listenerName))
}
return nil
}(),

// TODO: set port after gateway port matching implemented:
// https://github.com/Kong/kubernetes-ingress-controller/issues/3016
},
ControllerName: GetControllerName(),
Conditions: []metav1.Condition{
condition,
},
}
setRouteParentInStatusForParent(route, newParentStatus, g)

routeParentStatuses = append(routeParentStatuses, newParentStatus)
parentStatuses[parentRefKey] = &newParentStatus
statusChanged = true
}
}

// update status if needed.
if statusChanged {
if err := client.Update(ctx, route); err != nil {
return false, err
}
return true, nil
}
// no need to update if no status is changed.
return false, nil
}
75 changes: 6 additions & 69 deletions internal/controllers/gateway/grpcroute_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/samber/lo"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -27,7 +26,6 @@ import (

"github.com/kong/kubernetes-ingress-controller/v3/internal/controllers"
"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
k8sobj "github.com/kong/kubernetes-ingress-controller/v3/internal/util/kubernetes/object"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util/kubernetes/object/status"
)
Expand Down Expand Up @@ -424,78 +422,17 @@ func (r *GRPCRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
// GRPCRouteReconciler - Status Helpers
// -----------------------------------------------------------------------------

// grpcrouteParentKind indicates the only object KIND that this GRPCRoute
// implementation supports for route object parent references.
var grpcrouteParentKind = "Gateway"

// ensureGatewayReferenceStatus takes any number of Gateways that should be
// considered "attached" to a given GRPCRoute and ensures that the status
// for the GRPCRoute is updated appropriately.
func (r *GRPCRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Context, grpcroute *gatewayapi.GRPCRoute, gateways ...supportedGatewayWithCondition) (bool, error) {
// map the existing parentStatues to avoid duplications
parentStatuses := getParentStatuses(grpcroute, grpcroute.Status.Parents)

// overlay the parent ref statuses for all new gateway references
statusChangesWereMade := false
for _, gateway := range gateways {
// build a new status for the parent Gateway
gatewayParentStatus := &gatewayapi.RouteParentStatus{
ParentRef: gatewayapi.ParentReference{
Group: (*gatewayapi.Group)(&gatewayv1.GroupVersion.Group),
Kind: util.StringToGatewayAPIKindPtr(grpcrouteParentKind),
Namespace: (*gatewayapi.Namespace)(&gateway.gateway.Namespace),
Name: gatewayapi.ObjectName(gateway.gateway.Name),
},
ControllerName: GetControllerName(),
Conditions: []metav1.Condition{{
Type: string(gatewayapi.RouteConditionAccepted),
Status: metav1.ConditionTrue,
ObservedGeneration: grpcroute.Generation,
LastTransitionTime: metav1.Now(),
Reason: string(gatewayapi.RouteReasonAccepted),
}},
}

if gateway.listenerName != "" {
sectionName := gatewayapi.SectionName(gateway.listenerName)
gatewayParentStatus.ParentRef.SectionName = &sectionName
}

// if the reference already exists and doesn't require any changes
// then just leave it alone.
parentRefKey := fmt.Sprintf("%s/%s/%s", gateway.gateway.Namespace, gateway.gateway.Name, gateway.listenerName)
if existingGatewayParentStatus, exists := parentStatuses[parentRefKey]; exists {
// check if the parentRef and controllerName are equal, and whether the new condition is present in existing conditions
if reflect.DeepEqual(existingGatewayParentStatus.ParentRef, gatewayParentStatus.ParentRef) &&
existingGatewayParentStatus.ControllerName == gatewayParentStatus.ControllerName &&
lo.ContainsBy(existingGatewayParentStatus.Conditions, func(condition metav1.Condition) bool {
return sameCondition(gatewayParentStatus.Conditions[0], condition)
}) {
continue
}
}

// otherwise overlay the new status on top the list of parentStatuses
parentStatuses[parentRefKey] = gatewayParentStatus
statusChangesWereMade = true
}
parentStatuses, statusChangesWereMade := parentStatusesForRoute(
grpcroute,
grpcroute.Status.Parents,
gateways...,
)

// initialize "programmed" condition to Unknown.
// do not update the condition If a "Programmed" condition is already present.
programmedConditionChanged := false
programmedConditionUnknown := metav1.Condition{
Type: ConditionTypeProgrammed,
Status: metav1.ConditionUnknown,
Reason: string(ConditionReasonProgrammedUnknown),
ObservedGeneration: grpcroute.Generation,
LastTransitionTime: metav1.Now(),
}
for _, parentStatus := range parentStatuses {
if !parentStatusHasProgrammedCondition(parentStatus) {
programmedConditionChanged = true
parentStatus.Conditions = append(parentStatus.Conditions, programmedConditionUnknown)
}
}
programmedConditionChanged := initializeParentStatusesWithProgrammedCondition(grpcroute, parentStatuses)

// if we didn't have to actually make any changes, no status update is needed
if !statusChangesWereMade && !programmedConditionChanged {
Expand Down
73 changes: 6 additions & 67 deletions internal/controllers/gateway/httproute_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/samber/lo"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -518,10 +517,6 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
// HTTPRouteReconciler - Status Helpers
// -----------------------------------------------------------------------------

// httprouteParentKind indicates the only object KIND that this HTTPRoute
// implementation supports for route object parent references.
var httprouteParentKind = "Gateway"

// ensureGatewayReferenceStatus takes any number of Gateways that should be
// considered "attached" to a given HTTPRoute and ensures that the status
// for the HTTPRoute is updated appropriately.
Expand All @@ -530,74 +525,18 @@ var httprouteParentKind = "Gateway"
func (r *HTTPRouteReconciler) ensureGatewayReferenceStatusAdded(
ctx context.Context, httproute *gatewayapi.HTTPRoute, gateways ...supportedGatewayWithCondition,
) (bool, ctrl.Result, error) {
// map the existing parentStatues to avoid duplications
parentStatuses := getParentStatuses(httproute, httproute.Status.Parents)

// overlay the parent ref statuses for all new gateway references
statusChangesWereMade := false
for _, gateway := range gateways {
// build a new status for the parent Gateway
gatewayParentStatus := &gatewayapi.RouteParentStatus{
ParentRef: gatewayapi.ParentReference{
Group: (*gatewayapi.Group)(&gatewayv1.GroupVersion.Group),
Kind: util.StringToGatewayAPIKindPtr(httprouteParentKind),
Namespace: (*gatewayapi.Namespace)(&gateway.gateway.Namespace),
Name: gatewayapi.ObjectName(gateway.gateway.Name),
},
ControllerName: GetControllerName(),
Conditions: []metav1.Condition{{
Type: gateway.condition.Type,
Status: gateway.condition.Status,
ObservedGeneration: httproute.Generation,
LastTransitionTime: metav1.Now(),
Reason: gateway.condition.Reason,
}},
}
if gateway.listenerName != "" {
gatewayParentStatus.ParentRef.SectionName = lo.ToPtr(gatewayapi.SectionName(gateway.listenerName))
}

key := fmt.Sprintf("%s/%s/%s", gateway.gateway.Namespace, gateway.gateway.Name, gateway.listenerName)

// if the reference already exists and doesn't require any changes
// then just leave it alone.
if existingGatewayParentStatus, exists := parentStatuses[key]; exists {
// check if the parentRef and controllerName are equal, and whether the new condition is present in existing conditions
if reflect.DeepEqual(existingGatewayParentStatus.ParentRef, gatewayParentStatus.ParentRef) &&
existingGatewayParentStatus.ControllerName == gatewayParentStatus.ControllerName &&
lo.ContainsBy(existingGatewayParentStatus.Conditions, func(condition metav1.Condition) bool {
return sameCondition(gatewayParentStatus.Conditions[0], condition)
}) {
continue
}
}

// otherwise overlay the new status on top the list of parentStatuses
parentStatuses[key] = gatewayParentStatus
statusChangesWereMade = true
}
parentStatuses, statusChangesWereMade := parentStatusesForRoute(
httproute,
httproute.Status.Parents,
gateways...,
)

parentStatuses, resolvedRefsChanged, err := r.setRouteConditionResolvedRefsCondition(ctx, httproute, parentStatuses)
if err != nil {
return false, ctrl.Result{}, err
}

// initialize "programmed" condition to Unknown.
// do not update the condition If a "Programmed" condition is already present.
programmedConditionChanged := false
programmedConditionUnknown := metav1.Condition{
Type: ConditionTypeProgrammed,
Status: metav1.ConditionUnknown,
Reason: string(ConditionReasonProgrammedUnknown),
ObservedGeneration: httproute.Generation,
LastTransitionTime: metav1.Now(),
}
for _, parentStatus := range parentStatuses {
if !parentStatusHasProgrammedCondition(parentStatus) {
programmedConditionChanged = true
parentStatus.Conditions = append(parentStatus.Conditions, programmedConditionUnknown)
}
}
programmedConditionChanged := initializeParentStatusesWithProgrammedCondition(httproute, parentStatuses)

// if we didn't have to actually make any changes, no status update is needed
if !statusChangesWereMade && !resolvedRefsChanged && !programmedConditionChanged {
Expand Down
Loading
Loading