Skip to content

Commit

Permalink
sharding: fix the VW URLs in the SyncTarget
Browse files Browse the repository at this point in the history 
Signed-off-by: David Festal <dfestal@redhat.com>
  • Loading branch information
@davidfestal
davidfestal committed Mar 29, 2023
1 parent 75bb452 commit 2c895bd
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 27 deletions.
31 changes: 21 additions & 10 deletions pkg/reconciler/workload/synctarget/synctarget_reconcile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,28 @@ func (c *Controller) reconcile(ctx context.Context, syncTarget *workloadv1alpha1

desiredURLs := map[string]workloadv1alpha1.VirtualWorkspace{}

var rootShardKey string
for _, workspaceShard := range workspaceShards {
if workspaceShard.Spec.ExternalURL != "" {
sharedExternalURL, err := url.Parse(workspaceShard.Spec.ExternalURL)
if workspaceShard.Spec.VirtualWorkspaceURL != "" {
shardVWURL, err := url.Parse(workspaceShard.Spec.VirtualWorkspaceURL)
if err != nil {
logger.Error(err, "failed to parse workspaceShard.Spec.ExternalURL")
logger.Error(err, "failed to parse workspaceShard.Spec.VirtualWorkspaceURL")
return nil, err
}

syncerVirtualWorkspaceURL := *sharedExternalURL
syncerVirtualWorkspaceURL := *shardVWURL
syncerVirtualWorkspaceURL.Path = path.Join(
sharedExternalURL.Path,
shardVWURL.Path,
virtualworkspacesoptions.DefaultRootPathPrefix,
syncerbuilder.SyncerVirtualWorkspaceName,
logicalcluster.From(syncTargetCopy).String(),
syncTargetCopy.Name,
string(syncTargetCopy.UID),
)

upsyncerVirtualWorkspaceURL := *sharedExternalURL
upsyncerVirtualWorkspaceURL := *shardVWURL
(&upsyncerVirtualWorkspaceURL).Path = path.Join(
sharedExternalURL.Path,
shardVWURL.Path,
virtualworkspacesoptions.DefaultRootPathPrefix,
syncerbuilder.UpsyncerVirtualWorkspaceName,
logicalcluster.From(syncTargetCopy).String(),
Expand All @@ -77,16 +78,26 @@ func (c *Controller) reconcile(ctx context.Context, syncTarget *workloadv1alpha1
syncerURL := (&syncerVirtualWorkspaceURL).String()
upsyncerURL := (&upsyncerVirtualWorkspaceURL).String()

desiredURLs[sharedExternalURL.String()] = workloadv1alpha1.VirtualWorkspace{
if workspaceShard.Name == corev1alpha1.RootShard {
rootShardKey = shardVWURL.String()
}
desiredURLs[shardVWURL.String()] = workloadv1alpha1.VirtualWorkspace{
SyncerURL: syncerURL,
UpsyncerURL: upsyncerURL,
}
}
}

// Let's always add the desired URL in the same order, which will be the order of the
// corresponding shard URLs
// Let's always add the desired URLs in the same order:
// - urls for the root shard will always be added at the first place,
// in order to ensure compatibility with the shard-unaware Syncer
// - urls for other shards which will be added in the lexical order of the
// corresponding shard URLs.
var desiredVirtualWorkspaces []workloadv1alpha1.VirtualWorkspace //nolint:prealloc
if rootShardVWURLs, ok := desiredURLs[rootShardKey]; ok {
desiredVirtualWorkspaces = append(desiredVirtualWorkspaces, rootShardVWURLs)
delete(desiredURLs, rootShardKey)
}
for _, shardURL := range sets.StringKeySet(desiredURLs).List() {
desiredVirtualWorkspaces = append(desiredVirtualWorkspaces, desiredURLs[shardURL])
}
Expand Down
112 changes: 95 additions & 17 deletions pkg/reconciler/workload/synctarget/synctarget_reconcile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ func TestReconciler(t *testing.T) {
Name: "root",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
ExternalURL: "http://external-host/",
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host/",
},
},
},
Expand Down Expand Up @@ -96,26 +96,26 @@ func TestReconciler(t *testing.T) {
Name: "root2",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
ExternalURL: "http://external-host-2/",
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-2/",
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "root3",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
ExternalURL: "http://external-host-3/",
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-3/",
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "root",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
ExternalURL: "http://external-host-1/",
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-1/",
},
},
},
Expand Down Expand Up @@ -167,33 +167,111 @@ func TestReconciler(t *testing.T) {
},
expectError: false,
},
"SyncTarget with multiple Shards with duplicated ExternalURLs results in a deduplicated list of URLs on the SyncTarget": {
"SyncTarget and multiple Shards, but root shard always first": {
workspaceShards: []*corev1alpha1.Shard{
{
ObjectMeta: metav1.ObjectMeta{
Name: "root2",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-2/",
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "root3",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-3/",
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "root",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-10/",
},
},
},
syncTarget: &workloadv1alpha1.SyncTarget{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Annotations: map[string]string{
logicalcluster.AnnotationKey: "demo:root:yourworkspace",
},
},
Spec: workloadv1alpha1.SyncTargetSpec{
Unschedulable: false,
EvictAfter: nil,
},
Status: workloadv1alpha1.SyncTargetStatus{
VirtualWorkspaces: []workloadv1alpha1.VirtualWorkspace{},
},
},
expectedSyncTarget: &workloadv1alpha1.SyncTarget{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Annotations: map[string]string{
logicalcluster.AnnotationKey: "demo:root:yourworkspace",
},
Labels: map[string]string{
"internal.workload.kcp.io/key": "aPXkBdRsTD8gXESO47r9qXmkr2kaG5qaox5C8r",
},
},
Spec: workloadv1alpha1.SyncTargetSpec{
Unschedulable: false,
EvictAfter: nil,
},
Status: workloadv1alpha1.SyncTargetStatus{
VirtualWorkspaces: []workloadv1alpha1.VirtualWorkspace{
{
SyncerURL: "http://external-host-10/services/syncer/demo:root:yourworkspace/test-cluster",
UpsyncerURL: "http://external-host-10/services/upsyncer/demo:root:yourworkspace/test-cluster",
},
{
SyncerURL: "http://external-host-2/services/syncer/demo:root:yourworkspace/test-cluster",
UpsyncerURL: "http://external-host-2/services/upsyncer/demo:root:yourworkspace/test-cluster",
},
{
SyncerURL: "http://external-host-3/services/syncer/demo:root:yourworkspace/test-cluster",
UpsyncerURL: "http://external-host-3/services/upsyncer/demo:root:yourworkspace/test-cluster",
},
},
},
},
expectError: false,
},
"SyncTarget with multiple Shards with duplicated VirtualWorkspaceURLs results in a deduplicated list of URLs on the SyncTarget": {
workspaceShards: []*corev1alpha1.Shard{
{
ObjectMeta: metav1.ObjectMeta{
Name: "root",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
ExternalURL: "http://external-host-1/",
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-1/",
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "root2",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
ExternalURL: "http://external-host-1/",
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-1/",
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "root3",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
ExternalURL: "http://external-host-3/",
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-3/",
},
},
},
Expand Down Expand Up @@ -282,8 +360,8 @@ func TestReconciler(t *testing.T) {
Name: "root",
},
Spec: corev1alpha1.ShardSpec{
BaseURL: "http://1.2.3.4/",
ExternalURL: "http://external-host-1/",
BaseURL: "http://1.2.3.4/",
VirtualWorkspaceURL: "http://external-host-1/",
},
},
},
Expand Down

0 comments on commit 2c895bd

Please sign in to comment.