Add asm-ready as the status signal of asm mode.

cmd/e2e-test/asm_test.go

@@ -30,27 +30,32 @@ func TestASMConfig(t *testing.T) {
 		for _, tc := range []struct {
 			desc                string
 			configMap           map[string]string
+			wantASMReady        bool
 			wantConfigMapEvents []string
 		}{
 			{
 				desc:                "Invalid ConfigMap value equals to disable",
 				configMap:           map[string]string{"enable-asm": "INVALID"},
+				wantASMReady:        false,
 				wantConfigMapEvents: []string{"The map provided a unvalid value for field: enable-asm, value: INVALID"},
 			},
 			{
 				desc:                "Invalid ConfigMap filed equals to disable",
 				configMap:           map[string]string{"enable-unknow-feild": "INVALID1"},
+				wantASMReady:        false,
 				wantConfigMapEvents: []string{"The map contains a unknown key-value pair: enable-unknow-feild:INVALID1"},
 			},
 			{
-				desc:                "Set enable-asm to true should restart the controller",
-				configMap:           map[string]string{"enable-asm": "true"},
-				wantConfigMapEvents: []string{"ConfigMapConfigController: Get a update on the ConfigMapConfig, Restarting Ingress controller"},
+				desc:         "Set enable-asm to true should restart the controller",
+				configMap:    map[string]string{"enable-asm": "true"},
+				wantASMReady: true,
+				wantConfigMapEvents: []string{"ConfigMapConfigController: Get a update on the ConfigMapConfig, Restarting Ingress controller",
+					"NEG controller is running in ASM Mode with Istio API: v1alpha3"},
 			},
-			// TODO(koonwah): The below case is not fully tested, should update the neg controller to include a enable-asm event to the target CM.
 			{
-				desc:      "Invalid ConfigMap value equals to disable",
-				configMap: map[string]string{"enable-asm": "INVALID2"},
+				desc:         "Invalid ConfigMap value equals to disable",
+				configMap:    map[string]string{"enable-asm": "INVALID2"},
+				wantASMReady: false,
 				wantConfigMapEvents: []string{"The map provided a unvalid value for field: enable-asm, value: INVALID2",
 					"ConfigMapConfigController: Get a update on the ConfigMapConfig, Restarting Ingress controller"},
 			},
@@ -59,10 +64,26 @@ func TestASMConfig(t *testing.T) {
 			if err := e2e.EnsureConfigMap(s, asmConfigNamespace, asmConfigName, tc.configMap); err != nil {
 				t.Errorf("Failed to ensure ConfigMap, error: %s", err)
 			}
+
+			if err := wait.Poll(5*time.Second, 3*time.Minute, func() (bool, error) {
+				cmData, err := e2e.GetConfigMap(s, asmConfigNamespace, asmConfigName)
+				if err != nil {
+					return false, err
+				}
+				if val, ok := cmData["asm-ready"]; ok {
+					return val == strconv.FormatBool(tc.wantASMReady), nil
+				}
+				return false, nil
+
+			}); err != nil {
+				t.Fatalf("Failed to validate asm-ready = %s. Error: %s", strconv.FormatBool(tc.wantASMReady), err)
+			}
+
 			if err := e2e.WaitConfigMapEvents(s, asmConfigNamespace, asmConfigName, tc.wantConfigMapEvents, negControllerRestartTimeout); err != nil {
 				t.Fatalf("Failed to get events: %v; Error %e", strings.Join(tc.wantConfigMapEvents, ";"), err)
 			}
 		}
+		e2e.DeleteConfigMap(s, asmConfigNamespace, asmConfigName)
 	})
 }
 
@@ -229,13 +250,13 @@ func TestNoIstioASM(t *testing.T) {
 			if err != nil {
 				return false, err
 			}
-			if val, ok := cmData["enable-asm"]; ok && val == "false" {
+			if val, ok := cmData["asm-ready"]; ok && val == "false" {
 				return true, nil
 			}
-			return false, fmt.Errorf("ConfigMap: %s/%s, nable-asm is not false. Value: %v", asmConfigNamespace, asmConfigName, cmData)
+			return false, fmt.Errorf("ConfigMap: %s/%s, asm-ready is not false. Value: %v", asmConfigNamespace, asmConfigName, cmData)
 
 		}); err != nil {
-			t.Fatalf("Failed to validate enable-asm = false. Error: %s", err)
+			t.Fatalf("Failed to validate asm-ready = false. Error: %s", err)
 		}
 
 	})

pkg/cmconfig/config.go

@@ -11,6 +11,7 @@ import (
 // Config holds configmap based configurations.
 type Config struct {
 	EnableASM                   bool
+	ASMReady                    bool
 	ASMServiceNEGSkipNamespaces []string
 }
 
@@ -19,6 +20,7 @@ const (
 	falseValue = "false"
 
 	enableASM         = "enable-asm"
+	asmReady          = "asm-ready"
 	asmSkipNamespaces = "asm-skip-namespaces"
 )
 
@@ -29,7 +31,7 @@ func NewConfig() Config {
 
 // Equals returns true if c equals to other.
 func (c *Config) Equals(other *Config) bool {
-	return reflect.DeepEqual(c, other)
+	return c.EnableASM == other.EnableASM && reflect.DeepEqual(c.ASMServiceNEGSkipNamespaces, other.ASMServiceNEGSkipNamespaces)
 }
 
 // LoadValue loads configs from a map, it will ignore any unknow/unvalid field.
@@ -46,6 +48,9 @@ func (c *Config) LoadValue(m map[string]string) error {
 			}
 		} else if k == asmSkipNamespaces {
 			c.ASMServiceNEGSkipNamespaces = strings.Split(v, ",")
+		} else if k == asmReady {
+			// Ignore this because it's a status.
+			continue
 		} else {
 			errList = append(errList, fmt.Errorf("The map contains a unknown key-value pair: %s:%s", k, v))
 		}

pkg/cmconfig/config_test.go

@@ -31,6 +31,12 @@ func TestLoadValue(t *testing.T) {
 			wantConfig: Config{EnableASM: false, ASMServiceNEGSkipNamespaces: []string{"kube-system", "istio-system"}},
 			wantLog:    "The map provided a unvalid value for field: enable-asm, value: f, valid values are: true/false",
 		},
+		{
+			desc:       "LoadValue and compare func should ignore the asm-ready field",
+			inputMap:   map[string]string{"enable-asm": "f", "asm-ready": "true"},
+			wantConfig: Config{EnableASM: false, ASMServiceNEGSkipNamespaces: []string{"kube-system", "istio-system"}},
+			wantLog:    "",
+		},
 		{
 			desc:       "LoadValue should be tolerant for unknow field.",
 			inputMap:   map[string]string{"A": "B"},

pkg/cmconfig/controller.go

@@ -63,21 +63,38 @@ func (c *ConfigMapConfigController) GetConfig() Config {
 	return *c.currentConfig
 }
 
-// DisableASMMode disables the ASM Mode by updating the ConfigMap and setting the internal flag.
-func (c *ConfigMapConfigController) DisableASMMode() {
-	patchBytes, err := utils.StrategicMergePatchBytes(v1.ConfigMap{Data: map[string]string{enableASM: trueValue}},
-		v1.ConfigMap{Data: map[string]string{enableASM: falseValue}}, v1.ConfigMap{})
+func (c *ConfigMapConfigController) updateASMReady(status string) {
+	patchBytes, err := utils.StrategicMergePatchBytes(v1.ConfigMap{Data: map[string]string{}},
+		v1.ConfigMap{Data: map[string]string{asmReady: status}}, v1.ConfigMap{})
 	if err != nil {
-		c.RecordEvent("Warning", "FailedDisableASMMode", fmt.Sprintf("Failed to disable ASM Mode, failed to create patch for ASM ConfigMap, error: %s", err))
+		c.RecordEvent("Warning", "FailedToUpdateASMStatus", fmt.Sprintf("Failed to update ASM Status, failed to create patch for ASM ConfigMap, error: %s", err))
 		return
 	}
 	cm, err := c.kubeClient.CoreV1().ConfigMaps(c.configMapNamespace).Patch(c.configMapName, apimachinerytypes.MergePatchType, patchBytes, "")
 	if err != nil {
-		c.RecordEvent("Warning", "FailedDisableASMMode", fmt.Sprintf("Failed to patch ASM ConfigMap, error: %s", err))
+		if errors.IsNotFound(err) {
+			return
+		}
+		c.RecordEvent("Warning", "FailedToUpdateASMStatus", fmt.Sprintf("Failed to patch ASM ConfigMap, error: %s", err))
 		return
 	}
 	c.currentConfigMapObject = cm
+}
+
+// DisableASM sets the internal ASM mode to off and update the ASMReady to False.
+func (c *ConfigMapConfigController) DisableASM() {
 	c.currentConfig.EnableASM = false
+	c.updateASMReady(falseValue)
+}
+
+// SetASMReadyTrue update the ASMReady to True.
+func (c *ConfigMapConfigController) SetASMReadyTrue() {
+	c.updateASMReady(trueValue)
+}
+
+// SetASMReadyFalse update the ASMReady to False.
+func (c *ConfigMapConfigController) SetASMReadyFalse() {
+	c.updateASMReady(falseValue)
 }
 
 // RecordEvent records a event to the ASMConfigmap
@@ -133,5 +150,12 @@ func (c *ConfigMapConfigController) processItem(obj interface{}, cancel func())
 	if !config.Equals(c.currentConfig) {
 		c.RecordEvent("Normal", "ASMConfigMapTiggerRestart", "ConfigMapConfigController: Get a update on the ConfigMapConfig, Restarting Ingress controller")
 		cancel()
+	} else {
+		// If the config has no change, make sure the ASMReady is updated.
+		if config.EnableASM {
+			c.SetASMReadyTrue()
+		} else {
+			c.SetASMReadyFalse()
+		}
 	}
 }

pkg/cmconfig/controller_test.go

@@ -9,7 +9,7 @@ import (
 	"bytes"
 	"os"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	informerv1 "k8s.io/client-go/informers/core/v1"
 	"k8s.io/client-go/kubernetes/fake"

pkg/context/context.go

@@ -147,6 +147,8 @@ func (ctx *ControllerContext) Init() {
 		cmConfig := ctx.ASMConfigController.GetConfig()
 		if cmConfig.EnableASM {
 			ctx.initEnableASM()
+		} else {
+			ctx.ASMConfigController.SetASMReadyFalse()
 		}
 	}
 
@@ -165,7 +167,7 @@ func (ctx *ControllerContext) initEnableASM() {
 	if err != nil {
 		msg := fmt.Sprintf("Failed to create ApiextensionClient for DestinationRule, disabling ASM Mode, error: %s", err)
 		ctx.ASMConfigController.RecordEvent("Warning", "FailedValidateDestinationRuleCRD", msg)
-		ctx.ASMConfigController.DisableASMMode()
+		ctx.ASMConfigController.DisableASM()
 		return
 	}
 	destinationRuleCRD, err := apiextensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(destinationRuleCRDName, metav1.GetOptions{})
@@ -175,22 +177,22 @@ func (ctx *ControllerContext) initEnableASM() {
 		} else {
 			ctx.ASMConfigController.RecordEvent("Warning", "FailedValidateDestinationRuleCRD", fmt.Sprintf("Failed to load DestinationRule CRD, disabling the ASM Mode, please check Istio setup. Error: %s", err))
 		}
-		ctx.ASMConfigController.DisableASMMode()
+		ctx.ASMConfigController.DisableASM()
 		return
 	}
 	if destinationRuleCRD.Spec.Version != destinationRuleAPIVersion {
 		ctx.ASMConfigController.RecordEvent("Warning", "FailedValidateDestinationRuleCRD", fmt.Sprintf("Only Support Istio API: %s, but found %s, disabling the ASM Mode, please check Istio setup.",
 			destinationRuleAPIVersion, destinationRuleCRD.Spec.Version))
-		ctx.ASMConfigController.DisableASMMode()
+		ctx.ASMConfigController.DisableASM()
 		return
 	}
 
 	dynamicClient, err := dynamic.NewForConfig(ctx.KubeConfig)
 	if err != nil {
 		msg := fmt.Sprintf("Failed to create kubernetes dynamic client, disabling ASM Mode, please retry. Error: %v", err)
-		klog.Fatalf(msg)
+		klog.Error(msg)
 		ctx.ASMConfigController.RecordEvent("Warning", "FailedCreateDynamicClient", msg)
-		ctx.ASMConfigController.DisableASMMode()
+		ctx.ASMConfigController.DisableASM()
 		return
 	}
 
@@ -201,6 +203,8 @@ func (ctx *ControllerContext) initEnableASM() {
 		nil)
 	ctx.DestinationRuleInformer = drDynamicInformer.Informer()
 	ctx.DestinationRuleClient = dynamicClient.Resource(destrinationGVR)
+	ctx.ASMConfigController.RecordEvent("Normal", "ASMModeOn", fmt.Sprintf("NEG controller is running in ASM Mode with Istio API: %s.", destinationRuleAPIVersion))
+	ctx.ASMConfigController.SetASMReadyTrue()
 }
 
 // HasSynced returns true if all relevant informers has been synced.

pkg/e2e/helpers.go

@@ -588,7 +588,7 @@ func WaitConfigMapEvents(s *Sandbox, namespace, name string, msgs []string, time
 		for _, event := range events[len(events)-len(msgs):] {
 			allMsg += event.Message
 		}
-		klog.Infof("WaitDestinationRuleAnnotation, allMsg: %s, want: %v", allMsg, msgs)
+		klog.Infof("WaitConfigMapEvents, allMsg: %s, want: %v", allMsg, msgs)
 
 		for _, msg := range msgs {
 			if !strings.Contains(allMsg, msg) {