open-cluster-management-io / registration

hub / spoke registration controllers
Apache License 2.0
42 stars 58 forks source link

fix cluster labels update conflict. #287

Open morvencao opened 1 year ago

morvencao commented 1 year ago

fix cluster labels update conflict:

E1122 13:49:38.188870       1 base_controller.go:270] "AddOnFeatureDiscoveryController" controller failed to sync "ocm-controlplane-1-mc-4/work-manager", err: Operation cannot be fulfilled on managedclusters.cluster.open-cluster-management.io "ocm-controlplane-1-mc-4": the object has been modified; please apply your changes to the latest version and try again
E1122 13:49:45.423603       1 base_controller.go:270] "AddOnFeatureDiscoveryController" controller failed to sync "ocm-controlplane-1-mc-3/governance-policy-framework", err: Operation cannot be fulfilled on managedclusters.cluster.open-cluster-management.io "ocm-controlplane-1-mc-3": the object has been modified; please apply your changes to the latest version and try again
E1122 13:49:45.438568       1 base_controller.go:270] "AddOnFeatureDiscoveryController" controller failed to sync "ocm-controlplane-1-mc-1/config-policy-controller", err: Operation cannot be fulfilled on managedclusters.cluster.open-cluster-management.io "ocm-controlplane-1-mc-1": the object has been modified; please apply your changes to the latest version and try again
E1122 13:49:45.451226       1 base_controller.go:270] "AddOnFeatureDiscoveryController" controller failed to sync "ocm-controlplane-1-mc-3/config-policy-controller", err: Operation cannot be fulfilled on managedclusters.cluster.open-cluster-management.io "ocm-controlplane-1-mc-3": the object has been modified; please apply your changes to the latest version and try again
E1122 13:49:45.460923       1 base_controller.go:270] "AddOnFeatureDiscoveryController" controller failed to sync "ocm-controlplane-1-mc-1/config-policy-controller", err: Operation cannot be fulfilled on managedclusters.cluster.open-cluster-management.io "ocm-controlplane-1-mc-1": the object has been modified; please apply your changes to the latest version and try again
E1122 13:49:45.477055       1 base_controller.go:270] "AddOnFeatureDiscoveryController" controller failed to sync "ocm-controlplane-1-mc-1/governance-policy-framework", err: Operation cannot be fulfilled on managedclusters.cluster.open-cluster-management.io "ocm-controlplane-1-mc-1": the object has been modified; please apply your changes to the latest version and try again
E1122 13:49:45.488734       1 base_controller.go:270] "AddOnFeatureDiscoveryController" controller failed to sync "ocm-controlplane-1-mc-3", err: Operation cannot be fulfilled on managedclusters.cluster.open-cluster-management.io "ocm-controlplane-1-mc-3": the object has been modified; please apply your changes to the latest version and try again
E1122 13:49:45.503787       1 base_controller.go:270] "AddOnFeatureDiscoveryController" controller failed to sync "ocm-controlplane-1-mc-1/governance-policy-framework", err: Operation cannot be fulfilled on managedclusters.cluster.open-cluster-management.io "ocm-controlplane-1-mc-1": the object has been modified; please apply your changes to the latest version and try again
E1122 13:49:45.511774       1 base_controller.go:270] "AddOnFeatureDiscoveryController" controller failed to sync "ocm-controlplane-1-mc-3", err: Operation cannot be fulfilled on managedclusters.cluster.open-cluster-management.io "ocm-controlplane-1-mc-3": the object has been modified; please apply your changes to the latest version and try again

Signed-off-by: morvencao lcao@redhat.com

openshift-ci[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: morvencao Once this PR has been reviewed and has the lgtm label, please assign qiujian16 for approval by writing /assign @qiujian16 in a comment. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/open-cluster-management-io/registration/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
morvencao commented 1 year ago

/hold

morvencao commented 1 year ago

is the integration test failure relevant?

morvencao commented 1 year ago

/hold cancel

skeeey commented 1 year ago

it seems not, restart the test to have a try

morvencao commented 1 year ago

@qiujian16 Another look? all comments addressed except the one set to nil for label key, because labels is known type(map[string]string), set value to nil, it will complain:

pkg/hub/addon/discovery_controller.go:144:17: cannot use nil as string value in assignment
pkg/hub/addon/discovery_controller.go:148:17: cannot use nil as string value in assignment
pkg/hub/addon/discovery_controller.go:219:23: cannot use nil as string value in assignment
pkg/hub/addon/discovery_controller.go:234:23: cannot use nil as string value in assignment
qiujian16 commented 1 year ago

you cannot set nil in map[string]string, but you can do it in map[string]interface{}. The patch you generate should be based on a map[string]interface{}.

morvencao commented 1 year ago

Any benefit to use map[string]interface{} instead of map[string]string? In labels update case, the labels field is known type, and the JSON encoding can also generate patch bytes based on map[string]string type. We can convert map[string]string to map[string]interface{}, but that will also make it more complicated, especially we need to check whether labels are actually updated before launch patch request. I tried to use map[string]interface{}, it is more complicated and the UT cases are also need to updated accordingly.

qiujian16 commented 1 year ago

you can delete a key with map[string]interface{}