Labels sync targets with the SyncTargetKey for reverse lookup #1672

jmprusi · 2022-08-03T14:50:34Z

This PR adds a new label to SyncTarget resources internal.workload.kcp.dev/key this label contains the SyncTargetKey, a hash generated with the ToSyncTargetKey() helper func.

Generalises the virtualworkspaceURLs controller, now is used for interacting with synctargets in a more generic way.
Exposes the SyncTargetKey of a SyncTarget in the kubectl view:

> kubectl get synctargets -o wide
NAME       LOCATION   READY   SYNCED API RESOURCES   KEY                                      AGE
cluster1   cluster1   False                          adVHlRJKckuJCsdmmK0eWIfPom0mHOqz6wVYvC   19s

pkg/apis/workload/v1alpha1/types.go

config/crds/workload.kcp.dev_synctargets.yaml

pkg/reconciler/workload/virtualworkspaceurls/virtualworkspaceurls_reconcile.go

pkg/reconciler/workload/virtualworkspaceurls/virtualworkspaceurls_controller.go

sttts · 2022-08-03T16:55:09Z

pkg/reconciler/workload/synctarget/synctarget_controller.go

+			klog.Errorf("failed to patch sync target: %v", err)
+			return err
+		}
+		klog.V(4).InfoS("patched synctarget", "name", newSyncTarget.Name, "workspace", logicalcluster.From(newSyncTarget))


nit: would do that before the patch and include the actual patch bytes, and V(2).

sttts · 2022-08-03T16:55:14Z

pkg/reconciler/workload/synctarget/synctarget_controller.go

+			klog.Errorf("failed to patch sync target status: %v", err)
+			return err
+		}
+		klog.V(4).InfoS("patched synctarget status", "name", newSyncTarget.Name, "workspace", logicalcluster.From(newSyncTarget))


pkg/reconciler/workload/synctarget/synctarget_controller.go

sttts · 2022-08-03T16:56:16Z

pkg/reconciler/workload/synctarget/synctarget_reconcile.go

@@ -34,6 +34,13 @@ import (
 func (c *Controller) reconcile(syncTarget *workloadv1alpha1.SyncTarget, workspaceShards []*v1alpha1.ClusterWorkspaceShard) (*workloadv1alpha1.SyncTarget, error) {
 	syncTargetCopy := syncTarget.DeepCopy()

+	labels := syncTargetCopy.GetLabels()


you can edit syncTargetCopy.Labels directly. No need for Get/SetLabel.

that panics.

no. Of course you have to create the map if it is nil. But no need for calling setter and getter.

…ting synctargets with kubectl

…bel to SyncTargets

sttts · 2022-08-04T05:54:24Z

/lgtm
/approve
/retest

openshift-ci · 2022-08-04T05:54:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sttts

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~~ [sttts]
~~config/crds/OWNERS~~ [sttts]
~~pkg/apis/OWNERS~~ [sttts]

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

openshift-ci bot requested review from qiujian16 and shawn-hurley August 3, 2022 14:50

openshift-ci bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Aug 3, 2022