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

Refacter code to use new ObjectType #2121

Merged
merged 3 commits into from
Jun 12, 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
5 changes: 3 additions & 2 deletions internal/framework/controller/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/nginxinc/nginx-gateway-fabric/internal/framework/events"
ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
)

// NamespacedNameFilterFunc is a function that returns true if the resource should be processed by the reconciler.
Expand All @@ -24,7 +25,7 @@ type ReconcilerConfig struct {
// Getter gets a resource from the k8s API.
Getter Getter
// ObjectType is the type of the resource that the reconciler will reconcile.
ObjectType client.Object
ObjectType ngftypes.ObjectType
// EventCh is the channel where the reconciler will send events.
EventCh chan<- interface{}
// NamespacedNameFilter filters resources the controller will process. Can be nil.
Expand Down Expand Up @@ -52,7 +53,7 @@ func NewReconciler(cfg ReconcilerConfig) *Reconciler {
}
}

func (r *Reconciler) newObject(objectType client.Object) client.Object {
func (r *Reconciler) newObject(objectType ngftypes.ObjectType) ngftypes.ObjectType {
if r.cfg.OnlyMetadata {
partialObj := &metav1.PartialObjectMetadata{}
partialObj.SetGroupVersionKind(objectType.GetObjectKind().GroupVersionKind())
Expand Down
5 changes: 3 additions & 2 deletions internal/framework/controller/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/predicate"

"github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index"
ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
)

const (
Expand Down Expand Up @@ -82,7 +83,7 @@ func defaultConfig() config {
// The registered controller will send events to the provided channel.
func Register(
ctx context.Context,
objectType client.Object,
objectType ngftypes.ObjectType,
mgr manager.Manager,
eventCh chan<- interface{},
options ...Option,
Expand Down Expand Up @@ -137,7 +138,7 @@ func Register(
func addIndex(
ctx context.Context,
indexer client.FieldIndexer,
objectType client.Object,
objectType ngftypes.ObjectType,
field string,
indexerFunc client.IndexerFunc,
) error {
Expand Down
4 changes: 2 additions & 2 deletions internal/framework/controller/register_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
v1 "sigs.k8s.io/gateway-api/apis/v1"
Expand All @@ -24,6 +23,7 @@ import (
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/predicate"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
)

func TestRegister(t *testing.T) {
Expand Down Expand Up @@ -62,7 +62,7 @@ func TestRegister(t *testing.T) {

tests := []struct {
fakes fakes
objectType client.Object
objectType ngftypes.ObjectType
expectedErr error
msg string
expectedMgrAddCallCount int
Expand Down
4 changes: 3 additions & 1 deletion internal/framework/events/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package events
import (
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
)

// EventBatch is a batch of events to be handled at once.
Expand All @@ -17,7 +19,7 @@ type UpsertEvent struct {
// DeleteEvent representing deleting a resource.
type DeleteEvent struct {
// Type is the resource type. For example, if the event is for *v1.HTTPRoute, pass &v1.HTTPRoute{} as Type.
Type client.Object
Type ngftypes.ObjectType
// NamespacedName is the namespace & name of the deleted resource.
NamespacedName types.NamespacedName
}
5 changes: 3 additions & 2 deletions internal/framework/status/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller"
ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
)

// UpdateRequest is a request to update the status of a resource.
type UpdateRequest struct {
ResourceType client.Object
ResourceType ngftypes.ObjectType
Setter Setter
NsName types.NamespacedName
}
Expand Down Expand Up @@ -83,7 +84,7 @@ func (u *Updater) Update(ctx context.Context, reqs ...UpdateRequest) {
func (u *Updater) writeStatuses(
ctx context.Context,
nsname types.NamespacedName,
resourceType client.Object,
resourceType ngftypes.ObjectType,
statusSetter Setter,
) {
obj := resourceType.DeepCopyObject().(client.Object)
Expand Down
4 changes: 4 additions & 0 deletions internal/framework/types/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/*
Package types contains types that are shared by the provisioner and static modes.
*/
package types
7 changes: 7 additions & 0 deletions internal/framework/types/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package types

import "sigs.k8s.io/controller-runtime/pkg/client"

// ObjectType is used when we only care about the type of client.Object.
// The fields of the client.Object may be empty.
type ObjectType client.Object
3 changes: 2 additions & 1 deletion internal/mode/provisioner/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/events"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/status"
ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
)

// Config is configuration for the provisioner mode.
Expand Down Expand Up @@ -63,7 +64,7 @@
// Note: for any new object type or a change to the existing one,
// make sure to also update firstBatchPreparer creation below
controllerRegCfgs := []struct {
objectType client.Object
objectType ngftypes.ObjectType

Check warning on line 67 in internal/mode/provisioner/manager.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/provisioner/manager.go#L67

Added line #L67 was not covered by tests
options []controller.Option
}{
{
Expand Down
3 changes: 2 additions & 1 deletion internal/mode/static/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/runnables"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/status"
ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/metrics/collectors"
ngxcfg "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config"
Expand Down Expand Up @@ -356,7 +357,7 @@
controlConfigNSName types.NamespacedName,
) error {
type ctlrCfg struct {
objectType client.Object
objectType ngftypes.ObjectType

Check warning on line 360 in internal/mode/static/manager.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/manager.go#L360

Added line #L360 was not covered by tests
options []controller.Option
}

Expand Down
9 changes: 5 additions & 4 deletions internal/mode/static/state/change_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation"
Expand Down Expand Up @@ -50,7 +51,7 @@ type ChangeProcessor interface {
// CaptureDeleteChange captures a delete change to a resource.
// The method panics if the resource is of unsupported type or if the passed Gateway is different from the one
// this ChangeProcessor was created for.
CaptureDeleteChange(resourceType client.Object, nsname types.NamespacedName)
CaptureDeleteChange(resourceType ngftypes.ObjectType, nsname types.NamespacedName)
// Process produces a graph-like representation of GatewayAPI resources.
// If no changes were captured, the changed return argument will be NoChange and graph will be empty.
Process() (changeType ChangeType, graphCfg *graph.Graph)
Expand Down Expand Up @@ -114,11 +115,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
clusterState: clusterStore,
}

isReferenced := func(obj client.Object, nsname types.NamespacedName) bool {
isReferenced := func(obj ngftypes.ObjectType, nsname types.NamespacedName) bool {
return processor.latestGraph != nil && processor.latestGraph.IsReferenced(obj, nsname)
}

isNGFPolicyRelevant := func(obj client.Object, nsname types.NamespacedName) bool {
isNGFPolicyRelevant := func(obj ngftypes.ObjectType, nsname types.NamespacedName) bool {
pol, ok := obj.(policies.Policy)
if !ok {
return false
Expand Down Expand Up @@ -241,7 +242,7 @@ func (c *ChangeProcessorImpl) CaptureUpsertChange(obj client.Object) {
c.updater.Upsert(obj)
}

func (c *ChangeProcessorImpl) CaptureDeleteChange(resourceType client.Object, nsname types.NamespacedName) {
func (c *ChangeProcessorImpl) CaptureDeleteChange(resourceType ngftypes.ObjectType, nsname types.NamespacedName) {
c.lock.Lock()
defer c.lock.Unlock()

Expand Down
3 changes: 2 additions & 1 deletion internal/mode/static/state/change_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state"
staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph"
Expand Down Expand Up @@ -2328,7 +2329,7 @@ var _ = Describe("ChangeProcessor", func() {

DescribeTable(
"CaptureDeleteChange must panic",
func(resourceType client.Object, nsname types.NamespacedName) {
func(resourceType ngftypes.ObjectType, nsname types.NamespacedName) {
process := func() {
processor.CaptureDeleteChange(resourceType, nsname)
}
Expand Down
12 changes: 8 additions & 4 deletions internal/mode/static/state/changed_predicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@ package state
import (
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
)

// stateChangedPredicate determines whether upsert and delete events constitute a change in state.
type stateChangedPredicate interface {
// upsert returns true if the newObject changes state.
upsert(oldObject, newObject client.Object) bool
// delete returns true if the deletion of the object changes state.
delete(object client.Object, nsname types.NamespacedName) bool
delete(object ngftypes.ObjectType, nsname types.NamespacedName) bool
}

// funcPredicate applies the stateChanged function on upsert and delete. On upsert, the newObject is passed.
// Implements stateChangedPredicate.
type funcPredicate struct {
stateChanged func(object client.Object, nsname types.NamespacedName) bool
stateChanged func(object ngftypes.ObjectType, nsname types.NamespacedName) bool
}

func (f funcPredicate) upsert(_, newObject client.Object) bool {
Expand All @@ -27,7 +29,7 @@ func (f funcPredicate) upsert(_, newObject client.Object) bool {
return f.stateChanged(newObject, client.ObjectKeyFromObject(newObject))
}

func (f funcPredicate) delete(object client.Object, nsname types.NamespacedName) bool {
func (f funcPredicate) delete(object ngftypes.ObjectType, nsname types.NamespacedName) bool {
return f.stateChanged(object, nsname)
}

Expand All @@ -53,4 +55,6 @@ func (a annotationChangedPredicate) upsert(oldObject, newObject client.Object) b
return oldAnnotation != newAnnotation
}

func (a annotationChangedPredicate) delete(_ client.Object, _ types.NamespacedName) bool { return true }
func (a annotationChangedPredicate) delete(_ ngftypes.ObjectType, _ types.NamespacedName) bool {
return true
}
6 changes: 4 additions & 2 deletions internal/mode/static/state/changed_predicate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
)

func TestFuncPredicate(t *testing.T) {
alwaysTrueFunc := func(_ client.Object, _ types.NamespacedName) bool { return true }
alwaysTrueFunc := func(_ ngftypes.ObjectType, _ types.NamespacedName) bool { return true }
emptyObject := &v1.Pod{}

p := funcPredicate{stateChanged: alwaysTrueFunc}
Expand All @@ -23,7 +25,7 @@ func TestFuncPredicate(t *testing.T) {
}

func TestFuncPredicate_Panic(t *testing.T) {
alwaysTrueFunc := func(_ client.Object, _ types.NamespacedName) bool { return true }
alwaysTrueFunc := func(_ ngftypes.ObjectType, _ types.NamespacedName) bool { return true }

p := funcPredicate{stateChanged: alwaysTrueFunc}

Expand Down
3 changes: 2 additions & 1 deletion internal/mode/static/state/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation"
)
Expand Down Expand Up @@ -79,7 +80,7 @@ type Graph struct {
type ProtectedPorts map[int32]string

// IsReferenced returns true if the Graph references the resource.
func (g *Graph) IsReferenced(resourceType client.Object, nsname types.NamespacedName) bool {
func (g *Graph) IsReferenced(resourceType ngftypes.ObjectType, nsname types.NamespacedName) bool {
switch obj := resourceType.(type) {
case *v1.Secret:
_, exists := g.ReferencedSecrets[nsname]
Expand Down
19 changes: 10 additions & 9 deletions internal/mode/static/state/statefakes/fake_change_processor.go

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

Loading
Loading