From 5ead13bcdaa8212272a5e7b42248925b169b4586 Mon Sep 17 00:00:00 2001 From: Damian Badura Date: Tue, 9 Jan 2024 14:37:00 +0100 Subject: [PATCH 1/5] implement proper port for prometheus --- .../controllers/serverless/service.go | 2 ++ .../controllers/serverless/service_test.go | 4 +-- .../controllers/serverless/system_state.go | 7 +++-- .../internal/controllers/serverless/utils.go | 27 +++++++++++++++++-- 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/components/serverless/internal/controllers/serverless/service.go b/components/serverless/internal/controllers/serverless/service.go index 36e87f3b9..a6db3f1fe 100644 --- a/components/serverless/internal/controllers/serverless/service.go +++ b/components/serverless/internal/controllers/serverless/service.go @@ -49,6 +49,8 @@ func buildStateFnUpdateService(newService corev1.Service) stateFn { svc.Spec.Type = newService.Spec.Type svc.ObjectMeta.Labels = newService.GetLabels() + updateMapWithNewValues(svc.ObjectMeta.Annotations, newService.GetAnnotations()) + //svc.ObjectMeta.Annotations = newService.GetAnnotations() r.log.Info(fmt.Sprintf("Updating Service %s", svc.GetName())) diff --git a/components/serverless/internal/controllers/serverless/service_test.go b/components/serverless/internal/controllers/serverless/service_test.go index d66ca085b..9f14e9d31 100644 --- a/components/serverless/internal/controllers/serverless/service_test.go +++ b/components/serverless/internal/controllers/serverless/service_test.go @@ -26,7 +26,7 @@ func TestFunctionReconciler_equalServices(t *testing.T) { want bool }{ { - name: "simple case", + name: "simple equal case", args: args{ existing: corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -406,7 +406,7 @@ func TestFunctionReconciler_equalServices(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := gomega.NewGomegaWithT(t) got := equalServices(tt.args.existing, tt.args.expected) - g.Expect(got).To(gomega.Equal(tt.want)) + g.Expect(got).To(gomega.Equal(tt.want).FailureMessage()) }) } } diff --git a/components/serverless/internal/controllers/serverless/system_state.go b/components/serverless/internal/controllers/serverless/system_state.go index d0e35f911..adb3fb1ff 100644 --- a/components/serverless/internal/controllers/serverless/system_state.go +++ b/components/serverless/internal/controllers/serverless/system_state.go @@ -56,13 +56,16 @@ func (s *systemState) functionLabels() map[string]string { } func (s *systemState) functionAnnotations() map[string]string { + return prometheusSvcAnnotations() +} + +func prometheusSvcAnnotations() map[string]string { return map[string]string{ - "prometheus.io/port": "80", + "prometheus.io/port": "8080", "prometheus.io/path": "/metrics", "prometheus.io/scrape": "true", } } - func (s *systemState) buildImageAddress(registryAddress string) string { var imageTag string isGitType := s.instance.TypeOf(serverlessv1alpha2.FunctionTypeGit) diff --git a/components/serverless/internal/controllers/serverless/utils.go b/components/serverless/internal/controllers/serverless/utils.go index 79aab43e2..7426e7862 100644 --- a/components/serverless/internal/controllers/serverless/utils.go +++ b/components/serverless/internal/controllers/serverless/utils.go @@ -132,11 +132,11 @@ func didNotFail(j batchv1.Job) bool { func countJobs(l batchv1.JobList, predicates ...func(batchv1.Job) bool) int { var out int -processing_next_item: +processingNextItem: for _, j := range l.Items { for _, p := range predicates { if !p(j) { - continue processing_next_item + continue processingNextItem } } out++ @@ -186,6 +186,28 @@ func mapsEqual(existing, expected map[string]string) bool { return true } +func updateMapWithNewValues(existing, newValues map[string]string) { + for key, value := range newValues { + if _, ok := existing[key]; ok { + existing[key] = value + } + } +} + +func mapsContains(existing, contains map[string]string) bool { + if len(existing) < len(contains) { + return false + } + + for key, value := range contains { + if v, ok := existing[key]; !ok || v != value { + return false + } + } + + return true +} + // TODO refactor to make this code more readable func equalDeployments(existing appsv1.Deployment, expected appsv1.Deployment) bool { return len(existing.Spec.Template.Spec.Containers) == 1 && @@ -202,6 +224,7 @@ func equalDeployments(existing appsv1.Deployment, expected appsv1.Deployment) bo func equalServices(existing corev1.Service, expected corev1.Service) bool { return mapsEqual(existing.Spec.Selector, expected.Spec.Selector) && + mapsContains(existing.Annotations, prometheusSvcAnnotations()) && mapsEqual(existing.Labels, expected.Labels) && len(existing.Spec.Ports) == len(expected.Spec.Ports) && len(expected.Spec.Ports) > 0 && From 2faa7df062900d8694a58b57e2525edd77270abe Mon Sep 17 00:00:00 2001 From: Damian Badura Date: Tue, 9 Jan 2024 14:38:59 +0100 Subject: [PATCH 2/5] fix --- .../internal/controllers/serverless/service.go | 1 - .../controllers/serverless/service_test.go | 2 +- .../internal/controllers/serverless/utils.go | 16 ++++++++-------- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/components/serverless/internal/controllers/serverless/service.go b/components/serverless/internal/controllers/serverless/service.go index a6db3f1fe..a3b189074 100644 --- a/components/serverless/internal/controllers/serverless/service.go +++ b/components/serverless/internal/controllers/serverless/service.go @@ -50,7 +50,6 @@ func buildStateFnUpdateService(newService corev1.Service) stateFn { svc.ObjectMeta.Labels = newService.GetLabels() updateMapWithNewValues(svc.ObjectMeta.Annotations, newService.GetAnnotations()) - //svc.ObjectMeta.Annotations = newService.GetAnnotations() r.log.Info(fmt.Sprintf("Updating Service %s", svc.GetName())) diff --git a/components/serverless/internal/controllers/serverless/service_test.go b/components/serverless/internal/controllers/serverless/service_test.go index 9f14e9d31..c37b9438b 100644 --- a/components/serverless/internal/controllers/serverless/service_test.go +++ b/components/serverless/internal/controllers/serverless/service_test.go @@ -406,7 +406,7 @@ func TestFunctionReconciler_equalServices(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := gomega.NewGomegaWithT(t) got := equalServices(tt.args.existing, tt.args.expected) - g.Expect(got).To(gomega.Equal(tt.want).FailureMessage()) + g.Expect(got).To(gomega.Equal(tt.want)) }) } } diff --git a/components/serverless/internal/controllers/serverless/utils.go b/components/serverless/internal/controllers/serverless/utils.go index 7426e7862..6cf9d4845 100644 --- a/components/serverless/internal/controllers/serverless/utils.go +++ b/components/serverless/internal/controllers/serverless/utils.go @@ -186,14 +186,6 @@ func mapsEqual(existing, expected map[string]string) bool { return true } -func updateMapWithNewValues(existing, newValues map[string]string) { - for key, value := range newValues { - if _, ok := existing[key]; ok { - existing[key] = value - } - } -} - func mapsContains(existing, contains map[string]string) bool { if len(existing) < len(contains) { return false @@ -208,6 +200,14 @@ func mapsContains(existing, contains map[string]string) bool { return true } +func updateMapWithNewValues(existing, newValues map[string]string) { + for key, value := range newValues { + if _, ok := existing[key]; ok { + existing[key] = value + } + } +} + // TODO refactor to make this code more readable func equalDeployments(existing appsv1.Deployment, expected appsv1.Deployment) bool { return len(existing.Spec.Template.Spec.Containers) == 1 && From cd5fb51a41478e7536ced57b8fdd9973c5395815 Mon Sep 17 00:00:00 2001 From: Damian Badura Date: Tue, 9 Jan 2024 14:47:09 +0100 Subject: [PATCH 3/5] fix unit tests --- .../internal/controllers/serverless/service_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/serverless/internal/controllers/serverless/service_test.go b/components/serverless/internal/controllers/serverless/service_test.go index c37b9438b..516c34edc 100644 --- a/components/serverless/internal/controllers/serverless/service_test.go +++ b/components/serverless/internal/controllers/serverless/service_test.go @@ -30,8 +30,9 @@ func TestFunctionReconciler_equalServices(t *testing.T) { args: args{ existing: corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: "svc-name", - Namespace: "svc-ns", + Name: "svc-name", + Namespace: "svc-ns", + Annotations: prometheusSvcAnnotations(), Labels: map[string]string{ "label1": "label1", }, @@ -54,6 +55,7 @@ func TestFunctionReconciler_equalServices(t *testing.T) { Labels: map[string]string{ "label1": "label1", }, + Annotations: prometheusSvcAnnotations(), }, Spec: corev1.ServiceSpec{ Ports: []corev1.ServicePort{{ From 8d0db2a46274faaa1e4b8a7c8632f72fce5183e1 Mon Sep 17 00:00:00 2001 From: Damian Badura Date: Tue, 9 Jan 2024 17:48:10 +0100 Subject: [PATCH 4/5] improve code --- .../controllers/serverless/service.go | 2 +- .../internal/controllers/serverless/utils.go | 31 ++++++++++--------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/components/serverless/internal/controllers/serverless/service.go b/components/serverless/internal/controllers/serverless/service.go index a3b189074..668cd51f5 100644 --- a/components/serverless/internal/controllers/serverless/service.go +++ b/components/serverless/internal/controllers/serverless/service.go @@ -49,7 +49,7 @@ func buildStateFnUpdateService(newService corev1.Service) stateFn { svc.Spec.Type = newService.Spec.Type svc.ObjectMeta.Labels = newService.GetLabels() - updateMapWithNewValues(svc.ObjectMeta.Annotations, newService.GetAnnotations()) + mergeMapWithNewValues(svc.ObjectMeta.Annotations, newService.GetAnnotations()) r.log.Info(fmt.Sprintf("Updating Service %s", svc.GetName())) diff --git a/components/serverless/internal/controllers/serverless/utils.go b/components/serverless/internal/controllers/serverless/utils.go index 6cf9d4845..c75657280 100644 --- a/components/serverless/internal/controllers/serverless/utils.go +++ b/components/serverless/internal/controllers/serverless/utils.go @@ -200,26 +200,29 @@ func mapsContains(existing, contains map[string]string) bool { return true } -func updateMapWithNewValues(existing, newValues map[string]string) { +func mergeMapWithNewValues(existing, newValues map[string]string) { for key, value := range newValues { - if _, ok := existing[key]; ok { - existing[key] = value - } + existing[key] = value } } // TODO refactor to make this code more readable func equalDeployments(existing appsv1.Deployment, expected appsv1.Deployment) bool { - return len(existing.Spec.Template.Spec.Containers) == 1 && - len(existing.Spec.Template.Spec.Containers) == len(expected.Spec.Template.Spec.Containers) && - existing.Spec.Template.Spec.Containers[0].Image == expected.Spec.Template.Spec.Containers[0].Image && - envsEqual(existing.Spec.Template.Spec.Containers[0].Env, expected.Spec.Template.Spec.Containers[0].Env) && - mapsEqual(existing.GetLabels(), expected.GetLabels()) && - mapsEqual(existing.Spec.Template.GetLabels(), expected.Spec.Template.GetLabels()) && - equalResources(existing.Spec.Template.Spec.Containers[0].Resources, expected.Spec.Template.Spec.Containers[0].Resources) && - equalInt32Pointer(existing.Spec.Replicas, expected.Spec.Replicas) && - equalSecretMounts(existing.Spec.Template.Spec, expected.Spec.Template.Spec) && - mapsEqual(existing.Spec.Template.GetAnnotations(), expected.Spec.Template.GetAnnotations()) + result := true + result = result && len(existing.Spec.Template.Spec.Containers) == 1 + result = result && len(existing.Spec.Template.Spec.Containers) == len(expected.Spec.Template.Spec.Containers) + + result = result && existing.Spec.Template.Spec.Containers[0].Image == expected.Spec.Template.Spec.Containers[0].Image + result = result && envsEqual(existing.Spec.Template.Spec.Containers[0].Env, expected.Spec.Template.Spec.Containers[0].Env) + result = result && equalResources(existing.Spec.Template.Spec.Containers[0].Resources, expected.Spec.Template.Spec.Containers[0].Resources) + + result = result && mapsEqual(existing.GetLabels(), expected.GetLabels()) + result = result && mapsEqual(existing.Spec.Template.GetLabels(), expected.Spec.Template.GetLabels()) + result = result && equalInt32Pointer(existing.Spec.Replicas, expected.Spec.Replicas) + + result = result && mapsEqual(existing.Spec.Template.GetAnnotations(), expected.Spec.Template.GetAnnotations()) + result = result && equalSecretMounts(existing.Spec.Template.Spec, expected.Spec.Template.Spec) + return result } func equalServices(existing corev1.Service, expected corev1.Service) bool { From 97d0cc3d81107fedd212a0db8c1a4fe52d5abbb1 Mon Sep 17 00:00:00 2001 From: Damian Badura Date: Wed, 10 Jan 2024 13:42:54 +0100 Subject: [PATCH 5/5] improve parameters names --- .../serverless/internal/controllers/serverless/utils.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/serverless/internal/controllers/serverless/utils.go b/components/serverless/internal/controllers/serverless/utils.go index c75657280..7df884bb8 100644 --- a/components/serverless/internal/controllers/serverless/utils.go +++ b/components/serverless/internal/controllers/serverless/utils.go @@ -186,13 +186,13 @@ func mapsEqual(existing, expected map[string]string) bool { return true } -func mapsContains(existing, contains map[string]string) bool { - if len(existing) < len(contains) { +func mapsContains(mapSet, mapSubset map[string]string) bool { + if len(mapSet) < len(mapSubset) { return false } - for key, value := range contains { - if v, ok := existing[key]; !ok || v != value { + for key, value := range mapSubset { + if v, ok := mapSet[key]; !ok || v != value { return false } }