store configuration when ensure routes

Signed-off-by: Kuromesi <[email protected]>

pkg/controller/trafficrouting/trafficrouting_controller.go

@@ -125,8 +125,8 @@ func (r *TrafficRoutingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	case v1alpha1.TrafficRoutingPhaseFinalizing:
 		done, err = r.trafficRoutingManager.FinalisingTrafficRouting(newTrafficRoutingContext(tr), false)
 		if done {
-			newStatus.Phase = v1alpha1.TrafficRoutingPhaseInitial
-			newStatus.Message = "TrafficRouting is Initializing"
+			newStatus.Phase = v1alpha1.TrafficRoutingPhaseHealthy
+			newStatus.Message = "TrafficRouting is Healthy"
 		}
 	case v1alpha1.TrafficRoutingPhaseTerminating:
 		done, err = r.trafficRoutingManager.FinalisingTrafficRouting(newTrafficRoutingContext(tr), false)

pkg/trafficrouting/network/custom/custom.go

@@ -87,7 +87,6 @@ func NewCustomController(client client.Client, conf Config) (network.NetworkProv
 
 // when initializing, first check lua and get all custom providers, then store custom providers
 func (r *customController) Initialize(ctx context.Context) error {
-	customNetworkRefList := make([]*unstructured.Unstructured, 0)
 	for _, ref := range r.conf.TrafficConf {
 		obj := &unstructured.Unstructured{}
 		obj.SetAPIVersion(ref.APIVersion)
@@ -102,15 +101,6 @@ func (r *customController) Initialize(ctx context.Context) error {
 			klog.Errorf("failed to get lua script for custom network provider %s(%s/%s): %s", ref.Kind, r.conf.RolloutNs, ref.Name, err.Error())
 			return err
 		}
-		customNetworkRefList = append(customNetworkRefList, obj)
-	}
-	for i := 0; i < len(customNetworkRefList); i++ {
-		nObj := customNetworkRefList[i].DeepCopy()
-		err := r.storeObject(nObj)
-		if err != nil {
-			klog.Errorf("failed to store custom network provider %s(%s/%s): %s", customNetworkRefList[i].GetKind(), r.conf.RolloutNs, customNetworkRefList[i].GetName(), err.Error())
-			return err
-		}
 	}
 	return nil
 }
@@ -124,16 +114,33 @@ func (r *customController) EnsureRoutes(ctx context.Context, strategy *rolloutv1
 		return true, nil
 	}
 	var err error
-	nSpecList := make([]Data, 0)
 	customNetworkRefList := make([]*unstructured.Unstructured, 0)
-	// first execute lua for new spec
+	// first get all custom network provider object
 	for _, ref := range r.conf.TrafficConf {
 		obj := &unstructured.Unstructured{}
 		obj.SetAPIVersion(ref.APIVersion)
 		obj.SetKind(ref.Kind)
 		if err = r.Get(ctx, types.NamespacedName{Namespace: r.conf.RolloutNs, Name: ref.Name}, obj); err != nil {
 			return false, err
 		}
+		customNetworkRefList = append(customNetworkRefList, obj)
+	}
+	// check if original configuration is stored in annotation, store it if not.
+	for i := 0; i < len(customNetworkRefList); i++ {
+		obj := customNetworkRefList[i]
+		if _, ok := obj.GetAnnotations()[OriginalSpecAnnotation]; !ok {
+			err := r.storeObject(obj)
+			if err != nil {
+				klog.Errorf("failed to store custom network provider %s(%s/%s): %s", customNetworkRefList[i].GetKind(), r.conf.RolloutNs, customNetworkRefList[i].GetName(), err.Error())
+				return false, err
+			}
+		}
+	}
+	nSpecList := make([]Data, 0)
+	// first execute lua for new spec
+	for i := 0; i < len(customNetworkRefList); i++ {
+		obj := customNetworkRefList[i]
+		ref := r.conf.TrafficConf[i]
 		specStr := obj.GetAnnotations()[OriginalSpecAnnotation]
 		if specStr == "" {
 			return false, fmt.Errorf("failed to get original spec from annotation for %s(%s/%s)", ref.Kind, r.conf.RolloutNs, ref.Name)
@@ -151,7 +158,6 @@ func (r *customController) EnsureRoutes(ctx context.Context, strategy *rolloutv1
 			return false, err
 		}
 		nSpecList = append(nSpecList, nSpec)
-		customNetworkRefList = append(customNetworkRefList, obj)
 	}
 	// update CustomNetworkRefs then
 	for i := 0; i < len(nSpecList); i++ {
@@ -181,7 +187,7 @@ func (r *customController) Finalise(ctx context.Context) error {
 				klog.Infof("custom network provider %s(%s/%s) not found when finalising", ref.Kind, r.conf.RolloutNs, ref.Name)
 				continue
 			}
-			klog.Errorf("failed to get %s(%s/%s) when finalising, proccess next first", ref.Kind, r.conf.RolloutNs, ref.Name)
+			klog.Errorf("failed to get %s(%s/%s) when finalising, process next first", ref.Kind, r.conf.RolloutNs, ref.Name)
 			done = false
 			continue
 		}
@@ -228,7 +234,7 @@ func (r *customController) storeObject(obj *unstructured.Unstructured) error {
 func (r *customController) restoreObject(obj *unstructured.Unstructured) error {
 	annotations := obj.GetAnnotations()
 	if annotations == nil || annotations[OriginalSpecAnnotation] == "" {
-		klog.Infof("OriginalSpecAnnotation not found in custom network provider %s(%s/%s)")
+		klog.Infof("OriginalSpecAnnotation not found in custom network provider %s(%s/%s)", obj.GetKind(), r.conf.RolloutNs, obj.GetName())
 		return nil
 	}
 	oSpecStr := annotations[OriginalSpecAnnotation]

pkg/trafficrouting/network/custom/custom_test.go

@@ -116,11 +116,10 @@ func init() {
 
 func TestInitialize(t *testing.T) {
 	cases := []struct {
-		name               string
-		getUnstructured    func() *unstructured.Unstructured
-		getConfig          func() Config
-		getConfigMap       func() *corev1.ConfigMap
-		expectUnstructured func() *unstructured.Unstructured
+		name            string
+		getUnstructured func() *unstructured.Unstructured
+		getConfig       func() Config
+		getConfigMap    func() *corev1.ConfigMap
 	}{
 		{
 			name: "test1, find lua script locally",
@@ -153,23 +152,13 @@ func TestInitialize(t *testing.T) {
 					},
 				}
 			},
-			expectUnstructured: func() *unstructured.Unstructured {
-				u := &unstructured.Unstructured{}
-				_ = u.UnmarshalJSON([]byte(virtualServiceDemo))
-				annotations := map[string]string{
-					OriginalSpecAnnotation: `{"spec":{"hosts":["echoserver.example.com"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]},"annotations":{"virtual":"test"}}`,
-					"virtual":              "test",
-				}
-				u.SetAnnotations(annotations)
-				return u
-			},
 		},
 		{
 			name: "test2, find lua script in ConfigMap",
 			getUnstructured: func() *unstructured.Unstructured {
 				u := &unstructured.Unstructured{}
 				_ = u.UnmarshalJSON([]byte(virtualServiceDemo))
-				u.SetAPIVersion("networking.istio.io/v1alpha3")
+				u.SetAPIVersion("networking.test.io/v1alpha3")
 				return u
 			},
 			getConfig: func() Config {
@@ -178,7 +167,7 @@ func TestInitialize(t *testing.T) {
 					CanaryService: "echoserver-canary",
 					TrafficConf: []rolloutsv1alpha1.CustomNetworkRef{
 						{
-							APIVersion: "networking.istio.io/v1alpha3",
+							APIVersion: "networking.test.io/v1alpha3",
 							Kind:       "VirtualService",
 							Name:       "echoserver",
 						},
@@ -192,21 +181,10 @@ func TestInitialize(t *testing.T) {
 						Namespace: util.GetRolloutNamespace(),
 					},
 					Data: map[string]string{
-						fmt.Sprintf("%s.%s.%s", configuration.LuaTrafficRoutingIngressTypePrefix, "VirtualService", "networking.istio.io"): "ExpectedLuaScript",
+						fmt.Sprintf("%s.%s.%s", configuration.LuaTrafficRoutingCustomTypePrefix, "VirtualService", "networking.test.io"): "ExpectedLuaScript",
 					},
 				}
 			},
-			expectUnstructured: func() *unstructured.Unstructured {
-				u := &unstructured.Unstructured{}
-				_ = u.UnmarshalJSON([]byte(virtualServiceDemo))
-				u.SetAPIVersion("networking.istio.io/v1alpha3")
-				annotations := map[string]string{
-					OriginalSpecAnnotation: `{"spec":{"hosts":["echoserver.example.com"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]},"annotations":{"virtual":"test"}}`,
-					"virtual":              "test",
-				}
-				u.SetAnnotations(annotations)
-				return u
-			},
 		},
 	}
 
@@ -226,7 +204,6 @@ func TestInitialize(t *testing.T) {
 			if err != nil {
 				t.Fatalf("Initialize failed: %s", err.Error())
 			}
-			checkEqual(fakeCli, t, cs.expectUnstructured())
 		})
 	}
 }
@@ -422,7 +399,7 @@ func TestEnsureRoutes(t *testing.T) {
 			if !expectHasError && err != nil {
 				t.Fatalf("EnsureRoutes failed: %s", err.Error())
 			} else if expectHasError && err == nil {
-				t.Fatalf("expect error occured but not")
+				t.Fatalf("expect error occurred but not")
 			} else if done != expectDone {
 				t.Fatalf("expect(%v), but get(%v)", expectDone, done)
 			}