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 18, 2023
1 parent 59d047f commit 9e43fed
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 12 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ Adding a new version? You'll need three changes:
- Change existing `resolvedRefs` condition in status `HTTPRoute` if there is
already one to avoid multiple appearance of conditions with same type
[#3386](https://github.com/Kong/kubernetes-ingress-controller/pull/3386)
- Event messages for invalid multi-Service backends now indicate their derived
Kong resource name.
[#3318](https://github.com/Kong/kubernetes-ingress-controller/pull/3318)

### Deprecated

Expand Down
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
3 changes: 2 additions & 1 deletion test/integration/translation_failures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ func TestTranslationFailures(t *testing.T) {
return expectedTranslationFailure{
// expect event for service2 as it doesn't have annotations that service1 has
causingObjects: []client.Object{service2},
reasonContains: "when multiple services comprise a backend all kong annotations between them must be set to the same value",
reasonContains: "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.",
}
},
},
Expand Down

0 comments on commit 9e43fed

Please sign in to comment.