Is the Using Finalizers doc up-to-date?

[rayterrill]

What broke? What's expected?

Attempted to follow the documentation to configure a finalizer for my controller:

    controlPlane := *controlplanev1beta1.AWSManagedControlPlane
    if err := r.Get(ctx, req.NamespacedName, controlPlane); err != nil {

This was resulting in the following:

expected pointer but got nil

I flipped this to the following (looking at the controller-runtime docs here: https://github.com/kubernetes-sigs/controller-runtime/blob/f236f0345ad2933912ebf34bfcf0f93620769654/example_test.go#L123):

	controlPlane := &controlplanev1beta1.AWSManagedControlPlane{}
	if err := r.Get(ctx, req.NamespacedName, controlPlane); err != nil {

This works fine.

Reproducing this issue

No response

KubeBuilder (CLI) Version

Version: main.version{KubeBuilderVersion:"3.3.0", KubernetesVendor:"unknown", GitCommit:"47859bf2ebf96a64db69a2f7074ffdec7f15c1ec", BuildDate:"2022-01-17T18:05:10Z", GoOs:"darwin", GoArch:"amd64"}

PROJECT version

3

Plugin versions

go.kubebuilder.io/v3

Other versions

No response

Extra Labels

No response

[camilamacedo86]

HI @rayterrill,

Before you use the client to fetch the resource in the cluster your variable will be nil so that shows expected.
We have in this project the doc: https://book.kubebuilder.io/reference/using-finalizers.html?highlight=finalizer#using-finalizers which speaks about finalizers.

So, could you please let us know if it is the doc that you were using? If yes, what change do you suggest on it to avoid others facing the same?

[rayterrill]

Hi @camilamacedo86! I'm pretty new to kubebuilder (and to Go in general), so this could just be me messing this up. But I wasn't able to build and deploy my project without making the change above - it was failing with that "expected pointer but got nil" until I made the change above.

I'm actually not sure what the best answer is here - just noting this confused me trying to follow the docs, just was hoping someone could confirm if it was correct or not. :)

[camilamacedo86]

Hi @rayterrill,

It shows that when you face the error you declare a nil var versus the fix that is an empty instance.

Sorry, but I do not see how it can be related to the project itself or how can we improve the docs in this case.
I hope that you do not mind we close this one. However, please feel free to raise new questions or report new issues.

[nayihz]

Controller will indeed panic if we follow the documentation to configure a finalizer.
Because cronJob is nil pointer. I think we shoud change var cronJob *batchv1.CronJob to cronJob := new(batchv1.CronJob).

var cronJob *batchv1.CronJob
if err := r.Get(ctx, req.NamespacedName, cronJob); err != nil {
    log.Error(err, "unable to fetch CronJob")
    // we'll ignore not-found errors, since they can't be fixed by an immediate
    // requeue (we'll need to wait for a new notification), and we can get them
    // on deleted requests.
    return ctrl.Result{}, client.IgnoreNotFound(err)
}

[sj14]

Indeed, what @cmssczy said or cronJob := &batchv1.CronJob{}
@camilamacedo86 please consider reopening the issue until the docs are fixed.

[camilamacedo86]

HI @rayterrill,

We can update it for:
cronJob := &batchv1.CronJob{}

Also, would you like to contribute to this one? Please, feel free to push the PR.