🐛 run make update fail because deepcopy doesn't support generic …

[xuezhaojun]

…type.

Summary

Run make update fail because of deepcopy-gen doesn't support generic type yet. kubernetes/gengo#225

It will log errors:

W1017 14:10:32.195161   62703 parse.go:863] Making unsupported type entry "T" for: &types.TypeParam{check:(*types.Checker)(nil), id:0x1, obj:(*types.TypeName)(0xc009651900), index:0, bound:(*types.Interface)(0xc00009a960)}
F1017 14:10:32.221298   62703 deepcopy.go:890] Hit an unsupported type open-cluster-management.io/api/cluster/v1alpha1.ClusterRolloutStatusFunc[T] for open-cluster-management.io/api/cluster/v1alpha1.ClusterRolloutStatusFunc[T], from open-cluster-management.io/api/cluster/v1alpha1.RolloutHandler[T]

The generic type was introduced by this PR: #276

In this PR, we change the strategy from 'opt-out'(via deepcopy-gen=false comment) to 'opt-in'(via deepcopy-gen=true comment)

Related issue(s)

Fixes #

[xuezhaojun]

/assign @qiujian16

cc @haoqing0110 @serngawy

[xuezhaojun]

After this PR merged, we should add make update as a ci step.

[serngawy]

Thanks @xuezhaojun , I'm able to reproduce the error as well. It should be enough to set the // +k8s:deepcopy-gen=false to ClusterRolloutStatusFunc[T any] type similar to RolloutHandler[T any] type here .
It is strange that it didn't show up with original PR .
Based on the deepCopy code doc here setting the deepcopy-gen=false should work.

[xuezhaojun]

Thanks @xuezhaojun , I'm able to reproduce the error as well. It should be enough to set the // +k8s:deepcopy-gen=false to ClusterRolloutStatusFunc[T any] type similar to RolloutHandler[T any] type here . It is strange that it didn't show up with original PR . Based on the deepCopy code doc here setting the deepcopy-gen=false should work.

Hi, @serngawy , unfortunately, adding // +k8s:deepcopy-gen=false to ClusterRolloutStatusFunc[T any] doesn't work, because the root cause is generic type, when deepcopy-gen parsing the code of a generic type, it end up with getting 2 types:

I1017 16:48:42.317012    5707 deepcopy.go:273] Type open-cluster-management.io/api/cluster/v1alpha1.RolloutHandler[T any] is not copyable
I1017 16:48:42.317023    5707 deepcopy.go:276] Type open-cluster-management.io/api/cluster/v1alpha1.RolloutHandler[T] is copyable

You can see from the above log, // +k8s:deepcopy-gen=false only works for RolloutHandler[T any] so it's shown as not capable. But for RolloutHandler[T](which I think is a result of incorrect parsering) is copyable.

[qiujian16]

hrm, I thought make verify already cover the process of make update...

[qiujian16]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, xuezhaojun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [qiujian16]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[xuezhaojun]

This PR is not the final solution, I will follow up to watch the progress of code-gen libs's generic type supporting, and we will solve this before upgrade these generic types to v1beta1.

[xuezhaojun]

/assign @haoqing0110

[haoqing0110]

/lgtm

[dhaiducek]

@xuezhaojun I noticed this PR merged, but I opened a PR off of my branch in case there was interest in going the controller-gen route instead:

• 🐛 Use controller-runtime for deepcopy generation for cluster:v1alpha1 #291

[serngawy]

@xuezhaojun I noticed this PR merged, but I opened a PR off of my branch in case there was interest in going the controller-gen route instead:

• 🐛 Use controller-runtime for deepcopy generation for cluster:v1alpha1 #291

+1 for using controller-gen it's more cleaner , we don't need to set deep-copy for every struct

[xuezhaojun]

@serngawy @dhaiducek Thanks for submitting the fix! vote to this alternative, great work!