updates: don't use subgraph if no existing version

is installed this prevented you from installing a version if there was no edge to update _to_ it, even if there was an edge to update _frome_ it.
project-kessel · Feb 8, 2023 · d82ed23 · d82ed23
1 parent a57d2dd
commit d82ed23
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 2 deletions.
diff --git a/pkg/updates/file.go b/pkg/updates/file.go
@@ -218,7 +218,19 @@ func (g *UpdateGraph) ComputeTarget(defaultBaseImage, image, version, channel, e
 		}
 	}
 
-	// Default to the currentVersion we're working toward.
+	// If version is explicit,and there's no current version yet, or the
+	// explicit version matches the current version, just install it
+	if len(version) > 0 && currentVersion == nil ||
+		(currentVersion != nil && currentVersion.Name == version && currentVersion.Channel == channel) {
+		state = updateSource.State(version)
+		target = &v1alpha1.SpiceDBVersion{
+			Name:    state.ID,
+			Channel: channel,
+		}
+		return
+	}
+
+	// Default to the currentVersion we're working towards.
 	target = currentVersion
 
 	var currentState State
@@ -242,7 +254,7 @@ func (g *UpdateGraph) ComputeTarget(defaultBaseImage, image, version, channel, e
 
 	// If currentVersion is set, we only use the subset of the update graph that leads
 	// to that currentVersion.
-	if len(version) > 0 {
+	if currentVersion != nil && len(version) > 0 {
 		updateSource, err = updateSource.Subgraph(version)
 		if err != nil {
 			err = fmt.Errorf("error finding update path from %s to %s", currentVersion.Name, version)
@@ -283,8 +295,12 @@ func (g *UpdateGraph) Difference(other *UpdateGraph) *UpdateGraph {
 
 	// Find matching channels between the graphs
 	for _, thisChannel := range g.Channels {
+
+		foundMatchingChannel := false
+
 		for _, otherChannel := range other.Channels {
 			if thisChannel.EqualIdentity(otherChannel) {
+				foundMatchingChannel = true
 				// Determine which edges are in this channel but not the other
 				keepEdges := make(map[string][]string, 0)
 
@@ -330,6 +346,11 @@ func (g *UpdateGraph) Difference(other *UpdateGraph) *UpdateGraph {
 				})
 			}
 		}
+
+		// if there's no matching channel, the whole channel is new
+		if !foundMatchingChannel {
+			diffGraph.Channels = append(diffGraph.Channels, thisChannel)
+		}
 	}
 	return diffGraph
 }
diff --git a/pkg/updates/file_test.go b/pkg/updates/file_test.go
@@ -297,6 +297,39 @@ func TestComputeTarget(t *testing.T) {
 			expectedTarget:    &v1alpha1.SpiceDBVersion{Name: "v1.0.0", Channel: "cockroachdb"},
 			expectedState:     State{ID: "v1.0.0"},
 		},
+		{
+			name: "version specified and no current version",
+			graph: &UpdateGraph{Channels: []Channel{{
+				Name:     "cockroachdb",
+				Metadata: map[string]string{"datastore": "cockroachdb"},
+				Edges:    EdgeSet{"v1.0.0": {"v1.0.1"}},
+				Nodes:    []State{{ID: "v1.0.1"}, {ID: "v1.0.0"}},
+			}}},
+			engine:            "cockroachdb",
+			channel:           "cockroachdb",
+			version:           "v1.0.0",
+			baseImage:         "ghcr.io/authzed/spicedb",
+			expectedBaseImage: "ghcr.io/authzed/spicedb",
+			currentVersion:    &v1alpha1.SpiceDBVersion{Name: "v1.0.0", Channel: "cockroachdb"},
+			expectedTarget:    &v1alpha1.SpiceDBVersion{Name: "v1.0.0", Channel: "cockroachdb"},
+			expectedState:     State{ID: "v1.0.0"},
+		},
+		{
+			name: "version specified and current version is the same",
+			graph: &UpdateGraph{Channels: []Channel{{
+				Name:     "cockroachdb",
+				Metadata: map[string]string{"datastore": "cockroachdb"},
+				Edges:    EdgeSet{"v1.0.0": {"v1.0.1"}},
+				Nodes:    []State{{ID: "v1.0.1"}, {ID: "v1.0.0"}},
+			}}},
+			engine:            "cockroachdb",
+			channel:           "cockroachdb",
+			version:           "v1.0.0",
+			baseImage:         "ghcr.io/authzed/spicedb",
+			expectedBaseImage: "ghcr.io/authzed/spicedb",
+			expectedTarget:    &v1alpha1.SpiceDBVersion{Name: "v1.0.0", Channel: "cockroachdb"},
+			expectedState:     State{ID: "v1.0.0"},
+		},
 		{
 			name: "head returns same currentVersion",
 			graph: &UpdateGraph{Channels: []Channel{{
@@ -328,6 +361,22 @@ func TestComputeTarget(t *testing.T) {
 			expectedTarget:    &v1alpha1.SpiceDBVersion{Name: "v1.0.1", Channel: "cockroachdb"},
 			expectedState:     State{ID: "v1.0.1"},
 		},
+		{
+			name: "no currentVersion returns spec.version if specified",
+			graph: &UpdateGraph{Channels: []Channel{{
+				Name:     "cockroachdb",
+				Metadata: map[string]string{"datastore": "cockroachdb"},
+				Edges:    EdgeSet{"v1.0.0": {"v1.0.1"}},
+				Nodes:    []State{{ID: "v1.0.1"}, {ID: "v1.0.0"}},
+			}}},
+			engine:            "cockroachdb",
+			channel:           "cockroachdb",
+			version:           "v1.0.0",
+			baseImage:         "ghcr.io/authzed/spicedb",
+			expectedBaseImage: "ghcr.io/authzed/spicedb",
+			expectedTarget:    &v1alpha1.SpiceDBVersion{Name: "v1.0.0", Channel: "cockroachdb"},
+			expectedState:     State{ID: "v1.0.0"},
+		},
 	}
 
 	for _, tt := range table {
@@ -382,6 +431,26 @@ func TestUpdateGraphDifference(t *testing.T) {
 			}},
 			want: []Channel{},
 		},
+		{
+			name: "new channel",
+			first: []Channel{{
+				Name:     "test",
+				Metadata: map[string]string{DatastoreMetadataKey: "test"},
+				Nodes:    []State{{ID: "A", Tag: "A"}, {ID: "B", Tag: "B"}},
+				Edges: map[string][]string{
+					"A": {"B"},
+				},
+			}},
+			second: []Channel{},
+			want: []Channel{{
+				Name:     "test",
+				Metadata: map[string]string{DatastoreMetadataKey: "test"},
+				Nodes:    []State{{ID: "A", Tag: "A"}, {ID: "B", Tag: "B"}},
+				Edges: map[string][]string{
+					"A": {"B"},
+				},
+			}},
+		},
 		{
 			name: "channel has new nodes",
 			first: []Channel{{