fix: the event order of sync (#17)

* fix: the event order of sync

* fix: code style

* fix: order

* fix: review

* fix: ci

cmd/sync.go

@@ -6,6 +6,7 @@ package cmd
 import (
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/fatih/color"
 	"github.com/spf13/cobra"
@@ -93,6 +94,7 @@ func sync(cmd *cobra.Command, dryRun bool) error {
 				color.Red("Failed to apply: %v", err)
 				return err
 			}
+			time.Sleep(100 * time.Millisecond)
 		}
 
 		for _, line := range strings.Split(str, "\n") {

internal/pkg/differ/differ.go

@@ -1,13 +1,36 @@
 package differ
 
 import (
+	"fmt"
 	"reflect"
+	"sort"
 
 	"github.com/api7/adc/internal/pkg/db"
 	"github.com/api7/adc/pkg/api/apisix/types"
 	"github.com/api7/adc/pkg/data"
 )
 
+func _key(typ data.ResourceType, option int) string {
+	return fmt.Sprintf("%s:%d", typ, option)
+}
+
+// Since the routes is related to the services, we need to sort the events.
+// The order is:
+// 1. Services Create
+// 2. Routes Create
+// 3. Services Update
+// 4. Routes Update
+// 5. Routes Delete
+// 6. Services Delete
+var order = map[string]int{
+	_key(data.ServiceResourceType, data.CreateOption): 6,
+	_key(data.RouteResourceType, data.CreateOption):   5,
+	_key(data.ServiceResourceType, data.UpdateOption): 4,
+	_key(data.RouteResourceType, data.UpdateOption):   3,
+	_key(data.RouteResourceType, data.DeleteOption):   2,
+	_key(data.ServiceResourceType, data.DeleteOption): 1,
+}
+
 // Differ is the object of comparing two configurations.
 type Differ struct {
 	localDB      *db.DB
@@ -31,6 +54,12 @@ func NewDiffer(local, remote *types.Configuration) (*Differ, error) {
 	}, nil
 }
 
+func sortEvents(events []*data.Event) {
+	sort.Slice(events, func(i, j int) bool {
+		return order[_key(events[i].ResourceType, events[i].Option)] > order[_key(events[j].ResourceType, events[j].Option)]
+	})
+}
+
 // Diff compares the local configuration and remote configuration, and returns the events.
 func (d *Differ) Diff() ([]*data.Event, error) {
 	var events []*data.Event
@@ -48,6 +77,8 @@ func (d *Differ) Diff() ([]*data.Event, error) {
 	events = append(events, serviceEvents...)
 	events = append(events, routeEvents...)
 
+	sortEvents(events)
+
 	return events, nil
 }
 

internal/pkg/differ/differ_test.go

@@ -38,6 +38,64 @@ var (
 	}
 )
 
+func TestSortEvents(t *testing.T) {
+	events := []*data.Event{
+		{
+			ResourceType: data.RouteResourceType,
+			Option:       data.CreateOption,
+		},
+		{
+			ResourceType: data.ServiceResourceType,
+			Option:       data.CreateOption,
+		},
+		{
+			ResourceType: data.RouteResourceType,
+			Option:       data.DeleteOption,
+		},
+		{
+			ResourceType: data.ServiceResourceType,
+			Option:       data.DeleteOption,
+		},
+		{
+			ResourceType: data.RouteResourceType,
+			Option:       data.UpdateOption,
+		},
+		{
+			ResourceType: data.ServiceResourceType,
+			Option:       data.UpdateOption,
+		},
+	}
+
+	sortEvents(events)
+	assert.Equal(t, []*data.Event{
+		{
+			ResourceType: data.ServiceResourceType,
+			Option:       data.CreateOption,
+		},
+		{
+			ResourceType: data.RouteResourceType,
+			Option:       data.CreateOption,
+		},
+		{
+			ResourceType: data.ServiceResourceType,
+			Option:       data.UpdateOption,
+		},
+		{
+			ResourceType: data.RouteResourceType,
+			Option:       data.UpdateOption,
+		},
+		{
+			ResourceType: data.RouteResourceType,
+			Option:       data.DeleteOption,
+		},
+		{
+			ResourceType: data.ServiceResourceType,
+			Option:       data.DeleteOption,
+		},
+	}, events, "check the content of sorted events")
+
+}
+
 func TestDiff(t *testing.T) {
 	// Test case 1: delete events
 	localConfig := &types.Configuration{
@@ -58,19 +116,19 @@ func TestDiff(t *testing.T) {
 	assert.Equal(t, 3, len(events), "check the number of delete events")
 	assert.Equal(t, []*data.Event{
 		{
-			ResourceType: data.ServiceResourceType,
-			Option:       data.DeleteOption,
-			OldValue:     svc,
+			ResourceType: data.RouteResourceType,
+			Option:       data.CreateOption,
+			Value:        route,
 		},
 		{
 			ResourceType: data.RouteResourceType,
 			Option:       data.DeleteOption,
 			OldValue:     &route1,
 		},
 		{
-			ResourceType: data.RouteResourceType,
-			Option:       data.CreateOption,
-			Value:        route,
+			ResourceType: data.ServiceResourceType,
+			Option:       data.DeleteOption,
+			OldValue:     svc,
 		},
 	}, events, "check the content of delete events")
 }

pkg/api/apisix/resource_client.go

@@ -109,7 +109,7 @@ func (u *resourceClient[T]) Update(ctx context.Context, id string, obj *T) (*T,
 	return svc, err
 }
 
-func getResourceNameOrID(resource interface{}) string {
+func GetResourceNameOrID(resource interface{}) string {
 	value := reflect.ValueOf(resource)
 	value = reflect.Indirect(value)
 	nameOrID := value.FieldByName("Name")
@@ -122,7 +122,7 @@ func getResourceNameOrID(resource interface{}) string {
 func (u *resourceClient[T]) Validate(ctx context.Context, resource *T) error {
 	err := u.client.validate(ctx, u.validateURL, resource)
 	if err != nil {
-		return fmt.Errorf("failed to validate resource '%s (%s)': %s", u.resourceName, getResourceNameOrID(resource), err.Error())
+		return fmt.Errorf("failed to validate resource '%s (%s)': %s", u.resourceName, GetResourceNameOrID(resource), err.Error())
 	}
 	return nil
 }

pkg/data/events.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"reflect"
 
 	"github.com/hexops/gotextdiff"
 	"github.com/hexops/gotextdiff/myers"
@@ -42,14 +41,6 @@ type Event struct {
 	Value        interface{}  `json:"value"`
 }
 
-func getName(field string, value interface{}) string {
-	v := reflect.ValueOf(value)
-	if v.Kind() == reflect.Ptr {
-		v = v.Elem()
-	}
-	return v.FieldByName(field).String()
-}
-
 // Output returns the output of event,
 // if the event is create, it will return the message of creating resource.
 // if the event is update, it will return the diff of old value and new value.
@@ -58,9 +49,9 @@ func (e *Event) Output() (string, error) {
 	var output string
 	switch e.Option {
 	case CreateOption:
-		output = fmt.Sprintf("creating %s: \"%s\"", e.ResourceType, getName("Name", e.Value))
+		output = fmt.Sprintf("creating %s: \"%s\"", e.ResourceType, apisix.GetResourceNameOrID(e.Value))
 	case DeleteOption:
-		output = fmt.Sprintf("deleting %s: \"%s\"", e.ResourceType, getName("Name", e.OldValue))
+		output = fmt.Sprintf("deleting %s: \"%s\"", e.ResourceType, apisix.GetResourceNameOrID(e.OldValue))
 	case UpdateOption:
 		remote, err := json.MarshalIndent(e.OldValue, "", "\t")
 		if err != nil {
@@ -76,36 +67,36 @@ func (e *Event) Output() (string, error) {
 
 		edits := myers.ComputeEdits(span.URIFromPath("remote"), string(remote), string(local))
 		diff := fmt.Sprint(gotextdiff.ToUnified("remote", "local", string(remote), edits))
-		output = fmt.Sprintf("updating %s: \"%s\"\n%s", e.ResourceType, getName("Name", e.Value), diff)
+		output = fmt.Sprintf("updating %s: \"%s\"\n%s", e.ResourceType, apisix.GetResourceNameOrID(e.Value), diff)
 	}
 
 	return output, nil
 }
 
-func applyService(cluster apisix.Cluster, option int, value interface{}) error {
+func applyService(cluster apisix.Cluster, event *Event) error {
 	var err error
-	switch option {
+	switch event.Option {
 	case CreateOption:
-		_, err = cluster.Service().Create(context.Background(), value.(*types.Service))
+		_, err = cluster.Service().Create(context.Background(), event.Value.(*types.Service))
 	case DeleteOption:
-		err = cluster.Service().Delete(context.Background(), getName("Name", value))
+		err = cluster.Service().Delete(context.Background(), apisix.GetResourceNameOrID(event.OldValue))
 	case UpdateOption:
-		_, err = cluster.Service().Update(context.Background(), value.(*types.Service))
+		_, err = cluster.Service().Update(context.Background(), event.Value.(*types.Service))
 		return err
 	}
 
 	return errors.Wrap(err, "failed to apply service")
 }
 
-func applyRoute(cluster apisix.Cluster, option int, value interface{}) error {
+func applyRoute(cluster apisix.Cluster, event *Event) error {
 	var err error
-	switch option {
+	switch event.Option {
 	case CreateOption:
-		_, err = cluster.Route().Create(context.Background(), value.(*types.Route))
+		_, err = cluster.Route().Create(context.Background(), event.Value.(*types.Route))
 	case DeleteOption:
-		err = cluster.Route().Delete(context.Background(), getName("Name", value))
+		err = cluster.Route().Delete(context.Background(), apisix.GetResourceNameOrID(event.OldValue))
 	case UpdateOption:
-		_, err = cluster.Route().Update(context.Background(), value.(*types.Route))
+		_, err = cluster.Route().Update(context.Background(), event.Value.(*types.Route))
 	}
 
 	return errors.Wrap(err, "failed to apply route")
@@ -114,9 +105,9 @@ func applyRoute(cluster apisix.Cluster, option int, value interface{}) error {
 func (e *Event) Apply(cluster apisix.Cluster) error {
 	switch e.ResourceType {
 	case ServiceResourceType:
-		return applyService(cluster, e.Option, e.Value)
+		return applyService(cluster, e)
 	case RouteResourceType:
-		return applyRoute(cluster, e.Option, e.Value)
+		return applyRoute(cluster, e)
 	}
 
 	return nil

test/cli/suites/diff.go

@@ -18,8 +18,10 @@ var _ = ginkgo.Describe("`adc diff` tests", func() {
 			err := cmd.Run()
 			gomega.Expect(err).To(gomega.BeNil())
 			gomega.Expect(out.String()).To(gomega.Equal(`creating service: "svc1"
+creating service: "svc2"
 creating route: "route1"
-Summary: created 2, updated 0, deleted 0
+creating route: "route2"
+Summary: created 4, updated 0, deleted 0
 `))
 		})
 	})

test/cli/suites/dump.go

@@ -10,7 +10,7 @@ import (
 
 var _ = ginkgo.Describe("`adc dump` tests", func() {
 	ginkgo.Context("Test the dump command", func() {
-		_ = NewScaffold()
+		s := NewScaffold()
 		ginkgo.It("should dump APISIX resources", func() {
 			var syncOutput bytes.Buffer
 			cmd := exec.Command("adc", "sync", "-f", "testdata/test.yaml")
@@ -28,12 +28,19 @@ routes:
 - id: route1
   methods:
   - GET
+  - PUT
   name: route1
   service_id: svc1
   uri: /get
+- id: route2
+  methods:
+  - GET
+  name: route2
+  service_id: svc2
+  uri: /get
 services:
 - hosts:
-  - foo.com
+  - foo1.com
   id: svc1
   name: svc1
   upstream:
@@ -46,8 +53,30 @@ services:
       weight: 1
     scheme: http
     type: roundrobin
+- hosts:
+  - svc.com
+  id: svc2
+  name: svc2
+  upstream:
+    hash_on: vars
+    id: httpbin
+    name: httpbin
+    nodes:
+    - host: httpbin
+      port: 80
+      weight: 1
+    scheme: http
+    type: roundrobin
 version: ""
 `))
+
+			err = s.DeleteRoute("route1")
+			gomega.Expect(err).To(gomega.BeNil(), "check route delete")
+			err = s.DeleteRoute("route2")
+			gomega.Expect(err).To(gomega.BeNil(), "check route delete")
+			err = s.DeleteService("svc1")
+			gomega.Expect(err).To(gomega.BeNil(), "check service delete")
+			err = s.DeleteService("svc2")
 		})
 	})
 })

test/cli/suites/scaffold.go

@@ -9,6 +9,7 @@ import (
 	"github.com/onsi/gomega"
 
 	"github.com/api7/adc/pkg/api/apisix"
+	"github.com/api7/adc/pkg/api/apisix/types"
 )
 
 type Scaffold struct {
@@ -32,6 +33,22 @@ func NewScaffold() *Scaffold {
 	}
 }
 
+func (s *Scaffold) CreateRoute(route *types.Route) (*types.Route, error) {
+	return s.cluster.Route().Create(context.Background(), route)
+}
+
+func (s *Scaffold) UpdateRoute(route *types.Route) (*types.Route, error) {
+	return s.cluster.Route().Update(context.Background(), route)
+}
+
+func (s *Scaffold) CreateService(service *types.Service) (*types.Service, error) {
+	return s.cluster.Service().Create(context.Background(), service)
+}
+
+func (s *Scaffold) UpdateService(service *types.Service) (*types.Service, error) {
+	return s.cluster.Service().Update(context.Background(), service)
+}
+
 func (s *Scaffold) DeleteService(id string) error {
 	return s.cluster.Service().Delete(context.Background(), id)
 }