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 5581c8c commit e0f4e21
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 11 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ Adding a new version? You'll need three changes:
This is all the way at the bottom. It's the thing we always forget.
--->

<<<<<<< HEAD
- [2.8.1](#281)
=======
- [2.9.0](#290)
>>>>>>> f2c58efe (feat(events) list Kong name for multi-Services)
- [2.8.0](#280)
- [2.7.0](#270)
- [2.6.0](#260)
Expand Down Expand Up @@ -78,6 +82,9 @@ Adding a new version? You'll need three changes:
- Parse `ttl` field of key-auth credentials in secrets to `int` type before filling
to kong credentials to fix the invalid type error.
[#3319](https://github.com/Kong/kubernetes-ingress-controller/pull/3319)
- Event messages for invalid multi-Service backends now indicate their derived
Kong resource name.
[#3318](https://github.com/Kong/kubernetes-ingress-controller/pull/3318)

## [2.8.0]

Expand Down Expand Up @@ -2164,6 +2171,7 @@ Please read the changelog and test in your environment.
- The initial versions were rapildy iterated to deliver
a working ingress controller.

[2.9.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.7.0...v2.9.0
[2.8.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.7.0...v2.8.0
[2.7.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.6.0...v2.7.0
[2.6.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.5.0...v2.6.0
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

0 comments on commit e0f4e21

Please sign in to comment.