Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: updating httproute may generate redundant resources #49

Draft
wants to merge 2 commits into
base: release-v2-dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/conformance-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ jobs:
make conformance-test

- name: Upload Gateway API Conformance Report
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@v4
with:
name: api7-ingress-controller-conformance-report.yaml
path: api7-ingress-controller-conformance-report.yaml
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ENVTEST_K8S_VERSION = 1.30.0
KIND_NAME ?= api7-ingress-cluster
GATEAY_API_VERSION ?= v1.1.0

DASHBOARD_VERSION ?= v3.2.14.2
DASHBOARD_VERSION ?= v3.2.14.6

# go
VERSYM="github.com/api7/api7-ingress-controller/internal/version._buildVersion"
Expand Down Expand Up @@ -90,7 +90,7 @@ e2e-test:

.PHONY: conformance-test
conformance-test:
DASHBOARD_VERSION=v3.2.14.2 go test -v ./test/conformance -tags=conformance
DASHBOARD_VERSION=$(DASHBOARD_VERSION) go test -v ./test/conformance -tags=conformance

.PHONY: lint
lint: golangci-lint ## Run golangci-lint linter
Expand Down
92 changes: 70 additions & 22 deletions internal/controlplane/controlplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@ package controlplane
import (
"context"

"go.uber.org/zap"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"

"github.com/api7/api7-ingress-controller/internal/controller/config"
"github.com/api7/api7-ingress-controller/internal/controlplane/translator"
"github.com/api7/api7-ingress-controller/pkg/dashboard"
"github.com/api7/gopkg/pkg/log"
)

type Controlplane interface {
Expand Down Expand Up @@ -38,8 +41,10 @@ func NewDashboard() (Controlplane, error) {
}

return &dashboardClient{
translator: &translator.Translator{},
c: control,
translator: &translator.Translator{
Log: ctrl.Log.WithName("controlplane").WithName("translator"),
},
c: control,
}, nil
}

Expand All @@ -53,15 +58,63 @@ func (d *dashboardClient) Update(ctx context.Context, tctx *translator.Translate
if err != nil {
return err
}
// TODO: support diff resources
name := "default"
for _, service := range result.Services {
if _, err := d.c.Cluster(name).Service().Update(ctx, service); err != nil {

clusterName := "default"

kind := obj.GetObjectKind().GroupVersionKind().Kind
namespace := obj.GetNamespace()
name := obj.GetName()

KindLabel := dashboard.ListByKindLabelOptions{
Kind: kind,
Namespace: namespace,
Name: name,
}

routes, err := d.c.Cluster(clusterName).Route().List(ctx, dashboard.ListOptions{
From: dashboard.ListFromCache,
KindLabel: KindLabel,
})
log.Infow("route", zap.Any("route", routes))
if err != nil && err != dashboard.ErrNotFound {
return err
}

service, err := d.c.Cluster(clusterName).Service().List(ctx, dashboard.ListOptions{
From: dashboard.ListFromCache,
KindLabel: KindLabel,
})
log.Infow("service", zap.Any("service", service))
if err != nil && err != dashboard.ErrNotFound {
return err
}

// Delete the routes and services that are not in the result
for _, route := range routes {
if _, ok := result.RouteMap[route.Name]; ok {
continue
}
if err := d.c.Cluster(clusterName).Route().Delete(ctx, route); err != nil {
return err
}
}
for _, route := range result.Routes {
if _, err := d.c.Cluster(name).Route().Update(ctx, route); err != nil {

for _, service := range service {
if _, ok := result.ServiceMap[service.Name]; ok {
continue
}
if err := d.c.Cluster(clusterName).Service().Delete(ctx, service); err != nil {
return err
}
}

for _, service := range result.ServiceMap {
if _, err := d.c.Cluster(clusterName).Service().Update(ctx, service); err != nil {
return err
}
}
for _, route := range result.RouteMap {
if _, err := d.c.Cluster(clusterName).Route().Update(ctx, route); err != nil {
return err
}
}
Expand All @@ -70,15 +123,15 @@ func (d *dashboardClient) Update(ctx context.Context, tctx *translator.Translate

func (d *dashboardClient) Delete(ctx context.Context, obj client.Object) error {
clusters := d.c.ListClusters()
kindLabel := dashboard.ListByKindLabelOptions{
Kind: obj.GetObjectKind().GroupVersionKind().Kind,
Namespace: obj.GetNamespace(),
Name: obj.GetName(),
}
for _, cluster := range clusters {
routes, _ := cluster.Route().List(ctx, dashboard.ListOptions{
From: dashboard.ListFromCache,
Args: []interface{}{
"label",
obj.GetObjectKind().GroupVersionKind().Kind,
obj.GetNamespace(),
obj.GetName(),
},
From: dashboard.ListFromCache,
KindLabel: kindLabel,
})

for _, route := range routes {
Expand All @@ -88,13 +141,8 @@ func (d *dashboardClient) Delete(ctx context.Context, obj client.Object) error {
}

services, _ := cluster.Service().List(ctx, dashboard.ListOptions{
From: dashboard.ListFromCache,
Args: []interface{}{
"label",
obj.GetObjectKind().GroupVersionKind().Kind,
obj.GetNamespace(),
obj.GetName(),
},
From: dashboard.ListFromCache,
KindLabel: kindLabel,
})

for _, service := range services {
Expand Down
6 changes: 3 additions & 3 deletions internal/controlplane/translator/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (t *Translator) translateBackendRef(tctx *TranslateContext, ref gatewayv1.B
}

func (t *Translator) TranslateGatewayHTTPRoute(tctx *TranslateContext, httpRoute *gatewayv1.HTTPRoute) (*TranslateResult, error) {
result := &TranslateResult{}
result := NewTranslateResult()

hosts := make([]string, 0, len(httpRoute.Spec.Hostnames))
for _, hostname := range httpRoute.Spec.Hostnames {
Expand Down Expand Up @@ -248,7 +248,7 @@ func (t *Translator) TranslateGatewayHTTPRoute(tctx *TranslateContext, httpRoute
service.Hosts = hosts
service.Plugins = t.generatePluginsFromHTTPRouteFilters(httpRoute.GetNamespace(), rule.Filters)

result.Services = append(result.Services, service)
result.ServiceMap[service.ID] = service

matches := rule.Matches
if len(matches) == 0 {
Expand All @@ -275,7 +275,7 @@ func (t *Translator) TranslateGatewayHTTPRoute(tctx *TranslateContext, httpRoute
route.ID = id.GenID(name)
route.Labels = label.GenLabel(httpRoute)
route.ServiceID = service.ID
result.Routes = append(result.Routes, route)
result.RouteMap[route.ID] = route
}
}

Expand Down
17 changes: 15 additions & 2 deletions internal/controlplane/translator/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ type TranslateContext struct {
}

type TranslateResult struct {
Routes []*v1.Route
Services []*v1.Service
RouteMap map[string]*v1.Route
ServiceMap map[string]*v1.Service
}

func NewTranslator(log logr.Logger) *Translator {
return &Translator{
Log: log,
}
}

func NewTranslateResult() *TranslateResult {
return &TranslateResult{
RouteMap: make(map[string]*v1.Route),
ServiceMap: make(map[string]*v1.Service),
}
}
21 changes: 20 additions & 1 deletion pkg/dashboard/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,30 @@ func (r *routeClient) Get(ctx context.Context, name string) (*v1.Route, error) {

// List is only used in cache warming up. So here just pass through
// to APISIX.
func (r *routeClient) List(ctx context.Context, args ...interface{}) ([]*v1.Route, error) {
func (r *routeClient) List(ctx context.Context, listOptions ...interface{}) ([]*v1.Route, error) {

var options ListOptions
if len(listOptions) > 0 {
options = listOptions[0].(ListOptions)
}

if options.From == ListFromCache {
log.Debugw("try to list services in cache",
zap.String("cluster", r.cluster.name),
zap.String("url", r.url),
)
return r.cluster.cache.ListRoutes(
"label",
options.KindLabel.Kind,
options.KindLabel.Namespace,
options.KindLabel.Name,
)
}
log.Debugw("try to list routes in APISIX",
zap.String("cluster", r.cluster.name),
zap.String("url", r.url),
)

routeItems, err := r.cluster.listResource(ctx, r.url, "route")
if err != nil {
log.Errorf("failed to list routes: %s", err)
Expand Down
17 changes: 14 additions & 3 deletions pkg/dashboard/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,14 @@ var (
)

type ListOptions struct {
From ListFrom
Args []interface{}
From ListFrom
KindLabel ListByKindLabelOptions
}

type ListByKindLabelOptions struct {
Kind string
Namespace string
Name string
}

// List is only used in cache warming up. So here just pass through
Expand All @@ -74,7 +80,12 @@ func (u *serviceClient) List(ctx context.Context, listOptions ...interface{}) ([
zap.String("cluster", u.cluster.name),
zap.String("url", u.url),
)
return u.cluster.cache.ListServices()
return u.cluster.cache.ListServices(
"label",
options.KindLabel.Kind,
options.KindLabel.Namespace,
options.KindLabel.Name,
)
}

log.Debugw("try to list upstreams in APISIX",
Expand Down
Loading
Loading