-
Notifications
You must be signed in to change notification settings - Fork 593
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: generate fallback cache snapshot excluding affected objects
- Loading branch information
Showing
7 changed files
with
525 additions
and
153 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
package fallback | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
|
||
"github.com/dominikbraun/graph" | ||
"sigs.k8s.io/controller-runtime/pkg/client" | ||
|
||
"github.com/kong/kubernetes-ingress-controller/v3/internal/store" | ||
) | ||
|
||
// DefaultCacheGraphProvider is a default implementation of the CacheGraphProvider interface. | ||
type DefaultCacheGraphProvider struct{} | ||
|
||
func NewDefaultCacheGraphProvider() *DefaultCacheGraphProvider { | ||
return &DefaultCacheGraphProvider{} | ||
} | ||
|
||
// CacheToGraph creates a new ConfigGraph from the given cache stores. It adds all objects | ||
// from the cache stores to the graph as vertices as well as edges between objects and their dependencies | ||
// resolved by the ResolveDependencies function. | ||
func (p DefaultCacheGraphProvider) CacheToGraph(c store.CacheStores) (*ConfigGraph, error) { | ||
g := NewConfigGraph() | ||
|
||
for _, s := range c.ListAllStores() { | ||
for _, o := range s.List() { | ||
obj, ok := o.(client.Object) | ||
if !ok { | ||
// Should not happen since all objects in the cache are client.Objects, but better safe than sorry. | ||
return nil, fmt.Errorf("expected client.Object, got %T", o) | ||
} | ||
// Add the object to the graph. It can happen that the object is already in the graph (i.e. was already added | ||
// as a dependency of another object), in which case we ignore the error. | ||
if err := g.AddVertex(obj); err != nil && !errors.Is(err, graph.ErrVertexAlreadyExists) { | ||
return nil, fmt.Errorf("failed to add %s to the graph: %w", GetObjectHash(obj), err) | ||
} | ||
|
||
deps, err := ResolveDependencies(c, obj) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to resolve dependencies for %s: %w", GetObjectHash(obj), err) | ||
} | ||
// Add the object's dependencies to the graph. | ||
for _, dep := range deps { | ||
// Add the dependency to the graph in case it wasn't added before. If it was added before, we ignore the | ||
// error. | ||
if err := g.AddVertex(dep); err != nil && !errors.Is(err, graph.ErrVertexAlreadyExists) { | ||
return nil, fmt.Errorf("failed to add %s to the graph: %w", GetObjectHash(obj), err) | ||
} | ||
|
||
// Add an edge from a dependency to the object. If the edge was already added before, we ignore the error. | ||
// It's on purpose that we add the edge from the dependency to the object, as it makes it easier to traverse | ||
// the graph from the object to its dependants once it is broken. | ||
if err := g.AddEdge(GetObjectHash(dep), GetObjectHash(obj)); err != nil && !errors.Is(err, graph.ErrEdgeAlreadyExists) { | ||
return nil, fmt.Errorf("failed to add edge from %s to %s: %w", GetObjectHash(obj), GetObjectHash(dep), err) | ||
} | ||
} | ||
} | ||
} | ||
|
||
return g, nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
package fallback_test | ||
|
||
import ( | ||
"sort" | ||
"testing" | ||
|
||
"github.com/samber/lo" | ||
"github.com/stretchr/testify/require" | ||
corev1 "k8s.io/api/core/v1" | ||
netv1 "k8s.io/api/networking/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
||
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/fallback" | ||
"github.com/kong/kubernetes-ingress-controller/v3/internal/store" | ||
incubatorv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/incubator/v1alpha1" | ||
) | ||
|
||
func TestDefaultCacheGraphProvider_CacheToGraph(t *testing.T) { | ||
// adjacencyGraphStrings returns a map of stringified vertices and their neighbours | ||
// in the given graph for easy comparison. | ||
adjacencyGraphStrings := func(t *testing.T, g *fallback.ConfigGraph) map[string][]string { | ||
am, err := g.AdjacencyMap() | ||
require.NoError(t, err) | ||
adjacencyMapStrings := make(map[string][]string, len(am)) | ||
for v, neighbours := range am { | ||
neighboursStrings := lo.Map(neighbours, func(n fallback.ObjectHash, _ int) string { | ||
return n.String() | ||
}) | ||
sort.Strings(neighboursStrings) // Sort for deterministic output. | ||
adjacencyMapStrings[v.String()] = neighboursStrings | ||
} | ||
return adjacencyMapStrings | ||
} | ||
|
||
testCases := []struct { | ||
name string | ||
cache store.CacheStores | ||
expectedAdjacencyMap map[string][]string | ||
}{ | ||
{ | ||
name: "empty cache", | ||
cache: store.NewCacheStores(), | ||
expectedAdjacencyMap: map[string][]string{}, | ||
}, | ||
{ | ||
name: "cache with Ingress and its dependencies", | ||
cache: cacheStoresFromObjs(t, | ||
&netv1.Ingress{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: "test-ingress", | ||
Namespace: "test-namespace", | ||
}, | ||
Spec: netv1.IngressSpec{ | ||
IngressClassName: lo.ToPtr("test-ingress-class"), | ||
Rules: []netv1.IngressRule{ | ||
{ | ||
IngressRuleValue: netv1.IngressRuleValue{ | ||
HTTP: &netv1.HTTPIngressRuleValue{ | ||
Paths: []netv1.HTTPIngressPath{ | ||
{ | ||
Backend: netv1.IngressBackend{ | ||
Service: &netv1.IngressServiceBackend{ | ||
Name: "test-service", | ||
}, | ||
}, | ||
}, | ||
{ | ||
Backend: netv1.IngressBackend{ | ||
Resource: &corev1.TypedLocalObjectReference{ | ||
Name: "test-kong-service-facade", | ||
Kind: "KongServiceFacade", | ||
APIGroup: lo.ToPtr(incubatorv1alpha1.GroupVersion.Group), | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
&netv1.IngressClass{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: "test-ingress-class", | ||
}, | ||
}, | ||
&corev1.Service{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: "test-service", | ||
Namespace: "test-namespace", | ||
}, | ||
}, | ||
&incubatorv1alpha1.KongServiceFacade{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: "test-kong-service-facade", | ||
Namespace: "test-namespace", | ||
}, | ||
}, | ||
), | ||
expectedAdjacencyMap: map[string][]string{ | ||
"Ingress:test-namespace/test-ingress": {}, | ||
"IngressClass:test-ingress-class": { | ||
"Ingress:test-namespace/test-ingress", | ||
}, | ||
"Service:test-namespace/test-service": { | ||
"Ingress:test-namespace/test-ingress", | ||
}, | ||
"KongServiceFacade:test-namespace/test-kong-service-facade": { | ||
"Ingress:test-namespace/test-ingress", | ||
}, | ||
}, | ||
}, | ||
} | ||
for _, tc := range testCases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
p := fallback.NewDefaultCacheGraphProvider() | ||
g, err := p.CacheToGraph(tc.cache) | ||
require.NoError(t, err) | ||
require.NotNil(t, g) | ||
require.Equal(t, tc.expectedAdjacencyMap, adjacencyGraphStrings(t, g)) | ||
}) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package fallback | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/kong/kubernetes-ingress-controller/v3/internal/store" | ||
) | ||
|
||
type CacheGraphProvider interface { | ||
// CacheToGraph returns a new ConfigGraph instance built from the given cache snapshot. | ||
CacheToGraph(cache store.CacheStores) (*ConfigGraph, error) | ||
} | ||
|
||
// Generator is responsible for generating fallback cache snapshots. | ||
type Generator struct { | ||
cacheGraphProvider CacheGraphProvider | ||
} | ||
|
||
func NewGenerator(cacheGraphProvider CacheGraphProvider) *Generator { | ||
return &Generator{ | ||
cacheGraphProvider: cacheGraphProvider, | ||
} | ||
} | ||
|
||
// GenerateExcludingAffected generates a new cache snapshot that excludes all objects that depend on the broken objects. | ||
func (g *Generator) GenerateExcludingAffected( | ||
cache store.CacheStores, | ||
brokenObjects []ObjectHash, | ||
) (store.CacheStores, error) { | ||
graph, err := g.cacheGraphProvider.CacheToGraph(cache) | ||
if err != nil { | ||
return store.CacheStores{}, fmt.Errorf("failed to build cache graph: %w", err) | ||
} | ||
|
||
fallbackCache, err := cache.TakeSnapshot() | ||
if err != nil { | ||
return store.CacheStores{}, fmt.Errorf("failed to take cache snapshot: %w", err) | ||
} | ||
|
||
for _, brokenObject := range brokenObjects { | ||
subgraphObjects, err := graph.SubgraphObjects(brokenObject) | ||
if err != nil { | ||
return store.CacheStores{}, fmt.Errorf("failed to find dependants for %s: %w", brokenObject, err) | ||
} | ||
for _, obj := range subgraphObjects { | ||
if err := fallbackCache.Delete(obj); err != nil { | ||
return store.CacheStores{}, fmt.Errorf("failed to delete %s from the cache: %w", GetObjectHash(obj), err) | ||
} | ||
} | ||
} | ||
|
||
return fallbackCache, nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
package fallback_test | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/fallback" | ||
"github.com/kong/kubernetes-ingress-controller/v3/internal/store" | ||
) | ||
|
||
// mockGraphProvider is a mock implementation of the CacheGraphProvider interface. | ||
type mockGraphProvider struct { | ||
graph *fallback.ConfigGraph | ||
lastCalledWithStore store.CacheStores | ||
} | ||
|
||
// CacheToGraph returns the graph that was set on the mockGraphProvider. It also records the last store that was passed to it. | ||
func (m *mockGraphProvider) CacheToGraph(s store.CacheStores) (*fallback.ConfigGraph, error) { | ||
m.lastCalledWithStore = s | ||
return m.graph, nil | ||
} | ||
|
||
func TestGenerateFallbackCacheStores(t *testing.T) { | ||
// We have to use real-world object types here as we're testing integration with store.CacheStores. | ||
ingressClass := testIngressClass(t, "ingressClass") | ||
service := testService(t, "service") | ||
serviceFacade := testKongServiceFacade(t, "serviceFacade") | ||
plugin := testKongPlugin(t, "kongPlugin") | ||
inputCacheStores := cacheStoresFromObjs(t, ingressClass, service, serviceFacade, plugin) | ||
|
||
// This graph doesn't reflect real dependencies between the objects - it's only used for testing purposes. | ||
// It will be injected into the Generator via the mockGraphProvider. | ||
// Dependency resolving between the objects is tested in TestResolveDependencies_* tests. | ||
// | ||
// Graph structure (edges define dependency -> dependant relationship): | ||
// ┌────────────┐ ┌──────┐ | ||
// │ingressClass│ │plugin│ | ||
// └──────┬─────┘ └──────┘ | ||
// │ | ||
// ┌───▼───┐ | ||
// │service│ | ||
// └───┬───┘ | ||
// │ | ||
// ┌──────▼──────┐ | ||
// │serviceFacade│ | ||
// └─────────────┘ | ||
graph, err := NewGraphBuilder(). | ||
WithVertices(ingressClass, service, serviceFacade, plugin). | ||
WithEdge(ingressClass, service). | ||
WithEdge(service, serviceFacade). | ||
Build() | ||
require.NoError(t, err) | ||
|
||
graphProvider := &mockGraphProvider{graph: graph} | ||
g := fallback.NewGenerator(graphProvider) | ||
|
||
t.Run("ingressClass is broken", func(t *testing.T) { | ||
fallbackCache, err := g.GenerateExcludingAffected(inputCacheStores, []fallback.ObjectHash{fallback.GetObjectHash(ingressClass)}) | ||
require.NoError(t, err) | ||
require.Equal(t, inputCacheStores, graphProvider.lastCalledWithStore, "expected the generator to call CacheToGraph with the input cache stores") | ||
require.NotSame(t, inputCacheStores, fallbackCache) | ||
require.Empty(t, fallbackCache.IngressClassV1.List(), "ingressClass should be excluded as it's broken") | ||
require.Empty(t, fallbackCache.Service.List(), "service should be excluded as it depends on ingressClass") | ||
require.Empty(t, fallbackCache.KongServiceFacade.List(), "serviceFacade should be excluded as it depends on service") | ||
require.ElementsMatch(t, fallbackCache.Plugin.List(), []any{plugin}, "plugin shouldn't be excluded as it doesn't depend on ingressClass") | ||
}) | ||
|
||
t.Run("service is broken", func(t *testing.T) { | ||
fallbackCache, err := g.GenerateExcludingAffected(inputCacheStores, []fallback.ObjectHash{fallback.GetObjectHash(service)}) | ||
require.NoError(t, err) | ||
require.Equal(t, inputCacheStores, graphProvider.lastCalledWithStore, "expected the generator to call CacheToGraph with the input cache stores") | ||
require.NotSame(t, inputCacheStores, fallbackCache) | ||
require.Empty(t, fallbackCache.Service.List(), "service should be excluded as it's broken") | ||
require.Empty(t, fallbackCache.KongServiceFacade.List(), "serviceFacade should be excluded as it depends on service") | ||
require.ElementsMatch(t, fallbackCache.IngressClassV1.List(), []any{ingressClass}, "ingressClass shouldn't be excluded as it doesn't depend on service") | ||
require.ElementsMatch(t, fallbackCache.Plugin.List(), []any{plugin}, "plugin shouldn't be excluded as it doesn't depend on service") | ||
}) | ||
|
||
t.Run("serviceFacade is broken", func(t *testing.T) { | ||
fallbackCache, err := g.GenerateExcludingAffected(inputCacheStores, []fallback.ObjectHash{fallback.GetObjectHash(serviceFacade)}) | ||
require.NoError(t, err) | ||
require.Equal(t, inputCacheStores, graphProvider.lastCalledWithStore, "expected the generator to call CacheToGraph with the input cache stores") | ||
require.NotSame(t, inputCacheStores, fallbackCache) | ||
require.Empty(t, fallbackCache.KongServiceFacade.List(), "serviceFacade should be excluded as it's broken") | ||
require.ElementsMatch(t, fallbackCache.IngressClassV1.List(), []any{ingressClass}, "ingressClass shouldn't be excluded as it doesn't depend on service") | ||
require.ElementsMatch(t, fallbackCache.Service.List(), []any{service}, "service shouldn't be excluded as it doesn't depend on serviceFacade") | ||
require.ElementsMatch(t, fallbackCache.Plugin.List(), []any{plugin}, "plugin shouldn't be excluded as it doesn't depend on serviceFacade") | ||
}) | ||
|
||
t.Run("plugin is broken", func(t *testing.T) { | ||
fallbackCache, err := g.GenerateExcludingAffected(inputCacheStores, []fallback.ObjectHash{fallback.GetObjectHash(plugin)}) | ||
require.NoError(t, err) | ||
require.Equal(t, inputCacheStores, graphProvider.lastCalledWithStore, "expected the generator to call CacheToGraph with the input cache stores") | ||
require.NotSame(t, inputCacheStores, fallbackCache) | ||
require.Empty(t, fallbackCache.Plugin.List(), "plugin should be excluded as it's broken") | ||
require.ElementsMatch(t, fallbackCache.IngressClassV1.List(), []any{ingressClass}, "ingressClass shouldn't be excluded as it doesn't depend on plugin") | ||
require.ElementsMatch(t, fallbackCache.Service.List(), []any{service}, "service shouldn't be excluded as it doesn't depend on plugin") | ||
require.ElementsMatch(t, fallbackCache.KongServiceFacade.List(), []any{serviceFacade}, "serviceFacade shouldn't be excluded as it doesn't depend on plugin") | ||
}) | ||
|
||
t.Run("multiple objects are broken", func(t *testing.T) { | ||
fallbackCache, err := g.GenerateExcludingAffected(inputCacheStores, []fallback.ObjectHash{fallback.GetObjectHash(ingressClass), fallback.GetObjectHash(service)}) | ||
require.NoError(t, err) | ||
require.Equal(t, inputCacheStores, graphProvider.lastCalledWithStore, "expected the generator to call CacheToGraph with the input cache stores") | ||
require.NotSame(t, inputCacheStores, fallbackCache) | ||
require.Empty(t, fallbackCache.IngressClassV1.List(), "ingressClass should be excluded as it's broken") | ||
require.Empty(t, fallbackCache.Service.List(), "service should be excluded as it's broken") | ||
require.Empty(t, fallbackCache.KongServiceFacade.List(), "serviceFacade should be excluded as it depends on service") | ||
require.ElementsMatch(t, fallbackCache.Plugin.List(), []any{plugin}, "plugin shouldn't be excluded as it doesn't depend on either ingressClass or service") | ||
}) | ||
} |
Oops, something went wrong.