From fe5a044dc99d403d34194676fbdf634a64b2a7a2 Mon Sep 17 00:00:00 2001 From: leoporoli Date: Thu, 17 Oct 2024 12:07:08 -0300 Subject: [PATCH] feat: add the Istio labels for baseline Services and Deployments (#8) * add Istion labels for Services and Deployments * fix test * revert c counter line to previous version * adding set -x * fix app var name * moving the logic to the resource package and adding unit tests for the Istio labels enrurer * fixing imports --- .github/workflows/ci-tests.yml | 6 +- ci/obd-demo.yaml | 90 +++++++--------------------- go.mod | 6 +- go.sum | 4 +- kardinal/reconciler/reconciler.go | 5 ++ kardinal/resources/resources.go | 84 ++++++++++++++++++++++++-- kardinal/resources/resources_test.go | 82 +++++++++++++++++++++++++ 7 files changed, 198 insertions(+), 79 deletions(-) create mode 100644 kardinal/resources/resources_test.go diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 8e2414e..015dae5 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -50,15 +50,17 @@ jobs: - name: Validate that the boutique demo is up and running run: | + set -x # Check that the four baseline service pods are running and ready while [ $(kubectl get pods -n baseline --no-headers -o custom-columns=NAMESPACE:metadata.namespace,POD:metadata.name,PodIP:status.podIP,READY-true:status.containerStatuses[*].ready | grep "true" | wc -l) -ne 4 ] do - echo "Waiting for baseline pods to run..." + echo "Waiting for baseline pods to run, iteration number $c..." kubectl get pods -n baseline -o custom-columns=NAMESPACE:metadata.namespace,POD:metadata.name,PodIP:status.podIP,READY-true:status.containerStatuses[*].ready ((c++)) && ((c==12)) && exit 1 sleep 10 done - apps=$(kubectl get pods -n baseline -o custom-columns=:metadata.labels.app | tr " " "\n" | sort -g | tr "\n" " " | xargs) + echo "All baseline pods are running and ready." + apps=$(kubectl get pods -n baseline -o jsonpath='{.items[*].metadata.labels.app\.kubernetes\.io/name}') echo ${apps} if [ "${apps}" != "cartservice frontend postgres productcatalogservice" ]; then exit 1; fi diff --git a/ci/obd-demo.yaml b/ci/obd-demo.yaml index efd4d10..aa4df3e 100644 --- a/ci/obd-demo.yaml +++ b/ci/obd-demo.yaml @@ -3,18 +3,15 @@ kind: Deployment metadata: name: cartservice-v1 labels: - app: cartservice - version: v1 + app.kubernetes.io/name: cartservice spec: selector: matchLabels: - app: cartservice - version: v1 + app.kubernetes.io/name: cartservice template: metadata: labels: - app: cartservice - version: v1 + app.kubernetes.io/name: cartservice spec: terminationGracePeriodSeconds: 5 containers: @@ -66,14 +63,13 @@ kind: Service metadata: name: cartservice labels: - app: cartservice - version: v1 + app.kubernetes.io/name: cartservice annotations: kardinal.dev.service/dependencies: "postgres:tcp" spec: type: ClusterIP selector: - app: cartservice + app.kubernetes.io/name: cartservice ports: - name: http port: 8090 @@ -86,18 +82,15 @@ kind: Deployment metadata: name: frontend-v1 labels: - app: frontend - version: v1 + app.kubernetes.io/name: frontend spec: selector: matchLabels: - app: frontend - version: v1 + app.kubernetes.io/name: frontend template: metadata: labels: - app: frontend - version: v1 + app.kubernetes.io/name: frontend annotations: sidecar.istio.io/rewriteAppHTTPProbers: "true" spec: @@ -139,20 +132,14 @@ kind: Service metadata: name: frontend labels: - app: frontend - version: v1 + app.kubernetes.io/name: frontend annotations: kardinal.dev.service/dependencies: "productcatalogservice:http,cartservice:http" - kardinal.dev.service/plugins: | - - name: https://github.com/kurtosis-tech/free-currency-api-plugin.git - type: external - servicename: free-currency-api - args: - api_key: fca_live_VKZlykCWEiFcpBHnw74pzd4vLi04q1h9JySbVHDF + kardinal.dev.service/plugins: "jsdelivr-api" spec: type: ClusterIP selector: - app: frontend + app.kubernetes.io/name: frontend ports: - name: http port: 80 @@ -166,19 +153,16 @@ kind: Deployment metadata: name: postgres-v1 labels: - app: postgres - version: v1 + app.kubernetes.io/name: postgres spec: replicas: 1 selector: matchLabels: - app: postgres - version: v1 + app.kubernetes.io/name: postgres template: metadata: labels: - app: postgres - version: v1 + app.kubernetes.io/name: postgres spec: containers: - name: postgres @@ -206,36 +190,10 @@ kind: Service metadata: name: postgres labels: - app: postgres - version: v1 + app.kubernetes.io/name: postgres annotations: kardinal.dev.service/stateful: "true" - kardinal.dev.service/plugins: | - - name: github.com/kurtosis-tech/postgres-seed-plugin - args: - seed_script: | - -- create the table - CREATE TABLE IF NOT EXISTS public.items( - id bigserial PRIMARY KEY, - created_at TIMESTAMP WITH TIME ZONE, - updated_at TIMESTAMP WITH TIME ZONE, - deleted_at TIMESTAMP WITH TIME ZONE, - user_id TEXT, - product_id TEXT, - quantity INTEGER - ); - - INSERT INTO public.items (id, created_at, updated_at, deleted_at, user_id, product_id, quantity) - VALUES (1, '2024-08-02 13:02:07.656104 +00:00', '2024-08-02 13:02:07.656104 +00:00', null, '0494c5e0-dde0-48fa-a6d8-f7962f5476bf', '66VCHSJNUP', 1); - - INSERT INTO public.items (id, created_at, updated_at, deleted_at, user_id, product_id, quantity) - VALUES (2, '2024-08-02 13:02:10.891407 +00:00', '2024-08-02 13:02:10.891407 +00:00', null, '0494c5e0-dde0-48fa-a6d8-f7962f5476bf', '2ZYFJ3GM2N', 1); - - -- Set the sequence to the correct value after inserting records - SELECT setval('public.items_id_seq', (SELECT MAX(id) FROM public.items)); - db_name: "cart" - db_user: "postgresuser" - db_password: "postgrespass" + kardinal.dev.service/plugins: "postgres-seed-plugin" spec: type: ClusterIP @@ -245,7 +203,7 @@ spec: targetPort: 5432 protocol: TCP selector: - app: postgres + app.kubernetes.io/name: postgres --- apiVersion: apps/v1 @@ -253,18 +211,15 @@ kind: Deployment metadata: name: productcatalogservice-v1 labels: - app: productcatalogservice - version: v1 + app.kubernetes.io/name: productcatalogservice spec: selector: matchLabels: - app: productcatalogservice - version: v1 + app.kubernetes.io/name: productcatalogservice template: metadata: labels: - app: productcatalogservice - version: v1 + app.kubernetes.io/name: productcatalogservice spec: terminationGracePeriodSeconds: 5 containers: @@ -299,12 +254,11 @@ kind: Service metadata: name: productcatalogservice labels: - app: productcatalogservice - version: v1 + app.kubernetes.io/name: productcatalogservice spec: type: ClusterIP selector: - app: productcatalogservice + app.kubernetes.io/name: productcatalogservice ports: - name: http port: 8070 diff --git a/go.mod b/go.mod index 4b7b57e..e6d0dc1 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,9 @@ require ( github.com/onsi/gomega v1.33.1 github.com/samber/lo v1.47.0 github.com/sirupsen/logrus v1.9.3 + github.com/stretchr/testify v1.9.0 + istio.io/api v1.23.2 + istio.io/client-go v1.23.2 k8s.io/api v0.31.0 k8s.io/apimachinery v0.31.0 k8s.io/client-go v0.31.0 @@ -55,6 +58,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.19.1 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.55.0 // indirect @@ -90,8 +94,6 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - istio.io/api v1.23.2 // indirect - istio.io/client-go v1.23.2 // indirect k8s.io/apiextensions-apiserver v0.31.0 // indirect k8s.io/apiserver v0.31.0 // indirect k8s.io/component-base v0.31.0 // indirect diff --git a/go.sum b/go.sum index 3602659..5c123f5 100644 --- a/go.sum +++ b/go.sum @@ -22,8 +22,8 @@ github.com/dominikbraun/graph v0.23.0 h1:TdZB4pPqCLFxYhdyMFb1TBdFxp8XLcJfTTBQucV github.com/dominikbraun/graph v0.23.0/go.mod h1:yOjYyogZLY1LSG9E33JWZJiq5k83Qy2C6POAuiViluc= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= -github.com/evanphx/json-patch v0.5.2 h1:xVCHIVMUu1wtM/VkR9jVZ45N3FhZfYMMYGorLCR8P3k= -github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ= +github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U= +github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg= github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= diff --git a/kardinal/reconciler/reconciler.go b/kardinal/reconciler/reconciler.go index 17f1178..ac188b3 100644 --- a/kardinal/reconciler/reconciler.go +++ b/kardinal/reconciler/reconciler.go @@ -20,6 +20,11 @@ func Reconcile(ctx context.Context, cl client.Client) error { if err != nil { return stacktrace.Propagate(err, "An error occurred retrieving the list of resources") } + + if err := resources.InjectIstioLabelsInServicesAndDeployments(ctx, cl, clusterResources); err != nil { + return stacktrace.Propagate(err, "An error occurred injecting the Istio labels in the services and deployments") + } + // Generate base cluster topology logrus.Info("Generate base cluster topology") baseClusterTopology, err := topology.NewClusterTopologyFromResources(clusterResources) diff --git a/kardinal/resources/resources.go b/kardinal/resources/resources.go index 1b56d7f..611f762 100644 --- a/kardinal/resources/resources.go +++ b/kardinal/resources/resources.go @@ -18,11 +18,23 @@ import ( ) const ( - BaselineNamespace = "baseline" - trueStr = "true" - kardinalManagedLabelKey = "kardinal.dev/managed" + BaselineNamespace = "baseline" + defaultVersionLabelValue = "baseline" + trueStr = "true" + kardinalManagedLabelKey = "kardinal.dev/managed" + + // Thi is a common label used in several applications and recommended by Kubernetes: https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/ + appNameKubernetesLabelKey = "app.kubernetes.io/name" + appLabelKey = "app" + versionLabelKey = "version" ) +type labeledResources interface { + GetLabels() map[string]string + SetLabels(labels map[string]string) + GetName() string +} + type Resources struct { Namespaces []*Namespace } @@ -176,8 +188,6 @@ func ApplyServiceResources(ctx context.Context, clusterResources *Resources, clu } } } - // OPERATOR-TODO: Set app and version labels on non-managed service if not already set. - // Those labels are required by Istio. } } } @@ -372,3 +382,67 @@ func ApplyIngressResources(ctx context.Context, clusterResources *Resources, clu return nil } + +// OPERATOR-TODO make sure to execute this again once we connect the operator to listen to k8s Deployments and Services events +// OPERATOR-TODO there is another approach we could take, if it doesn't works for all use cases, which is to use MutatingAdmissionWebHooks +// related info for this here: https://book.kubebuilder.io/cronjob-tutorial/webhook-implementation and particularly this https://book.kubebuilder.io/reference/webhook-for-core-types +// for creating and webhook for these core types. +func InjectIstioLabelsInServicesAndDeployments(ctx context.Context, cl client.Client, clusterResources *Resources) error { + for _, namespace := range clusterResources.Namespaces { + for _, service := range namespace.Services { + shouldUpdateLabels := ensureIstioLabelsForResource(service) + if shouldUpdateLabels { + if err := cl.Update(ctx, service); err != nil { + return stacktrace.Propagate(err, "An error occurred adding Istio labels to service '%s'", service.GetName()) + } + } + + } + + for _, deployment := range namespace.Deployments { + shouldUpdateLabels := ensureIstioLabelsForResource(deployment) + if shouldUpdateLabels { + if err := cl.Update(ctx, deployment); err != nil { + return stacktrace.Propagate(err, "An error occurred adding Istio labels to deployment '%s'", deployment.GetName()) + } + } + } + } + return nil +} + +func ensureIstioLabelsForResource(resource labeledResources) bool { + + var areLabelsUpdated bool + + labels := resource.GetLabels() + if labels == nil { + labels = map[string]string{} + } + + // The 'app' label + _, ok := labels[appLabelKey] + if !ok { + areLabelsUpdated = true + appNameKubernetesLabelValue, ok := labels[appNameKubernetesLabelKey] + if ok { + labels[appLabelKey] = appNameKubernetesLabelValue + } else { + labels[appLabelKey] = resource.GetName() + } + } + + // The 'version' label + // OPERATOR-TODO how are we going to handle when a non-managed resource already has the "version" label and + // this value is different from the value needed for managing the baseline traffic + _, ok = labels[versionLabelKey] + if !ok { + areLabelsUpdated = true + labels[versionLabelKey] = defaultVersionLabelValue + } + if areLabelsUpdated { + resource.SetLabels(labels) + } + + return areLabelsUpdated +} diff --git a/kardinal/resources/resources_test.go b/kardinal/resources/resources_test.go new file mode 100644 index 0000000..9a1d3a3 --- /dev/null +++ b/kardinal/resources/resources_test.go @@ -0,0 +1,82 @@ +package resources + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +type labeledResourcesForTest struct { + labels map[string]string + name string +} + +func newLabeledResourcesForTest(labels map[string]string, name string) *labeledResourcesForTest { + return &labeledResourcesForTest{labels: labels, name: name} +} + +func (l *labeledResourcesForTest) GetLabels() map[string]string { + return l.labels +} + +func (l *labeledResourcesForTest) SetLabels(labels map[string]string) { + l.labels = labels +} + +func (l *labeledResourcesForTest) GetName() string { + return l.name +} + +func TestIstioLabelsEnsurer(t *testing.T) { + + withoutAnyIstioLabel := map[string]string{} + labeledResourceWithoutAnyIstioLabel := newLabeledResourcesForTest(withoutAnyIstioLabel, "my-service") + expectedLabels := map[string]string{ + "app": "my-service", + "version": "baseline", + } + + expectedBool := ensureIstioLabelsForResource(labeledResourceWithoutAnyIstioLabel) + require.True(t, expectedBool) + require.Equal(t, expectedLabels, labeledResourceWithoutAnyIstioLabel.GetLabels()) + + withK8sAppLabel := map[string]string{ + "app.kubernetes.io/name": "my-service-using-k8s-app", + } + labeledResourceWithK8sAppLabel := newLabeledResourcesForTest(withK8sAppLabel, "my-service") + expectedLabels = map[string]string{ + "app.kubernetes.io/name": "my-service-using-k8s-app", + "app": "my-service-using-k8s-app", + "version": "baseline", + } + expectedBool = ensureIstioLabelsForResource(labeledResourceWithK8sAppLabel) + require.True(t, expectedBool) + require.Equal(t, expectedLabels, labeledResourceWithK8sAppLabel.GetLabels()) + + withVersionLabel := map[string]string{ + "version": "v0.2.1", + } + labeledResourceWithVersionLabel := newLabeledResourcesForTest(withVersionLabel, "my-versioned-service") + expectedLabels = map[string]string{ + "app": "my-versioned-service", + "version": "v0.2.1", + } + + expectedBool = ensureIstioLabelsForResource(labeledResourceWithVersionLabel) + require.True(t, expectedBool) + require.Equal(t, expectedLabels, labeledResourceWithVersionLabel.GetLabels()) + + withVersionAndAppLabel := map[string]string{ + "app": "my-app", + "version": "v0.2.1", + } + labeledResourceWithVersionAndAppLabel := newLabeledResourcesForTest(withVersionAndAppLabel, "my-app-service") + expectedLabels = map[string]string{ + "app": "my-app", + "version": "v0.2.1", + } + + expectedBool = ensureIstioLabelsForResource(labeledResourceWithVersionAndAppLabel) + require.False(t, expectedBool) + require.Equal(t, expectedLabels, labeledResourceWithVersionAndAppLabel.GetLabels()) +}