Skip to content

Commit

Permalink
Merge pull request #3095 from mjudeikis/mjudeikis/fix.workspace.reload
Browse files Browse the repository at this point in the history 
🐛  fix mount workspace reload
  • Loading branch information
@kcp-ci-bot
kcp-ci-bot authored Mar 16, 2024
2 parents f8f0a65 + e1e2763 commit 35584e0
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 15 deletions.
6 changes: 5 additions & 1 deletion pkg/index/index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ func (c *State) UpsertWorkspace(shard string, ws *tenancyv1alpha1.Workspace) {
mountObjString := c.clusterWorkspaceMountAnnotation[clusterName][ws.Name] // experimental feature
c.lock.RUnlock()

if (cluster.String() == ws.Spec.Cluster) || (ws.Annotations[tenancyv1alpha1.ExperimentalWorkspaceMountAnnotationKey] != "" && mountObjString == ws.Annotations[tenancyv1alpha1.ExperimentalWorkspaceMountAnnotationKey]) {
// TODO(mjudeikis): we are allowing upsert in 2 cases:
// 1. cluster name is different
// 2. mount object string is different (updated, added, or removed)
// When we promote this to workspace structure, we should make this check smarter and better tested.
if (cluster.String() == ws.Spec.Cluster) && (ws.Annotations[tenancyv1alpha1.ExperimentalWorkspaceMountAnnotationKey] != "" && mountObjString == ws.Annotations[tenancyv1alpha1.ExperimentalWorkspaceMountAnnotationKey]) {
return
}

Expand Down
38 changes: 24 additions & 14 deletions pkg/index/index_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,15 +317,15 @@ func TestDeleteShard(t *testing.T) {
target.UpsertLogicalCluster("amber", newLogicalCluster("43"))

r, found := target.Lookup(logicalcluster.NewPath("root:org1"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "amber", "43", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "amber", "43", "", true)

// delete the shard and ensure we cannot look up a path on it
target.DeleteShard("amber")
r, found = target.Lookup(logicalcluster.NewPath("root:org1"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "", "", false)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "", "", "", false)

r, found = target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "root", "34", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "root", "34", "", true)
}

func TestDeleteLogicalCluster(t *testing.T) {
Expand All @@ -340,16 +340,16 @@ func TestDeleteLogicalCluster(t *testing.T) {
target.UpsertLogicalCluster("root", newLogicalCluster("34"))

r, found := target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "root", "34", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "root", "34", "", true)

// ensure that after deleting the logical cluster it cannot be looked up
target.DeleteLogicalCluster("root", newLogicalCluster("34"))

r, found = target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "", "", false)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "", "", "", false)

r, found = target.Lookup(logicalcluster.NewPath("root"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "root", "root", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "root", "root", "", true)
}

func TestDeleteWorkspace(t *testing.T) {
Expand All @@ -366,15 +366,15 @@ func TestDeleteWorkspace(t *testing.T) {
target.UpsertLogicalCluster("root", newLogicalCluster("43"))

r, found := target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "root", "34", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "root", "34", "", true)

target.DeleteWorkspace("root", newWorkspace("org", "root", "34"))

r, found = target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "", "", false)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "", "", "", false)

r, found = target.Lookup(logicalcluster.NewPath("root:org1"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "root", "43", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "root", "43", "", true)
}

func TestUpsertLogicalCluster(t *testing.T) {
Expand All @@ -387,11 +387,11 @@ func TestUpsertLogicalCluster(t *testing.T) {
target.UpsertLogicalCluster("root", newLogicalCluster("34"))

r, found := target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "root", "34", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "root", "34", "", true)

target.UpsertLogicalCluster("amber", newLogicalCluster("34"))
r, found = target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "amber", "34", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "amber", "34", "", true)
}

// Since LookupURL uses Lookup method the following test is just a smoke tests.
Expand Down Expand Up @@ -456,14 +456,21 @@ func TestUpsertWorkspace(t *testing.T) {
target.UpsertLogicalCluster("root", newLogicalCluster("44"))

r, found := target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "root", "34", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "root", "34", "", true)

target.UpsertWorkspace("root", newWorkspace("org", "root", "44"))
r, found = target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, found, "root", "44", true)
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "root", "44", "", true)

// Upsert workspace with changed mount annotation
target.UpsertWorkspace("root", newWorkspaceWithAnnotation("org", "root", "44", map[string]string{
"experimental.tenancy.kcp.io/mount": `{"spec":{"ref":{"kind":"KubeCluster","name":"prod-cluster","apiVersion":"proxy.kcp.dev/v1alpha1"}},"status":{"phase":"Ready","url":"https://kcp.dev.local/services/custom-url/proxy"}}`,
}))
r, found = target.Lookup(logicalcluster.NewPath("root:org"))
validateLookupOutput(t, logicalcluster.NewPath("root:org"), r.Shard, r.Cluster, r.URL, found, "", "", "https://kcp.dev.local/services/custom-url/proxy", true)
}

func validateLookupOutput(t *testing.T, path logicalcluster.Path, shard string, cluster logicalcluster.Name, found bool, expectedShard string, expectedCluster logicalcluster.Name, expectToFind bool) {
func validateLookupOutput(t *testing.T, path logicalcluster.Path, shard string, cluster logicalcluster.Name, url string, found bool, expectedShard string, expectedCluster logicalcluster.Name, expectedURL string, expectToFind bool) {
t.Helper()

if found != expectToFind {
Expand All @@ -475,6 +482,9 @@ func validateLookupOutput(t *testing.T, path logicalcluster.Path, shard string,
if shard != expectedShard {
t.Fatalf("unexpected shard = %v, expected = %v, for %q path", shard, expectedShard, path)
}
if url != expectedURL {
t.Fatalf("unexpected url = %v, expected = %v, for %q path", url, expectedURL, path)
}
}

func newWorkspace(name, cluster, scheduledCluster string) *tenancyv1alpha1.Workspace {
Expand Down

0 comments on commit 35584e0

Please sign in to comment.