From e3ec31cc8ee54350755295b9f213cb1ca9a2b4e9 Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Wed, 9 Oct 2024 10:23:27 -0400 Subject: [PATCH 1/4] feat: README + TODOs (#5) * README + todos * Typo --- README.md | 37 ++++++++++++++++++++++++++++++++- kardinal/resources/resources.go | 8 ++++--- kardinal/topology/service.go | 8 +++---- kardinal/topology/topology.go | 14 ++++++------- 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 972d668..0d2f873 100644 --- a/README.md +++ b/README.md @@ -1 +1,36 @@ -# kardinal-operator \ No newline at end of file +# Kardinal Operator + +Implementation of [Kardinal](https://github.com/kurtosis-tech/kardinal) as a K8S Operator. + +## Development + +Minikube + K8S manifest deployed. K8S context set to your local cluster. + +The following three commands are commonly used during development: + +``` +make lint (Run golangci linter. Can also be configured inside your IDE.) +make test (Run tests against local cluster) +make run (Run operator against your local cluster) +``` + +Manage custom resources with kubectl: + +```yaml +apiVersion: core.kardinal.dev/v1 +kind: Flow +metadata: + labels: + app.kubernetes.io/name: kardinal + app.kubernetes.io/managed-by: kustomize + name: flow-test + namespace: baseline +spec: + service: frontend + image: kurtosistech/frontend:demo-frontend +``` + +``` +kubectl create -f flow.yaml +kubectl delete -f flow.yaml +``` diff --git a/kardinal/resources/resources.go b/kardinal/resources/resources.go index 1a3a8a7..1b56d7f 100644 --- a/kardinal/resources/resources.go +++ b/kardinal/resources/resources.go @@ -149,8 +149,8 @@ func AddAnnotations(obj *metav1.ObjectMeta, annotations map[string]string) { } } -// TODO: Add create, update and delete global options -// TODO: Refactor the Apply... functions +// OPERATOR-TODO: Add create, update and delete global options +// OPERATOR-TODO: Refactor the Apply... functions func ApplyServiceResources(ctx context.Context, clusterResources *Resources, clusterTopologyResources *Resources, cl client.Client) error { for _, namespace := range clusterResources.Namespaces { @@ -176,6 +176,8 @@ 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. } } } @@ -241,7 +243,7 @@ func ApplyDeploymentResources(ctx context.Context, clusterResources *Resources, /* else { annotationsToAdd := map[string]string{ "sidecar.istio.io/inject": "true", - // TODO: make this a flag to help debugging + // KARDINAL-TODO: make this a flag to help debugging // One can view the logs with: kubeclt logs -f -l app= -n -c istio-proxy "sidecar.istio.io/componentLogLevel": "lua:info", } diff --git a/kardinal/topology/service.go b/kardinal/topology/service.go index 72e4652..e586a01 100644 --- a/kardinal/topology/service.go +++ b/kardinal/topology/service.go @@ -101,7 +101,7 @@ func (service *Service) GetAppsV1Deployment(namespace string) *appsv1.Deployment deployment.Spec.Template.ObjectMeta = metav1.ObjectMeta{ Annotations: map[string]string{ "sidecar.istio.io/inject": trueStr, - // TODO: make this a flag to help debugging + // KARDINAL-TODO: make this a flag to help debugging // One can view the logs with: kubeclt logs -f -l app= -n -c istio-proxy "sidecar.istio.io/componentLogLevel": "lua:info", }, @@ -195,7 +195,7 @@ func (service *Service) GetVirtualService(services []*Service) (*istioclient.Vir destinationRule := service.GetDestinationRule(services) for _, svc := range services { - // TODO: Support for multiple ports + // KARDINAL-TODO: Support for multiple ports servicePort := &svc.ServiceSpec.Ports[0] var flowHost *string @@ -227,7 +227,7 @@ func (service *Service) GetVirtualService(services []*Service) (*istioclient.Vir } func (service *Service) GetDestinationRule(services []*Service) *istioclient.DestinationRule { - // TODO(shared-annotation) - we could store "shared" versions somewhere so that the pointers are the same + // KARDINAL-TODO(shared-annotation) - we could store "shared" versions somewhere so that the pointers are the same // if we do that then the render work around isn't necessary subsets := lo.UniqBy( lo.Map(services, func(svc *Service, _ int) *v1alpha3.Subset { @@ -238,7 +238,7 @@ func (service *Service) GetDestinationRule(services []*Service) *istioclient.Des }, } - // TODO Narrow down this configuration to only subsets created for telepresence intercepts or find a way to enable TLS for telepresence intercepts https://github.com/kurtosis-tech/kardinal-kontrol/issues/14 + // KARDINAL-TODO Narrow down this configuration to only subsets created for telepresence intercepts or find a way to enable TLS for telepresence intercepts https://github.com/kurtosis-tech/kardinal-kontrol/issues/14 // This config is necessary for Kardinal/Telepresence (https://www.telepresence.io/) integration if svc.IsManaged { newTrafficPolicy := &v1alpha3.TrafficPolicy{ diff --git a/kardinal/topology/topology.go b/kardinal/topology/topology.go index 80bee44..d0e4dc1 100644 --- a/kardinal/topology/topology.go +++ b/kardinal/topology/topology.go @@ -108,7 +108,7 @@ func (clusterTopology *ClusterTopology) UpdateWithFlow( clusterTopology.UpdateDependencies(service, modifiedService) // create versioned parents for non http stateful services - // TODO - this should be done for all non http services and not just the stateful ones + // KARDINAL-TODO - this should be done for all non http services and not just the stateful ones // every child should be copied; immediate parent duplicated // if children of non http services support http then our routing will have to be modified // we should treat those http services as non http; a hack could be to remove the appProtocol HTTP marking @@ -178,7 +178,7 @@ func (clusterTopology *ClusterTopology) GetResources() (*resources.Resources, er }) for _, services := range groupedServices { if len(services) > 0 { - // TODO: this assumes service specs didn't change. May we need a new version to ClusterTopology data structure + // KARDINAL-TODO: this assumes service specs didn't change. May we need a new version to ClusterTopology data structure // ServiceSpec is nil for external services - don't process anything bc theres nothing to add to the cluster if services[0].ServiceSpec == nil { @@ -190,7 +190,7 @@ func (clusterTopology *ClusterTopology) GetResources() (*resources.Resources, er resourceNamespace.VirtualServices = append(resourceNamespace.VirtualServices, virtualService) resourceNamespace.DestinationRules = append(resourceNamespace.DestinationRules, destinationRule) - // TODO: Add authz policies + // OPERATOR-TODO: Add authz policies } } @@ -239,7 +239,7 @@ func (clusterTopology *ClusterTopology) ApplyResources(ctx context.Context, clus return stacktrace.Propagate(err, "An error occurred applying the virtual service resources") } - // TODO: Apply ingress resources + // OPERATOR-TODO: Apply ingress resources /* err = resources.ApplyIngressResources(ctx, clusterResources, clusterTopologyResources, cl) if err != nil { return stacktrace.Propagate(err, "An error occurred applying the ingress resources") @@ -319,9 +319,7 @@ func (clusterTopology *ClusterTopology) Merge(clusterTopologies []*ClusterTopolo mergedClusterTopology.Ingress.ActiveFlowIDs = lo.Uniq(mergedClusterTopology.Ingress.ActiveFlowIDs) logrus.Infof("Services length: %d", len(mergedClusterTopology.Services)) - // TODO improve the filtering method, we could implement the `Service.Equal` method to compare and filter the services - // TODO and inside this method we could use the k8s service marshall method (https://pkg.go.dev/k8s.io/api/core/v1#Service.Marsha) and also the same for other k8s fields - // TODO it should be faster + // KARDINAL-TODO improve the filtering method, we could implement the `Service.Equal` method to compare and filter the services and inside this method we could use the k8s service marshall method (https://pkg.go.dev/k8s.io/api/core/v1#Service.Marsha) and also the same for other k8s fields it should be faster mergedClusterTopology.Services = lo.UniqBy(mergedClusterTopology.Services, func(service *Service) ServiceVersion { serviceVersion := ServiceVersion{ ServiceID: service.ServiceID, @@ -473,7 +471,7 @@ func processServices(services []*corev1.Service, deployments []*appsv1.Deploymen clusterTopologyServices = append(clusterTopologyServices, clusterTopologyService) } - // TODO: Use the dependency CRs instead + // OPERATOR-TODO: Use the dependency CRs instead for _, svcWithDependenciesAnnotation := range serviceWithDependencies { serviceAndPorts := strings.Split(svcWithDependenciesAnnotation.dependenciesAnnotation, ",") From e7a3b53ccbc23a7050df1e03374757a083e07e26 Mon Sep 17 00:00:00 2001 From: leoporoli Date: Wed, 16 Oct 2024 09:14:57 -0300 Subject: [PATCH 2/4] docs: adding instructions for updating CRDs API and RBAC permissions (#6) * how to update flow API * removed duplicated group-version file * rbac instructions * replace the deprecated kubernetes.io/ingress.class in the obd-demo.yaml file * adding a OPERATOR-TODO to fix service version * fix after lint warnings * adding flow-test manifest file * add more kubectl cmds in README --- .dockerignore | 1 + README.md | 46 +++++++++++++++++++++++++++++--- api/v1/groupversion_info.go | 36 ------------------------- ci/flow-test.yaml | 11 ++++++++ ci/obd-demo.yaml | 2 +- config/samples/core_v1_flow.yaml | 3 ++- kardinal/topology/topology.go | 5 +++- 7 files changed, 62 insertions(+), 42 deletions(-) delete mode 100644 api/v1/groupversion_info.go create mode 100644 ci/flow-test.yaml diff --git a/.dockerignore b/.dockerignore index a3aab7a..1416cb2 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,3 +1,4 @@ # More info: https://docs.docker.com/engine/reference/builder/#dockerignore-file # Ignore build and test binaries. bin/ +.idea diff --git a/README.md b/README.md index 0d2f873..729c562 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,10 @@ Implementation of [Kardinal](https://github.com/kurtosis-tech/kardinal) as a K8S ## Development -Minikube + K8S manifest deployed. K8S context set to your local cluster. +Minikube + K8S manifest deployed. K8S context set to your local cluster. +``` +make install (to install the CRDs into the cluster) +``` The following three commands are commonly used during development: @@ -31,6 +34,43 @@ spec: ``` ``` -kubectl create -f flow.yaml -kubectl delete -f flow.yaml +# Create a flow +kubectl create -f ./ci/flow-test.yaml + +# Delete a flow +kubectl delete -f ./ci/flow-test.yaml + +# Get all flows in namespace +kubectl get flows -n baseline + +# Describe a flow by its name +kubectl describe flows flow-test -n baseline +``` + +Deploy the operator inside the cluster +``` +make deploy (when you want to test it inside the cluster) ``` + +## Update the CRDs API + +1. Read [this document][api-design-doc] to follow the design rules. +2. The CRDs API files are inside the `./api/core/v1` folder. +3. You can edit the `flow` API for example: + 1. Add, update or remove fields in the `FlowSpec` inside the `flow_types.go` file. Don't forget to add the json tags. + 2. Run `make manifests` to include your changes in the auto generated `./config/crd/bases/core.kardinal.dev_flows.yaml` manifest file. + 3. Update the spec example inside `./config/samples/core_v1_flow.yaml` +4. If you are adding a new CRD make sure its schema has been added in the `init` function in the `./cmd/main.go` file + + +## Update the RBAC permissions + +1. Read [this document][rbac-markers-doc] to understand what are the RBAC markers and how to compose them. +2. Add, update or remove the RBAC markers, for instance the `flow` controller: + 1. Open the flow controller file `./internal/controller/core/flow_controller.go` + 2. Edit the markers inside of it. + 3. Run `make manifests` to include your changes in the auto generated `./config/rbac/role.yaml` manifest file. + 4. NOTE: If you receive an error, please run the specified command in the error and re-run make manifests. + +[api-design-doc]: https://book.kubebuilder.io/cronjob-tutorial/api-design +[rbac-markers-doc]: https://book.kubebuilder.io/reference/markers/rbac \ No newline at end of file diff --git a/api/v1/groupversion_info.go b/api/v1/groupversion_info.go deleted file mode 100644 index 6893d39..0000000 --- a/api/v1/groupversion_info.go +++ /dev/null @@ -1,36 +0,0 @@ -/* -Copyright 2024. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Package v1 contains API Schema definitions for the core v1 API group -// +kubebuilder:object:generate=true -// +groupName=core.kardinal.dev -package v1 - -import ( - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/scheme" -) - -var ( - // GroupVersion is group version used to register these objects - GroupVersion = schema.GroupVersion{Group: "core.kardinal.dev", Version: "v1"} - - // SchemeBuilder is used to add go types to the GroupVersionKind scheme - SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} - - // AddToScheme adds the types in this group-version to the given scheme. - AddToScheme = SchemeBuilder.AddToScheme -) diff --git a/ci/flow-test.yaml b/ci/flow-test.yaml new file mode 100644 index 0000000..45d4010 --- /dev/null +++ b/ci/flow-test.yaml @@ -0,0 +1,11 @@ +apiVersion: core.kardinal.dev/v1 +kind: Flow +metadata: + labels: + app.kubernetes.io/name: kardinal + app.kubernetes.io/managed-by: kustomize + name: flow-test + namespace: baseline +spec: + service: frontend + image: kurtosistech/frontend:demo-frontend \ No newline at end of file diff --git a/ci/obd-demo.yaml b/ci/obd-demo.yaml index e1b766d..efd4d10 100644 --- a/ci/obd-demo.yaml +++ b/ci/obd-demo.yaml @@ -317,10 +317,10 @@ apiVersion: networking.k8s.io/v1 kind: Ingress metadata: annotations: - kubernetes.io/ingress.class: nginx kardinal.dev.service/ingress: "true" name: ingress spec: + ingressClassName: nginx rules: - host: prod.app.localhost http: diff --git a/config/samples/core_v1_flow.yaml b/config/samples/core_v1_flow.yaml index 996d083..b668d65 100644 --- a/config/samples/core_v1_flow.yaml +++ b/config/samples/core_v1_flow.yaml @@ -6,4 +6,5 @@ metadata: app.kubernetes.io/managed-by: kustomize name: flow-sample spec: - # TODO(user): Add fields here + service: "frontend" + image: "kurtosistech/frontend:demo-frontend" diff --git a/kardinal/topology/topology.go b/kardinal/topology/topology.go index d0e4dc1..aced8fd 100644 --- a/kardinal/topology/topology.go +++ b/kardinal/topology/topology.go @@ -365,7 +365,10 @@ func (clusterTopology *ClusterTopology) GetNetIngresses() ([]*net.Ingress, []*co for _, pathOriginal := range ruleOriginal.HTTP.Paths { target := clusterTopology.GetServiceByVersion(namespace, pathOriginal.Backend.Service.Name, activeFlowID) // fallback to baseline if backend not found at the active flow - // the baseline topology (or prod topology) flow ID and flow version are equal to the namespace these three should use same value + // OPERATOR-TODO + // the baseline topology (or prod topology) flow ID and flow version are equal to the namespace these three should use same value //TODO this doesn't apply for the current approach + // TODO with Kardinal-Kontrol the baselineFlowVersion and namespace where equal because Kontrol modify the Service's metadata like the 'version' label + // TODO we need to find a way to get the real version, not modified baselineFlowVersion := namespace if target == nil { target = clusterTopology.GetServiceByVersion(namespace, pathOriginal.Backend.Service.Name, baselineFlowVersion) From 7c10b561264afd70fa4dc44ea9f5ef2eef649af5 Mon Sep 17 00:00:00 2001 From: leoporoli Date: Wed, 16 Oct 2024 09:22:14 -0300 Subject: [PATCH 3/4] fix: fallback to baseline in GetNetIngresses (#7) * fix fallback to baseline in GetNetIngresses * fix after lint warnings --- kardinal/topology/topology.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/kardinal/topology/topology.go b/kardinal/topology/topology.go index aced8fd..5ef189f 100644 --- a/kardinal/topology/topology.go +++ b/kardinal/topology/topology.go @@ -63,6 +63,16 @@ func (clusterTopology *ClusterTopology) GetServiceByVersion(namespace string, na return nil } +func (clusterTopology *ClusterTopology) GetBaselineFlowService(namespace string, name string) *Service { + for _, service := range clusterTopology.Services { + if service.Namespace == namespace && service.ServiceID == name && !service.IsManaged { + return service + } + } + + return nil +} + func (clusterTopology *ClusterTopology) UpdateWithFlow( clusterGraph graph.Graph[ServiceHash, *Service], flowId string, @@ -365,13 +375,8 @@ func (clusterTopology *ClusterTopology) GetNetIngresses() ([]*net.Ingress, []*co for _, pathOriginal := range ruleOriginal.HTTP.Paths { target := clusterTopology.GetServiceByVersion(namespace, pathOriginal.Backend.Service.Name, activeFlowID) // fallback to baseline if backend not found at the active flow - // OPERATOR-TODO - // the baseline topology (or prod topology) flow ID and flow version are equal to the namespace these three should use same value //TODO this doesn't apply for the current approach - // TODO with Kardinal-Kontrol the baselineFlowVersion and namespace where equal because Kontrol modify the Service's metadata like the 'version' label - // TODO we need to find a way to get the real version, not modified - baselineFlowVersion := namespace if target == nil { - target = clusterTopology.GetServiceByVersion(namespace, pathOriginal.Backend.Service.Name, baselineFlowVersion) + target = clusterTopology.GetBaselineFlowService(namespace, pathOriginal.Backend.Service.Name) } if target != nil { path := *pathOriginal.DeepCopy() From fe5a044dc99d403d34194676fbdf634a64b2a7a2 Mon Sep 17 00:00:00 2001 From: leoporoli Date: Thu, 17 Oct 2024 12:07:08 -0300 Subject: [PATCH 4/4] 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()) +}