Cluster version controller code restructering

[nb-ohad]

• Seperate the different reconcile concerns into functions
• Preload all input resources
• Optimize memory managemnt for console resources
• Add reconsole tracking log messages
• Fix some issues with object metadata overwrites

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nb-ohad

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [nb-ohad]

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

[leelavg]

@nb-ohad

1. Refactor CSI package code to reduce memory, GC overhead, and "order of operations" considerations #107 introduced a bug wherein ceph deploy & ds .spec.template.metadata.labels are missed during refactor which kinda makes current main undeployable

2024-03-15T03:27:04Z    ERROR   Reconciler error        {"controller": "clusterversion", "controllerGroup": "config.openshift.io", "controllerKind": "ClusterVersion", "ClusterVersion": {"name":"version"}, "namespace": "", "name": "version", "reconcileID": "4c22d638-60c8-40c0-9855-2ae425f8cb09", "error": "Deployment.apps \"csi-cephfsplugin-provisioner\" is invalid: spec.template.metadata.labels: Invalid value: map[string]string(nil): `selector` does not match template `labels`"}

2. w/ current PR I see codepath in point 1 isn't hit (or at-least I don't see any logs) at all which is more puzzling and a new error in the log appeared

2024-03-15T03:33:03Z    INFO    Starting reconciliation {"controller": "clusterversion", "controllerGroup": "config.openshift.io", "controllerKind": "ClusterVersion", "ClusterVersion": {"name":"version"}, "namespace": "", "name": "version", "reconcileID": "7cc80ef1-57e4-4a1d-b7e2-75aead0b1854", "ClusterVersion": {"name":"version"}}
2024-03-15T03:33:03Z    INFO    Reconciling NginxConfigMap      {"controller": "clusterversion", "controllerGroup": "config.openshift.io", "controllerKind": "ClusterVersion", "ClusterVersion": {"name":"version"}, "namespace": "", "name": "version", "reconcileID": "7cc80ef1-57e4-4a1d-b7e2-75aead0b1854", "ClusterVersion": {"name":"version"}}
2024-03-15T03:33:03Z    ERROR   failed to create nginx config map       {"controller": "clusterversion", "controllerGroup": "config.openshift.io", "controllerKind": "ClusterVersion", "ClusterVersion": {"name":"version"}, "namespace": "", "name": "version", "reconcileID": "7cc80ef1-57e4-4a1d-b7e2-75aead0b1854", "ClusterVersion": {"name":"version"}, "error": "Object openshift-storage-client/ocs-client-operator-console-nginx-conf is already owned by another Deployment controller ocs-client-operator-console"}
github.com/red-hat-storage/ocs-client-operator/controllers.(*ClusterVersionReconciler).reconcileNginxConfigMap
        /workspace/controllers/clusterversion_controller.go:231
github.com/red-hat-storage/ocs-client-operator/controllers.(*ClusterVersionReconciler).reconcilPhases
        /workspace/controllers/clusterversion_controller.go:165
github.com/red-hat-storage/ocs-client-operator/controllers.(*ClusterVersionReconciler).Reconcile
        /workspace/controllers/clusterversion_controller.go:153
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile
        /workspace/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:119
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
        /workspace/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:316
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
        /workspace/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
        /workspace/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:227
2024-03-15T03:33:03Z    INFO    Reconciliation completed successfully   {"controller": "clusterversion", "controllerGroup": "config.openshift.io", "controllerKind": "ClusterVersion", "ClusterVersion": {"name":"version"}, "namespace": "", "name": "version", "reconcileID": "7cc80ef1-57e4-4a1d-b7e2-75aead0b1854", "ClusterVersion": {"name":"version"}}
2024-03-15T03:33:03Z    INFO    Reconciling StorageClassClaim.  {"controller": "storageclassclaim", "controllerGroup": "ocs.openshift.io", "controllerKind": "StorageClassClaim", "StorageClassClaim": {"name":"ocs-storagecluster-cephfs"}, "namespace": "", "name": "ocs-storagecluster-cephfs", "reconcileID": "bb5c50e1-fe1b-444d-a21b-5c819d968ad6", "StorageClassClaim": {"name":"ocs-storagecluster-cephfs"}}

I think we really need a deployment test suite, can we make an effort to have client operator be able to run on plain k8s which can help a bit better?

Regarding point 1, I'll be raising a new PR to fix that specific issue.

[leelavg]

looks ordered overall. thanks.

[leelavg]

in existing code, console deployment owns the configmap and that seems to be correct and so setting another controller ref here always returns an error.

[nb-ohad]

Console deployment should not own the config map because the deployment controller is not the one that reconciles it. But I guess you mean that the execution fails because there is an existing controller ref.

[leelavg]

you mean that the execution fails because there is an existing controller ref.

• yes.

[openshift-merge-robot]

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[leelavg]

@nb-ohad will you be refreshing this PR?

[nb-ohad]

@nb-ohad will you be refreshing this PR?

@leelavg I am not sure I will do that as the changes to the code base are considerable. I might decide on an alternative of recoding these changes from the ground up on top of the current main. Either way, this work will get into the repo at one point or another.