Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests refactor #451

Merged
merged 6 commits into from
Oct 22, 2024
Merged

Add tests refactor #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3bf136f
add e2e for apps
klinch0 Oct 15, 2024
4512e10
Refactor e2e tests for applications
kvaps Oct 18, 2024
8251a22
fix tests
klinch0 Oct 22, 2024
f409573
fix app tests
klinch0 Oct 22, 2024
d640ef1
fix install yq jq
klinch0 Oct 22, 2024
95897d6
refactor
kvaps Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,14 @@ repos:
- repo: https://github.com/igorshubovych/markdownlint-cli
rev: v0.41.0
hooks:
- id: markdownlint
args: [--fix, --disable, MD013, MD041, --]
- id: markdownlint
args: [--fix, --disable, MD013, MD041, --]
- repo: local
hooks:
- id: gen-versions-map
name: Generate versions map and check for changes
entry: sh -c 'make -C packages/apps check-version-map && make -C packages/extra check-version-map'
language: system
types: [file]
pass_filenames: false
description: Run the script and fail if it generates changes
135 changes: 135 additions & 0 deletions hack/e2e.application.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
#!/bin/bash

RED='\033[0;31m'
GREEN='\033[0;32m'
RESET='\033[0m'
YELLOW='\033[0;33m'


ROOT_NS="tenant-root"
TEST_TENANT="tenant-e2e"

function clean() {
kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS
if true; then
echo -e "${GREEN}Cleanup successful!${RESET}"
return 0
else
echo -e "${RED}Cleanup failed!${RESET}"
return 1
fi
}
Comment on lines +12 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve error handling in the clean function.

The current implementation always reports success, regardless of whether the deletion was actually successful. This could lead to false positives.

Consider modifying the function to actually check the deletion status:

 function clean() {
-    kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS
-    if true; then
+    if kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS; then
         echo -e "${GREEN}Cleanup successful!${RESET}"
         return 0
     else
         echo -e "${RED}Cleanup failed!${RESET}"
         return 1
     fi
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function clean() {
kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS
if true; then
echo -e "${GREEN}Cleanup successful!${RESET}"
return 0
else
echo -e "${RED}Cleanup failed!${RESET}"
return 1
fi
}
function clean() {
if kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS; then
echo -e "${GREEN}Cleanup successful!${RESET}"
return 0
else
echo -e "${RED}Cleanup failed!${RESET}"
return 1
fi
}


function install_helmrelease() {
local release_name="$1"
local namespace="$2"
local chart_path="$3"
local repo_name="$4"
local repo_ns="$5"
local values_file="$6"

if [[ -z "$release_name" ]]; then
echo -e "${RED}Error: Release name is required.${RESET}"
exit 1
fi

if [[ -z "$namespace" ]]; then
echo -e "${RED}Error: Namespace name is required.${RESET}"
exit 1
fi

if [[ -z "$chart_path" ]]; then
echo -e "${RED}Error: Chart path name is required.${RESET}"
exit 1
fi

local helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
{
echo "apiVersion: helm.toolkit.fluxcd.io/v2"
echo "kind: HelmRelease"
echo "metadata:"
echo " labels:"
echo " cozystack.io/ui: \"true\""
echo " name: \"$release_name\""
echo " namespace: \"$namespace\""
echo "spec:"
echo " chart:"
echo " spec:"
echo " chart: \"$chart_path\""
echo " reconcileStrategy: Revision"
echo " sourceRef:"
echo " kind: HelmRepository"
echo " name: \"$repo_name\""
echo " namespace: \"$repo_ns\""
echo " version: '*'"
echo " interval: 1m0s"
echo " timeout: 5m0s"

if [[ -n "$values_file" && -f "$values_file" ]]; then
echo " values:"
cat "$values_file" | sed 's/^/ /'
fi
} > "$helmrelease_file"

kubectl apply -f "$helmrelease_file"

rm -f "$helmrelease_file"
}
Comment on lines +23 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve error handling in install_helmrelease function.

The function is well-structured, but there's room for improvement in error handling.

  1. Address the Shellcheck warning by separating the declaration and assignment of helmrelease_file:
-    local helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
+    local helmrelease_file
+    helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml) || { echo "Failed to create temp file"; exit 1; }
  1. Add error handling for the kubectl apply command:
-    kubectl apply -f "$helmrelease_file"
+    if ! kubectl apply -f "$helmrelease_file"; then
+        echo -e "${RED}Failed to apply HelmRelease${RESET}"
+        rm -f "$helmrelease_file"
+        return 1
+    fi

These changes will make the function more robust and prevent potential issues from silent failures.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function install_helmrelease() {
local release_name="$1"
local namespace="$2"
local chart_path="$3"
local repo_name="$4"
local repo_ns="$5"
local values_file="$6"
if [[ -z "$release_name" ]]; then
echo -e "${RED}Error: Release name is required.${RESET}"
exit 1
fi
if [[ -z "$namespace" ]]; then
echo -e "${RED}Error: Namespace name is required.${RESET}"
exit 1
fi
if [[ -z "$chart_path" ]]; then
echo -e "${RED}Error: Chart path name is required.${RESET}"
exit 1
fi
local helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
{
echo "apiVersion: helm.toolkit.fluxcd.io/v2"
echo "kind: HelmRelease"
echo "metadata:"
echo " labels:"
echo " cozystack.io/ui: \"true\""
echo " name: \"$release_name\""
echo " namespace: \"$namespace\""
echo "spec:"
echo " chart:"
echo " spec:"
echo " chart: \"$chart_path\""
echo " reconcileStrategy: Revision"
echo " sourceRef:"
echo " kind: HelmRepository"
echo " name: \"$repo_name\""
echo " namespace: \"$repo_ns\""
echo " version: '*'"
echo " interval: 1m0s"
echo " timeout: 5m0s"
if [[ -n "$values_file" && -f "$values_file" ]]; then
echo " values:"
cat "$values_file" | sed 's/^/ /'
fi
} > "$helmrelease_file"
kubectl apply -f "$helmrelease_file"
rm -f "$helmrelease_file"
}
function install_helmrelease() {
local release_name="$1"
local namespace="$2"
local chart_path="$3"
local repo_name="$4"
local repo_ns="$5"
local values_file="$6"
if [[ -z "$release_name" ]]; then
echo -e "${RED}Error: Release name is required.${RESET}"
exit 1
fi
if [[ -z "$namespace" ]]; then
echo -e "${RED}Error: Namespace name is required.${RESET}"
exit 1
fi
if [[ -z "$chart_path" ]]; then
echo -e "${RED}Error: Chart path name is required.${RESET}"
exit 1
fi
local helmrelease_file
helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml) || { echo "Failed to create temp file"; exit 1; }
{
echo "apiVersion: helm.toolkit.fluxcd.io/v2"
echo "kind: HelmRelease"
echo "metadata:"
echo " labels:"
echo " cozystack.io/ui: \"true\""
echo " name: \"$release_name\""
echo " namespace: \"$namespace\""
echo "spec:"
echo " chart:"
echo " spec:"
echo " chart: \"$chart_path\""
echo " reconcileStrategy: Revision"
echo " sourceRef:"
echo " kind: HelmRepository"
echo " name: \"$repo_name\""
echo " namespace: \"$repo_ns\""
echo " version: '*'"
echo " interval: 1m0s"
echo " timeout: 5m0s"
if [[ -n "$values_file" && -f "$values_file" ]]; then
echo " values:"
cat "$values_file" | sed 's/^/ /'
fi
} > "$helmrelease_file"
if ! kubectl apply -f "$helmrelease_file"; then
echo -e "${RED}Failed to apply HelmRelease${RESET}"
rm -f "$helmrelease_file"
return 1
fi
rm -f "$helmrelease_file"
}
🧰 Tools
🪛 Shellcheck

[warning] 46-46: Declare and assign separately to avoid masking return values.

(SC2155)


function install_tenant (){
local release_name="$1"
local namespace="$2"
local values_file="${3:-tenant.yaml}"
local repo_name="cozystack-apps"
local repo_ns="cozy-public"

install_helmrelease "$release_name" "$namespace" "tenant" "$repo_name" "$repo_ns" "$values_file"
}

function check_helmrelease_status() {
local release_name="$1"
local namespace="$2"
local timeout=300 # Timeout in seconds
local interval=5 # Interval between checks in seconds
local elapsed=0

while [[ $elapsed -lt $timeout ]]; do
local status_output
status_output=$(kubectl get helmrelease "$release_name" -n "$namespace" -o json | jq -r '.status.conditions[-1].reason')

if [[ "$status_output" == "InstallSucceeded" ]]; then
echo -e "${GREEN}Helm release '$release_name' is ready.${RESET}"
return 0
elif [[ "$status_output" == "InstallFailed" ]]; then
echo -e "${RED}Helm release '$release_name': InstallFailed${RESET}"
exit 1
else
echo -e "${YELLOW}Helm release '$release_name' is not ready. Current status: $status_output${RESET}"
fi

sleep "$interval"
elapsed=$((elapsed + interval))
done

echo -e "${RED}Timeout reached. Helm release '$release_name' is still not ready after $timeout seconds.${RESET}"
exit 1
}

chart_name="$1"

if [ -z "$chart_name" ]; then
echo -e "${RED}No chart name provided. Exiting...${RESET}"
exit 1
fi

echo "Running tests for chart: $chart_name"
install_tenant $TEST_TENANT $ROOT_NS
check_helmrelease_status $TEST_TENANT $ROOT_NS

repo_name="cozystack-apps"
repo_ns="cozy-public"

release_name="$chart_name-e2e"
install_helmrelease "$release_name" "$TEST_TENANT" "$chart_name" "$repo_name" "$repo_ns"

check_helmrelease_status "$release_name" "$TEST_TENANT"
6 changes: 6 additions & 0 deletions hack/values/tenant.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
host: ""
etcd: false
monitoring: false
ingress: false
seaweedfs: false
isolated: true
4 changes: 2 additions & 2 deletions packages/apps/ferretdb/templates/init-script.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ stringData:
DROP USER $user;
EOT
done

echo "== create roles"
psql -v ON_ERROR_STOP=1 --echo-all <<\EOT
SELECT 'CREATE ROLE app_admin NOINHERIT;'
Expand All @@ -83,7 +83,7 @@ stringData:
FOR schema_record IN SELECT schema_name FROM information_schema.schemata WHERE schema_name NOT IN ('pg_catalog', 'information_schema') LOOP
-- Changing Schema Ownership
EXECUTE format('ALTER SCHEMA %I OWNER TO %I', schema_record.schema_name, 'app_admin');

-- Add rights for the admin role
EXECUTE format('GRANT ALL ON SCHEMA %I TO %I', schema_record.schema_name, 'app_admin');
EXECUTE format('GRANT ALL ON ALL TABLES IN SCHEMA %I TO %I', schema_record.schema_name, 'app_admin');
Expand Down
Empty file modified packages/core/testing/Chart.yaml
100644 → 100755
View file
Open in desktop
Empty file.
16 changes: 13 additions & 3 deletions packages/core/testing/Makefile
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
NAMESPACE=cozy-e2e-tests
NAME := sandbox
CLEAN := 1
TESTING_APPS := $(shell find ../../apps -maxdepth 1 -mindepth 1 -type d | awk -F/ '{print $$NF}')

include ../../../scripts/common-envs.mk

Expand Down Expand Up @@ -32,10 +33,19 @@ image-e2e-sandbox:
yq -i '.e2e.image = strenv(IMAGE)' values.yaml
rm -f images/e2e-sandbox.json

test: wait-for-sandbox ## Run the end-to-end tests in existing sandbox.
cat ../../../hack/e2e.sh | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c 'cat > /e2e.sh && chmod +x /e2e.sh'
copy-hack-dir:
tar -C ../../../ -cf- hack | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- tar -xf-
Comment on lines +36 to +37
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using kubectl cp to simplify directory copying

Using kubectl cp can streamline the process of copying the hack directory into the Kubernetes deployment, making the command more readable and maintainable.

Apply this diff to use kubectl cp:

copy-hack-dir:
-	tar -C ../../../ -cf- hack | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- tar -xf-
+	kubectl cp ../../../hack $(NAMESPACE)/deploy/cozystack-e2e-$(NAME):/hack

Note: Ensure that the kubectl cp command works correctly in your environment and that the pod has the necessary permissions to receive files.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
copy-hack-dir:
tar -C ../../../ -cf- hack | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- tar -xf-
copy-hack-dir:
kubectl cp ../../../hack $(NAMESPACE)/deploy/cozystack-e2e-$(NAME):/hack


test: wait-for-sandbox copy-hack-dir ## Run the end-to-end tests in existing sandbox.
helm template -n cozy-system installer ../installer | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c 'cat > /cozystack-installer.yaml'
kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c 'export COZYSTACK_INSTALLER_YAML=$$(cat /cozystack-installer.yaml) && /e2e.sh'
kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c 'export COZYSTACK_INSTALLER_YAML=$$(cat /cozystack-installer.yaml) && /hack/e2e.sh'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid passing large files through environment variables

Assigning the contents of /cozystack-installer.yaml to an environment variable may lead to issues if the file size exceeds shell limits.

Consider passing the file path to the script instead. Apply this diff:

	kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c \
-	'export COZYSTACK_INSTALLER_YAML=$$(cat /cozystack-installer.yaml) && /hack/e2e.sh'
+	'/hack/e2e.sh /cozystack-installer.yaml'

Then, update /hack/e2e.sh to accept the installer file path as an argument and read from it internally.

This approach avoids potential issues with environment variable size limits and makes the script more flexible.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c 'export COZYSTACK_INSTALLER_YAML=$$(cat /cozystack-installer.yaml) && /hack/e2e.sh'
kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c \
'/hack/e2e.sh /cozystack-installer.yaml'


test-applications: wait-for-sandbox copy-hack-dir ## Run the end-to-end tests in existing sandbox for applications.
for app in $(TESTING_APPS); do \
echo "Running tests for $${app}"; \
kubectl exec -ti -n cozy-e2e-tests deploy/cozystack-e2e-sandbox -- bash -c "/hack/e2e.application.sh $${app}"; \
done
kubectl exec -ti -n cozy-e2e-tests deploy/cozystack-e2e-sandbox -- bash -c "kubectl get hr -A | grep -v 'True'"
Comment on lines +43 to +48
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use variables for namespace and deployment name for consistency

The hardcoded namespace cozy-e2e-tests and deployment name cozystack-e2e-sandbox should be replaced with $(NAMESPACE) and cozystack-e2e-$(NAME) to maintain consistency and ease of maintenance.

Apply this diff to utilize the predefined variables:

test-applications: wait-for-sandbox copy-hack-dir ## Run the end-to-end tests in existing sandbox for applications.
	for app in $(TESTING_APPS); do \
		echo "Running tests for $${app}"; \
-		kubectl exec -ti -n cozy-e2e-tests deploy/cozystack-e2e-sandbox -- bash -c "/hack/e2e.application.sh $${app}"; \
+		kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- bash -c "/hack/e2e.application.sh $${app}"; \
	done
-	kubectl exec -ti -n cozy-e2e-tests deploy/cozystack-e2e-sandbox -- bash -c "kubectl get hr -A | grep -v 'True'"
+	kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- bash -c "kubectl get hr -A | grep -v 'True'"

This change improves readability and ensures that any future updates to $(NAMESPACE) or $(NAME) are automatically reflected throughout the script.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test-applications: wait-for-sandbox copy-hack-dir ## Run the end-to-end tests in existing sandbox for applications.
for app in $(TESTING_APPS); do \
echo "Running tests for $${app}"; \
kubectl exec -ti -n cozy-e2e-tests deploy/cozystack-e2e-sandbox -- bash -c "/hack/e2e.application.sh $${app}"; \
done
kubectl exec -ti -n cozy-e2e-tests deploy/cozystack-e2e-sandbox -- bash -c "kubectl get hr -A | grep -v 'True'"
test-applications: wait-for-sandbox copy-hack-dir ## Run the end-to-end tests in existing sandbox for applications.
for app in $(TESTING_APPS); do \
echo "Running tests for $${app}"; \
kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- bash -c "/hack/e2e.application.sh $${app}"; \
done
kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- bash -c "kubectl get hr -A | grep -v 'True'"


delete: ## Remove sandbox from existing Kubernetes cluster.
kubectl delete deploy -n $(NAMESPACE) cozystack-e2e-$(NAME)
Expand Down
3 changes: 2 additions & 1 deletion packages/core/testing/images/e2e-sandbox/Dockerfile
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ ARG TALOSCTL_VERSION=1.7.6
ARG HELM_VERSION=3.15.4

RUN apt-get update
RUN apt-get -y install genisoimage qemu-kvm qemu-utils iproute2 iptables wget xz-utils netcat curl
RUN apt-get -y install genisoimage qemu-kvm qemu-utils iproute2 iptables wget xz-utils netcat curl jq
RUN curl -LO "https://github.com/siderolabs/talos/releases/download/v${TALOSCTL_VERSION}/talosctl-linux-amd64" \
&& chmod +x talosctl-linux-amd64 \
&& mv talosctl-linux-amd64 /usr/local/bin/talosctl
RUN curl -LO "https://dl.k8s.io/release/v${KUBECTL_VERSION}/bin/linux/amd64/kubectl" \
&& chmod +x kubectl \
&& mv kubectl /usr/local/bin/kubectl
RUN curl -sSL https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3 | bash -s - --version "v${HELM_VERSION}"
RUN wget https://github.com/mikefarah/yq/releases/download/v4.44.3/yq_linux_amd64 -O /usr/local/bin/yq && chmod +x /usr/local/bin/yq
Empty file modified packages/core/testing/templates/sandbox.yaml
100644 → 100755
View file
Open in desktop
Empty file.
2 changes: 1 addition & 1 deletion packages/core/testing/values.yaml
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
e2e:
image: ghcr.io/aenix-io/cozystack/e2e-sandbox:v0.17.1@sha256:c3390f0076f4a8445273d0cdeda7725ac6f5110ad35e1a286be4b158708c4402
image: ghcr.io/aenix-io/cozystack/e2e-sandbox:latest@sha256:1a26a511b9e269bcb607e2d80f878d7c2d993b7a2a7a3a2a1042470c8c56b061
Loading