Skip to content

Commit

Permalink
feat(events) list Kong name for multi-Services
Browse files Browse the repository at this point in the history
Include the Kong service name when emitting Events related to
multi-Service backend annotation inconsistency. This name indicates the
parent route resource. Reviewing the route resource will show the other
Services used by the problem rule.
  • Loading branch information
rainest committed Jan 4, 2023
1 parent 996bc14 commit 1c96456
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 11 deletions.
17 changes: 10 additions & 7 deletions internal/dataplane/parser/ingressrules.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer,

// if the Kubernetes services have been deemed invalid, log an error message
// and skip the current service.
if !servicesAllUseTheSameKongAnnotations(k8sServices, seenAnnotations, failuresCollector) {
if !servicesAllUseTheSameKongAnnotations(k8sServices, seenAnnotations, failuresCollector, key) {
// The Kong services not having all the k8s services correctly annotated must be marked
// as to be skipped.
serviceNamesToSkip[key] = nil
Expand Down Expand Up @@ -255,6 +255,7 @@ func servicesAllUseTheSameKongAnnotations(
services []*corev1.Service,
annotations map[string]string,
failuresCollector *failures.ResourceFailuresCollector,
kongServiceName string,
) bool {
match := true
for _, service := range services {
Expand All @@ -270,9 +271,10 @@ func servicesAllUseTheSameKongAnnotations(
valueForThisObject, ok := service.Annotations[k]
if !ok {
failuresCollector.PushResourceFailure(
fmt.Sprintf("in the backend group of %d kubernetes services some have the %s annotation while others don't. "+
"this is not supported: when multiple services comprise a backend all kong annotations "+
"between them must be set to the same value", len(services), k),
fmt.Sprintf("Service has inconsistent %s annotation and is used in multi-Service backend %s. "+
"All Services in a multi-Service backend must have matching Kong annotations. Review the "+
"associated route resource and align annotations in its multi-Service backends.",
k, kongServiceName),
service.DeepCopy(),
)
match = false
Expand All @@ -282,9 +284,10 @@ func servicesAllUseTheSameKongAnnotations(

if valueForThisObject != v {
failuresCollector.PushResourceFailure(
fmt.Sprintf("the value of annotation %s is different between the %d services which comprise this backend. "+
"this is not supported: when multiple services comprise a backend all kong annotations "+
"between them must be set to the same value", k, len(services)),
fmt.Sprintf("Service has inconsistent %s annotation and is used in multi-Service backend %s. "+
"All Services in a multi-Service backend must have matching Kong annotations. Review the "+
"associated route resource and align annotations in its multi-Service backends.",
k, kongServiceName),
service.DeepCopy(),
)
match = false
Expand Down
8 changes: 4 additions & 4 deletions internal/dataplane/parser/ingressrules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ func TestDoK8sServicesMatchAnnotations(t *testing.T) {
},
expected: false,
expectedLogEntries: []string{
"in the backend group of 3 kubernetes services some have the konghq.com/foo annotation while others don't",
"Service has inconsistent konghq.com/foo annotation and is used in multi-Service backend",
},
},
{
Expand Down Expand Up @@ -519,16 +519,16 @@ func TestDoK8sServicesMatchAnnotations(t *testing.T) {
},
expected: false,
expectedLogEntries: []string{
"the value of annotation konghq.com/foo is different between the 3 services which comprise this backend.",
"the value of annotation konghq.com/foo is different between the 3 services which comprise this backend.",
"Service has inconsistent konghq.com/foo annotation and is used in multi-Service backend",
"Service has inconsistent konghq.com/foo annotation and is used in multi-Service backend",
},
},
} {
t.Run(tt.name, func(t *testing.T) {
logger, loggerHook := test.NewNullLogger()
failuresCollector, err := failures.NewResourceFailuresCollector(logger)
require.NoError(t, err)
assert.Equal(t, tt.expected, servicesAllUseTheSameKongAnnotations(tt.services, tt.annotations, failuresCollector))
assert.Equal(t, tt.expected, servicesAllUseTheSameKongAnnotations(tt.services, tt.annotations, failuresCollector, ""))
assert.Len(t, failuresCollector.PopResourceFailures(), len(tt.expectedLogEntries), "expecting as many translation failures as log entries")
for i := range tt.expectedLogEntries {
assert.Contains(t, loggerHook.AllEntries()[i].Message, tt.expectedLogEntries[i])
Expand Down

0 comments on commit 1c96456

Please sign in to comment.