Add handling for non latest revisions with mutation bool

knative · Nov 29, 2021 · 42ca9ea · 42ca9ea
1 parent 83b6290
commit 42ca9ea
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 13 deletions.
diff --git a/pkg/kn/traffic/compute.go b/pkg/kn/traffic/compute.go
@@ -285,24 +285,27 @@ func verifyInput(trafficFlags *flags.Traffic, svc *servingv1.Service, revisions
 		return errorTrafficDistribution(sum, errorDistributionRevisionCount)
 	}
 
-	if specRevPercentCount == existingRevisionCount-1 {
-		if latestRefTraffic {
-			// if mutation is set, @latest tag is referring to the revision which
-			// will be created after service update. specRevPercentCount should have been
-			// equal to existingRevisionCount
-			if mutation {
-				return errorTrafficDistribution(sum, errorDistributionRevisionCount)
-			}
-			revisionRefMap[svc.Status.LatestReadyRevisionName] = 0
+	// if mutation is set, a new revision will be created after service update.
+	// specRevPercentCount should be equal to existingRevisionCount
+	if mutation {
+		if specRevPercentCount != existingRevisionCount {
+			return errorTrafficDistribution(sum, errorDistributionRevisionCount)
+		}
+		if _, ok := revisionRefMap[latestRevisionRef]; !ok {
+			// remaining % can only go to the @latest tag
+			trafficFlags.RevisionsPercentages = append(trafficFlags.RevisionsPercentages, fmt.Sprintf("%s=%d", latestRevisionRef, 100-sum))
+			return nil
 		}
 	}
 
-	if specRevPercentCount == existingRevisionCount && latestRefTraffic {
-		// if mutation is not set, @latest tag is referring to the revision which
-		// already exists. specRevPercentCount should have been equal to existingRevisionCount
-		if !mutation {
+	// if mutation is not set, specRevPercentCount should be equal to existingRevisionCount
+	if !mutation {
+		if specRevPercentCount != existingRevisionCount-1 {
 			return errorTrafficDistribution(sum, errorDistributionRevisionCount)
 		}
+		if latestRefTraffic {
+			revisionRefMap[svc.Status.LatestReadyRevisionName] = 0
+		}
 	}
 
 	// remaining % to 100

diff --git a/pkg/kn/traffic/compute_test.go b/pkg/kn/traffic/compute_test.go
@@ -318,6 +318,38 @@ func TestCompute(t *testing.T) {
 			mutation:       false,
 			latestRevision: "rev-00003",
 		},
+		{
+			name:             "traffic split sum < 100 without which mutates service should specify N revs. Remaining should go to @latest",
+			existingTraffic:  append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "rev-00001", 0, false), newTarget("", "rev-00002", 0, false), newTarget("", "rev-00003", 100, true)),
+			inputFlags:       []string{"--traffic", "rev-00001=10,rev-00003=40,rev-00002=30"},
+			desiredRevisions: []string{"rev-00001", "rev-00002", "", "rev-00003"},
+			desiredPercents:  []int64{10, 30, 20, 40},
+			desiredTags:      []string{"", "", "", ""},
+			existingRevisions: []servingv1.Revision{{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "rev-00001",
+					Labels: map[string]string{
+						"serving.knative.dev/service": "serviceName",
+					},
+				},
+			}, {
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "rev-00002",
+					Labels: map[string]string{
+						"serving.knative.dev/service": "serviceName",
+					},
+				},
+			}, {
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "rev-00003",
+					Labels: map[string]string{
+						"serving.knative.dev/service": "serviceName",
+					},
+				},
+			}},
+			mutation:       true,
+			latestRevision: "rev-00003",
+		},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {
 			if lper, lrev, ltag := len(testCase.desiredPercents), len(testCase.desiredRevisions), len(testCase.desiredTags); lper != lrev || lper != ltag {

diff --git a/test/e2e/traffic_split_test.go b/test/e2e/traffic_split_test.go
@@ -166,6 +166,23 @@ func TestTrafficSplit(t *testing.T) {
 		verifyTargets(r, serviceName, expectedTargets, false)
 		test.ServiceDelete(r, serviceName)
 	})
+	t.Run("45:55 automatic traffic split to @latest after mutation", func(t *testing.T) {
+		t.Log("direct 45% traffic explicitly to previous revision and remaining 55 will automatically be directed to @latest")
+		r := test.NewKnRunResultCollector(t, it)
+		defer r.DumpIfFailed()
+
+		serviceName := test.GetNextServiceName(serviceBase)
+
+		test.ServiceCreate(r, serviceName)
+
+		rev1 := fmt.Sprintf("%s-00001", serviceName)
+		rev2 := fmt.Sprintf("%s-00002", serviceName)
+		test.ServiceUpdate(r, serviceName, "--env", "TARGET=v1", "--traffic", fmt.Sprintf("%s=%d", rev1, 45))
+
+		expectedTargets := []TargetFields{newTargetFields("", rev2, 55, true), newTargetFields("", rev1, 45, false)}
+		verifyTargets(r, serviceName, expectedTargets, false)
+		test.ServiceDelete(r, serviceName)
+	})
 	t.Run("automatic traffic split failure", func(t *testing.T) {
 		t.Log("direct 50% traffic to one of the three revisions. Remaining will not be automatically redirected as only one revision should be missing from spec")
 		r := test.NewKnRunResultCollector(t, it)