Fix/default rev (#58123)

* fix

* stop and test

* wait for caches to sync

* Fix tests

* lint

Author: Stevenjin8

istioctl/pkg/cli/context.go

@@ -26,6 +26,7 @@ import (
 	"istio.io/istio/istioctl/pkg/util/handlers"
 	"istio.io/istio/pkg/cluster"
 	"istio.io/istio/pkg/kube"
+	"istio.io/istio/pkg/log"
 	"istio.io/istio/pkg/maps"
 	"istio.io/istio/pkg/ptr"
 	"istio.io/istio/pkg/revisions"
@@ -215,21 +216,27 @@ func (i *instance) RevisionOrDefault(rev string) string {
 
 	// Try to get the default revision from the default watcher
 	// We create a simple approach that doesn't cause circular dependencies
+
+	stop := make(chan struct{})
+	defer close(stop)
 	if i.defaultWatcher == nil {
 		// Try to initialize the default watcher using a basic client
 		if basicClient := i.getBasicClientForDefaultWatcher(); basicClient != nil {
 			i.defaultWatcher = revisions.NewDefaultWatcher(basicClient, "")
+			basicClient.RunAndWait(stop)
+			go i.defaultWatcher.Run(stop)
+			kube.WaitForCacheSync("default revision watcher", stop, i.defaultWatcher.HasSynced)
 		}
 	}
 
 	if i.defaultWatcher != nil {
 		if defaultRev := i.defaultWatcher.GetDefault(); defaultRev != "" {
 			return defaultRev
 		}
+		log.Warnf("default revision watcher not synced, falling back to \"default\"")
 	}
 
-	// If no default revision found, return empty string (which means default behavior)
-	return ""
+	return "default"
 }
 
 // getBasicClientForDefaultWatcher creates a basic kube client just for watching default revisions
@@ -318,7 +325,10 @@ func (f *fakeInstance) IstioNamespace() string {
 }
 
 func (f *fakeInstance) RevisionOrDefault(rev string) string {
-	// For fake instance, just return the revision as-is (no default resolution)
+	// For fake instance, return "default" if empty (consistent with real implementation fallback)
+	if rev == "" {
+		return "default"
+	}
 	return rev
 }
 

istioctl/pkg/cli/context_test.go

@@ -63,9 +63,9 @@ func TestRevisionOrDefault(t *testing.T) {
 			expectedResult: "test-revision",
 		},
 		{
-			description:    "return empty string when no revision provided and no default available",
+			description:    "return default when no revision provided and no default watcher available",
 			inputRevision:  "",
-			expectedResult: "",
+			expectedResult: "default",
 		},
 	}
 

istioctl/pkg/cli/revision_or_default_test.go

@@ -30,9 +30,9 @@ func TestRevisionOrDefaultWithMockDefaultRevision(t *testing.T) {
 			expectedResult: "test-revision",
 		},
 		{
-			description:    "return empty string when no revision provided (fake context has no default detection)",
+			description:    "return default when no revision provided (fake context has no default detection)",
 			inputRevision:  "",
-			expectedResult: "",
+			expectedResult: "default",
 		},
 	}
 
@@ -59,10 +59,10 @@ func TestRevisionOrDefaultCaching(t *testing.T) {
 		t.Fatalf("expected stable-revision, got %v", result1)
 	}
 
-	// Test that empty revisions return empty (since fake context has no default detection)
+	// Test that empty revisions return "default" (since fake context has no default detection)
 	result2 := ctx.RevisionOrDefault("")
-	if result2 != "" {
-		t.Fatalf("expected empty string, got %v", result2)
+	if result2 != "default" {
+		t.Fatalf("expected default, got %v", result2)
 	}
 
 	// Test consistency

istioctl/pkg/internaldebug/internal-debug_test.go

@@ -44,7 +44,8 @@ import (
 )
 
 type execTestCase struct {
-	args     []string
+	args []string
+	// revision should default to "default" if not set
 	revision string
 	noIstiod bool
 
@@ -83,6 +84,9 @@ func TestInternalDebug(t *testing.T) {
 	})
 
 	for i, c := range cases {
+		if c.revision == "" {
+			c.revision = "default"
+		}
 		t.Run(fmt.Sprintf("case %d %s", i, strings.Join(c.args, " ")), func(t *testing.T) {
 			ctx := cli.NewFakeContext(&cli.NewFakeContextOption{
 				IstioNamespace: "istio-system",
@@ -192,6 +196,9 @@ func TestInternalDebugWithMultiIstiod(t *testing.T) {
 	}
 
 	for _, c := range cases {
+		if c.testcase.revision == "" {
+			c.testcase.revision = "default"
+		}
 		t.Run(c.name, func(t *testing.T) {
 			defer func() {
 				getXdsResponseCall = 0
@@ -201,15 +208,16 @@ func TestInternalDebugWithMultiIstiod(t *testing.T) {
 			ctx := cli.NewFakeContext(&cli.NewFakeContextOption{
 				IstioNamespace: "istio-system",
 			})
-			client, err := ctx.CLIClient()
+			client, err := ctx.CLIClientWithRevision(c.testcase.revision)
 			require.NoError(t, err)
 
 			_, err = client.Kube().CoreV1().Pods("istio-system").Create(context.TODO(), &corev1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "istiod-test-1",
 					Namespace: "istio-system",
 					Labels: map[string]string{
-						"app": "istiod",
+						"app":                 "istiod",
+						label.IoIstioRev.Name: c.testcase.revision,
 					},
 				},
 				Status: corev1.PodStatus{
@@ -223,7 +231,8 @@ func TestInternalDebugWithMultiIstiod(t *testing.T) {
 					Name:      "istiod-test-2",
 					Namespace: "istio-system",
 					Labels: map[string]string{
-						"app": "istiod",
+						"app":                 "istiod",
+						label.IoIstioRev.Name: c.testcase.revision,
 					},
 				},
 				Status: corev1.PodStatus{

istioctl/pkg/proxystatus/proxystatus_test.go

@@ -58,7 +58,7 @@ type execTestCase struct {
 
 func TestProxyStatus(t *testing.T) {
 	cases := []execTestCase{
-		{ // case 0, with no Isitod instance
+		{ // case 0, with no Istiod instance
 			args:           []string{},
 			noIstiod:       true,
 			expectedOutput: "Error: no running Istio pods in \"istio-system\"\n",
@@ -106,6 +106,9 @@ func TestProxyStatus(t *testing.T) {
 
 	for i, c := range cases {
 		t.Run(fmt.Sprintf("case %d %s", i, strings.Join(c.args, " ")), func(t *testing.T) {
+			if c.revision == "" {
+				c.revision = "default"
+			}
 			ctx := cli.NewFakeContext(&cli.NewFakeContextOption{
 				IstioNamespace: "istio-system",
 			})

istioctl/pkg/workload/workload_test.go

@@ -400,7 +400,7 @@ func runTestCmd(t *testing.T, createResourceFunc func(client kube.CLIClient), re
 	})
 	rootCmd := Cmd(ctx)
 	rootCmd.SetArgs(args)
-	client, err := ctx.CLIClientWithRevision(rev)
+	client, err := ctx.CLIClientWithRevision(ctx.RevisionOrDefault(rev))
 	if err != nil {
 		return "", err
 	}