Skip to content

Commit

Permalink
Refactor Status Updater to be resource agnostic (nginxinc#1814)
Browse files Browse the repository at this point in the history
Problem:
Currently, the status updater is aware of what resource it is updating
the status of. This makes it difficult to extend because for each new
resource we add support for, we have to modify the status updater.

Solution:

Replace old status updater with two new ones

(1) Regular Updater, to update statuses of multiple resources via
UpdateRequest type.

(2) LeaderAwareGroupUpdater to update statuses of groups of resources
and save any pending UpdateRequests until the updater is enabled
(when pod becomes leader). It uses Regular Updater.

Using groups allow replacing UpdateRequests of subset of resources,
without the need to recompute UpdateRequests for all resources.

The new status package is
resource agnostic. This is accomplished by representing status update
as a function (Setter).

Other updates:
- provisioner manager uses regular Updater.
- static manager uses LeaderAwareGroupUpdater (because it needs to
  support leader election)
- framework Status setter were simplified and moved to static/status
  package
- status related functions in static package were moved to
  static/status package.
- Previous Build* functions were replaced with PrepareRequests
  functions. Such functions don't create any intermediate status
  representation (like it was before), but create status UpdateRequests
  which directly set the resource statuses.
- static manager handler conditionally updates statuses GatewayClass
  based on manager configuration. Previously, the conditional logic was
  implemented in the Status Updater.

CLOSES -- nginxinc#1071
  • Loading branch information
pleshakov authored Apr 16, 2024
1 parent 3d180b2 commit 2277050
Show file tree
Hide file tree
Showing 36 changed files with 3,035 additions and 3,268 deletions.
22 changes: 22 additions & 0 deletions internal/framework/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,25 @@ func NewGatewayClassConflict() Condition {
Message: GatewayClassMessageGatewayClassConflict,
}
}

// ConvertConditions converts conditions to Kubernetes API conditions.
func ConvertConditions(
conds []Condition,
observedGeneration int64,
transitionTime metav1.Time,
) []metav1.Condition {
apiConds := make([]metav1.Condition, len(conds))

for i := range conds {
apiConds[i] = metav1.Condition{
Type: conds[i].Type,
Status: conds[i].Status,
ObservedGeneration: observedGeneration,
LastTransitionTime: transitionTime,
Reason: conds[i].Reason,
Message: conds[i].Message,
}
}

return apiConds
}
48 changes: 46 additions & 2 deletions internal/framework/conditions/conditions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
)

func TestDeduplicateConditions(t *testing.T) {
g := NewWithT(t)

conds := []Condition{
{
Type: "Type1",
Expand Down Expand Up @@ -56,6 +54,52 @@ func TestDeduplicateConditions(t *testing.T) {
},
}

g := NewWithT(t)

result := DeduplicateConditions(conds)
g.Expect(result).Should(Equal(expected))
}

func TestConvertConditions(t *testing.T) {
conds := []Condition{
{
Type: "Type1",
Status: metav1.ConditionTrue,
Reason: "Reason1",
Message: "Message1",
},
{
Type: "Type2",
Status: metav1.ConditionFalse,
Reason: "Reason2",
Message: "Message2",
},
}

const generation = 3
time := metav1.Now()

expected := []metav1.Condition{
{
Type: "Type1",
Status: metav1.ConditionTrue,
Reason: "Reason1",
Message: "Message1",
LastTransitionTime: time,
ObservedGeneration: generation,
},
{
Type: "Type2",
Status: metav1.ConditionFalse,
Reason: "Reason2",
Message: "Message2",
LastTransitionTime: time,
ObservedGeneration: generation,
},
}

g := NewWithT(t)

result := ConvertConditions(conds, generation, time)
g.Expect(result).Should(Equal(expected))
}
12 changes: 11 additions & 1 deletion internal/framework/helpers/helpers.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// Package helpers contains helper functions for unit tests.
// Package helpers contains helper functions
package helpers

import (
"fmt"

"github.com/google/go-cmp/cmp"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// Diff prints the diff between two structs.
Expand Down Expand Up @@ -41,3 +42,12 @@ func PrepareTimeForFakeClient(t metav1.Time) metav1.Time {

return t
}

// MustCastObject casts the client.Object to the specified type that implements it.
func MustCastObject[T client.Object](object client.Object) T {
if obj, ok := object.(T); ok {
return obj
}

panic(fmt.Errorf("unexpected object type %T", object))
}
25 changes: 25 additions & 0 deletions internal/framework/helpers/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package helpers

import (
"testing"

. "github.com/onsi/gomega"

"sigs.k8s.io/controller-runtime/pkg/client"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
)

func TestMustCastObject(t *testing.T) {
g := NewWithT(t)

var obj client.Object = &gatewayv1.Gateway{}

g.Expect(func() {
_ = MustCastObject[*gatewayv1.Gateway](obj)
}).ToNot(Panic())

g.Expect(func() {
_ = MustCastObject[*gatewayv1alpha2.BackendTLSPolicy](obj)
}).To(Panic())
}
45 changes: 0 additions & 45 deletions internal/framework/status/backend_tls.go

This file was deleted.

51 changes: 0 additions & 51 deletions internal/framework/status/backend_tls_test.go

This file was deleted.

25 changes: 0 additions & 25 deletions internal/framework/status/clock.go

This file was deleted.

39 changes: 21 additions & 18 deletions internal/framework/status/conditions.go
Original file line number Diff line number Diff line change
@@ -1,28 +1,31 @@
package status

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"slices"

"github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func convertConditions(
conds []conditions.Condition,
observedGeneration int64,
transitionTime metav1.Time,
) []metav1.Condition {
apiConds := make([]metav1.Condition, len(conds))
// ConditionsEqual compares conditions.
// It doesn't check the last transition time of conditions.
func ConditionsEqual(prev, cur []metav1.Condition) bool {
return slices.EqualFunc(prev, cur, func(c1, c2 metav1.Condition) bool {
if c1.ObservedGeneration != c2.ObservedGeneration {
return false
}

if c1.Type != c2.Type {
return false
}

if c1.Status != c2.Status {
return false
}

for i := range conds {
apiConds[i] = metav1.Condition{
Type: conds[i].Type,
Status: conds[i].Status,
ObservedGeneration: observedGeneration,
LastTransitionTime: transitionTime,
Reason: conds[i].Reason,
Message: conds[i].Message,
if c1.Message != c2.Message {
return false
}
}

return apiConds
return c1.Reason == c2.Reason
})
}
Loading

0 comments on commit 2277050

Please sign in to comment.