[vmcluster]: About the HasSpecChanges method and the AnnotationsFiltered method

[z-anshun]

Hi, everyone!
When I read the source code, I had some doubts about vmcluster, they may be bugs:

1. The HasSpecChanges method does not judge null values, which may cause an error when querying crd for the first time.
   code:
   https://github.com/VictoriaMetrics/operator/blob/master/api/v1beta1/vmcluster_types.go#L947
   error:

1.6944398628889349e+09  ERROR   controller      failed to check if cluster spec changed {"Request.Namespace": "vm", "Request.Name": "example-vmcluster-persistent", "error": "cannot parse last applied cluster spec value:  : unexpected end of JSON input"}
...

2. annotationFilterPrefixes will filter annotations prefixed by operator.victoriametrics.com/, but in the LastAppliedSpecAsPatch function, the key of annotation is set to operator.victoriametrics/last-applied-spec. I think annotationFilterPrefixes expects to filter it out.

[Haleygo]

Hello, thanks for the issue!

The HasSpecChanges method does not judge null values, which may cause an error when querying crd for the first time.

That's a nosiy message for new resource, but it also print error if LastAppliedSpecAsPatch wasn't ever success or annotation got lost for some reason, so I think it's ok to leave it there.

annotationFilterPrefixes will filter annotations prefixed by operator.victoriametrics.com/, but in the LastAppliedSpecAsPatch function, the key of annotation is set to operator.victoriametrics/last-applied-spec. I think annotationFilterPrefixes expects to filter it out.

Yeah, should be fixed in #754, please take a look.

[z-anshun]

Thank you.

That's a nosiy message for new resource, but it also print error if LastAppliedSpecAsPatch wasn't ever success or annotation got lost for some reason, so I think it's ok to leave it there.

By the way, because Reconcile may execute r.Client.Patch at the end, but there is no waitExpanding like creating or updating other components. This may cause the next time the Reconcile method is executed, the VMCluster obtained may not be the latest version, which may cause the error log to be printed twice.

The error log is as follows:

1.694585952956201e+09   ERROR   controller      failed to check if cluster spec changed {"Request.Namespace": "vm", "Request.Name": "example-vmcluster-persistent", "error": "cannot parse last applied cluster spec value:  : unexpected end of JSON input"}
github.com/VictoriaMetrics/operator/controllers.(*VMClusterReconciler).Reconcile
        /github/operator/controllers/vmcluster_controller.go:63
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:121
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:320
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:273
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:234
1.69458595818442e+09    INFO    controller      Reconciling VMCluster   {"Request.Namespace": "vm", "Request.Name": "example-vmcluster-persistent"}
1.6945859964140449e+09  ERROR   controller      failed to check if cluster spec changed {"Request.Namespace": "vm", "Request.Name": "example-vmcluster-persistent", "error": "cannot parse last applied cluster spec value:  : unexpected end of JSON input"}
github.com/VictoriaMetrics/operator/controllers.(*VMClusterReconciler).Reconcile
        /github/operator/controllers/vmcluster_controller.go:63
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile
       /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:121
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:320
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:273
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
       /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:234
1.694585996437922e+09   ERROR   Reconciler error        {"controller": "vmcluster", "controllerGroup": "operator.victoriametrics.com", "controllerKind": "VMCluster", "VMCluster": {"name":"example-vmcluster-persistent","namespace":"vm"}, "namespace": "vm", "name": "example-vmcluster-persistent", "reconcileID": "367f79fb-01ad-40cd-a3ee-a1c00a16e668", "error": "cannot set expanding status for cluster: Operation cannot be fulfilled on vmclusters.operator.victoriametrics.com \"example-vmcluster-persistent\": the object has been modified; please apply your changes to the latest version and try again"}
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:326
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:273
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
       /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:234

This isn't a panic mistake, but I feel like there should be a better way to avoid this.

[Haleygo]

Hmm, that's right. But there is no pods or other resources for vmcluster to wait or show it's ready, I can only come up the idea to have a hardcode timeout or recheck it's spec, both sound not so good.
Please let me know if you get other idea)

[f41gh7]

Linked commit must remove misleading error message.

[f41gh7]

Fix included into the https://github.com/VictoriaMetrics/operator/releases/tag/v0.39.0 release