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(controllers) expand TypeMeta population #4767

Merged
merged 7 commits into from
Oct 13, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ Adding a new version? You'll need three changes:

- No more "log.SetLogger(...) was never called..." log entry during shutdown of KIC
[#4738](https:/Kong/kubernetes-ingress-controller/pull/4738)
- Changes to referenced Secrets are now tracked independent of their referent.
[#4758](https:/Kong/kubernetes-ingress-controller/pull/4758)
- When Kong returns a flattened error related to a Kong entity, the entity's type and name
will be included in the message reported in `KongConfigurationApplyFailed` Kubernetes event
generated for it.
Expand Down
6 changes: 0 additions & 6 deletions hack/generators/controllers/networking/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,6 @@ import (
discoveryv1 "k8s.io/api/discovery/v1"
netv1 "k8s.io/api/networking/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
k8stypes "k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -550,11 +549,6 @@ func (r *{{.PackageAlias}}{{.Kind}}Reconciler) Reconcile(ctx context.Context, re

// get the relevant object
obj := new({{.PackageImportAlias}}.{{.Kind}})
// set type meta to the object
obj.TypeMeta = metav1.TypeMeta{
APIVersion: {{.PackageImportAlias}}.SchemeGroupVersion.String(),
Kind: "{{.Kind}}",
}

if err := r.Get(ctx, req.NamespacedName, obj); err != nil {
if apierrors.IsNotFound(err) {
Expand Down
1 change: 1 addition & 0 deletions internal/controllers/configuration/secret_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func (r *CoreV1SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request
}
return ctrl.Result{}, err
}

log.V(util.DebugLevel).Info("reconciling resource", "namespace", req.Namespace, "name", req.Name)

// clean the object up if it's being deleted
Expand Down
61 changes: 0 additions & 61 deletions internal/controllers/configuration/zz_generated_controllers.go

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

10 changes: 2 additions & 8 deletions internal/controllers/gateway/gateway_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,7 @@ import (
// Vars & Consts
// -----------------------------------------------------------------------------

var (
gatewayV1beta1Group = gatewayapi.Group(gatewayv1beta1.GroupName)
gatewayTypeMeta = metav1.TypeMeta{
APIVersion: gatewayv1beta1.GroupVersion.String(),
Kind: "Gateway",
}
)
var gatewayV1beta1Group = gatewayapi.Group(gatewayv1beta1.GroupName)

// -----------------------------------------------------------------------------
// Gateway Controller - GatewayReconciler
Expand Down Expand Up @@ -341,7 +335,6 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
// gather the gateway object based on the reconciliation trigger. It's possible for the object
// to be gone at this point in which case it will be ignored.
gateway := new(gatewayapi.Gateway)
gateway.TypeMeta = gatewayTypeMeta
if err := r.Get(ctx, req.NamespacedName, gateway); err != nil {
if apierrors.IsNotFound(err) {
gateway.Namespace = req.Namespace
Expand All @@ -356,6 +349,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
}
return ctrl.Result{Requeue: true}, err
}

debug(log, gateway, "processing gateway")

// though our watch configuration eliminates reconciliation of unsupported gateways it's
Expand Down
1 change: 1 addition & 0 deletions internal/controllers/gateway/gatewayclass_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ func (r *GatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request
}
return ctrl.Result{}, err
}

log.V(util.DebugLevel).Info("processing gatewayclass", "name", req.Name)

if isGatewayClassControlledAndUnmanaged(gwc) {
Expand Down
1 change: 1 addition & 0 deletions internal/controllers/gateway/grpcroute_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ func (r *GRPCRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
// for any error other than 404, requeue
return ctrl.Result{}, err
}

debug(log, grpcroute, "processing grpcroute")

// if there's a present deletion timestamp then we need to update the proxy cache
Expand Down
1 change: 1 addition & 0 deletions internal/controllers/gateway/httproute_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
// for any error other than 404, requeue
return ctrl.Result{}, err
}

debug(log, httproute, "processing httproute")

// if there's a present deletion timestamp then we need to update the proxy cache
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func (r *ReferenceGrantReconciler) Reconcile(ctx context.Context, req ctrl.Reque
// for any error other than 404, requeue
return ctrl.Result{}, err
}

debug(log, grant, "processing referencegrant")

debug(log, grant, "checking deletion timestamp")
Expand Down
1 change: 1 addition & 0 deletions internal/controllers/gateway/tcproute_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
// for any error other than 404, requeue
return ctrl.Result{}, err
}

debug(log, tcproute, "processing tcproute")

// if there's a present deletion timestamp then we need to update the proxy cache
Expand Down
1 change: 1 addition & 0 deletions internal/controllers/gateway/tlsroute_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ func (r *TLSRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
// for any error other than 404, requeue
return ctrl.Result{}, err
}

debug(log, tlsroute, "processing tlsroute")

// if there's a present deletion timestamp then we need to update the proxy cache
Expand Down
1 change: 1 addition & 0 deletions internal/controllers/gateway/udproute_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ func (r *UDPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
// for any error other than 404, requeue
return ctrl.Result{}, err
}

debug(log, udproute, "processing udproute")

// if there's a present deletion timestamp then we need to update the proxy cache
Expand Down
62 changes: 52 additions & 10 deletions internal/controllers/reference/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kong/kubernetes-ingress-controller/v2/internal/controllers"
"github.com/kong/kubernetes-ingress-controller/v2/internal/manager/scheme"
"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
)

Expand All @@ -29,9 +30,25 @@ type ObjectReference struct {
// objectKeyFunc returns a k8s object key in the following format:
// group/version,Kind=kind/namespace/name.
// the combination is unique inside a kubernetes cluster.
func objectKeyFunc(obj client.Object) string {
return obj.GetObjectKind().GroupVersionKind().String() + "/" +
obj.GetNamespace() + "/" + obj.GetName()
func objectKeyFunc(obj client.Object) (string, error) {
// TypeMeta is necessary to generate the correct key for references, but we can't use the original object.
// controller-runtime's client provides the same object to both predicates and the admission webhook, and can result
// in a race condition if this uses the original
o := obj.DeepCopyObject()
metaObj, ok := o.(client.Object)
if !ok {
return "", fmt.Errorf("could not convert %s/%s back to client Object", obj.GetNamespace(), obj.GetName())
}
s, err := scheme.Get()
if err != nil {
return "", fmt.Errorf("could not get scheme for %s/%s metadata: %w", obj.GetNamespace(), obj.GetName(), err)
}
err = util.PopulateTypeMeta(o, s)
if err != nil {
return "", fmt.Errorf("could not populate %s/%s metadata: %w", obj.GetNamespace(), obj.GetName(), err)
}
return metaObj.GetObjectKind().GroupVersionKind().String() + "/" +
metaObj.GetNamespace() + "/" + metaObj.GetName(), nil
}

// CacheIndexers implements a reference cache to store reference relationship between k8s objects
Expand Down Expand Up @@ -62,8 +79,14 @@ func ObjectReferenceKeyFunc(obj interface{}) (string, error) {
if !ok {
return "", ErrTypeNotObjectReference
}
referrerKey := objectKeyFunc(ref.Referrer)
referentKey := objectKeyFunc(ref.Referent)
referrerKey, err := objectKeyFunc(ref.Referrer)
if err != nil {
return "", err
}
referentKey, err := objectKeyFunc(ref.Referent)
if err != nil {
return "", err
}

return referrerKey + ":" + referentKey, nil
}
Expand All @@ -76,7 +99,11 @@ func ObjectReferenceIndexerReferrer(obj interface{}) ([]string, error) {
if !ok {
return nil, ErrTypeNotObjectReference
}
return []string{objectKeyFunc(ref.Referrer)}, nil
referrerKey, err := objectKeyFunc(ref.Referrer)
if err != nil {
return []string{}, err
}
return []string{referrerKey}, nil
}

// ObjectReferenceIndexerReferent is the index function to index by the referent,
Expand All @@ -87,7 +114,11 @@ func ObjectReferenceIndexerReferent(obj interface{}) ([]string, error) {
if !ok {
return nil, ErrTypeNotObjectReference
}
return []string{objectKeyFunc(ref.Referent)}, nil
referentKey, err := objectKeyFunc(ref.Referent)
if err != nil {
return []string{}, err
}
return []string{referentKey}, nil
}

// SetObjectReference adds or updates a reference record between referrer and referent in reference cache.
Expand Down Expand Up @@ -127,7 +158,11 @@ func (c CacheIndexers) DeleteObjectReference(referrer client.Object, referent cl
// ObjectReferred returns true if an object is referenced (being the referent)
// in at least one reference record.
func (c CacheIndexers) ObjectReferred(obj client.Object) (bool, error) {
refs, err := c.indexer.ByIndex(IndexNameReferent, objectKeyFunc(obj))
objKey, err := objectKeyFunc(obj)
if err != nil {
return false, err
}
refs, err := c.indexer.ByIndex(IndexNameReferent, objKey)
if err != nil {
return false, err
}
Expand All @@ -138,7 +173,11 @@ func (c CacheIndexers) ObjectReferred(obj client.Object) (bool, error) {
// ListReferencesByReferrer lists all reference records where referrer has the same key
// (GroupVersionKind+NamespacedName, that means the same k8s object).
func (c CacheIndexers) ListReferencesByReferrer(referrer client.Object) ([]*ObjectReference, error) {
refList, err := c.indexer.ByIndex(IndexNameReferrer, objectKeyFunc(referrer))
referrerKey, err := objectKeyFunc(referrer)
if err != nil {
return nil, err
}
refList, err := c.indexer.ByIndex(IndexNameReferrer, referrerKey)
if err != nil {
return nil, err
}
Expand All @@ -157,7 +196,10 @@ func (c CacheIndexers) ListReferencesByReferrer(referrer client.Object) ([]*Obje
// (GroupVersionKind+NamespacedName, that means the same k8s object).
// called when a k8s object deleted in cluster, or when we do not care about it anymore.
func (c CacheIndexers) DeleteReferencesByReferrer(referrer client.Object) error {
key := objectKeyFunc(referrer)
key, err := objectKeyFunc(referrer)
if err != nil {
return err
}
refs, err := c.indexer.ByIndex(IndexNameReferrer, key)
if err != nil {
return err
Expand Down
Loading