From afca44d86970536c9938a9d75341de744f43b394 Mon Sep 17 00:00:00 2001 From: Ron Federman Date: Sun, 15 Dec 2024 12:44:37 +0200 Subject: [PATCH 1/3] Use GOMAXPROCS in gateway based on limits.cpu --- autoscaler/controllers/gateway/deployment.go | 13 +++++++++++++ tests/common/assert/pipeline-ready.yaml | 7 +++++++ .../03-assert-action-created.yaml | 6 ++++++ 3 files changed, 26 insertions(+) diff --git a/autoscaler/controllers/gateway/deployment.go b/autoscaler/controllers/gateway/deployment.go index 002ec1b5c..a89a66fd1 100644 --- a/autoscaler/controllers/gateway/deployment.go +++ b/autoscaler/controllers/gateway/deployment.go @@ -171,6 +171,19 @@ func getDesiredDeployment(dests *odigosv1.DestinationList, configDataHash string Name: "GOMEMLIMIT", Value: fmt.Sprintf("%dMiB", gateway.Spec.ResourcesSettings.GomemlimitMiB), }, + { + // let the Go runtime know how many CPUs are available, + // without this, Go will assume all the cores are available. + Name: "GOMAXPROCS", + ValueFrom: &corev1.EnvVarSource{ + ResourceFieldRef: &corev1.ResourceFieldSelector{ + ContainerName: containerName, + // limitCPU, Kubernetes automatically rounds up the value to an integer + // (700m -> 1, 1200m -> 2) + Resource: "limits.cpu", + }, + }, + }, }, SecurityContext: &corev1.SecurityContext{ AllowPrivilegeEscalation: boolPtr(false), diff --git a/tests/common/assert/pipeline-ready.yaml b/tests/common/assert/pipeline-ready.yaml index 2c52b1386..3995d8106 100644 --- a/tests/common/assert/pipeline-ready.yaml +++ b/tests/common/assert/pipeline-ready.yaml @@ -55,12 +55,19 @@ spec: fieldPath: metadata.name - name: GOMEMLIMIT (value != null): true + - name: GOMAXPROCS + valueFrom: + resourceFieldRef: + containerName: gateway + divisor: "0" + resource: limits.cpu name: gateway resources: requests: (memory != null): true limits: (memory != null): true + (cpu != null): true volumeMounts: - mountPath: /conf name: collector-conf diff --git a/tests/e2e/workload-lifecycle/03-assert-action-created.yaml b/tests/e2e/workload-lifecycle/03-assert-action-created.yaml index 8729b705a..74e66e758 100644 --- a/tests/e2e/workload-lifecycle/03-assert-action-created.yaml +++ b/tests/e2e/workload-lifecycle/03-assert-action-created.yaml @@ -63,6 +63,12 @@ spec: fieldPath: metadata.name - name: GOMEMLIMIT (value != null): true + - name: GOMAXPROCS + valueFrom: + resourceFieldRef: + containerName: gateway + divisor: "0" + resource: limits.cpu name: gateway resources: requests: From 244fc0a0b27c13d21d9441708c506ffa589201e5 Mon Sep 17 00:00:00 2001 From: Ron Federman Date: Sun, 15 Dec 2024 13:54:37 +0200 Subject: [PATCH 2/3] remove pipeline ready assert in cli-upgrade before the actual upgrade --- tests/e2e/cli-upgrade/chainsaw-test.yaml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/e2e/cli-upgrade/chainsaw-test.yaml b/tests/e2e/cli-upgrade/chainsaw-test.yaml index b9907593f..ad18173f9 100644 --- a/tests/e2e/cli-upgrade/chainsaw-test.yaml +++ b/tests/e2e/cli-upgrade/chainsaw-test.yaml @@ -103,10 +103,6 @@ spec: try: - apply: file: ../../common/apply/add-tempo-traces-destination.yaml - - name: Odigos pipeline ready - try: - - assert: - file: ../../common/assert/pipeline-ready.yaml - name: Simple-demo instrumented after destination added try: - assert: From 98314efe6288b7ca6011dfd6a87f7b8bf3f781fe Mon Sep 17 00:00:00 2001 From: Ron Federman Date: Sun, 15 Dec 2024 15:25:03 +0200 Subject: [PATCH 3/3] Add assert_pipeline_pods_ready.sh to run before the upgrade --- tests/common/assert/pipeline-ready.yaml | 1 + tests/common/assert_pipeline_pods_ready.sh | 45 ++++++++++++++++++++++ tests/e2e/cli-upgrade/chainsaw-test.yaml | 9 +++++ 3 files changed, 55 insertions(+) create mode 100755 tests/common/assert_pipeline_pods_ready.sh diff --git a/tests/common/assert/pipeline-ready.yaml b/tests/common/assert/pipeline-ready.yaml index 3995d8106..6c023c690 100644 --- a/tests/common/assert/pipeline-ready.yaml +++ b/tests/common/assert/pipeline-ready.yaml @@ -65,6 +65,7 @@ spec: resources: requests: (memory != null): true + (cpu != null): true limits: (memory != null): true (cpu != null): true diff --git a/tests/common/assert_pipeline_pods_ready.sh b/tests/common/assert_pipeline_pods_ready.sh new file mode 100755 index 000000000..43024745e --- /dev/null +++ b/tests/common/assert_pipeline_pods_ready.sh @@ -0,0 +1,45 @@ +#!/bin/bash + +DEFAULT_NAMESPACE="odigos-test" +DEFAULT_TIMEOUT="120s" +CHECK_INTERVAL=5 + +NAMESPACE=${1:-$DEFAULT_NAMESPACE} +TIMEOUT=${2:-$DEFAULT_TIMEOUT} + +# Define expected labels for pods (adjust as needed) +EXPECTED_LABELS=( + "odigos.io/collector-role=NODE_COLLECTOR" # For odigos-data-collection pods + "odigos.io/collector-role=CLUSTER_GATEWAY" # For odigos-gateway pods + "app.kubernetes.io/name=odigos-autoscaler" + "app.kubernetes.io/name=odigos-instrumentor" + "app.kubernetes.io/name=odigos-scheduler" + "app=odigos-ui" +) + +echo "Waiting for all expected pods to be ready in namespace '$NAMESPACE' with timeout of $TIMEOUT..." + +for label in "${EXPECTED_LABELS[@]}"; do + echo "Waiting for pods with label: $label..." + # Wait for pods to exist before using kubectl wait + EXISTS=false + while [[ "$EXISTS" == "false" ]]; do + POD_COUNT=$(kubectl get pods -l "$label" -n "$NAMESPACE" --no-headers 2>/dev/null | wc -l) + if [[ "$POD_COUNT" -gt 0 ]]; then + EXISTS=true + echo "Found $POD_COUNT pod(s) with label '$label'. Proceeding to wait for readiness..." + else + echo "No pods found with label '$label'. Checking again in $CHECK_INTERVAL seconds..." + sleep $CHECK_INTERVAL + fi + done + + # Use `kubectl wait` to check all pods matching the label + kubectl wait --for=condition=Ready pods -l "$label" -n "$NAMESPACE" --timeout="$TIMEOUT" + if [[ $? -ne 0 ]]; then + echo "Pods with label '$label' did not become ready within $TIMEOUT in namespace '$NAMESPACE'" + exit 1 + fi +done + +echo "All expected pods are ready!" diff --git a/tests/e2e/cli-upgrade/chainsaw-test.yaml b/tests/e2e/cli-upgrade/chainsaw-test.yaml index ad18173f9..e4171fe75 100644 --- a/tests/e2e/cli-upgrade/chainsaw-test.yaml +++ b/tests/e2e/cli-upgrade/chainsaw-test.yaml @@ -103,6 +103,15 @@ spec: try: - apply: file: ../../common/apply/add-tempo-traces-destination.yaml + - name: Odigos pipeline pods ready + # We make sure that the pipeline pods are ready before proceeding with the next steps + # This is intentionally different from pipeline-ready.yaml, which checks for the pipeline CRDs + # When adding a feature related to the pipeline, if we would use the same assert before the upgrade, the test would fail. + # Since the version installed here is latest official one. + try: + - script: + content: ../../common/assert_pipeline_pods_ready.sh + timeout: 60s - name: Simple-demo instrumented after destination added try: - assert: