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

Include Kong service name in multi-Service Events #3318

Merged
merged 1 commit into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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),
rainest marked this conversation as resolved.
Show resolved Hide resolved
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