reconciler during runtime causes a panic

neowulf commented 4 months ago

Describe the bug

Go 1.22.0 reconciler-runtime 1.17.0

When Istio types are used with a ChildReconciler, there is a panic that happens here:

panic: interface conversion: interface {} is **v1beta1.DestinationRule, not *v1beta1.DestinationRule [recovered]
        panic: interface conversion: interface {} is **v1beta1.DestinationRule, not *v1beta1.DestinationRule

goroutine 96 [running]:
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()
        /Users/siva/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.0/pkg/internal/controller/controller.go:116 +0x1e5
panic({0x34aed40?, 0xc000633d70?})
        /Users/siva/go/go1.22.0/src/runtime/panic.go:770 +0x132
github.com/vmware-labs/reconciler-runtime/reconcilers.extractItems[...]({0x36dbf20, 0xc00020f570?})
        /Users/siva/go/pkg/mod/github.com/vmware-labs/reconciler-runtime@v0.17.0/reconcilers/child.go:391 +0x21d
github.com/vmware-labs/reconciler-runtime/reconcilers.(*ChildReconciler[...]).filterChildren(0x36e9060, 0xc000440c60, 0x36dbf20)
        /Users/siva/go/pkg/mod/github.com/vmware-labs/reconciler-runtime@v0.17.0/reconcilers/child.go:357 +0x45
github.com/vmware-labs/reconciler-runtime/reconcilers.(*ChildReconciler[...]).reconcile(0x36e9060, {0x36d4d18, 0xc000633830}, 0xc000440c60)
        /Users/siva/go/pkg/mod/github.com/vmware-labs/reconciler-runtime@v0.17.0/reconcilers/child.go:303 +0x2e5
github.com/vmware-labs/reconciler-runtime/reconcilers.(*ChildReconciler[...]).Reconcile(0x36e9060, {0x36d4d18, 0xc000633740}, 0xc000440c60)
        /Users/siva/go/pkg/mod/github.com/vmware-labs/reconciler-runtime@v0.17.0/reconcilers/child.go:258 +0x4f2
github.com/vmware-labs/reconciler-runtime/reconcilers.Sequence[...].Reconcile(0x0?, {0x36d4d18, 0xc000633650?}, 0xc000440c60?)
        /Users/siva/go/pkg/mod/github.com/vmware-labs/reconciler-runtime@v0.17.0/reconcilers/sequence.go:47 +0x206
github.com/vmware-labs/reconciler-runtime/reconcilers.(*ResourceReconciler[...]).reconcile(0x36e6f00, {0x36d4d18, 0xc000633650}, 0xc000440c60)
        /Users/siva/go/pkg/mod/github.com/vmware-labs/reconciler-runtime@v0.17.0/reconcilers/resource.go:273 +0x82
github.com/vmware-labs/reconciler-runtime/reconcilers.(*ResourceReconciler[...]).Reconcile(0x36e6f00, {0x36d4d18, 0xc0006333e0}, {{{0xc000136b58, 0x0?}, {0xc000138fd8?, 0x5?}}})

Reproduction steps

Create a simple ChildController managing DestinationRules

func DestinationRule() reconcilers.SubReconciler[*v1alpha1.CustomCR] {
    return &reconcilers.ChildReconciler[*v1alpha1.CustomCR, *v1beta1.DestinationRule, *v1beta1.DestinationRuleList]{
        Name: "SimpleDestinationRule",
        DesiredChild: func(ctx context.Context, resource *v1alpha1.CustomCR) (*v1beta1.DestinationRule, error) {
            return &v1beta1.DestinationRule{}, nil
        },
        MergeBeforeUpdate: func(current, desired *v1beta1.DestinationRule) {},
        ReflectChildStatusOnParent: func(ctx context.Context, parent *v1alpha1.CustomCR, child *v1beta1.DestinationRule, err error) {
        },
    }
}

Running this reconciler will cause the panic

Expected behavior

Running the reconciler should not cause a panic.

Additional context

I believe this line using reflection is causing the issue:

https://github.com/vmware-labs/reconciler-runtime/blob/main/reconcilers/child.go#L391

Do note that the Istio Lists are a slice of pointers rather than the expected slice of types that are found in the k8s

https://github.com/istio/client-go/blob/d0d3864f139c210f15a0316565ede3da19beca31/pkg/apis/networking/v1beta1/types.gen.go#L75

neowulf commented 4 months ago

Using these packages:

    istio.io/client-go v1.20.0
    k8s.io/api v0.29.0
    sigs.k8s.io/controller-runtime v0.17.0

Here's a scratch code simulating this issue:

package main

import (
    "fmt"
    "reflect"

    "istio.io/client-go/pkg/apis/networking/v1beta1"
    v1 "k8s.io/api/core/v1"
    "sigs.k8s.io/controller-runtime/pkg/client"
)

func filterChildren[CT client.Object, CLT client.ObjectList](children CLT) []CT {
    items := []CT{}
    for _, child := range extractItems[CT](children) {
        items = append(items, child)
    }
    return items
}

func extractItems[T client.Object](list client.ObjectList) []T {
    items := []T{}
    listValue := reflect.ValueOf(list).Elem()
    itemsValue := listValue.FieldByName("Items")
    for i := 0; i < itemsValue.Len(); i++ {
        item := itemsValue.Index(i).Addr().Interface().(T)
        items = append(items, item)
    }
    return items
}

func main() {
    pods := v1.PodList{
        Items: []v1.Pod{
            v1.Pod{},
        },
    }

    drs := v1beta1.DestinationRuleList{
        Items: []*v1beta1.DestinationRule{
            &v1beta1.DestinationRule{},
        },
    }

    fmt.Println("checking pods now...")

    filterChildren[*v1.Pod, *v1.PodList](&pods)

    fmt.Println("checking drs now...")

    filterChildren[*v1beta1.DestinationRule, *v1beta1.DestinationRuleList](&drs)
}

causes this output:

checking pods now...
checking drs now...
panic: interface conversion: interface {} is **v1beta1.DestinationRule, not *v1beta1.DestinationRule

goroutine 1 [running]:
main.extractItems[...]({0x400e368, 0xc000528850?})
        /Users/siva/Library/Application Support/JetBrains/GoLand2023.3/scratches/scratch_2.go:25 +0x21d
main.filterChildren[...](0xc00011c020)
        /Users/siva/Library/Application Support/JetBrains/GoLand2023.3/scratches/scratch_2.go:14 +0x2c
main.main()
        /Users/siva/Library/Application Support/JetBrains/GoLand2023.3/scratches/scratch_2.go:51 +0x25f

scothis commented 4 months ago

The canonical type for items within a ObjectList is a slice of structs rather than a slice of struct pointers. That said, we should be able to handle this better.

vmware-labs / reconciler-runtime