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.

Sign up for GitHub

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

Jump to bottom

Use cache only for service business service - unit test refactor #5930

Merged
merged 1 commit into from
Mar 28, 2023
Merged

Use cache only for service business service - unit test refactor #5930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 7 additions & 4 deletions business/apps_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,13 @@ func TestGetAppListFromDeployments(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

conf := config.NewConfig()
// Auxiliar fake* tests defined in workload_test.go
objects := []runtime.Object{
&core_v1.Namespace{ObjectMeta: v1.ObjectMeta{Name: "Namespace"}},
&osproject_v1.Project{ObjectMeta: v1.ObjectMeta{Name: "Namespace"}},
}
for _, obj := range FakeDeployments() {
for _, obj := range FakeDeployments(*conf) {
o := obj
objects = append(objects, &o)
}
Expand Down Expand Up @@ -98,7 +99,7 @@ func TestGetAppFromDeployments(t *testing.T) {
objects := []runtime.Object{
&osproject_v1.Project{ObjectMeta: v1.ObjectMeta{Name: "Namespace"}},
}
for _, obj := range FakeDeployments() {
for _, obj := range FakeDeployments(*conf) {
o := obj
objects = append(objects, &o)
}
Expand Down Expand Up @@ -134,13 +135,14 @@ func TestGetAppFromDeployments(t *testing.T) {
func TestGetAppListFromReplicaSets(t *testing.T) {
require := require.New(t)
assert := assert.New(t)
conf := config.NewConfig()

// Setup mocks
// Auxiliar fake* tests defined in workload_test.go
objects := []runtime.Object{
&osproject_v1.Project{ObjectMeta: v1.ObjectMeta{Name: "Namespace"}},
}
for _, obj := range FakeReplicaSets() {
for _, obj := range FakeReplicaSets(*conf) {
o := obj
objects = append(objects, &o)
}
Expand All @@ -167,12 +169,13 @@ func TestGetAppListFromReplicaSets(t *testing.T) {
func TestGetAppFromReplicaSets(t *testing.T) {
require := require.New(t)
assert := assert.New(t)
conf := config.NewConfig()

// Setup mocks
objects := []runtime.Object{
&osproject_v1.Project{ObjectMeta: v1.ObjectMeta{Name: "Namespace"}},
}
for _, obj := range FakeReplicaSets() {
for _, obj := range FakeReplicaSets(*conf) {
o := obj
objects = append(objects, &o)
}
Expand Down
8 changes: 6 additions & 2 deletions business/health_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/prometheus/common/model"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
apps_v1 "k8s.io/api/apps/v1"
core_v1 "k8s.io/api/core/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -236,8 +237,8 @@ func TestGetWorkloadHealthWithoutIstio(t *testing.T) {
}

func TestGetNamespaceAppHealthWithoutIstio(t *testing.T) {
require := require.New(t)
conf := config.NewConfig()
conf.KubernetesConfig.CacheEnabled = false
config.Set(conf)

objects := []runtime.Object{
Expand All @@ -255,12 +256,14 @@ func TestGetNamespaceAppHealthWithoutIstio(t *testing.T) {
k8s := kubetest.NewFakeK8sClient(objects...)
k8s.OpenShift = true
prom := new(prometheustest.PromClientMock)
SetupBusinessLayer(t, k8s, *conf)

clients := make(map[string]kubernetes.ClientInterface)
clients[kubernetes.HomeClusterName] = k8s
hs := HealthService{prom: prom, businessLayer: NewWithBackends(clients, clients, prom, nil)}
criteria := NamespaceHealthCriteria{Namespace: "ns", RateInterval: "1m", QueryTime: time.Date(2017, 1, 15, 0, 0, 0, 0, time.UTC), IncludeMetrics: true}
_, _ = hs.GetNamespaceAppHealth(context.TODO(), criteria)
_, err := hs.GetNamespaceAppHealth(context.TODO(), criteria)
require.NoError(err)

// Make sure unnecessary call isn't performed
prom.AssertNumberOfCalls(t, "GetAllRequestRates", 0)
Expand All @@ -280,6 +283,7 @@ func TestGetNamespaceServiceHealthWithNA(t *testing.T) {
)
k8s.OpenShift = true
prom := new(prometheustest.PromClientMock)
SetupBusinessLayer(t, k8s, *config.NewConfig())

prom.On("GetNamespaceServicesRequestRates", "tutorial", mock.AnythingOfType("string"), mock.AnythingOfType("time.Time")).Return(serviceRates, nil)

Expand Down
43 changes: 24 additions & 19 deletions business/istio_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

"github.com/kiali/kiali/config"
"github.com/kiali/kiali/kubernetes"
"github.com/kiali/kiali/kubernetes/cache"
"github.com/kiali/kiali/log"
"github.com/kiali/kiali/models"
"github.com/kiali/kiali/observability"
Expand All @@ -29,6 +30,7 @@ const allResources string = "*"

type IstioConfigService struct {
k8s kubernetes.ClientInterface
cache cache.KialiCache
businessLayer *Layer
}

Expand Down Expand Up @@ -241,7 +243,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
var err error
// Check if namespace is cached
if IsResourceCached(criteria.Namespace, kubernetes.DestinationRules) {
istioConfigList.DestinationRules, err = kialiCache.GetDestinationRules(criteria.Namespace, criteria.LabelSelector)
istioConfigList.DestinationRules, err = in.cache.GetDestinationRules(criteria.Namespace, criteria.LabelSelector)
} else {
drl, e := in.k8s.Istio().NetworkingV1beta1().DestinationRules(criteria.Namespace).List(ctx, listOpts)
istioConfigList.DestinationRules = drl.Items
Expand All @@ -258,7 +260,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
if criteria.Include(kubernetes.EnvoyFilters) {
var err error
if IsResourceCached(criteria.Namespace, kubernetes.EnvoyFilters) {
istioConfigList.EnvoyFilters, err = kialiCache.GetEnvoyFilters(criteria.Namespace, criteria.LabelSelector)
istioConfigList.EnvoyFilters, err = in.cache.GetEnvoyFilters(criteria.Namespace, criteria.LabelSelector)
} else {
efl, e := in.k8s.Istio().NetworkingV1alpha3().EnvoyFilters(criteria.Namespace).List(ctx, listOpts)
istioConfigList.EnvoyFilters = efl.Items
Expand All @@ -280,8 +282,9 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
var err error
// Check if namespace is cached
if IsResourceCached(criteria.Namespace, kubernetes.Gateways) {
istioConfigList.Gateways, err = kialiCache.GetGateways(criteria.Namespace, criteria.LabelSelector)
istioConfigList.Gateways, err = in.cache.GetGateways(criteria.Namespace, criteria.LabelSelector)
} else {
log.Debugf("Listing Gateways for namespace [%s] with labelSelector [%s]", criteria.Namespace, criteria.LabelSelector)
gwl, e := in.k8s.Istio().NetworkingV1beta1().Gateways(criteria.Namespace).List(ctx, listOpts)
istioConfigList.Gateways = gwl.Items
err = e
Expand All @@ -303,7 +306,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
// ignore an error as system could not be configured to support K8s Gateway API
// Check if namespace is cached
if IsResourceCached(criteria.Namespace, kubernetes.K8sGateways) {
istioConfigList.K8sGateways, err = kialiCache.GetK8sGateways(criteria.Namespace, criteria.LabelSelector)
istioConfigList.K8sGateways, err = in.cache.GetK8sGateways(criteria.Namespace, criteria.LabelSelector)
}
// TODO gwl.Items, there is conflict itself in Gateway API between returned types referenced or not
//else {
Expand All @@ -324,7 +327,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
// ignore an error as system could not be configured to support K8s Gateway API
// Check if namespace is cached
if IsResourceCached(criteria.Namespace, kubernetes.K8sHTTPRoutes) {
istioConfigList.K8sHTTPRoutes, err = kialiCache.GetK8sHTTPRoutes(criteria.Namespace, criteria.LabelSelector)
istioConfigList.K8sHTTPRoutes, err = in.cache.GetK8sHTTPRoutes(criteria.Namespace, criteria.LabelSelector)
}
// TODO gwl.Items, there is conflict itself in Gateway API between returned types referenced or not
//else {
Expand All @@ -344,7 +347,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
var err error
// Check if namespace is cached
if IsResourceCached(criteria.Namespace, kubernetes.ServiceEntries) {
istioConfigList.ServiceEntries, err = kialiCache.GetServiceEntries(criteria.Namespace, criteria.LabelSelector)
istioConfigList.ServiceEntries, err = in.cache.GetServiceEntries(criteria.Namespace, criteria.LabelSelector)
} else {
sel, e := in.k8s.Istio().NetworkingV1beta1().ServiceEntries(criteria.Namespace).List(ctx, listOpts)
istioConfigList.ServiceEntries = sel.Items
Expand All @@ -361,7 +364,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
if criteria.Include(kubernetes.Sidecars) {
var err error
if IsResourceCached(criteria.Namespace, kubernetes.Sidecars) {
istioConfigList.Sidecars, err = kialiCache.GetSidecars(criteria.Namespace, criteria.LabelSelector)
istioConfigList.Sidecars, err = in.cache.GetSidecars(criteria.Namespace, criteria.LabelSelector)
} else {
scl, e := in.k8s.Istio().NetworkingV1beta1().Sidecars(criteria.Namespace).List(ctx, listOpts)
istioConfigList.Sidecars = scl.Items
Expand All @@ -383,7 +386,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
var err error
// Check if namespace is cached
if IsResourceCached(criteria.Namespace, kubernetes.VirtualServices) {
istioConfigList.VirtualServices, err = kialiCache.GetVirtualServices(criteria.Namespace, criteria.LabelSelector)
istioConfigList.VirtualServices, err = in.cache.GetVirtualServices(criteria.Namespace, criteria.LabelSelector)
} else {
vsl, e := in.k8s.Istio().NetworkingV1beta1().VirtualServices(criteria.Namespace).List(ctx, listOpts)
istioConfigList.VirtualServices = vsl.Items
Expand All @@ -400,7 +403,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
if criteria.Include(kubernetes.WorkloadEntries) {
var err error
if IsResourceCached(criteria.Namespace, kubernetes.WorkloadEntries) {
istioConfigList.WorkloadEntries, err = kialiCache.GetWorkloadEntries(criteria.Namespace, criteria.LabelSelector)
istioConfigList.WorkloadEntries, err = in.cache.GetWorkloadEntries(criteria.Namespace, criteria.LabelSelector)
} else {
wel, e := in.k8s.Istio().NetworkingV1beta1().WorkloadEntries(criteria.Namespace).List(ctx, listOpts)
istioConfigList.WorkloadEntries = wel.Items
Expand All @@ -417,7 +420,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
if criteria.Include(kubernetes.WorkloadGroups) {
var err error
if IsResourceCached(criteria.Namespace, kubernetes.WorkloadGroups) {
istioConfigList.WorkloadGroups, err = kialiCache.GetWorkloadGroups(criteria.Namespace, criteria.LabelSelector)
istioConfigList.WorkloadGroups, err = in.cache.GetWorkloadGroups(criteria.Namespace, criteria.LabelSelector)
} else {
wgl, e := in.k8s.Istio().NetworkingV1beta1().WorkloadGroups(criteria.Namespace).List(ctx, listOpts)
istioConfigList.WorkloadGroups = wgl.Items
Expand All @@ -434,7 +437,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
if criteria.Include(kubernetes.WasmPlugins) {
var err error
if IsResourceCached(criteria.Namespace, kubernetes.WasmPlugins) {
istioConfigList.WasmPlugins, err = kialiCache.GetWasmPlugins(criteria.Namespace, criteria.LabelSelector)
istioConfigList.WasmPlugins, err = in.cache.GetWasmPlugins(criteria.Namespace, criteria.LabelSelector)
} else {
wgl, e := in.k8s.Istio().ExtensionsV1alpha1().WasmPlugins(criteria.Namespace).List(ctx, listOpts)
istioConfigList.WasmPlugins = wgl.Items
Expand All @@ -451,7 +454,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
if criteria.Include(kubernetes.Telemetries) {
var err error
if IsResourceCached(criteria.Namespace, kubernetes.Telemetries) {
istioConfigList.Telemetries, err = kialiCache.GetTelemetries(criteria.Namespace, criteria.LabelSelector)
istioConfigList.Telemetries, err = in.cache.GetTelemetries(criteria.Namespace, criteria.LabelSelector)
} else {
wgl, e := in.k8s.Istio().TelemetryV1alpha1().Telemetries(criteria.Namespace).List(ctx, listOpts)
istioConfigList.Telemetries = wgl.Items
Expand All @@ -468,7 +471,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
if criteria.Include(kubernetes.AuthorizationPolicies) {
var err error
if IsResourceCached(criteria.Namespace, kubernetes.AuthorizationPolicies) {
istioConfigList.AuthorizationPolicies, err = kialiCache.GetAuthorizationPolicies(criteria.Namespace, criteria.LabelSelector)
istioConfigList.AuthorizationPolicies, err = in.cache.GetAuthorizationPolicies(criteria.Namespace, criteria.LabelSelector)
} else {
apl, e := in.k8s.Istio().SecurityV1beta1().AuthorizationPolicies(criteria.Namespace).List(ctx, listOpts)
istioConfigList.AuthorizationPolicies = apl.Items
Expand All @@ -489,7 +492,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
if criteria.Include(kubernetes.PeerAuthentications) {
var err error
if IsResourceCached(criteria.Namespace, kubernetes.PeerAuthentications) {
istioConfigList.PeerAuthentications, err = kialiCache.GetPeerAuthentications(criteria.Namespace, criteria.LabelSelector)
istioConfigList.PeerAuthentications, err = in.cache.GetPeerAuthentications(criteria.Namespace, criteria.LabelSelector)
} else {
pal, e := in.k8s.Istio().SecurityV1beta1().PeerAuthentications(criteria.Namespace).List(ctx, listOpts)
istioConfigList.PeerAuthentications = pal.Items
Expand All @@ -510,7 +513,7 @@ func (in *IstioConfigService) GetIstioConfigList(ctx context.Context, criteria I
if criteria.Include(kubernetes.RequestAuthentications) {
var err error
if IsResourceCached(criteria.Namespace, kubernetes.RequestAuthentications) {
istioConfigList.RequestAuthentications, err = kialiCache.GetRequestAuthentications(criteria.Namespace, criteria.LabelSelector)
istioConfigList.RequestAuthentications, err = in.cache.GetRequestAuthentications(criteria.Namespace, criteria.LabelSelector)
} else {
ral, e := in.k8s.Istio().SecurityV1beta1().RequestAuthentications(criteria.Namespace).List(ctx, listOpts)
istioConfigList.RequestAuthentications = ral.Items
Expand Down Expand Up @@ -918,12 +921,14 @@ func (in *IstioConfigService) DeleteIstioConfigDetail(namespace, resourceType, n
default:
err = fmt.Errorf("object type not found: %v", resourceType)
}
if err != nil {
return err
}

// Cache is stopped after a Create/Update/Delete operation to force a refresh
if kialiCache != nil && err == nil {
kialiCache.Refresh(namespace)
}
return err
kialiCache.Refresh(namespace)

return nil
}

func (in *IstioConfigService) UpdateIstioConfigDetail(namespace, resourceType, name, jsonPatch string) (models.IstioConfigDetails, error) {
Expand Down